On Tue, May 08, 2018 at 08:53:10PM +0200, Ævar Arnfjörð Bjarmason wrote:
> >> "X^{tree}" should *RESULT* in a tree, but it should *REQUIRE* X to
> >> be a tree-ish. It is unclear "should be tree" is about the former
> >> and I read (perhaps mis-read) it as saying "it should require X to
> >> be a tree"---that statement is utterly incorrect as we agreed above.
> >
> > FWIW, I had the same feeling as you when reading this, that this commit
> > (and the one after) are doing the wrong thing. And these paragraphs sum
> > it up. The "^{tree}" is about asking us to peel to a tree, not about
> > resolving X in the first place. We can use it as a _hint_ when resolving
> > X, but the correct hint is "something that can be peeled to a tree", not
> > "is definitely a tree".
>
> Maybe I'm just being dense, but I still don't get from this & Junio's
> E-Mails what the general rule should be.
Let me try to lay out my thinking a bit more clearly, and then I'll try
to respond to the points you laid out below.
Before we had any disambiguation code, resolving X^{tree} really was two
independent steps: resolve X, and then peel it to a tree. When we added
the disambiguation code, the goal was to provide a hint to the first
step in such a way that we could never eliminate any resolutions that
the user _might_ have meant. But it's OK to take a situation where every
case but one would result in an error, and assume the user meant that
case. Sort of a "do no harm" rule.
By disambiguating with just a tree and not a tree-ish, that hint is now
eliminating possibilities that would have worked in the second step,
which violates the rule.
Does thinking about it that way make more sense?
> I think a response to the part after "leaving that aside" of my upthread
> E-Mail
> (https://public-inbox.org/git/87lgczyfq6.fsf@xxxxxxxxxxxxxxxxxxx/) would
> help me out.
I'll quote that bit here:
> 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)
Yes, that makes sense to me conceptually, and would follow the rule I
gave above. And I think that's what we do now, with the exception that
there is no blobish disambiguation. Presumably nobody ever bothered
because probably because tagged blobs are pretty rare (and obviously
though trees point to blobs, you cannot disambiguate that way since
there's no one-to-one correspondence).
So I doubt anybody really cares in practice, but I agree that it would
improve consistency to write a patch to introduce GET_OID_BLOBISH and
have "^{blob}" parsing use it. And possibly add "blobish" to
core.disambiguate (or is it "blobbish"?), though that's almost certainly
something nobody would ever use.
> My understanding of what you two are saying is that somehow the peel
> semantics should be preserved when we take this beyond the 1=1 mapping
> case, but I don't see how if we run with that how we wouldn't need to
> introduce the concept of blobish for consistency as I noted upthread.
Yeah, I think the lack of blobish is a bug, just one that nobody has
ever really cared about.
> So it would be very useful to me if you or someone who understands the
> behavior you & Junio seem to want could write a version of the patch I
> have above where the last paragraph is different, and describes the
> desired semantics, because I still don't get it. Why would we 1=many
> peel commits to trees as a special case, but not 1=many do the same for
> trees & blobs?
I'm not sure I understand the mention of trees in the final sentence.
AFAICT tree disambiguation is consistent with the peeling rules.
-Peff
