On Mon, Nov 7, 2016 at 4:30 PM, Jeff King <peff@xxxxxxxx> wrote:
> On Mon, Nov 07, 2016 at 03:42:35PM -0800, Bryan Turner wrote:
>
>> > @@ -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"
>
> That shouldn't happen, though, because the path we are normalizing has
> been converted to an absolute path via strbuf_add_absolute_path. IOW, if
> your relative path is "../../../foo", we should be feeding something
> like "/path/to/repo/.git/objects/../../../foo" and normalizing that to
> "/path/to/foo".
>
> But in your example, you see:
>
> error: unable to normalize alternate object path: ../0/objects
>
> which cannot come from the code above, which calls die(). It should be
> coming from the call in link_alt_odb_entry().
Ah, of course. I should have looked more closely at the call.
<snip>
> No, I had no intention of disallowing relative alternates (and as you
> noticed, a commit from the same series actually expands the use of
> relative alternates). My use has been entirely within info/alternates
> files, though, not via the environment.
>
> As I said, I'm not sure this was ever meant to work, but as far as I can
> tell it mostly _has_ worked, modulo some quirks. So I think we should
> consider it a regression for it to stop working in v2.11.
>
> The obvious solution is one of:
>
> 1. Stop calling normalize() at all when we do not have a relative base
> and the path is not absolute. This restores the original quirky
> behavior (plus makes the "foo/../../bar" case work).
>
> If we want to do the minimum before releasing v2.11, it would be
> that. I'm not sure it leaves things in a very sane state, but at
> least v2.11 does no harm, and anybody who cares can build saner
> semantics for v2.12.
>
> 2. Fix it for real. Pass a real relative_base when linking from the
> environment. The question is: what is the correct relative base? I
> suppose "getcwd() at the time we prepare the alt odb" is
> reasonable, and would behave similarly to older versions ($GIT_DIR
> for bare repos, top of the working tree otherwise).
>
> If we were designing from scratch, I think saner semantics would
> probably be always relative from $GIT_DIR, or even always relative
> from the object directory (i.e., behave as if the paths were given
> in objects/info/alternates). But that breaks compatibility with
> older versions. If we are treating this as a regression, it is not
> very friendly to say "you are still broken, but you might just need
> to add an extra '..' to your path".
>
> So I dunno. I guess that inclines me towards (1), as it lets us punt on
> the harder question.
Is there anything I can do to help? I'm happy to test out changes.
I've got a set of ~1,040 tests that verify all sorts of different Git
behaviors (those tests flagged this change, for example, and found a
regression in git diff-tree in 2.0.2/2.0.3, among other things). I run
them on the "newest" patch release for every feature-bearing line of
Git from 1.8.x up to 2.10 (currently 1.8.0.3, 1.8.1.5, 1.8.2.3,
1.8.3.4, 1.8.4.5, 1.8.5.6, 1.9.5, 2.0.5, 2.1.4, 2.2.3, 2.3.10, 2.4.11,
2.5.5, 2.6.6, 2.7.4, 2.8.4, 2.9.3 and 2.10.2), and I add in RCs of new
as soon as they become available. (I also test Git for Windows; at the
moment I've got 1.8.0, 1.8.1.2, 1.8.3, 1.8.4, 1.8.5.2 and 1.9.5.1 from
msysgit and 2.3.7.1, 2.4.6, 2.5.3, 2.6.4, 2.7.4, 2.8.4, 2.9.3 and
2.10.2 from Git for Windows. 2.11.0-rc0 on Windows passes my test
suite; it looks like it's not tagging the same git/git commit as
v2.11.0-rc0 is.) I wish there was an easy way for me to open this up.
At the moment, it's something I can really only run in-house, as it
were.
At the moment I have limited ability to actually try to submit patches
myself. I really need to sit down and setup a working development
environment for Git. (My current dream, if I could get such an
environment running, is to follow up on your git blame-tree work.
>
> -Peff