From: Stefan Sperling <stsp@stsp.name>
Subject: Re: got_repo_map_path()
To: Christian Weisgerber <naddy@mips.inka.de>
Cc: gameoftrees@openbsd.org
Date: Thu, 22 Oct 2020 01:11:24 +0200

On Thu, Oct 22, 2020 at 12:28:07AM +0200, Christian Weisgerber wrote:
> Stefan Sperling:
> 
> > The file logged in your example is located in a Got work tree.
> > I think good a first step towards clearing up the confusion is to
> > avoid calling got_repo_map_path() if we have a work tree.
> 
> But the unveil/realpath problem arises when the work tree is
> irrelevant!
> 
> Actually, the example I gave was bad because of the subdirectory.
> Let's try again:
> 
> $ cd /home/naddy/got    # work tree
> $ got log -r /home/naddy/got.git Makefile
> got: home/naddy/got/Makefile: no such entry found in tree
> 
> The trigger of the problem is "-r repo", which causes the worktree==NULL
> branch to be taken.
> 
> So in the example above, the -r repo is redundant since it just
> specifies the very same repository that can be derived from the
> work tree.  However, it makes perfect sense to reference a DIFFERENT
> repository while in a worktree:
> 
> $ cd /home/naddy/got    # worktree
> $ got log -r /home/naddy/openbsd.git Makefile
> got: home/naddy/got/Makefile: no such entry found in tree
> 
> It's irrelevant whether a file ./Makefile exists in the worktree.

OK, I see. This is a bug in got_map_repo_path(). If it matches an
otherwise unrelated on-disk path it should indeed preserve what
was passed in, instead of returning some path it has calculated. 

> Is there any interpretation for "log -r repo path" other than "path
> is relative to repo" that still makes sense?

Yes. A case where a mapped path is required instead is if you are
in a subdirectory of a non-bare Git repository and do something
like this:
	cd ~/some-git-repo/doc
	got log -r ~/some-git-repo readme.txt
This needs to log the in-repo path 'doc/readme.txt', not 'readme.txt'.

The patch below makes your test case work. The test suite is passing
both with and without your reproducing patch which disables unveil.

I think my other diff is valid regardless?
It just addresses a different issue.

diff 60ca2a50c4fa6806752129de244576f0d289ffa0 /home/stsp/src/got
blob - 036305e904556fd6910f4e907610f8973a3b2e8a
file + lib/repository.c
--- lib/repository.c
+++ lib/repository.c
@@ -769,8 +769,11 @@ got_repo_map_path(char **in_repo_path, struct got_repo
 			if (got_repo_is_bare(repo)) {
 				/*
 				 * Matched an on-disk path inside repository
-				 * database. Treat as repository-relative.
+				 * database. Treat input as repository-relative.
 				 */
+				 free(path);
+				 path = canonpath;
+				 canonpath = NULL;
 			} else {
 				char *child;
 				/* Strip common prefix with repository path. */
@@ -784,8 +787,11 @@ got_repo_map_path(char **in_repo_path, struct got_repo
 		} else {
 			/*
 			 * Matched unrelated on-disk path.
-			 * Treat it as repository-relative.
+			 * Treat input as repository-relative.
 			 */
+			 free(path);
+			 path = canonpath;
+			 canonpath = NULL;
 		}
 	}