Unexpected behavior with :/<text> references

Unexpected behavior with :/<text> references

[William Chargin]

Hello,

I'm experiencing strange behavior with :/<text> references, which seems
to be inconsistent with the explanation in the docs on two counts.
First, sometimes the matched commit is not the youngest. Second, some
commits cannot be found at all, even if they are reachable from HEAD.

Here is a script to reproduce the behavior (Git built from current pu):

    #!/bin/sh
    export GIT_CONFIG_NOSYSTEM=1
    export GIT_ATTR_NOSYSTEM=1
    cd "$(mktemp -d --suffix .gitrepro)"

    git --version
    git init

    git commit -q --allow-empty -m initial
    git commit -q --allow-empty -m foo
    git checkout -q -b early-branch
    git commit -q --allow-empty -m foobar
    git checkout -q --detach
    git commit -q --allow-empty -m foobarbaz

    echo
    echo "The following should all print 'foobarbaz':"
    git show --format=%s ':/foo' --
    git show --format=%s ':/foobar' --
    git show --format=%s ':/foobarbaz' --

    echo
    echo "With an explicit branch:"
    git branch late-branch
    git show --format=%s ':/foo' --
    git show --format=%s ':/foobar' --
    git show --format=%s ':/foobarbaz' --

Here is the output on my machine:

    git version 2.18.0.516.g6fb7f6652
    Initialized empty Git repository in /tmp/tmp.WeCD0QZPIf.gitrepro/.git/

    The following should all print 'foobarbaz':
    foo
    foobar
    fatal: bad revision ':/foobarbaz'

    With an explicit branch:
    foo
    foobarbaz
    foobarbaz

First, the commit with message "foobar" clearly matches the regular
expression /foo/ as well as /foobar/, but ":/foo" resolves to an older
commit. However, Documentation/revisions.txt indicates that a :/<text>
reference should resolve to the _youngest_ matching commit.

Second, the commit with message "foobarbaz" is reachable from HEAD, and
yet the regular expression /foobarbaz/ fails to match it. The same
documentation indicates that :/<text> references find commits reachable
from any ref, and the glossary entry for "ref" states that HEAD is a
"special-purpose ref" even though it does not begin with "refs/".

It looks to me like references reachable from `master` are always picked
over other references, and that references reachable only from HEAD are
not matched at all.

Is the observed behavior intentional? If so, what am I misunderstanding?

Thanks!
WC

(for searchability: colon slash text references)

Re: Unexpected behavior with :/<text> references

[William Chargin]

After further investigation, it appears that ":/foo" indeed resolves to
the commit with message "foobar" (in the above example) if the commits
are not all created at the same time: e.g., by adding `sleep 1` between
the commit commands, or exporting `GIT_AUTHOR_DATE`.

This leaves only the question of why :/<text> references don't resolve
to commits reachable only from HEAD, and whether this is the best
behavior.

Re: Unexpected behavior with :/<text> references

[Jeff King]

On Sun, Jul 08, 2018 at 10:13:12PM -0700, William Chargin wrote:

> After further investigation, it appears that ":/foo" indeed resolves to
> the commit with message "foobar" (in the above example) if the commits
> are not all created at the same time: e.g., by adding `sleep 1` between
> the commit commands, or exporting `GIT_AUTHOR_DATE`.

I suspect it's actually GIT_COMMITTER_DATE (but I didn't dig into it, so
I could be wrong). "Youngest" in terms of commits generally refers to
the committer date.

> This leaves only the question of why :/<text> references don't resolve
> to commits reachable only from HEAD, and whether this is the best
> behavior.

I agree that it would make sense to look at HEAD here. The point of ":/"
is to search everything reachable, and HEAD is part of that
reachability. _Usually_ it doesn't matter because HEAD is pointing to
another ref, but the detached case is an exception.

Doing this would bring us in line with the behavior of other parts of
Git. E.g., f0298cf1c6 (revision walker: include a detached HEAD
in --all, 2009-01-16).

The patch is probably this:

diff --git a/sha1-name.c b/sha1-name.c
index 60d9ef3c7e..641ca12f91 100644
--- a/sha1-name.c
+++ b/sha1-name.c
@@ -1650,6 +1650,7 @@ static int get_oid_with_context_1(const char *name,
 			struct commit_list *list = NULL;
 
 			for_each_ref(handle_one_ref, &list);
+			head_ref(handle_one_ref, &list);
 			commit_list_sort_by_date(&list);
 			return get_oid_oneline(name + 2, oid, list);
 		}

but I didn't test it at all. Do you want to try adding a case for this
to our test suite (probably in t4208) and wrapping it up with a commit
message?

-Peff

Re: Unexpected behavior with :/<text> references

[William Chargin]

Yep, I agree with your analysis. I'd be happy to test and commit this,
probably later today.

Thanks for your input, and for the patch!

Best,
WC