Re: [PATCH v3 7/8] push: add DWYM support for "git push refs/remotes/...:<dst>"

From: Junio C Hamano <gitster@pobox.com>
To: "Ævar Arnfjörð Bjarmason" <avarab@gmail.com>
Cc: git@vger.kernel.org, Jeff King <peff@peff.net>,
	Stefan Beller <sbeller@google.com>
Subject: Re: [PATCH v3 7/8] push: add DWYM support for "git push refs/remotes/...:<dst>"
Date: Thu, 01 Nov 2018 13:18:35 +0900	[thread overview]
Message-ID: <xmqqy3adtopw.fsf@gitster-ct.c.googlers.com> (raw)
In-Reply-To: <87in1lkw54.fsf@evledraar.gmail.com> (=?utf-8?B?IsOGdmFyIEFy?= =?utf-8?B?bmZqw7Zyw7A=?= Bjarmason"'s message of "Mon, 29 Oct 2018 09:13:59 +0100")

Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:

>> On the other hand, I do not think I mind all that much if a src that
>> is a tag object to automatically go to refs/tags/ (having a tag
>> object in refs/remotes/** is rare enough to matter in the first
>> place).
>
> Yeah maybe this is going too far. I don't think so, but happy to me
> challenged on that point :)
>
> I don't think so because the only reason I've ever needed this is
> because I deleted some branch accidentally and am using a push from
> "remotes/*" to bring it back. I.e. I'll always want branch-for-branch,
> not to push that as a tag.

Oh, I didn't consider pushing it out as a tag, but now you bring it
up, I think that it also would make sense in a workflow to tell your
colleages to look at (sort of like how people use pastebin---"look
here, this commit has the kind of change I have in mind in this
discussion") some random commit and the commit happens to be sitting
at a tip of a remote-trackig branch.  Instead of pushing it out as a
branch or a remote-tracking branch, which has strong connotations of
inviting others to build on top, pushing it out as a tag would make
more sense in that context.

And as I mentioned already, I think it would equally likely, if not
more likely, for people like me to push remotes/** out as a
remote-tracking branch (rather than a local branch) of the
repository I'm pushing into.

So I tend to agree that this is going too far.  If the original
motivating example was not an ingredient of everyday workflow, but
was an one-off "recovery", I'd rather see people forced to be more
careful by requiring "push origin/frotz:refs/heads/frotz" rather
than incorrectly DWIDNM "push origin/frotz:frotz" and ending up with
creating refs/tags/frotz or refs/remotes/origin/frotz, which also
are plausible choices depending on what the user is trying to
recover from, which the sending end would not know (the side on
which the accidental loss of a ref happened earlier is on the remote
repository that would be receiving this push, and it _might_ know).

As to the entirety of the series,

 - I do not think this step 7, and its documentation in step 8, are
   particularly a good idea, in their current shape.  Pushing tag
   objects to refs/tags/ is probably a good idea, but pushing a
   commit as local branch heads are necessarily not.

 - Step 6 is probably a good documentation on the cases in which we
   make and do not make guess on the unqualified push destination.

 - Step 5 and earlier looked like good changes.

If we were to salvage some parts of step 7 (and step 8), we'd
probably need fb7c2268 ("SQUASH???", 2018-10-29) to number all the
placeholders in the printf format string.