threads / discuss / 28691

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

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

## tl;dr

10 messages between Oct 16, 2011 and Oct 19, 2011.

replies: 9people: 4as markdown or json

Junio C Hamano· Oct 16, 2011, 07:20 UTC · lore

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)?
Marc Branchaud· Oct 17, 2011, 14:35 UTC · re: Junio C Hamano · lore

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

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.

Show 22 quoted lines
> 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.
Ramkumar Ramachandra· Oct 17, 2011, 16:15 UTC · re: Marc Branchaud · lore

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

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
Junio C Hamano· Oct 17, 2011, 17:09 UTC · re: Marc Branchaud · lore

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

Marc Branchaud <marcnarc@xiplink.com> writes:
Show 7 quoted lines
> 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.

Ramkumar Ramachandra· Oct 17, 2011, 16:10 UTC · re: Junio C Hamano · lore

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

Hi Junio,
Junio C Hamano writes:
Show 10 quoted lines
> 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.
Show 9 quoted lines
>  (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.
Show 6 quoted lines
>  (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
Jeff King· Oct 17, 2011, 17:10 UTC · re: Junio C Hamano · lore

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

On Sun, Oct 16, 2011 at 12:20:02AM -0700, Junio C Hamano wrote:
Show 10 quoted lines
> 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
Junio C Hamano· Oct 17, 2011, 18:34 UTC · re: Jeff King · lore

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

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.

Marc Branchaud· Oct 17, 2011, 22:02 UTC · re: Junio C Hamano · lore

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

On 11-10-17 02:34 PM, Junio C Hamano wrote:
Show 12 quoted lines
> 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.
Junio C Hamano· Oct 19, 2011, 04:31 UTC · re: Marc Branchaud · lore

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

Marc Branchaud <marcnarc@xiplink.com> writes:
Show 5 quoted lines
> 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.

Marc Branchaud· Oct 19, 2011, 13:45 UTC · re: Junio C Hamano · lore

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

On 11-10-19 12:31 AM, Junio C Hamano wrote:
Show 21 quoted lines
> 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.
Show 5 quoted lines
>> 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.

← back to recent threads