{"thread":{"id":"28691","subject":"What should \"git fetch origin +next\" should do?","startedAt":"2011-10-16T07:20:02Z","lastAt":"2011-10-19T13:45:34Z","messageCount":10,"participants":["Junio C Hamano","Marc Branchaud","Ramkumar Ramachandra","Jeff King"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"177763","messageId":"7v7h45s8rh.fsf@alter.siamese.dyndns.org","threadId":"28691","inReplyTo":null,"subject":"What should \"git fetch origin +next\" should do?","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-10-16T07:20:02Z","receivedAt":"2011-10-16T07:20:02Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"As some might know, I use the traditional non-separate-remotes layout in\nmany of my working trees. I am old fashioned.\n\nI just tried to update one of them with \"git pull --ff-only\", and after\nseeing that the fetch phase failed with non-ff on 'next', ran\n\n\t$ git fetch origin +next\n\nwhich happily copied the tip of updated next to FETCH_HEAD and nowhere\nelse. Of course, a colon-less refspec means do not store it anywhere,\ni.e. \"<colon-less-refspec>\" === \"<colon-less refspec>:\", so prefixing it\nwith '+' to force would logically be a no-op.  But it nevertheless was\nsomewhat surprising and irritating.\n\nThis is one of the many things that is so minor that it probably is not\nworth risking backward compatibility issues to change, but something that\nwe would design differently if we were starting from scratch. Maybe in Git\n2.0.\n\nThe question however is what should it do. I can see three possibilities:\n\n (1) Forcing to fetch into FETCH_HEAD does not make any sense, so instead\n     of silently ignoring the '+' prefix, error it out and do not fetch\n     anything. This is easy to explain and logically makes more sense than\n     the current behaviour.\n\n (2) Do notice '+' and understand that it is a request to force fetch into\n     some ref locally, and from the lack of colon and RHS, assume that the\n     user wants Git to infer the RHS using the configured refspec (in my\n     case, \"refs/heads/next:refs/heads/next\" is one of the configured\n     fetch refspec; \"refs/heads/*:refs/remotes/origin/*\" would be the one\n     that would match in the separate-remotes layout). In other words,\n     treat it as if the user typed \"+refs/heads/next:refs/heads/next\" (or\n     \"+refs/heads/next:refs/remotes/origin/next\" in the separate-remote\n     layout) from the command line.\n\n (3) Do notice '+' is applied to 'next' but otherwise ignore the fact that\n     it is a command line pathspec, which would cause us to ignore\n     configured refspecs. In other words, fetch normally as if there were\n     no refspec on the command line, but when updating refs/heads/next (or\n     refs/remotes/origin/next in the separate-remotes layout), allow non\n     fast-forward updates.\n\nThe latter two are hard to explain and more error prone. I tend to think\nboth would be a mistake. If \"+next\" from the command line is modified to\nupdate our remote tracking branch using the configured refspec, \"next\"\nwithout '+' prefix also should. Otherwise the behaviour becomes too\ninconsistent, but it is often convenient to fetch one-off even from a\nconfigured remote using \"git fetch origin next\" into FETCH_HEAD without\nupdating our remote tracking branches, and change along the lines of\neither (2) or (3) will forbid such a workflow.\n\nAdditionally (3) has very little advantage over \"git fetch --force\".  If\n'next' and 'master' both do not fast-forward, and the configured fetch\nrefspecs require both to fast-forward, \"git fetch --force\" would update\nboth, and \"git fetch +next\" with interpretation (3) would error out by\nnoticing that 'master' does not fast-forward. But anybody who is doing\n\"git fetch +next\" already knows that 'next' does not fast-forward, and\nthat is because he has tried regular \"git fetch\" and saw both 'next' and\n'master' fail back then, so he either wants to update only 'next', in\nwhich case interpretation in (3) would not be helpful, or wants to update\nboth, in which case --force would be a better choice and exists already.\n\nPerhaps we can/want to implement (1)?\n"},{"id":"177861","messageId":"4E9C3D27.3060504@xiplink.com","threadId":"28691","inReplyTo":"7v7h45s8rh.fsf@alter.siamese.dyndns.org","subject":"Re: What should \"git fetch origin +next\" should do?","fromName":"Marc Branchaud","fromEmail":"marcnarc@xiplink.com","sentAt":"2011-10-17T14:35:19Z","receivedAt":"2011-10-17T14:35:19Z","isPatch":false,"sender":{"key":"marcnarc@xiplink.com","avatar":"https://avatars.githubusercontent.com/u/14980203?v=4"},"body":"On 11-10-16 03:20 AM, Junio C Hamano wrote:\n> As some might know, I use the traditional non-separate-remotes layout in\n> many of my working trees. I am old fashioned.\n\nBeing hip and modern :) I use separate remote refspecs.  As I read your post,\nI kept thinking that it makes no sense for fetch to ever update local refs\nand that you're a victim of your stodgy old ways.\n\n> I just tried to update one of them with \"git pull --ff-only\", and after\n> seeing that the fetch phase failed with non-ff on 'next', ran\n> \n> \t$ git fetch origin +next\n> \n> which happily copied the tip of updated next to FETCH_HEAD and nowhere\n> else. Of course, a colon-less refspec means do not store it anywhere,\n> i.e. \"<colon-less-refspec>\" === \"<colon-less refspec>:\", so prefixing it\n> with '+' to force would logically be a no-op.  But it nevertheless was\n> somewhat surprising and irritating.\n> \n> This is one of the many things that is so minor that it probably is not\n> worth risking backward compatibility issues to change, but something that\n> we would design differently if we were starting from scratch. Maybe in Git\n> 2.0.\n> \n> The question however is what should it do. I can see three possibilities:\n> \n>  (1) Forcing to fetch into FETCH_HEAD does not make any sense, so instead\n>      of silently ignoring the '+' prefix, error it out and do not fetch\n>      anything. This is easy to explain and logically makes more sense than\n>      the current behaviour.\n\nI think this makes the most sense.\n\nI'd even go so far as to make fetch error out if there's a colon in the\nrefspec.  Fetch has no business updating local refs.  There are other\ncommands for that, and which command you use depends on how you want your\nlocal ref updated.  I think it would be a mistake to start going down a path\nwhere fetch learns different ways to update a local ref.  Madness!\n\n\t\tM.\n"},{"id":"177862","messageId":"CALkWK0kXyovFrJO+5AD3gOs1YuutBZouH8aESXSKA-1nX3T6rw@mail.gmail.com","threadId":"28691","inReplyTo":"7v7h45s8rh.fsf@alter.siamese.dyndns.org","subject":"Re: What should \"git fetch origin +next\" should do?","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2011-10-17T16:10:56Z","receivedAt":"2011-10-17T16:10:56Z","isPatch":false,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"Hi Junio,\n\nJunio C Hamano writes:\n> I just tried to update one of them with \"git pull --ff-only\", and after\n> seeing that the fetch phase failed with non-ff on 'next', ran\n>\n>        $ git fetch origin +next\n>\n> which happily copied the tip of updated next to FETCH_HEAD and nowhere\n> else. Of course, a colon-less refspec means do not store it anywhere,\n> i.e. \"<colon-less-refspec>\" === \"<colon-less refspec>:\", so prefixing it\n> with '+' to force would logically be a no-op.  But it nevertheless was\n> somewhat surprising and irritating.\n\nInteresting: I wouldn't have expected this behavior either.  I'll see\nif I can add something useful to this.\n\n> As some might know, I use the traditional non-separate-remotes layout in\n> many of my working trees. I am old fashioned.\n\nAs an interesting aside, I recently stopped using the word 'origin' to\nname a remote since I started using multiple remotes: your remote is\ncalled 'junio', mine's called 'ram', Jonathan's is called 'jrn', and\nso on.  I personally use a variation of `git fetch junio master:master\nnext:next +pu:pu`.  It \"fails\" when:\n1. Some uncommitted work is left:  I'm a bit messy with multiple\nworktrees (git-new-workdir).\n2. I'm doing some work directly on top of `master` or some other\nupstream branch, and haven't forked out yet (I only fork out and name\nthe branch if the volume of work justifies it).\n3. Sometimes `next` is non-ff, and I'm curious about what happened.  I\ninspect the changes before invariably using a `git reset --hard\njunio/next` to throw away the useless commits.\n\n>  (2) Do notice '+' and understand that it is a request to force fetch into\n>     some ref locally, and from the lack of colon and RHS, assume that the\n>     user wants Git to infer the RHS using the configured refspec (in my\n>     case, \"refs/heads/next:refs/heads/next\" is one of the configured\n>     fetch refspec; \"refs/heads/*:refs/remotes/origin/*\" would be the one\n>     that would match in the separate-remotes layout). In other words,\n>     treat it as if the user typed \"+refs/heads/next:refs/heads/next\" (or\n>     \"+refs/heads/next:refs/remotes/origin/next\" in the separate-remote\n>     layout) from the command line.\n\nUgh, no.  Such smartness is probably desirable at the `pull` level.\n\n>  (3) Do notice '+' is applied to 'next' but otherwise ignore the fact that\n>     it is a command line pathspec, which would cause us to ignore\n>     configured refspecs. In other words, fetch normally as if there were\n>     no refspec on the command line, but when updating refs/heads/next (or\n>     refs/remotes/origin/next in the separate-remotes layout), allow non\n>     fast-forward updates.\n\nThis is unnecessarily complicated and ugly imho.  I think `git fetch`\nis trying to be over-smart here: If I don't choose to update my local\nrefs by hand immediately after the fetch, I'll be surprised later.\n\n> Perhaps we can/want to implement (1)?\n\nYeah, I think it's the right thing to do.  For the implementation,\nshould we update the condition in fetch.c:451 or try to implement it\nat the refspec-parsing level?\n\nThanks.\n\n-- Ram\n"},{"id":"177863","messageId":"CALkWK0ke03kPwFommtVUB5iidgqzaqqjBgAot_4F=LjxcAogsA@mail.gmail.com","threadId":"28691","inReplyTo":"4E9C3D27.3060504@xiplink.com","subject":"Re: What should \"git fetch origin +next\" should do?","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2011-10-17T16:15:16Z","receivedAt":"2011-10-17T16:15:16Z","isPatch":false,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"Hi Marc,\n\nMarc Branchaud writes:\n> Being hip and modern :) I use separate remote refspecs.  As I read your post,\n> I kept thinking that it makes no sense for fetch to ever update local refs\n> and that you're a victim of your stodgy old ways.\n\nHm, I like the symmetry with the `git push` UI.  I wouldn't want to\ncheckout every non-ff upstream branch and merge by hand before\nrebasing my work on top of it!\n\n-- Ram\n"},{"id":"177865","messageId":"7v8vojo88p.fsf@alter.siamese.dyndns.org","threadId":"28691","inReplyTo":"4E9C3D27.3060504@xiplink.com","subject":"Re: What should \"git fetch origin +next\" should do?","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-10-17T17:09:26Z","receivedAt":"2011-10-17T17:09:26Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Marc Branchaud <marcnarc@xiplink.com> writes:\n\n> On 11-10-16 03:20 AM, Junio C Hamano wrote:\n>> As some might know, I use the traditional non-separate-remotes layout in\n>> many of my working trees. I am old fashioned.\n>\n> Being hip and modern :) I use separate remote refspecs.  As I read your post,\n> I kept thinking that it makes no sense for fetch to ever update local refs\n> and that you're a victim of your stodgy old ways.\n\nImagine a scenario where I run the same \"git fetch origin +next\" in a\nrepository with separate remotes layout, expecting that the remote\ntracking branch refs/remotes/origin/next to be force updated. The fetched\nvalue will be stored only in FETCH_HEAD, and I would feel exactly the same\nminor irritation.\n\nIn other words, the issue does not have anything to do with the layout.\nMy mention of layout variants was only to clarify that the ref to be force\nupdated on the local side is determined by the suggested behaviours (2)\nand (3) based on the fetch refspec (i.e. refs/heads/next in the\ntraditional layout, refs/remotes/origin/next in the separate remotes\nlayout).\n\nThis is a tangent but we have seen in the past some new people wonder why\ntheir configured remote tracking refs are not updated when they do\n\n\t$ git fetch origin next\n\nThis is a variant without '+', and in such a case, in addition to list the\nfetched tip in FETCH_HEAD, it might be more natural for the user to expect\nthat the usual remote tracking branch update to happen.  And I suspect\nthat the suggested semantics (2) might better match what the users expect\nin general.\n\nThat is, when fetching from a remote that has configured fetch refspecs,\ncolon-less refspecs are given from the command line, are first matched\nagainst the configured fetch refspecs for the remote, and used to update\nthe remote tracking branches. IOW, with the separate remote layout fetch\nrefspec, the above command line is re-written to\n\n\t$ git fetch origin refs/heads/next:refs/remotes/origin/next\n\nand leaves the fetched tip of 'next' in FETCH_HEAD and updates the remote\ntracking branch; nothing else is fetched.\n\n\nAnd if we apply the '+' prefix in this context, as the natural\nconsequence, my original example would be rewritten to:\n\n\t$ git fetch origin +refs/heads/next:refs/remotes/origin/next\n\nin a repository with the separate remote layout fetch refspec, and in a\nrepository with the non-separate remote layout, it would be rewritten to:\n\n\t$ git fetch origin +refs/heads/next:refs/heads/next\n\nHistorically, we never used configured fetch refspecs when refspecs are\ngiven on the command line, so such a change definitely breaks backward\ncompatibility, but possibly in a good way, and might deserve consideration\nfor Git 2.0.\n"},{"id":"177866","messageId":"20111017171041.GA12837@sigill.intra.peff.net","threadId":"28691","inReplyTo":"7v7h45s8rh.fsf@alter.siamese.dyndns.org","subject":"Re: What should \"git fetch origin +next\" should do?","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2011-10-17T17:10:41Z","receivedAt":"2011-10-17T17:10:41Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sun, Oct 16, 2011 at 12:20:02AM -0700, Junio C Hamano wrote:\n\n> I just tried to update one of them with \"git pull --ff-only\", and after\n> seeing that the fetch phase failed with non-ff on 'next', ran\n> \n> \t$ git fetch origin +next\n> \n> which happily copied the tip of updated next to FETCH_HEAD and nowhere\n> else. Of course, a colon-less refspec means do not store it anywhere,\n> i.e. \"<colon-less-refspec>\" === \"<colon-less refspec>:\", so prefixing it\n> with '+' to force would logically be a no-op.  But it nevertheless was\n> somewhat surprising and irritating.\n\nI don't see that this has anything to do with the \"+\" at all. If I said:\n\n  $ git fetch origin next\n\nI think the exact same confusion exists. I told git to update 'next'\nfrom origin, but it didn't touch refs/remotes/origin/next. You and I\nboth know what is happening, because we understand that \"next\" is a\nrefspec, and that it has no RHS.  But I suspect that is not how many git\nusers think of it.\n\nWe've discussed this before, of course:\n\n  http://thread.gmane.org/gmane.comp.version-control.git/127163/focus=127215\n\n-Peff\n"},{"id":"177870","messageId":"7vipnnmppx.fsf@alter.siamese.dyndns.org","threadId":"28691","inReplyTo":"20111017171041.GA12837@sigill.intra.peff.net","subject":"Re: What should \"git fetch origin +next\" should do?","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-10-17T18:34:50Z","receivedAt":"2011-10-17T18:34:50Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n> I think the exact same confusion exists. I told git to update 'next'\n> from origin, but it didn't touch refs/remotes/origin/next.\n\nExcept that you didn't tell git to *update* the remote tracking branch for\n'next'; you merely told it to fetch 'next' at the remote.\n\n> ...  But I suspect that is not how many git users think of it.\n\nI am inclined to agree that it might be the case; see my other message in\nthis thread.\n\n> We've discussed this before, of course:\n>\n>   http://thread.gmane.org/gmane.comp.version-control.git/127163/focus=127215\n\nYes, you brought up the \"remote state as of the time I told git to record\nit is a precious piece of information\" issue, and I share the reasoning,\nhence I am somewhat torn.\n\nWe might be better off biting the bullet and do the \"rewrite a command\nline colon-less refspec using a matching configured refspec iff exists\"\nand defer the history of remote tracking branches to its reflog in the\nlonger term.\n"},{"id":"177895","messageId":"4E9CA5E2.2020701@xiplink.com","threadId":"28691","inReplyTo":"7vipnnmppx.fsf@alter.siamese.dyndns.org","subject":"Re: What should \"git fetch origin +next\" should do?","fromName":"Marc Branchaud","fromEmail":"marcnarc@xiplink.com","sentAt":"2011-10-17T22:02:10Z","receivedAt":"2011-10-17T22:02:10Z","isPatch":false,"sender":{"key":"marcnarc@xiplink.com","avatar":"https://avatars.githubusercontent.com/u/14980203?v=4"},"body":"On 11-10-17 02:34 PM, Junio C Hamano wrote:\n> Jeff King <peff@peff.net> writes:\n> \n>> I think the exact same confusion exists. I told git to update 'next'\n>> from origin, but it didn't touch refs/remotes/origin/next.\n> \n> Except that you didn't tell git to *update* the remote tracking branch for\n> 'next'; you merely told it to fetch 'next' at the remote.\n> \n>> ...  But I suspect that is not how many git users think of it.\n> \n> I am inclined to agree that it might be the case; see my other message in\n> this thread.\n\nIndeed.  Apologies for missing that subtlety.\n\nSo now I think option (2) is the best choice.  To support one-off fetches,\nteach fetch to accept \"foo:\" refspecs as \"fetch ref foo from the remote and\nonly update FETCH_HEAD\" -- maybe allow \"foo:FETCH_HEAD\" too, for folks who\nlike to be explicit and can't remember the shorthand syntax.\n\nThe rest of this post explains my reasoning, which I think pretty much just\nrehashes what Junio said more efficiently in his initial message.\n\nOverall I'd expect \"git fetch origin next\" to be a subset of \"git fetch\norigin\".  That is, since the default fetch refspec is\n\t+refs/heads/*:refs/remotes/origin/*\nnormally \"git fetch origin\" gets all of origin's updated refs (ff or not) and\nputs them under the local remotes/origin/ space.  So I would expect \"git\nfetch origin next\" to only fetch the \"next\" ref from origin and update the\nlocal remotes/origin/next ref.\n\nGiven the default fetch refspec, I'd expect \"git fetch origin +next\" to do\nthe exact same thing.  The + on the command line is basically redundant.\n\nBut removing the + from the fetch refspec changes things.  Now I'd expect\nplain \"git fetch origin\" to fail if there are any non-ff updates, and \"git\nfetch origin next\" should also fail if origin's next ref is non-ff.  But \"git\nfetch origin +next\" would succeed.\n\nIn all cases if the command-line refspec has no RHS then git should try to\nfigure out which local ref to update from the config, and it should die if it\ncan't figure out a local ref to create or update.  (As I said above, maybe\nallow \"git fetch origin foo:\" to let the user put the tip of origin's foo ref\ninto FETCH_HEAD.)\n\nAll this gets a bit more complicated if the user has currently checked out\nthe a ref that should be updated (regardless of the presence of a LHS +).\nBut really only old-style git gurus should run in to that problem.\n\n\t\tM.\n"},{"id":"177979","messageId":"7v8vohfvqs.fsf@alter.siamese.dyndns.org","threadId":"28691","inReplyTo":"4E9CA5E2.2020701@xiplink.com","subject":"Re: What should \"git fetch origin +next\" should do?","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-10-19T04:31:07Z","receivedAt":"2011-10-19T04:31:07Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Marc Branchaud <marcnarc@xiplink.com> writes:\n\n> In all cases if the command-line refspec has no RHS then git should try to\n> figure out which local ref to update from the config, and it should die if it\n> can't figure out a local ref to create or update.  (As I said above, maybe\n> allow \"git fetch origin foo:\" to let the user put the tip of origin's foo ref\n> into FETCH_HEAD.)\n\nI'd agree with everything you said in your message, except for the above\n\"it should die\" part.\n\nYou are assuming that the user knows what his configured refspecs would\nnormally do and that is the whole reason why \"git fetch origin next\" that\nwould update the remote tracking branch specified for the upstream's next\nmight feel more natural than the current behaviour. I too think that is a\nreasonable assumption.\n\nWith the same assumption, if you said \"git fetch origin frotz\" when you\nknow you are not usually tracking the remote 'frotz' branch, the command\njust should store what is fetched in FETCH_HEAD (and nowhere else) without\ndying. I do not think how it helps the user to die in that situation.\n\n> All this gets a bit more complicated if the user has currently checked out\n> the a ref that should be updated (regardless of the presence of a LHS +).\n\nThat \"do not update the currently checked out branch\" already exists and\nis correctly handled by \"git fetch\", I think.\n"},{"id":"177994","messageId":"4E9ED47E.5050408@xiplink.com","threadId":"28691","inReplyTo":"7v8vohfvqs.fsf@alter.siamese.dyndns.org","subject":"Re: What should \"git fetch origin +next\" should do?","fromName":"Marc Branchaud","fromEmail":"marcnarc@xiplink.com","sentAt":"2011-10-19T13:45:34Z","receivedAt":"2011-10-19T13:45:34Z","isPatch":false,"sender":{"key":"marcnarc@xiplink.com","avatar":"https://avatars.githubusercontent.com/u/14980203?v=4"},"body":"On 11-10-19 12:31 AM, Junio C Hamano wrote:\n> Marc Branchaud <marcnarc@xiplink.com> writes:\n> \n>> In all cases if the command-line refspec has no RHS then git should try to\n>> figure out which local ref to update from the config, and it should die if it\n>> can't figure out a local ref to create or update.  (As I said above, maybe\n>> allow \"git fetch origin foo:\" to let the user put the tip of origin's foo ref\n>> into FETCH_HEAD.)\n> \n> I'd agree with everything you said in your message, except for the above\n> \"it should die\" part.\n> \n> You are assuming that the user knows what his configured refspecs would\n> normally do and that is the whole reason why \"git fetch origin next\" that\n> would update the remote tracking branch specified for the upstream's next\n> might feel more natural than the current behaviour. I too think that is a\n> reasonable assumption.\n> \n> With the same assumption, if you said \"git fetch origin frotz\" when you\n> know you are not usually tracking the remote 'frotz' branch, the command\n> just should store what is fetched in FETCH_HEAD (and nowhere else) without\n> dying. I do not think how it helps the user to die in that situation.\n\nSounds reasonable to me.\n\nIn all cases, I'd expect fetch to tell me what it did.\n\n>> All this gets a bit more complicated if the user has currently checked out\n>> the a ref that should be updated (regardless of the presence of a LHS +).\n> \n> That \"do not update the currently checked out branch\" already exists and\n> is correctly handled by \"git fetch\", I think.\n\nSweet!\n\n(I'd also be happy if fetch updated the ref, and left the checked-out HEAD\ndetached, with attendant messages.  But then I'm quite comfortable working\nwith detached HEADs.)\n\n\t\tM.\n"}]}