Re: [PATCH 07/18] link_alt_odb_entry: handle normalize_path errors

From: Bryan Turner <bturner@atlassian.com>
Date: Nov 7, 2016, 23:42 UTC
Message-ID: <CAGyf7-HWAMF8S+Bw3wcwJCS1Subc28KHjpSCc1__0qn-GSMyvA@mail.gmail.com>
In-Reply-To: <20161003203417.izcgwt4yz3yspdnm@sigill.intra.peff.net>

On Mon, Oct 3, 2016 at 1:34 PM, Jeff King <peff@peff.net> wrote:

Show 8 quoted lines

> When we add a new alternate to the list, we try to normalize
> out any redundant "..", etc. However, we do not look at the
> return value of normalize_path_copy(), and will happily
> continue with a path that could not be normalized. Worse,
> the normalizing process is done in-place, so we are left
> with whatever half-finished working state the normalizing
> function was in.
>

<snip>

Show 8 quoted lines

> @@ -335,7 +340,9 @@ static void link_alt_odb_entries(const char *alt, int len, int sep,
>         }
>
>         strbuf_add_absolute_path(&objdirbuf, get_object_directory());
> -       normalize_path_copy(objdirbuf.buf, objdirbuf.buf);
> +       if (strbuf_normalize_path(&objdirbuf) < 0)
> +               die("unable to normalize object directory: %s",
> +                   objdirbuf.buf);

This appears to break the ability to use a relative alternate via an environment variable, since normalize_path_copy_len is explicitly documented "Returns failure (non-zero) if a ".." component appears as first path"

For example, when trying to run a rev-list over commits in two repositories using GIT_ALTERNATE_OBJECT_DIRECTORIES, in 2.10.x and prior the following command works. I know the alternate worked previously because I'm passing a commit that does not exist in the repository I'm running the command in; it only exists in a repository linked by alternate, as shown by the "fatal: bad object" when the alternates are rejected.

Before, using Git 2.7.4 (but I've verified this behavior through to and including 2.10.2):

bturner@elysoun /tmp/1478561282706-0/shared/data/repositories/3 $ GIT_ALTERNATE_OBJECT_DIRECTORIES=../0/objects:../1/objects git rev-list --format="%H" 2d8897c9ac29ce42c3442cf80ac977057045e7f6 74de5497dfca9731e455d60552f9a8906e5dc1ac ^6053a1eaa1c009dd11092d09a72f3c41af1b59ad ^017caf31eca7c46eb3d1800fcac431cfa7147a01 -- commit 74de5497dfca9731e455d60552f9a8906e5dc1ac 74de5497dfca9731e455d60552f9a8906e5dc1ac commit 3528cf690cb37f6adb85b7bd40cc7a6118d4b598 3528cf690cb37f6adb85b7bd40cc7a6118d4b598 commit 2d8897c9ac29ce42c3442cf80ac977057045e7f6 2d8897c9ac29ce42c3442cf80ac977057045e7f6 commit 9c05f43f859375e392d90d23a13717c16d0fdcda 9c05f43f859375e392d90d23a13717c16d0fdcda

Now, using Git 2.11.0-rc0

bturner@elysoun /tmp/1478561282706-0/shared/data/repositories/3 $ GIT_ALTERNATE_OBJECT_DIRECTORIES=../0/objects:../1/objects /opt/git/2.11.0-rc0/bin/git rev-list --format="%H" 2d8897c9ac29ce42c3442cf80ac977057045e7f6 74de5497dfca9731e455d60552f9a8906e5dc1ac ^6053a1eaa1c009dd11092d09a72f3c41af1b59ad ^017caf31eca7c46eb3d1800fcac431cfa7147a01 -- error: unable to normalize alternate object path: ../0/objects error: unable to normalize alternate object path: ../1/objects fatal: bad object 74de5497dfca9731e455d60552f9a8906e5dc1ac

Other commits, like [1], suggest the ability to use relative paths in alternates is something still actively developed and enhanced. Is it intentional that this breaks the ability to use relative alternates? If this is to be the "new normal", is there any other option when using environment variables besides using absolute paths?

Best regards, Bryan Turner

[1]: https://github.com/git/git/commit/087b6d584062f5b704356286d6445bcc84d686fb -- Also newly tagged in 2.11.0-rc0