Re: [PATCH v2] contrib/subtree bugfix: Can't `add` annotated tag

[Junio C Hamano <gitster@pobox.com>]

James Denholm <nod.helm@gmail.com> writes:

> On Tue, May 13, 2014 at 04:12:56PM -0700, Junio C Hamano wrote:
>> James Denholm <nod.helm@gmail.com> writes:
>> 
>> > I'm not sure that can actually happen - peel_committish is essentially
>> > implemented as `rev-parse $arg^0` (though with a bit of bling, of
>> > course), and to my understanding FETCH_HEAD will always parse to a
>> > committish - I could have missed something, of course.
>> 
>>  $ git fetch git://repo.or.cz/alt-git junio-gpg-pub
>>  $ git rev-parse FETCH_HEAD^0
>
> That would be a problem... Sadly I doubt I'll have time to develop a
> solution into subtree's overall design before the end of June. As that
> eventual change would probably involve altering the inclusions of this
> fix, and that users have a workaround in adding either squashed commits
> or referencing lightweight tags, would you rather drop the patch and
> wait for that?

Sorry, I am lost.  What would be a problem exactly?

A FETCH_HEAD can be pointing at an object that is not committish,
and users involved, both at the originating end who controls the
repository you fetched from and at the receiving end who wanted to
fetch the object, are *not* expeting to be able to make a merge of
such an object anyway.  My suggestion was not to ask you to come up
with a sane behaviour when the user told us to add a single blob
with "subtree add"; it was merely to detect such unintended use as
an error.

To me, it looks like all that is necessary is to accept your patch
but with a three-byte tightening to detect such a pathological case
and signal an error, which is what " &&", which I added to your new
line that sets revp=$(peel_committish ...), is about.

This patch, with or without these extra " &&" three bytes, will not
be part of the upcoming 2.0 release anyway, so we have enough time
to iron it out.