Re: [PATCH v3 09/12] get_short_oid / peel_onion: ^{tree} should be tree, not treeish

From: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Date: May 4, 2018, 08:42 UTC
Message-ID: <87lgczyfq6.fsf@evledraar.gmail.com>
In-Reply-To: <xmqqvac4tb64.fsf@gitster-ct.c.googlers.com>

On Fri, May 04 2018, Junio C Hamano wrote:

Show 15 quoted lines

> Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:
>
>> The reason I'm doing this is because I found it confusing that I can't
>> do:
>>
>>     for t in tag commit tree blob; do ./git --exec-path=$PWD rev-parse 7452^{$t}; done
>>
>> And get, respectively, only the SHAs that match the respective type, but
>> currently (with released git) you can do:
>>
>>     for t in tag commit committish treeish tree blob; do git -c core.disambiguate=$t rev-parse 7452; done
>
> Exactly.  The former asks "I (think I) know 7452 can be used to name
> an object of type $t, with peeling if necessary--give me the underlying
> object of type $t".

Right, and I'm with you so far, this makes sense to me for all existing uses of the peel syntax, otherwise v2.17.0^{tree} wouldn't be the same as rev-parse v2.17.0^{tree}^{tree}...

Show 19 quoted lines

> In short, the fact that you can write "$X^{$t}"
> says that $X is a $t-ish (otherwise $X cannot be used as a stand-in
> for an object of type $t) and that you fully expect that $X can merely
> be of type $t-ish and not exactly $t (otherwise you wouldn't be
> making sure to coerce $X into $t with ^{$t} notation).
>
> In *THAT* context, disambiguation help that lists objects whose name
> begins with "7452" you gave, hoping that it is a unique enough
> prefix when it wasn't in reality, *MUST* give $t-ish; restricting it
> to $t makes the help mostly useless.
>
>> 1) Am I missing some subtlety or am I correct that there was no way to
>> get git to return more than one SHA-1 for ^{commit} or ^{tree} before
>> this disambiguation feature was added?
>
> There is no such feature either before or after the disambiguation
> help.  I am not saying there shouldn't exist such a feature.  I am
> saying that breaking the existing feature and making it useless is
> not the way to add such a feature.

I still don't get how what you're proposing is going to be consistent, but let's fully enumerate the output of 7452 with my patch to take that case-by-case[1]:

    ^{tag}:
    7452b4b5786778d5d87f5c90a94fab8936502e20
    ^{commit}:
    hint:   74521eee4c commit 2007-12-01 - git-gui: install-sh from automake does not like -m755
    hint:   745224e04a commit 2014-06-18 - refs.c: SSE2 optimizations for check_refname_component
    ^{tree}:
    hint:   7452336aa3 tree
    hint:   74524f384d tree
    hint:   7452813bcd tree
    hint:   7452b1a701 tree
    hint:   7452b73c42 tree
    hint:   7452ca1557 tree
    ^{blob}:
    hint:   7452001351 blob
    hint:   745254665d blob
    hint:   7452a572c1 blob
    hint:   7452b9fd21 blob
    hint:   7452db13c8 blob
    hint:   7452fce0da blob

And[2]:

    core.disambiguate=tag:
    [same as ^{tag]
    core.disambiguate=commit:
    [same as ^{commit}]
    core.disambiguate=committish:
    hint:   7452b4b578 tag v2.1.0
    hint:   74521eee4c commit 2007-12-01 - git-gui: install-sh from automake does not like -m755
    hint:   745224e04a commit 2014-06-18 - refs.c: SSE2 optimizations for check_refname_component
    core.disambiguate=tree:
    [same as ^{tree}]
    core.disambiguate=treeish (same as $sha1:)
    hint:   7452b4b578 tag v2.1.0
    hint:   74521eee4c commit 2007-12-01 - git-gui: install-sh from automake does not like -m755
    hint:   745224e04a commit 2014-06-18 - refs.c: SSE2 optimizations for check_refname_component
    hint:   7452336aa3 tree
    hint:   74524f384d tree
    hint:   7452813bcd tree
    hint:   7452b1a701 tree
    hint:   7452b73c42 tree
    hint:   7452ca1557 tree
    core.disambiguate=blob:
    [same as ^{blob}]

So from my understanding of what you're saying you'd like to list tag, commits and trees given $sha1^{tree}, because they're all types that can be used to reach a tree.

I don't think that's very useful, yes it would "break" existing disambiguations, but this is such an obscure (and purely manual) use-case than I think that's fine.

Because I think to the extent anyone's going to use this it's because they know they have e.g. a short blob, commit etc. SHA-1 they're not going to use it because they have some short $SHA they know is a tree, and then want all SHA-1s of that *and* random tag & commit objects that happen to have the same object prefix just because tags and commits can also point to trees.

How does that make any sense? The entire reason for using the normal peel syntax is because you e.g. have v2.17.0 and want to get to the ^{tree} or the ^{commit} tht v2.17.0 directly points to. That's entirely orthogonal to what the disambiguation is doing. There with your proposed semantics you're peeling 7452 as 7452^{tree} because (IMO) you're looking for trees, just to get some entirely unrelated commits and tags.

But *leaving that aside*, i.e. I don't see why the use-case would make sense. What I *don't* get is why, if you think that, you only want to apply that rule to ^{tree}. I.e. wouldn't it then be consistent to say:

    # a)
    ^{tag}    = tag
    ^{commit} = tag, commit
    ^{tree}   = tag, commit, tree
    ^{blob}   = tag, blob (blobish)

Whereas my patch now does:

    # b)
    ^{tag}    = tag
    ^{commit} = commit
    ^{tree}   = tree
    ^{blob}   = blob

But from what you seem to be proposing (or maybe you just didn't have a chance to critique the ^{blob} and ^{commit} patches):

    # c)
    ^{tag}    = tag
    ^{commit} = commit
    ^{tree}   = tag, tree, commit
    ^{blob}   = blob

1. for type in tag commit tree blob; do echo "^{$type}:" && ./git --exec-path=$PWD rev-parse 7452^{$type} 2>&1|grep -E -e ^hint -e '^[0-9a-f]{40}$' |grep -v are:; done
2. for cfg in tag commit committish tree treeish blob; do echo "core.disambiguate=$cfg:" && ./git --exec-path=$PWD -c core.disambiguate=$cfg rev-parse 7452 2>&1|grep -E -e ^hint -e '^[0-9a-f]{40}$' |grep -v are:; done