git@vger.kernel.org mailing list mirror (one of many)
 help / Atom feed
From: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
To: Junio C Hamano <gitster@pobox.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: Mon, 05 Nov 2018 12:31:50 +0100
Message-ID: <87k1lr7obd.fsf@evledraar.gmail.com> (raw)
In-Reply-To: <xmqqy3adtopw.fsf@gitster-ct.c.googlers.com>


I'll re-roll this. Hopefully sooner than later. I'll leave out the later
part of this series as it's more controversial and we can discuss that
later on its own. Meanwhile just some replies to this (while I
remember):

> Æ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).

Yeah this example was bad, but now since I've written it I've become
more convinced that it's the right way to go, just that I didn't explain
it well.

E.g. just now I did:

    git push avar avar/some-branch:some-branch-wip
    git push avar HEAD -f # I was on 'some-branch'

Because I'd regretted taking the "some-branch" name with some WIP code,
but didn't want to lose the work, so I wanted to swap.

It's also arbitrary and contrary to the distributed nature of git that
we're treating branches from other repos differently than the ones from
ours.

After all sometimes "other" is just the repo on my laptop or server. I
shouldn't need to jump through hoops to re-push stuff from my "other"
repo anymore than from the local repo.

Yes refs/remotes/* isn't guaranteed to be "other repo's branches" in the
same way our local refs/heads/* is, and this series bends over backwards
to check if someone's (ab)used it for that, but I think for the purposes
of the default UI we should treat it as "branches from other remotes" if
we find commit objects there.

So I think the *only* thing we should be checking for the purposes of
this DWYM feature is what local type of object (branch or tag) we find
on the LHS of the refspec. And if we're sure of its type push to
refs/{heads,tags}/* as appropriate.

I don't think it makes any sense as you suggest to move away from "guess
based on existing type" to breaking the dwimmery because we found the
branch in some other place. The user's pushing "newref" from an existing
branch, let's just assume the new thing is a branch, whether the source
is local or not.

> 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.

Thanks. As noted will try to re-roll this soon.

  reply index

Thread overview: 53+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-10-10 10:41 [PATCH 0/2] add an advice on unqualified <dst> push Ævar Arnfjörð Bjarmason
2018-10-10 10:41 ` [PATCH 1/2] i18n: remote.c: mark error(...) messages for translation Ævar Arnfjörð Bjarmason
2018-10-10 20:55   ` Jeff King
2018-10-10 10:41 ` [PATCH 2/2] push: add an advice on unqualified <dst> push Ævar Arnfjörð Bjarmason
2018-10-10 20:55   ` Jeff King
2018-10-10 21:23     ` Ævar Arnfjörð Bjarmason
2018-10-11  0:16       ` Jeff King
2018-10-11 22:45       ` Junio C Hamano
2018-10-26 15:45         ` Ævar Arnfjörð Bjarmason
2018-10-29  1:00           ` Junio C Hamano
2018-10-29  4:17             ` Junio C Hamano
2018-10-26 19:27         ` [PATCH v2 0/7] fixes for " Ævar Arnfjörð Bjarmason
2018-10-26 23:07           ` [PATCH v3 0/8] " Ævar Arnfjörð Bjarmason
2018-11-02  6:52             ` Jeff King
2018-11-13 19:52             ` [PATCH v4 0/7] " Ævar Arnfjörð Bjarmason
2018-11-14  7:00               ` Junio C Hamano
2018-11-13 19:52             ` [PATCH v4 1/7] remote.c: add braces in anticipation of a follow-up change Ævar Arnfjörð Bjarmason
2018-11-13 19:52             ` [PATCH v4 2/7] i18n: remote.c: mark error(...) messages for translation Ævar Arnfjörð Bjarmason
2018-11-13 19:52             ` [PATCH v4 3/7] push: improve the error shown on unqualified <dst> push Ævar Arnfjörð Bjarmason
2018-11-13 19:52             ` [PATCH v4 4/7] push: move unqualified refname error into a function Ævar Arnfjörð Bjarmason
2018-11-13 19:52             ` [PATCH v4 5/7] push: add an advice on unqualified <dst> push Ævar Arnfjörð Bjarmason
2018-11-13 19:52             ` [PATCH v4 6/7] push: test that <src> doesn't DWYM if <dst> is unqualified Ævar Arnfjörð Bjarmason
2018-11-13 19:52             ` [PATCH v4 7/7] push doc: document the DWYM behavior pushing to unqualified <dst> Ævar Arnfjörð Bjarmason
2018-10-26 23:07           ` [PATCH v3 1/8] remote.c: add braces in anticipation of a follow-up change Ævar Arnfjörð Bjarmason
2018-10-26 23:07           ` [PATCH v3 2/8] i18n: remote.c: mark error(...) messages for translation Ævar Arnfjörð Bjarmason
2018-10-26 23:07           ` [PATCH v3 3/8] push: improve the error shown on unqualified <dst> push Ævar Arnfjörð Bjarmason
2018-10-29  5:03             ` Junio C Hamano
2018-10-26 23:07           ` [PATCH v3 4/8] push: move unqualified refname error into a function Ævar Arnfjörð Bjarmason
2018-10-26 23:07           ` [PATCH v3 5/8] push: add an advice on unqualified <dst> push Ævar Arnfjörð Bjarmason
2018-10-29  5:14             ` Junio C Hamano
2018-11-02  6:47               ` Jeff King
2018-10-26 23:07           ` [PATCH v3 6/8] push: test that <src> doesn't DWYM if <dst> is unqualified Ævar Arnfjörð Bjarmason
2018-10-29  5:19             ` Junio C Hamano
2018-10-26 23:07           ` [PATCH v3 7/8] push: add DWYM support for "git push refs/remotes/...:<dst>" Ævar Arnfjörð Bjarmason
2018-10-29  5:24             ` Junio C Hamano
2018-10-29  8:13               ` Ævar Arnfjörð Bjarmason
2018-11-01  4:18                 ` Junio C Hamano
2018-11-05 11:31                   ` Ævar Arnfjörð Bjarmason [this message]
2018-11-05 12:29                     ` Junio C Hamano
2018-10-29  7:06             ` Junio C Hamano
2018-10-29  7:57               ` Junio C Hamano
2018-10-29  8:05               ` Ævar Arnfjörð Bjarmason
2018-10-26 23:07           ` [PATCH v3 8/8] push doc: document the DWYM behavior pushing to unqualified <dst> Ævar Arnfjörð Bjarmason
2018-10-26 19:27         ` [PATCH v2 1/7] remote.c: add braces in anticipation of a follow-up change Ævar Arnfjörð Bjarmason
2018-10-26 21:05           ` Stefan Beller
2018-10-26 19:27         ` [PATCH v2 2/7] i18n: remote.c: mark error(...) messages for translation Ævar Arnfjörð Bjarmason
2018-10-26 19:27         ` [PATCH v2 3/7] push: improve the error shown on unqualified <dst> push Ævar Arnfjörð Bjarmason
2018-10-26 19:27         ` [PATCH v2 4/7] push: move unqualified refname error into a function Ævar Arnfjörð Bjarmason
2018-10-26 19:27         ` [PATCH v2 5/7] push: add an advice on unqualified <dst> push Ævar Arnfjörð Bjarmason
2018-10-26 19:27         ` [PATCH v2 6/7] push: test that <src> doesn't DWYM if <dst> is unqualified Ævar Arnfjörð Bjarmason
2018-10-26 19:27         ` [PATCH v2 7/7] push: add DWYM support for "git push refs/remotes/...:<dst>" Ævar Arnfjörð Bjarmason
2018-10-10 21:54     ` [PATCH 2/2] push: add an advice on unqualified <dst> push Junio C Hamano
2018-10-11  0:19       ` Jeff King

Reply instructions:

You may reply publically to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=87k1lr7obd.fsf@evledraar.gmail.com \
    --to=avarab@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=peff@peff.net \
    --cc=sbeller@google.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

git@vger.kernel.org mailing list mirror (one of many)

Archives are clonable:
	git clone --mirror https://public-inbox.org/git
	git clone --mirror http://ou63pmih66umazou.onion/git
	git clone --mirror http://czquwvybam4bgbro.onion/git
	git clone --mirror http://hjrcffqmbrq6wope.onion/git

Newsgroups are available over NNTP:
	nntp://news.public-inbox.org/inbox.comp.version-control.git
	nntp://ou63pmih66umazou.onion/inbox.comp.version-control.git
	nntp://czquwvybam4bgbro.onion/inbox.comp.version-control.git
	nntp://hjrcffqmbrq6wope.onion/inbox.comp.version-control.git
	nntp://news.gmane.org/gmane.comp.version-control.git

 note: .onion URLs require Tor: https://www.torproject.org/
       or Tor2web: https://www.tor2web.org/

AGPL code for this site: git clone https://public-inbox.org/ public-inbox