"GOT", but the "O" is a cute, smiling pufferfish. Index | Thread | Search

From:
Stefan Sperling <stsp@stsp.name>
Subject:
Re: Disabling unveil breaks "got log -r repo entry"
To:
Christian Weisgerber <naddy@mips.inka.de>
Cc:
gameoftrees@openbsd.org
Date:
Wed, 16 Sep 2020 00:35:26 +0200

Download raw body.

Thread
On Tue, Sep 15, 2020 at 11:15:16PM +0200, Christian Weisgerber wrote:
> If unveil() is replaced by a dummy that does nothing and always
> returns 0, "got log -r repo entry" in a work tree breaks.
> 
> regress/cmdline/log.sh reproduces this, specifically
> test_log_in_worktree_different_repo:
> 
> test_log_in_worktree_different_repo got: tmp/got-test-log_in_worktree_different_repo-ERkpS88v/wt/alpha: no such entry found in tree
> --- /tmp/got-test-log_in_worktree_different_repo-ERkpS88v/stdout.expected      Tue Sep 15 22:42:32 2020
> +++ /tmp/got-test-log_in_worktree_different_repo-ERkpS88v/stdout        Tue Sep 15 22:42:32 2020
> @@ -1 +0,0 @@
> -commit 4b955087b291a4cc5568d53b625e66d4e59caa95 (master)
> test failed; leaving test data in /tmp/got-test-log_in_worktree_different_repo-ERkpS88v
> 
> The call chain is this:
> cmd_log() -> got_repo_map_path() -> realpath()
>
> With unveil(), the realpath() call fails.
> 
> Without unveil(), realpath() succeeds.  This effectively expands
> "got log -r repo entry" into "got log -r entry /path/entry", which
> then fails because there is no /path/entry in the repository.

map_path() is not great. I am not really happy with that function
and bugs like this one aren't very surprising to me.
I think this function is one of the earliest bits of code written for got.
It definitely dates from before use of unveil was introduced into the
code base.

Perhaps we should take a fresh look at the problem this code is
trying to solve (map an input to some path inside the repository)
and think about how we could handle it best and in a portable way?

It doesn't need to be a single function either. We could identify a
set of cases we need to handle, and write a separate function for each.
If it helps we could re-define specific requirements, like requiring
the function to be called before, or after, unveil takes effect.