What should "git fetch origin +next" should do?

What should "git fetch origin +next" should do?

[Junio C Hamano]

As some might know, I use the traditional non-separate-remotes layout in
many of my working trees. I am old fashioned.

I just tried to update one of them with "git pull --ff-only", and after
seeing that the fetch phase failed with non-ff on 'next', ran

	$ git fetch origin +next

which happily copied the tip of updated next to FETCH_HEAD and nowhere
else. Of course, a colon-less refspec means do not store it anywhere,
i.e. "<colon-less-refspec>" === "<colon-less refspec>:", so prefixing it
with '+' to force would logically be a no-op.  But it nevertheless was
somewhat surprising and irritating.

This is one of the many things that is so minor that it probably is not
worth risking backward compatibility issues to change, but something that
we would design differently if we were starting from scratch. Maybe in Git
2.0.

The question however is what should it do. I can see three possibilities:

 (1) Forcing to fetch into FETCH_HEAD does not make any sense, so instead
     of silently ignoring the '+' prefix, error it out and do not fetch
     anything. This is easy to explain and logically makes more sense than
     the current behaviour.

 (2) Do notice '+' and understand that it is a request to force fetch into
     some ref locally, and from the lack of colon and RHS, assume that the
     user wants Git to infer the RHS using the configured refspec (in my
     case, "refs/heads/next:refs/heads/next" is one of the configured
     fetch refspec; "refs/heads/*:refs/remotes/origin/*" would be the one
     that would match in the separate-remotes layout). In other words,
     treat it as if the user typed "+refs/heads/next:refs/heads/next" (or
     "+refs/heads/next:refs/remotes/origin/next" in the separate-remote
     layout) from the command line.

 (3) Do notice '+' is applied to 'next' but otherwise ignore the fact that
     it is a command line pathspec, which would cause us to ignore
     configured refspecs. In other words, fetch normally as if there were
     no refspec on the command line, but when updating refs/heads/next (or
     refs/remotes/origin/next in the separate-remotes layout), allow non
     fast-forward updates.

The latter two are hard to explain and more error prone. I tend to think
both would be a mistake. If "+next" from the command line is modified to
update our remote tracking branch using the configured refspec, "next"
without '+' prefix also should. Otherwise the behaviour becomes too
inconsistent, but it is often convenient to fetch one-off even from a
configured remote using "git fetch origin next" into FETCH_HEAD without
updating our remote tracking branches, and change along the lines of
either (2) or (3) will forbid such a workflow.

Additionally (3) has very little advantage over "git fetch --force".  If
'next' and 'master' both do not fast-forward, and the configured fetch
refspecs require both to fast-forward, "git fetch --force" would update
both, and "git fetch +next" with interpretation (3) would error out by
noticing that 'master' does not fast-forward. But anybody who is doing
"git fetch +next" already knows that 'next' does not fast-forward, and
that is because he has tried regular "git fetch" and saw both 'next' and
'master' fail back then, so he either wants to update only 'next', in
which case interpretation in (3) would not be helpful, or wants to update
both, in which case --force would be a better choice and exists already.

Perhaps we can/want to implement (1)?

Re: What should "git fetch origin +next" should do?

[Marc Branchaud]

On 11-10-16 03:20 AM, Junio C Hamano wrote:
> As some might know, I use the traditional non-separate-remotes layout in
> many of my working trees. I am old fashioned.

Being hip and modern :) I use separate remote refspecs.  As I read your post,
I kept thinking that it makes no sense for fetch to ever update local refs
and that you're a victim of your stodgy old ways.

> I just tried to update one of them with "git pull --ff-only", and after
> seeing that the fetch phase failed with non-ff on 'next', ran
> 
> 	$ git fetch origin +next
> 
> which happily copied the tip of updated next to FETCH_HEAD and nowhere
> else. Of course, a colon-less refspec means do not store it anywhere,
> i.e. "<colon-less-refspec>" === "<colon-less refspec>:", so prefixing it
> with '+' to force would logically be a no-op.  But it nevertheless was
> somewhat surprising and irritating.
> 
> This is one of the many things that is so minor that it probably is not
> worth risking backward compatibility issues to change, but something that
> we would design differently if we were starting from scratch. Maybe in Git
> 2.0.
> 
> The question however is what should it do. I can see three possibilities:
> 
>  (1) Forcing to fetch into FETCH_HEAD does not make any sense, so instead
>      of silently ignoring the '+' prefix, error it out and do not fetch
>      anything. This is easy to explain and logically makes more sense than
>      the current behaviour.

I think this makes the most sense.

I'd even go so far as to make fetch error out if there's a colon in the
refspec.  Fetch has no business updating local refs.  There are other
commands for that, and which command you use depends on how you want your
local ref updated.  I think it would be a mistake to start going down a path
where fetch learns different ways to update a local ref.  Madness!

		M.

Re: What should "git fetch origin +next" should do?

[Ramkumar Ramachandra]

Hi Junio,

Junio C Hamano writes:
> I just tried to update one of them with "git pull --ff-only", and after
> seeing that the fetch phase failed with non-ff on 'next', ran
>
>        $ git fetch origin +next
>
> which happily copied the tip of updated next to FETCH_HEAD and nowhere
> else. Of course, a colon-less refspec means do not store it anywhere,
> i.e. "<colon-less-refspec>" === "<colon-less refspec>:", so prefixing it
> with '+' to force would logically be a no-op.  But it nevertheless was
> somewhat surprising and irritating.

Interesting: I wouldn't have expected this behavior either.  I'll see
if I can add something useful to this.

> As some might know, I use the traditional non-separate-remotes layout in
> many of my working trees. I am old fashioned.

As an interesting aside, I recently stopped using the word 'origin' to
name a remote since I started using multiple remotes: your remote is
called 'junio', mine's called 'ram', Jonathan's is called 'jrn', and
so on.  I personally use a variation of `git fetch junio master:master
next:next +pu:pu`.  It "fails" when:
1. Some uncommitted work is left:  I'm a bit messy with multiple
worktrees (git-new-workdir).
2. I'm doing some work directly on top of `master` or some other
upstream branch, and haven't forked out yet (I only fork out and name
the branch if the volume of work justifies it).
3. Sometimes `next` is non-ff, and I'm curious about what happened.  I
inspect the changes before invariably using a `git reset --hard
junio/next` to throw away the useless commits.

>  (2) Do notice '+' and understand that it is a request to force fetch into
>     some ref locally, and from the lack of colon and RHS, assume that the
>     user wants Git to infer the RHS using the configured refspec (in my
>     case, "refs/heads/next:refs/heads/next" is one of the configured
>     fetch refspec; "refs/heads/*:refs/remotes/origin/*" would be the one
>     that would match in the separate-remotes layout). In other words,
>     treat it as if the user typed "+refs/heads/next:refs/heads/next" (or
>     "+refs/heads/next:refs/remotes/origin/next" in the separate-remote
>     layout) from the command line.

Ugh, no.  Such smartness is probably desirable at the `pull` level.

>  (3) Do notice '+' is applied to 'next' but otherwise ignore the fact that
>     it is a command line pathspec, which would cause us to ignore
>     configured refspecs. In other words, fetch normally as if there were
>     no refspec on the command line, but when updating refs/heads/next (or
>     refs/remotes/origin/next in the separate-remotes layout), allow non
>     fast-forward updates.

This is unnecessarily complicated and ugly imho.  I think `git fetch`
is trying to be over-smart here: If I don't choose to update my local
refs by hand immediately after the fetch, I'll be surprised later.

> Perhaps we can/want to implement (1)?

Yeah, I think it's the right thing to do.  For the implementation,
should we update the condition in fetch.c:451 or try to implement it
at the refspec-parsing level?

Thanks.

-- Ram

Re: What should "git fetch origin +next" should do?

[Ramkumar Ramachandra]

Hi Marc,

Marc Branchaud writes:
> Being hip and modern :) I use separate remote refspecs.  As I read your post,
> I kept thinking that it makes no sense for fetch to ever update local refs
> and that you're a victim of your stodgy old ways.

Hm, I like the symmetry with the `git push` UI.  I wouldn't want to
checkout every non-ff upstream branch and merge by hand before
rebasing my work on top of it!

-- Ram

Re: What should "git fetch origin +next" should do?

[Junio C Hamano]

Marc Branchaud <marcnarc@xiplink.com> writes:

> On 11-10-16 03:20 AM, Junio C Hamano wrote:
>> As some might know, I use the traditional non-separate-remotes layout in
>> many of my working trees. I am old fashioned.
>
> Being hip and modern :) I use separate remote refspecs.  As I read your post,
> I kept thinking that it makes no sense for fetch to ever update local refs
> and that you're a victim of your stodgy old ways.

Imagine a scenario where I run the same "git fetch origin +next" in a
repository with separate remotes layout, expecting that the remote
tracking branch refs/remotes/origin/next to be force updated. The fetched
value will be stored only in FETCH_HEAD, and I would feel exactly the same
minor irritation.

In other words, the issue does not have anything to do with the layout.
My mention of layout variants was only to clarify that the ref to be force
updated on the local side is determined by the suggested behaviours (2)
and (3) based on the fetch refspec (i.e. refs/heads/next in the
traditional layout, refs/remotes/origin/next in the separate remotes
layout).

This is a tangent but we have seen in the past some new people wonder why
their configured remote tracking refs are not updated when they do

	$ git fetch origin next

This is a variant without '+', and in such a case, in addition to list the
fetched tip in FETCH_HEAD, it might be more natural for the user to expect
that the usual remote tracking branch update to happen.  And I suspect
that the suggested semantics (2) might better match what the users expect
in general.

That is, when fetching from a remote that has configured fetch refspecs,
colon-less refspecs are given from the command line, are first matched
against the configured fetch refspecs for the remote, and used to update
the remote tracking branches. IOW, with the separate remote layout fetch
refspec, the above command line is re-written to

	$ git fetch origin refs/heads/next:refs/remotes/origin/next

and leaves the fetched tip of 'next' in FETCH_HEAD and updates the remote
tracking branch; nothing else is fetched.


And if we apply the '+' prefix in this context, as the natural
consequence, my original example would be rewritten to:

	$ git fetch origin +refs/heads/next:refs/remotes/origin/next

in a repository with the separate remote layout fetch refspec, and in a
repository with the non-separate remote layout, it would be rewritten to:

	$ git fetch origin +refs/heads/next:refs/heads/next

Historically, we never used configured fetch refspecs when refspecs are
given on the command line, so such a change definitely breaks backward
compatibility, but possibly in a good way, and might deserve consideration
for Git 2.0.

Re: What should "git fetch origin +next" should do?

[Jeff King]

On Sun, Oct 16, 2011 at 12:20:02AM -0700, Junio C Hamano wrote:

> I just tried to update one of them with "git pull --ff-only", and after
> seeing that the fetch phase failed with non-ff on 'next', ran
> 
> 	$ git fetch origin +next
> 
> which happily copied the tip of updated next to FETCH_HEAD and nowhere
> else. Of course, a colon-less refspec means do not store it anywhere,
> i.e. "<colon-less-refspec>" === "<colon-less refspec>:", so prefixing it
> with '+' to force would logically be a no-op.  But it nevertheless was
> somewhat surprising and irritating.

I don't see that this has anything to do with the "+" at all. If I said:

  $ git fetch origin next

I think the exact same confusion exists. I told git to update 'next'
from origin, but it didn't touch refs/remotes/origin/next. You and I
both know what is happening, because we understand that "next" is a
refspec, and that it has no RHS.  But I suspect that is not how many git
users think of it.

We've discussed this before, of course:

  http://thread.gmane.org/gmane.comp.version-control.git/127163/focus=127215

-Peff

Re: What should "git fetch origin +next" should do?

[Junio C Hamano]

Jeff King <peff@peff.net> writes:

> I think the exact same confusion exists. I told git to update 'next'
> from origin, but it didn't touch refs/remotes/origin/next.

Except that you didn't tell git to *update* the remote tracking branch for
'next'; you merely told it to fetch 'next' at the remote.

> ...  But I suspect that is not how many git users think of it.

I am inclined to agree that it might be the case; see my other message in
this thread.

> We've discussed this before, of course:
>
>   http://thread.gmane.org/gmane.comp.version-control.git/127163/focus=127215

Yes, you brought up the "remote state as of the time I told git to record
it is a precious piece of information" issue, and I share the reasoning,
hence I am somewhat torn.

We might be better off biting the bullet and do the "rewrite a command
line colon-less refspec using a matching configured refspec iff exists"
and defer the history of remote tracking branches to its reflog in the
longer term.

Re: What should "git fetch origin +next" should do?

[Marc Branchaud]

On 11-10-17 02:34 PM, Junio C Hamano wrote:
> Jeff King <peff@peff.net> writes:
> 
>> I think the exact same confusion exists. I told git to update 'next'
>> from origin, but it didn't touch refs/remotes/origin/next.
> 
> Except that you didn't tell git to *update* the remote tracking branch for
> 'next'; you merely told it to fetch 'next' at the remote.
> 
>> ...  But I suspect that is not how many git users think of it.
> 
> I am inclined to agree that it might be the case; see my other message in
> this thread.

Indeed.  Apologies for missing that subtlety.

So now I think option (2) is the best choice.  To support one-off fetches,
teach fetch to accept "foo:" refspecs as "fetch ref foo from the remote and
only update FETCH_HEAD" -- maybe allow "foo:FETCH_HEAD" too, for folks who
like to be explicit and can't remember the shorthand syntax.

The rest of this post explains my reasoning, which I think pretty much just
rehashes what Junio said more efficiently in his initial message.

Overall I'd expect "git fetch origin next" to be a subset of "git fetch
origin".  That is, since the default fetch refspec is
	+refs/heads/*:refs/remotes/origin/*
normally "git fetch origin" gets all of origin's updated refs (ff or not) and
puts them under the local remotes/origin/ space.  So I would expect "git
fetch origin next" to only fetch the "next" ref from origin and update the
local remotes/origin/next ref.

Given the default fetch refspec, I'd expect "git fetch origin +next" to do
the exact same thing.  The + on the command line is basically redundant.

But removing the + from the fetch refspec changes things.  Now I'd expect
plain "git fetch origin" to fail if there are any non-ff updates, and "git
fetch origin next" should also fail if origin's next ref is non-ff.  But "git
fetch origin +next" would succeed.

In all cases if the command-line refspec has no RHS then git should try to
figure out which local ref to update from the config, and it should die if it
can't figure out a local ref to create or update.  (As I said above, maybe
allow "git fetch origin foo:" to let the user put the tip of origin's foo ref
into FETCH_HEAD.)

All this gets a bit more complicated if the user has currently checked out
the a ref that should be updated (regardless of the presence of a LHS +).
But really only old-style git gurus should run in to that problem.

		M.

Re: What should "git fetch origin +next" should do?

[Junio C Hamano]

Marc Branchaud <marcnarc@xiplink.com> writes:

> In all cases if the command-line refspec has no RHS then git should try to
> figure out which local ref to update from the config, and it should die if it
> can't figure out a local ref to create or update.  (As I said above, maybe
> allow "git fetch origin foo:" to let the user put the tip of origin's foo ref
> into FETCH_HEAD.)

I'd agree with everything you said in your message, except for the above
"it should die" part.

You are assuming that the user knows what his configured refspecs would
normally do and that is the whole reason why "git fetch origin next" that
would update the remote tracking branch specified for the upstream's next
might feel more natural than the current behaviour. I too think that is a
reasonable assumption.

With the same assumption, if you said "git fetch origin frotz" when you
know you are not usually tracking the remote 'frotz' branch, the command
just should store what is fetched in FETCH_HEAD (and nowhere else) without
dying. I do not think how it helps the user to die in that situation.

> All this gets a bit more complicated if the user has currently checked out
> the a ref that should be updated (regardless of the presence of a LHS +).

That "do not update the currently checked out branch" already exists and
is correctly handled by "git fetch", I think.

Re: What should "git fetch origin +next" should do?

[Marc Branchaud]

On 11-10-19 12:31 AM, Junio C Hamano wrote:
> Marc Branchaud <marcnarc@xiplink.com> writes:
> 
>> In all cases if the command-line refspec has no RHS then git should try to
>> figure out which local ref to update from the config, and it should die if it
>> can't figure out a local ref to create or update.  (As I said above, maybe
>> allow "git fetch origin foo:" to let the user put the tip of origin's foo ref
>> into FETCH_HEAD.)
> 
> I'd agree with everything you said in your message, except for the above
> "it should die" part.
> 
> You are assuming that the user knows what his configured refspecs would
> normally do and that is the whole reason why "git fetch origin next" that
> would update the remote tracking branch specified for the upstream's next
> might feel more natural than the current behaviour. I too think that is a
> reasonable assumption.
> 
> With the same assumption, if you said "git fetch origin frotz" when you
> know you are not usually tracking the remote 'frotz' branch, the command
> just should store what is fetched in FETCH_HEAD (and nowhere else) without
> dying. I do not think how it helps the user to die in that situation.

Sounds reasonable to me.

In all cases, I'd expect fetch to tell me what it did.

>> All this gets a bit more complicated if the user has currently checked out
>> the a ref that should be updated (regardless of the presence of a LHS +).
> 
> That "do not update the currently checked out branch" already exists and
> is correctly handled by "git fetch", I think.

Sweet!

(I'd also be happy if fetch updated the ref, and left the checked-out HEAD
detached, with attendant messages.  But then I'm quite comfortable working
with detached HEADs.)

		M.