git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Jeff King <peff@peff.net>
To: Junio C Hamano <gitster@pobox.com>
Cc: "Ævar Arnfjörð Bjarmason" <avarab@gmail.com>,
	"Jonathan Tan" <jonathantanmy@google.com>,
	git@vger.kernel.org,
	"Felipe Contreras" <felipe.contreras@gmail.com>
Subject: Re: [PATCH] clone: in protocol v2, use remote's default branch
Date: Wed, 16 Dec 2020 13:39:47 -0500	[thread overview]
Message-ID: <X9pUc2HXUr3+WHbR@coredump.intra.peff.net> (raw)
In-Reply-To: <xmqq7dpi5tvl.fsf@gitster.c.googlers.com>

On Tue, Dec 15, 2020 at 07:09:50PM -0800, Junio C Hamano wrote:

> > I'm not sure if it's a good idea to change "git checkout origin" here or
> > not. It already does something useful. I was mostly suggesting that the
> > other thing might _also_ be useful, but I'm not sure if it is wise to
> > change the current behavior.
> 
> Well, "git checkout origin/HEAD" would also do something useful,
> which happens to be identical to "git checkout origin", to detach
> HEAD at the commit.

To be clear, I meant that both of those do the useful thing, and I'm not
sure if it would be confusing to users to change that (but see below).

> > I was thinking more like an explicit way to trigger the dwim-behavior,
> > like:
> >
> >   # same as "git checkout foo" magic that creates "foo", but we
> >   # have said explicitly both that we expect to make the new branch, and
> >   # also that we expect it to come from origin.
> >   git checkout --make-local origin/foo
> 
> By default I think --guess (formerly known as --dwim) is enabled, so
> "git checkout foo" is "git checkout --guess foo", which is making
> local 'foo' out of the uniquely found remote-tracking branch.  This
> new one is to reduce the "uniquely found" part from the magic and
> let you be a bit more explicit, but not explicit enough to say "-t"
> or "-b foo"?  I am not sure if this is all that useful.

I agree it's not all that useful in that example. What I was thinking is
that by giving the implicit/heuristic magic a more explicit verbose
name, then we make it natural to extend the explicit version to more
cases where it might be questionable to do it implicitly.

So no, I doubt anybody would normally type what I wrote above. But it
lets us explain it as:

  - there's a feature "--make-local" (I still hate the name) that makes
    a local branch from a remote one if it doesn't already exist

  - that feature knows how to resolve symrefs and create the branch from
    the pointed-to name

  - as a shortcut, "git checkout foo" is a synonym for "--make-local
    origin/foo" when "origin/foo" exists but "foo" does not

It's definitely not worth it, though, if we decide that there's an
implicit/heuristic syntax that should trigger the symref thing.

> If this were a slightly different proposal, I would see the
> convenience value in it, though.  Currently what "--guess" does is:
> 
>       If the name 'foo' given does not exist as a local branch,
>       and the name appears exactly once as a remote-tracking branch
>       from some remote (i.e. 'refs/remotes/origin/foo' exists, but
>       there is no other 'refs/remotes/*/foo'), create a local 'foo'
>       that builds on that remote-tracking branch and check it out.
> 
> What would happen if we tweaked the existing "--guess" behaviour
> slightly?
> 
>       "git checkout --guess origin/foo", even when there is a second
>       remote 'publish' that also has a remote-tracking branch for
>       its 'foo' (i.e. both 'refs/remotes/{origin,publish}/foo'
>       exists), can be used to disambiguate among these remotes with
>       'foo'.  You'd get local 'foo' that builds on 'foo' from the
>       remote 'origin' and check it out.

I forgot we had --guess. Piggy-backing on that might be sensible as a
stronger "explicit" signal that this is what the user wants (though
"--guess" is still a funny name here, because we're no longer guessing
at all; the user told us what they want).

But yeah, the semantics you outlined in the second paragraph match what
I was expecting "--make-local" to do.

> >   # similar, but because we are being explicit, we know it is reasonable
> >   # to dereference HEAD to find the actual branch name
> >   git checkout --make-local origin/HEAD
> 
> The user does not need "git symbolic-ref refs/remotes/origin/HEAD"
> if such a feature were available.  "git checkout --some-option origin"
> without having to say /HEAD may be a better UI, though.

Right. I'm assuming that "origin/HEAD" and "origin" could be used
interchangeably in my example.

> And "checkout" being a Porcelain, and the DWIM feature that is
> always on is subject to be improved for human use, I do not see why
> that --some-option cannot be --guess.  If I want to get the current
> behaviour, I can explicitly say "git checkout --detach origin"
> anyway, no?

I think:

  git checkout --guess origin

would make sense to dereference origin/HEAD to "foo", as if we had said
"git checkout foo". That's the explicit part that seems safe. My
question is whether:

  git checkout origin

should likewise do so. As you note, one can always use --detach to make
their intention clear, and checkout is a porcelain, so we are OK to
change it. But would users find that annoying? I frequently use "git
checkout origin" to get a detached HEAD pointing at your master (e.g.,
because I want to reproduce a bug, or do a "something like this..."
patch). I'm sure I could retrain my fingers, but I wonder if I'm not the
only one.

Doing it for only an explicit "--guess" turns that feature into a
tri-state (explicitly off, explicitly on, or "implicit, so be a little
more conservative"). Which perhaps is harder to explain, but I think
cleanly adds the new feature in a consistent way, without really
changing any existing behavior.

Related, I assume that:

  git checkout --guess origin/foo
  git checkout origin/foo

should behave the same as their "origin" or "origin/HEAD" counterparts
for consistency (obviously making "foo" in the former case, and either
detaching or making "foo" in the second case, depending on what you
think of the earlier paragraphs).

-Peff

  reply	other threads:[~2020-12-16 18:40 UTC|newest]

Thread overview: 109+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-12-08  1:31 Cloning empty repository uses locally configured default branch name Jonathan Tan
2020-12-08  2:16 ` Junio C Hamano
2020-12-08  2:32   ` brian m. carlson
2020-12-08 18:55   ` Jonathan Tan
2020-12-08 21:00     ` Junio C Hamano
2020-12-08 15:58 ` Jeff King
2020-12-08 20:06   ` Jonathan Tan
2020-12-08 21:15     ` Jeff King
2020-12-11 21:05 ` [PATCH] clone: in protocol v2, use remote's default branch Jonathan Tan
2020-12-11 23:41   ` Junio C Hamano
2020-12-14 12:38   ` Ævar Arnfjörð Bjarmason
2020-12-14 15:51     ` Felipe Contreras
2020-12-14 16:30     ` Junio C Hamano
2020-12-15  1:41       ` Ævar Arnfjörð Bjarmason
2020-12-15  2:22         ` Junio C Hamano
2020-12-15  2:38         ` Jeff King
2020-12-15  2:55           ` Junio C Hamano
2020-12-15  4:36             ` Jeff King
2020-12-16  3:09               ` Junio C Hamano
2020-12-16 18:39                 ` Jeff King [this message]
2020-12-16 20:56                   ` Junio C Hamano
2020-12-18  6:19                     ` Jeff King
2020-12-15  3:22         ` Felipe Contreras
2020-12-14 19:25     ` Jonathan Tan
2020-12-14 19:42       ` Felipe Contreras
2020-12-15  1:27   ` Jeff King
2020-12-15 19:10     ` Jonathan Tan
2020-12-16  2:07   ` [PATCH v2 0/3] Cloning with remote unborn HEAD Jonathan Tan
2020-12-16  2:07     ` [PATCH v2 1/3] ls-refs: report unborn targets of symrefs Jonathan Tan
2020-12-16  6:16       ` Junio C Hamano
2020-12-16 23:49         ` Jonathan Tan
2020-12-16 18:23       ` Jeff King
2020-12-16 23:54         ` Jonathan Tan
2020-12-17  1:32           ` Junio C Hamano
2020-12-18  6:16             ` Jeff King
2020-12-16  2:07     ` [PATCH v2 2/3] connect, transport: add no-op arg for future patch Jonathan Tan
2020-12-16  6:20       ` Junio C Hamano
2020-12-16  2:07     ` [PATCH v2 3/3] clone: respect remote unborn HEAD Jonathan Tan
2020-12-21 22:30   ` [PATCH v3 0/3] Cloning with " Jonathan Tan
2020-12-21 22:30     ` [PATCH v3 1/3] ls-refs: report unborn targets of symrefs Jonathan Tan
2020-12-21 22:31     ` [PATCH v3 2/3] connect, transport: add no-op arg for future patch Jonathan Tan
2020-12-21 22:31     ` [PATCH v3 3/3] clone: respect remote unborn HEAD Jonathan Tan
2020-12-21 23:48     ` [PATCH v3 0/3] Cloning with " Junio C Hamano
2021-01-21 20:14     ` Jeff King
2020-12-22 21:54   ` [PATCH v4 " Jonathan Tan
2020-12-22 21:54     ` [PATCH v4 1/3] ls-refs: report unborn targets of symrefs Jonathan Tan
2021-01-21 20:48       ` Jeff King
2021-01-26 18:13         ` Jonathan Tan
2021-01-26 23:16           ` Jeff King
2020-12-22 21:54     ` [PATCH v4 2/3] connect, transport: add no-op arg for future patch Jonathan Tan
2021-01-21 20:55       ` Jeff King
2021-01-26 18:16         ` Jonathan Tan
2020-12-22 21:54     ` [PATCH v4 3/3] clone: respect remote unborn HEAD Jonathan Tan
2021-01-21 21:02       ` Jeff King
2021-01-26 18:22         ` Jonathan Tan
2021-01-26 23:04           ` Jeff King
2021-01-28  5:50             ` Junio C Hamano
2020-12-22 22:06     ` [PATCH v4 0/3] Cloning with " Junio C Hamano
2021-01-26 18:55 ` [PATCH v5 " Jonathan Tan
2021-01-26 18:55   ` [PATCH v5 1/3] ls-refs: report unborn targets of symrefs Jonathan Tan
2021-01-26 21:38     ` Junio C Hamano
2021-01-26 23:03       ` Junio C Hamano
2021-01-30  3:55         ` Jonathan Tan
2021-01-26 23:20       ` Jeff King
2021-01-26 23:38         ` Junio C Hamano
2021-01-29 20:23       ` Jonathan Tan
2021-01-29 22:04         ` Junio C Hamano
2021-02-02  2:20           ` Jonathan Tan
2021-02-02  5:00             ` Junio C Hamano
2021-01-27  1:28     ` Ævar Arnfjörð Bjarmason
2021-01-30  4:04       ` Jonathan Tan
2021-01-26 18:55   ` [PATCH v5 2/3] connect, transport: encapsulate arg in struct Jonathan Tan
2021-01-26 21:54     ` Junio C Hamano
2021-01-30  4:06       ` Jonathan Tan
2021-01-26 18:55   ` [PATCH v5 3/3] clone: respect remote unborn HEAD Jonathan Tan
2021-01-26 22:24     ` Junio C Hamano
2021-01-30  4:27       ` Jonathan Tan
2021-01-27  1:11   ` [PATCH v5 0/3] Cloning with " Junio C Hamano
2021-01-27  4:25     ` Jeff King
2021-01-27  6:14       ` Junio C Hamano
2021-01-27  1:41   ` Ævar Arnfjörð Bjarmason
2021-01-30  4:41     ` Jonathan Tan
2021-01-30 11:13       ` Ævar Arnfjörð Bjarmason
2021-02-02  2:22       ` Jonathan Tan
2021-02-03 14:23         ` Ævar Arnfjörð Bjarmason
2021-02-05 22:28     ` Junio C Hamano
2021-02-02  2:14 ` [PATCH v6 " Jonathan Tan
2021-02-02  2:14   ` [PATCH v6 1/3] ls-refs: report unborn targets of symrefs Jonathan Tan
2021-02-02 16:55     ` Junio C Hamano
2021-02-02 18:34       ` Jonathan Tan
2021-02-02 22:17         ` Junio C Hamano
2021-02-03  1:04           ` Jonathan Tan
2021-02-02  2:15   ` [PATCH v6 2/3] connect, transport: encapsulate arg in struct Jonathan Tan
2021-02-02  2:15   ` [PATCH v6 3/3] clone: respect remote unborn HEAD Jonathan Tan
2021-02-05  4:58 ` [PATCH v7 0/3] Cloning with " Jonathan Tan
2021-02-05  4:58   ` [PATCH v7 1/3] ls-refs: report unborn targets of symrefs Jonathan Tan
2021-02-05 16:10     ` Jeff King
2021-02-05  4:58   ` [PATCH v7 2/3] connect, transport: encapsulate arg in struct Jonathan Tan
2021-02-05  4:58   ` [PATCH v7 3/3] clone: respect remote unborn HEAD Jonathan Tan
2021-02-05  5:25   ` [PATCH v7 0/3] Cloning with " Junio C Hamano
2021-02-05 16:15     ` Jeff King
2021-02-05 21:15     ` Ævar Arnfjörð Bjarmason
2021-02-05 23:07       ` Junio C Hamano
2021-02-05 20:48 ` [PATCH v8 " Jonathan Tan
2021-02-05 20:48   ` [PATCH v8 1/3] ls-refs: report unborn targets of symrefs Jonathan Tan
2021-02-05 20:48   ` [PATCH v8 2/3] connect, transport: encapsulate arg in struct Jonathan Tan
2021-02-05 20:48   ` [PATCH v8 3/3] clone: respect remote unborn HEAD Jonathan Tan
2021-02-06 18:51   ` [PATCH v8 0/3] Cloning with " Junio C Hamano
2021-02-08 22:28     ` Junio C Hamano

Reply instructions:

You may reply publicly 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

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

  git send-email \
    --in-reply-to=X9pUc2HXUr3+WHbR@coredump.intra.peff.net \
    --to=peff@peff.net \
    --cc=avarab@gmail.com \
    --cc=felipe.contreras@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=jonathantanmy@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
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).