{"thread":{"id":"21299","subject":"[PATCH] pull: refuse complete src:dst fetchspec arguments","startedAt":"2009-10-20T18:23:06Z","lastAt":"2009-12-29T16:58:43Z","messageCount":21,"participants":["Thomas Rast","Wesley J. Landaker","Sean Estabrooks","Junio C Hamano","Daniel Barkalow","Björn Steinbrink","Jeff King","Nanako Shiraishi"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"125503","messageId":"d561e70f0aa802ceb96eba16d3bb2316134d69c8.1256062808.git.trast@student.ethz.ch","threadId":"21299","inReplyTo":null,"subject":"[PATCH] pull: refuse complete src:dst fetchspec arguments","fromName":"Thomas Rast","fromEmail":"trast@student.ethz.ch","sentAt":"2009-10-20T18:23:06Z","receivedAt":"2009-10-20T18:23:06Z","isPatch":true,"sender":{"key":"tr@thomasrast.ch","avatar":"https://avatars.githubusercontent.com/u/153510?v=4"},"body":"git-pull has historically accepted full fetchspecs, meaning that you\ncould do\n\n git pull $repo A:B\n\nwhich would simultaneously fetch the remote branch A into the local\nbranch B and merge B into HEAD. This got especially confusing if B\nwas checked out. New users variously mistook pull for fetch or read\nthat command as \"merge the remote A into my B\", neither of which is\ncorrect.\n\nSince the above usage should be very rare and can be done with\nseparate calls to fetch and merge, we just disallow full fetchspecs in\ngit-pull.\n\nSigned-off-by: Thomas Rast \n---\n\nThis actually came up on IRC *twice* this week.\n\n\n git-pull.sh | 19 +++++++++++++++++++\n t/t5520-pull.sh | 12 ------------\n 2 files changed, 19 insertions(+), 12 deletions(-)\n\ndiff --git a/git-pull.sh b/git-pull.sh\nindex fc78592..8f06491 100755\n--- a/git-pull.sh\n+++ b/git-pull.sh\n@@ -131,6 +131,25 @@ error_on_no_merge_candidates () {\n \texit 1\n }\n \n+check_full_fetchspec () {\n+\tshift\t# discard remote argument, if any\n+\tfor arg in \"$@\"\n+\tdo\n+\t\tcase \"$arg\" in\n+\t\t*:*)\n+\t\t\techo \"$arg\"\n+\t\t\treturn\n+\t\t\t;;\n+\t\tesac\n+\tdone\n+}\n+\n+full_fetchspec=$(check_full_fetchspec \"$@\")\n+if test -n \"$full_fetchspec\"\n+then\n+\tdie \"full fetchspec '$full_fetchspec' not allowed\"\n+fi\n+\n test true = \"$rebase\" && {\n \tif ! git rev-parse -q --verify HEAD >/dev/null\n \tthen\ndiff --git a/t/t5520-pull.sh b/t/t5520-pull.sh\nindex dd2ee84..a566a99 100755\n--- a/t/t5520-pull.sh\n+++ b/t/t5520-pull.sh\n@@ -29,18 +29,6 @@ test_expect_success 'checking the results' '\n \tdiff file cloned/file\n '\n \n-test_expect_success 'pulling into void using master:master' '\n-\tmkdir cloned-uho &&\n-\t(\n-\t\tcd cloned-uho &&\n-\t\tgit init &&\n-\t\tgit pull .. master:master\n-\t) &&\n-\ttest -f file &&\n-\ttest -f cloned-uho/file &&\n-\ttest_cmp file cloned-uho/file\n-'\n-\n test_expect_success 'test . as a remote' '\n \n \tgit branch copy master &&\n-- \n1.6.5.1.144.g40216\n"},{"id":"125504","messageId":"200910202037.09140.trast@student.ethz.ch","threadId":"21299","inReplyTo":"d561e70f0aa802ceb96eba16d3bb2316134d69c8.1256062808.git.trast@student.ethz.ch","subject":"Re: [RFC! PATCH] pull: refuse complete src:dst fetchspec arguments","fromName":"Thomas Rast","fromEmail":"trast@student.ethz.ch","sentAt":"2009-10-20T18:37:08Z","receivedAt":"2009-10-20T18:37:08Z","isPatch":true,"sender":{"key":"tr@thomasrast.ch","avatar":"https://avatars.githubusercontent.com/u/153510?v=4"},"body":"Thomas Rast wrote:\n> git-pull has historically accepted full fetchspecs, meaning that you\n> could do\n> \n> git pull $repo A:B\n> \n> which would simultaneously fetch the remote branch A into the local\n> branch B and merge B into HEAD. This got especially confusing if B\n> was checked out. New users variously mistook pull for fetch or read\n> that command as \"merge the remote A into my B\", neither of which is\n> correct.\n> \n> Since the above usage should be very rare and can be done with\n> separate calls to fetch and merge, we just disallow full fetchspecs in\n> git-pull.\n> \n> Signed-off-by: Thomas Rast \n\nArgh. This was actually supposed to be an *RFC* patch.\n\n-- \nThomas Rast\ntrast@{inf,student}.ethz.ch\n"},{"id":"125510","messageId":"200910201329.16359.wjl@icecavern.net","threadId":"21299","inReplyTo":"d561e70f0aa802ceb96eba16d3bb2316134d69c8.1256062808.git.trast@student.ethz.ch","subject":"Re: [PATCH] pull: refuse complete src:dst fetchspec arguments","fromName":"Wesley J. Landaker","fromEmail":"wjl@icecavern.net","sentAt":"2009-10-20T19:29:15Z","receivedAt":"2009-10-20T19:29:15Z","isPatch":true,"sender":{"key":"wjl@icecavern.net","avatar":"https://avatars.githubusercontent.com/u/67229?v=4"},"body":"On Tuesday 20 October 2009 12:23:06 Thomas Rast wrote:\n> git-pull has historically accepted full fetchspecs, meaning that you\n> could do\n> \n> git pull $repo A:B\n> \n> which would simultaneously fetch the remote branch A into the local\n> branch B and merge B into HEAD. This got especially confusing if B\n> was checked out. New users variously mistook pull for fetch or read\n> that command as \"merge the remote A into my B\", neither of which is\n> correct.\n\nOne thought here is that if the change you suggested (and I personally like) \nin your \"[RFC] pull/fetch rename\" thread was made, then I would expect to be \nable to run this exact command to have git fetch the remote branch A into \nthe local branch B (with no merging taking place, because I didn't say --\nmerge). So basically, it would be like \"git fetch $repo A:B\" is now.\n\nI readily agree that the *current* behavior of that command would have \nprobably caught me off-guard, since I probably only would have typed that on \naccident (e.g. using \"pull\" when I meant \"fetch\").\n"},{"id":"125520","messageId":"BLU0-SMTP97AA2287062D9A104101C8AEC00@phx.gbl","threadId":"21299","inReplyTo":"d561e70f0aa802ceb96eba16d3bb2316134d69c8.1256062808.git.trast@student.ethz.ch","subject":"Re: [PATCH] pull: refuse complete src:dst fetchspec arguments","fromName":"Sean Estabrooks","fromEmail":"seanlkml@sympatico.ca","sentAt":"2009-10-20T20:30:53Z","receivedAt":"2009-10-20T20:30:53Z","isPatch":true,"sender":{"key":"seanlkml@sympatico.ca","avatar":"https://gravatar.com/avatar/f92923f54fc08c401fc59b71829d4b89e9b8087fbba45ff87c82e6a83aee02ae?d=mp&s=160"},"body":"On Tue, 20 Oct 2009 20:23:06 +0200\nThomas Rast wrote:\n\nHi Thomas,\n\n> git-pull has historically accepted full fetchspecs, meaning that you\n> could do\n> \n> git pull $repo A:B\n> \n> which would simultaneously fetch the remote branch A into the local\n> branch B and merge B into HEAD. This got especially confusing if B\n> was checked out. New users variously mistook pull for fetch or read\n> that command as \"merge the remote A into my B\", neither of which is\n> correct.\n> \n> Since the above usage should be very rare and can be done with\n> separate calls to fetch and merge, we just disallow full fetchspecs in\n> git-pull.\n\nIt is however a handy shortcut to be able to specify the full refspec\nand specify where you want the head stored locally. It seems a shame to\nthrow away that functionality because of one confusing case. Wouldn't\nit be better to test of the confusing case and instead error out if the\nlocal refname is already checked out?\n\n\n[...]\n> diff --git a/t/t5520-pull.sh b/t/t5520-pull.sh\n> index dd2ee84..a566a99 100755\n> --- a/t/t5520-pull.sh\n> +++ b/t/t5520-pull.sh\n> @@ -29,18 +29,6 @@ test_expect_success 'checking the results' '\n> \tdiff file cloned/file\n> '\n> \n> -test_expect_success 'pulling into void using master:master' '\n> -\tmkdir cloned-uho &&\n> -\t(\n> -\t\tcd cloned-uho &&\n> -\t\tgit init &&\n> -\t\tgit pull .. master:master\n> -\t) &&\n> -\ttest -f file &&\n> -\ttest -f cloned-uho/file &&\n> -\ttest_cmp file cloned-uho/file\n> -\n> -\n> test_expect_success 'test . as a remote' '\n> \n> \tgit branch copy master &&\n> -- \n> \n\nInstead of removing this test it should be modified or replaced\nwith a test that ensures the new functionality operates correctly.\nIn this case that would mean checking that using a full refspec\nerrors out.\n\nCheers,\nSean\n"},{"id":"125530","messageId":"7v1vkxn53p.fsf@alter.siamese.dyndns.org","threadId":"21299","inReplyTo":"BLU0-SMTP97AA2287062D9A104101C8AEC00@phx.gbl","subject":"Re: [PATCH] pull: refuse complete src:dst fetchspec arguments","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-10-20T21:11:06Z","receivedAt":"2009-10-20T21:11:06Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Sean Estabrooks writes:\n\n>> -test_expect_success 'pulling into void using master:master' '\n>> -\tmkdir cloned-uho &&\n>> -\t(\n>> -\t\tcd cloned-uho &&\n>> -\t\tgit init &&\n>> -\t\tgit pull .. master:master\n>> -\t) &&\n>> -\ttest -f file &&\n>> -\ttest -f cloned-uho/file &&\n>> -\ttest_cmp file cloned-uho/file\n>> -\n>> -\n>> test_expect_success 'test . as a remote' '\n>> \n>> \tgit branch copy master &&\n>> -- \n>> \n>\n> Instead of removing this test it should be modified or replaced\n> with a test that ensures the new functionality operates correctly.\n> In this case that would mean checking that using a full refspec\n> errors out.\n\nShouldn't \"git pull .. master\" still work in this case, too? So this test\nwill probably become two tests, one for \"git pull .. master:master\" that\ncorrectly fails, and the other for \"git pull .. master\" to still work as\nexpected.\n"},{"id":"125553","messageId":"alpine.LNX.2.00.0910202001050.14365@iabervon.org","threadId":"21299","inReplyTo":"BLU0-SMTP97AA2287062D9A104101C8AEC00@phx.gbl","subject":"Re: [PATCH] pull: refuse complete src:dst fetchspec arguments","fromName":"Daniel Barkalow","fromEmail":"barkalow@iabervon.org","sentAt":"2009-10-21T00:15:23Z","receivedAt":"2009-10-21T00:15:23Z","isPatch":true,"sender":{"key":"barkalow@iabervon.org","avatar":"https://avatars.githubusercontent.com/u/55364219?v=4"},"body":"On Tue, 20 Oct 2009, Sean Estabrooks wrote:\n\n> On Tue, 20 Oct 2009 20:23:06 +0200\n> Thomas Rast wrote:\n> \n> Hi Thomas,\n> \n> > git-pull has historically accepted full fetchspecs, meaning that you\n> > could do\n> > \n> > git pull $repo A:B\n> > \n> > which would simultaneously fetch the remote branch A into the local\n> > branch B and merge B into HEAD. This got especially confusing if B\n> > was checked out. New users variously mistook pull for fetch or read\n> > that command as \"merge the remote A into my B\", neither of which is\n> > correct.\n> > \n> > Since the above usage should be very rare and can be done with\n> > separate calls to fetch and merge, we just disallow full fetchspecs in\n> > git-pull.\n> \n> It is however a handy shortcut to be able to specify the full refspec\n> and specify where you want the head stored locally. It seems a shame to\n> throw away that functionality because of one confusing case. Wouldn't\n> it be better to test of the confusing case and instead error out if the\n> local refname is already checked out?\n\nSurely, \"where you want the head stored locally\" is somewhere that's \ninformation about a remote repository, and therefore under \"refs/remotes/\" \n(or \"refs/tags/\" or something) and therefore not possible to be checked \nout (in the \"HEAD is a symref to it\" sense).\n\nI don't think it should be possible to fast-forward or create a local \nbranch from a remote branch while simultaneously merging it into the \ncurrently-checked-out local branch.\n\nActually, I think it would be good to prohibit fetching into a new or \nexisting local branch, whether or not it is checked out. We'd probably \nneed to provide a plumbing method of doing a fetch, though, for script \nenvironments that aren't using the normal porcelain meanings of refs/ \nsubdirectories. (Defining a bare repo with --mirror as not having local \nbranches, of course)\n\n\t-Daniel\n*This .sig left intentionally blank*\n"},{"id":"125555","messageId":"BLU0-SMTP889B2109047E949E039EFDAEBF0@phx.gbl","threadId":"21299","inReplyTo":"alpine.LNX.2.00.0910202001050.14365@iabervon.org","subject":"Re: [PATCH] pull: refuse complete src:dst fetchspec arguments","fromName":"Sean Estabrooks","fromEmail":"seanlkml@sympatico.ca","sentAt":"2009-10-21T00:29:52Z","receivedAt":"2009-10-21T00:29:52Z","isPatch":true,"sender":{"key":"seanlkml@sympatico.ca","avatar":"https://gravatar.com/avatar/f92923f54fc08c401fc59b71829d4b89e9b8087fbba45ff87c82e6a83aee02ae?d=mp&s=160"},"body":"On Tue, 20 Oct 2009 20:15:23 -0400 (EDT)\nDaniel Barkalow wrote:\n\nHi Daniel,\n\n> Surely, \"where you want the head stored locally\" is somewhere that's \n> information about a remote repository, and therefore under \"refs/remotes/\" \n> (or \"refs/tags/\" or something) and therefore not possible to be checked \n> out (in the \"HEAD is a symref to it\" sense).\n\nMaybe, but it could also just be to create a temp local branch for\nmerging into additional branches afterward with \"checkout other;\nmerge temp\". This is especially helpful when pulling from an\nannoyingly long URL instead of from a configured remote.\n \n> I don't think it should be possible to fast-forward or create a local \n> branch from a remote branch while simultaneously merging it into the \n> currently-checked-out local branch.\n\nWhat is the harm? Nobody is forced to use the facility and it does\nhave some marginal utility. I'd not fight for it, but i don't yet\nunderstand the argument to prohibit it.\n\n> Actually, I think it would be good to prohibit fetching into a new or \n> existing local branch, whether or not it is checked out. We'd probably \n> need to provide a plumbing method of doing a fetch, though, for script \n> environments that aren't using the normal porcelain meanings of refs/ \n> subdirectories. (Defining a bare repo with --mirror as not having local \n> branches, of course)\n\nI'm hoping you don't mean that all fetching to a new local branch should\nbe prohibited and you're only talking about the current issue of full\nrefspecs on and the pull command. Otherwise i'd say it seems\nunnecessarily restrictive.\n\nCheers,\nSean\n"},{"id":"125556","messageId":"alpine.LNX.2.00.0910202044150.14365@iabervon.org","threadId":"21299","inReplyTo":"BLU0-SMTP889B2109047E949E039EFDAEBF0@phx.gbl","subject":"Re: [PATCH] pull: refuse complete src:dst fetchspec arguments","fromName":"Daniel Barkalow","fromEmail":"barkalow@iabervon.org","sentAt":"2009-10-21T00:55:25Z","receivedAt":"2009-10-21T00:55:25Z","isPatch":true,"sender":{"key":"barkalow@iabervon.org","avatar":"https://avatars.githubusercontent.com/u/55364219?v=4"},"body":"On Tue, 20 Oct 2009, Sean Estabrooks wrote:\n\n> On Tue, 20 Oct 2009 20:15:23 -0400 (EDT)\n> Daniel Barkalow wrote:\n> \n> Hi Daniel,\n> \n> > Surely, \"where you want the head stored locally\" is somewhere that's \n> > information about a remote repository, and therefore under \"refs/remotes/\" \n> > (or \"refs/tags/\" or something) and therefore not possible to be checked \n> > out (in the \"HEAD is a symref to it\" sense).\n> \n> Maybe, but it could also just be to create a temp local branch for\n> merging into additional branches afterward with \"checkout other;\n> merge temp\". This is especially helpful when pulling from an\n> annoyingly long URL instead of from a configured remote.\n\nMaybe it should be fine to do:\n\n$ git fetch long-url-here master:temp\n$ git merge temp\n$ git checkout other-branch-that-also-needs-it\n$ git merge temp\n\nBut \"temp\" is \"refs/remotes/temp\", not \"refs/heads/temp\"?\n\n> > Actually, I think it would be good to prohibit fetching into a new or \n> > existing local branch, whether or not it is checked out. We'd probably \n> > need to provide a plumbing method of doing a fetch, though, for script \n> > environments that aren't using the normal porcelain meanings of refs/ \n> > subdirectories. (Defining a bare repo with --mirror as not having local \n> > branches, of course)\n> \n> I'm hoping you don't mean that all fetching to a new local branch should\n> be prohibited and you're only talking about the current issue of full\n> refspecs on and the pull command. Otherwise i'd say it seems\n> unnecessarily restrictive.\n\nI think, actually, that creating or changing a local branch is really not \nwhat \"fetch\" (or the fetch part of pull) is about. I think that just leads \nto confusion about what's locally-controlled and what's a local memory of \nsomething remotely-controlled.\n\n\t-Daniel\n*This .sig left intentionally blank*\n"},{"id":"125558","messageId":"BLU0-SMTP37B36AD1D1000A723B9EBDAEBF0@phx.gbl","threadId":"21299","inReplyTo":"alpine.LNX.2.00.0910202044150.14365@iabervon.org","subject":"Re: [PATCH] pull: refuse complete src:dst fetchspec arguments","fromName":"Sean Estabrooks","fromEmail":"seanlkml@sympatico.ca","sentAt":"2009-10-21T01:35:42Z","receivedAt":"2009-10-21T01:35:42Z","isPatch":true,"sender":{"key":"seanlkml@sympatico.ca","avatar":"https://gravatar.com/avatar/f92923f54fc08c401fc59b71829d4b89e9b8087fbba45ff87c82e6a83aee02ae?d=mp&s=160"},"body":"On Tue, 20 Oct 2009 20:55:25 -0400 (EDT)\nDaniel Barkalow wrote:\n\n> > Maybe, but it could also just be to create a temp local branch for\n> > merging into additional branches afterward with \"checkout other;\n> > merge temp\". This is especially helpful when pulling from an\n> > annoyingly long URL instead of from a configured remote.\n> \n> Maybe it should be fine to do:\n> \n> $ git fetch long-url-here master:temp\n> $ git merge temp\n> $ git checkout other-branch-that-also-needs-it\n> $ git merge temp\n> \n> But \"temp\" is \"refs/remotes/temp\", not \"refs/heads/temp\"?\n\nWell that's only one example of possibile uses for fetching directly to\na local branch, perhaps as a new base of further development. Is there\nreally a compelling reason to force someone to fetch into refs/remotes\nand then do the extra step of checking it out locally?\n \n> I think, actually, that creating or changing a local branch is really not \n> what \"fetch\" (or the fetch part of pull) is about. I think that just leads \n> to confusion about what's locally-controlled and what's a local memory of \n> something remotely-controlled.\n\nWell it's a handy shortcut for several situations. There must be a way\nto protect less adroit Git users without removing functionality.\n\nSean\n \n"},{"id":"125562","messageId":"20091021031528.GB18997@atjola.homenet","threadId":"21299","inReplyTo":"alpine.LNX.2.00.0910202044150.14365@iabervon.org","subject":"Re: [PATCH] pull: refuse complete src:dst fetchspec arguments","fromName":"Björn Steinbrink","fromEmail":"b.steinbrink@gmx.de","sentAt":"2009-10-21T03:15:28Z","receivedAt":"2009-10-21T03:15:28Z","isPatch":true,"sender":{"key":"b.steinbrink@gmx.de","avatar":"https://avatars.githubusercontent.com/u/230962?v=4"},"body":"On 2009.10.20 20:55:25 -0400, Daniel Barkalow wrote:\n> Maybe it should be fine to do:\n> \n> $ git fetch long-url-here master:temp\n> $ git merge temp\n> $ git checkout other-branch-that-also-needs-it\n> $ git merge temp\n> \n> But \"temp\" is \"refs/remotes/temp\", not \"refs/heads/temp\"?\n\nOne (maybe important) difference there is that the \"pull\" gets you:\n\n Merge branch 'pu' of git://git.kernel.org/pub/scm/git/git\n\nEven with \"master:tmp\". But with fetch+merge (storing in refs/remotes):\n\n Merge remote branch 'tmp'\n\nAs a minor side-effect, having the \"tmp\" ref makes re-running the pull\n(for whatever reason) cheaper, as without it, the fetch step would\npossibly re-fetch the whole stuff (not reachable through any local ref).\n\nBjörn, undecided...\n"},{"id":"125564","messageId":"alpine.LNX.2.00.0910210023050.14365@iabervon.org","threadId":"21299","inReplyTo":"20091021031528.GB18997@atjola.homenet","subject":"Re: [PATCH] pull: refuse complete src:dst fetchspec arguments","fromName":"Daniel Barkalow","fromEmail":"barkalow@iabervon.org","sentAt":"2009-10-21T04:32:30Z","receivedAt":"2009-10-21T04:32:30Z","isPatch":true,"sender":{"key":"barkalow@iabervon.org","avatar":"https://avatars.githubusercontent.com/u/55364219?v=4"},"body":"On Wed, 21 Oct 2009, Björn Steinbrink wrote:\n\n> On 2009.10.20 20:55:25 -0400, Daniel Barkalow wrote:\n> > Maybe it should be fine to do:\n> > \n> > $ git fetch long-url-here master:temp\n> > $ git merge temp\n> > $ git checkout other-branch-that-also-needs-it\n> > $ git merge temp\n> > \n> > But \"temp\" is \"refs/remotes/temp\", not \"refs/heads/temp\"?\n> \n> One (maybe important) difference there is that the \"pull\" gets you:\n> \n> Merge branch 'pu' of git://git.kernel.org/pub/scm/git/git\n> \n> Even with \"master:tmp\". But with fetch+merge (storing in refs/remotes):\n> \n> Merge remote branch 'tmp'\n\nIt would be nice to improve that in general, I think. You may fetch before \nmerging in order to check out what you're getting, and then lose \nFETCH_HEAD (or have not specified the branch), and you have to contact the \nremote server again if you want the message with its url.\n\n> As a minor side-effect, having the \"tmp\" ref makes re-running the pull\n> (for whatever reason) cheaper, as without it, the fetch step would\n> possibly re-fetch the whole stuff (not reachable through any local ref).\n\nOnly if the merge failed, but yes.\n\n\t-Daniel\n*This .sig left intentionally blank*"},{"id":"125579","messageId":"200910211005.29053.trast@student.ethz.ch","threadId":"21299","inReplyTo":"20091021031528.GB18997@atjola.homenet","subject":"Re: [PATCH] pull: refuse complete src:dst fetchspec arguments","fromName":"Thomas Rast","fromEmail":"trast@student.ethz.ch","sentAt":"2009-10-21T08:05:27Z","receivedAt":"2009-10-21T08:05:27Z","isPatch":true,"sender":{"key":"tr@thomasrast.ch","avatar":"https://avatars.githubusercontent.com/u/153510?v=4"},"body":"Björn Steinbrink wrote:\n> One (maybe important) difference there is that the \"pull\" gets you:\n> \n> Merge branch 'pu' of git://git.kernel.org/pub/scm/git/git\n> \n> Even with \"master:tmp\". But with fetch+merge (storing in refs/remotes):\n> \n> Merge remote branch 'tmp'\n\nWhat if any combination of fetch and merge always gave you the long\nform? After all, even if you do have a tracking branch for whatever\nyou are merging, that information is probably useless and it would be\nnicer if all of the following resulted in the long form:\n\n* git fetch git://git.kernel.org/pub/scm/git/git pu\n git merge FETCH_HEAD\n\n* git remote add origin git://git.kernel.org/pub/scm/git/git\n git fetch origin\n git merge origin/pu\n\n* git fetch git://git.kernel.org/pub/scm/git/git pu:tmp\n git merge tmp\n\nand so on.\n\n-- \nThomas Rast\ntrast@{inf,student}.ethz.ch\n"},{"id":"125580","messageId":"200910211006.44398.trast@student.ethz.ch","threadId":"21299","inReplyTo":"BLU0-SMTP97AA2287062D9A104101C8AEC00@phx.gbl","subject":"Re: [PATCH] pull: refuse complete src:dst fetchspec arguments","fromName":"Thomas Rast","fromEmail":"trast@student.ethz.ch","sentAt":"2009-10-21T08:06:43Z","receivedAt":"2009-10-21T08:06:43Z","isPatch":true,"sender":{"key":"tr@thomasrast.ch","avatar":"https://avatars.githubusercontent.com/u/153510?v=4"},"body":"Sean Estabrooks wrote:\n> Instead of removing this test it should be modified or replaced\n> with a test that ensures the new functionality operates correctly.\n> In this case that would mean checking that using a full refspec\n> errors out.\n\nIndeed, sorry. I meant it to be flagged as an RFC patch, and with\nthose I usually go for the minimal possible effort to not break the\ntest suite outright. As such, it also lacks doc updates.\n\n-- \nThomas Rast\ntrast@{inf,student}.ethz.ch\n"},{"id":"125764","messageId":"20091023025434.GA29908@sigio.peff.net","threadId":"21299","inReplyTo":"200910211005.29053.trast@student.ethz.ch","subject":"Re: [PATCH] pull: refuse complete src:dst fetchspec arguments","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2009-10-23T02:54:36Z","receivedAt":"2009-10-23T02:54:36Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Oct 21, 2009 at 10:05:27AM +0200, Thomas Rast wrote:\n\n> What if any combination of fetch and merge always gave you the long\n> form? After all, even if you do have a tracking branch for whatever\n> you are merging, that information is probably useless and it would be\n> nicer if all of the following resulted in the long form:\n> \n> * git fetch git://git.kernel.org/pub/scm/git/git pu\n> git merge FETCH_HEAD\n> \n> * git remote add origin git://git.kernel.org/pub/scm/git/git\n> git fetch origin\n> git merge origin/pu\n> \n> * git fetch git://git.kernel.org/pub/scm/git/git pu:tmp\n> git merge tmp\n\nMaybe it's just me, but I actually prefer the shorthand names. Five\nyears from now when I browse the history and see that I merged\nremote branch \"mike/topic\", I'll know exactly what that means: developer\nMike's version of a certain topic branch. But I am not likely to care\nabout exactly where we were storing developer repos at that time.\n\nBut probably that is an artifact of the workflow. The scenario I am\ndescribing above implies a somewhat centralized workflow, where the\nshorthand contains all of the interesting information. In a totally\ndistributed, we-don't-share-anything-except-the-url-namespace setup of\nan open source repo, the full URL makes more sense.\n\nSo maybe it is something that should be optional.\n\n-Peff\n\n> \n> and so on.\n> \n> -- \n> Thomas Rast\n> trast@{inf,student}.ethz.ch\n> --\n> To unsubscribe from this list: send the line \"unsubscribe git\" in\n> the body of a message to majordomo@vger.kernel.org\n> More majordomo info at http://vger.kernel.org/majordomo-info.html\n"},{"id":"125765","messageId":"alpine.LNX.2.00.0910222334040.14365@iabervon.org","threadId":"21299","inReplyTo":"20091023025434.GA29908@sigio.peff.net","subject":"Re: [PATCH] pull: refuse complete src:dst fetchspec arguments","fromName":"Daniel Barkalow","fromEmail":"barkalow@iabervon.org","sentAt":"2009-10-23T03:43:05Z","receivedAt":"2009-10-23T03:43:05Z","isPatch":true,"sender":{"key":"barkalow@iabervon.org","avatar":"https://avatars.githubusercontent.com/u/55364219?v=4"},"body":"On Thu, 22 Oct 2009, Jeff King wrote:\n\n> On Wed, Oct 21, 2009 at 10:05:27AM +0200, Thomas Rast wrote:\n> \n> > What if any combination of fetch and merge always gave you the long\n> > form? After all, even if you do have a tracking branch for whatever\n> > you are merging, that information is probably useless and it would be\n> > nicer if all of the following resulted in the long form:\n> > \n> > * git fetch git://git.kernel.org/pub/scm/git/git pu\n> > git merge FETCH_HEAD\n> > \n> > * git remote add origin git://git.kernel.org/pub/scm/git/git\n> > git fetch origin\n> > git merge origin/pu\n> > \n> > * git fetch git://git.kernel.org/pub/scm/git/git pu:tmp\n> > git merge tmp\n> \n> Maybe it's just me, but I actually prefer the shorthand names. Five\n> years from now when I browse the history and see that I merged\n> remote branch \"mike/topic\", I'll know exactly what that means: developer\n> Mike's version of a certain topic branch. But I am not likely to care\n> about exactly where we were storing developer repos at that time.\n> \n> But probably that is an artifact of the workflow. The scenario I am\n> describing above implies a somewhat centralized workflow, where the\n> shorthand contains all of the interesting information. In a totally\n> distributed, we-don't-share-anything-except-the-url-namespace setup of\n> an open source repo, the full URL makes more sense.\n> \n> So maybe it is something that should be optional.\n\nSurely you ought to be able to get the short form with \"pull\", though, if \nyou happen to like short forms. So it would make sense to decide how to \nformat the merge message based entirely on an option, not at all on \nwhether you use pull or fetch+merge.\n\n\t-Daniel\n*This .sig left intentionally blank*\n"},{"id":"125820","messageId":"20091024004917.GA8012@sigio.peff.net","threadId":"21299","inReplyTo":"alpine.LNX.2.00.0910222334040.14365@iabervon.org","subject":"Re: [PATCH] pull: refuse complete src:dst fetchspec arguments","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2009-10-24T00:49:18Z","receivedAt":"2009-10-24T00:49:18Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Oct 22, 2009 at 11:43:05PM -0400, Daniel Barkalow wrote:\n\n> > But probably that is an artifact of the workflow. The scenario I am\n> > describing above implies a somewhat centralized workflow, where the\n> > shorthand contains all of the interesting information. In a totally\n> > distributed, we-don't-share-anything-except-the-url-namespace setup of\n> > an open source repo, the full URL makes more sense.\n> > \n> > So maybe it is something that should be optional.\n> \n> Surely you ought to be able to get the short form with \"pull\", though, if \n> you happen to like short forms. So it would make sense to decide how to \n> format the merge message based entirely on an option, not at all on \n> whether you use pull or fetch+merge.\n\nYeah, I think you are right. It _should_ be variable, but right now it\nvaries on something totally unrelated to what you want (how you invoked,\nand not what type of repo setup you are using). So I agree a patch to\nmake it more consistent across fetch+merge versus pull would be good,\nand then we can make a configuration option to choose one or the other.\n\n-Peff\n"},{"id":"125821","messageId":"7vbpjxa8ma.fsf@alter.siamese.dyndns.org","threadId":"21299","inReplyTo":"20091024004917.GA8012@sigio.peff.net","subject":"Re: [PATCH] pull: refuse complete src:dst fetchspec arguments","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-10-24T01:22:37Z","receivedAt":"2009-10-24T01:22:37Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> Yeah, I think you are right. It _should_ be variable, but right now it\n> varies on something totally unrelated to what you want (how you invoked,\n> and not what type of repo setup you are using). So I agree a patch to\n> make it more consistent across fetch+merge versus pull would be good,\n> and then we can make a configuration option to choose one or the other.\n\nI have been looong wondering why I somehow thught that I saw a patch that makes\n\n $ git merge origin/topic\n\npretend as if you did\n\n $ git pull origin topic\n\nwhen you have this mapping\n\n [remote \"origin\"]\n fetch = refs/heads/*:refs/remotes/origin/*\n\nin your configuration file, since it is a very obvious thing to do...\n"},{"id":"127606","messageId":"200911151324.05109.trast@student.ethz.ch","threadId":"21299","inReplyTo":"d561e70f0aa802ceb96eba16d3bb2316134d69c8.1256062808.git.trast@student.ethz.ch","subject":"Re: [PATCH] pull: refuse complete src:dst fetchspec arguments","fromName":"Thomas Rast","fromEmail":"trast@student.ethz.ch","sentAt":"2009-11-15T12:24:03Z","receivedAt":"2009-11-15T12:24:03Z","isPatch":true,"sender":{"key":"tr@thomasrast.ch","avatar":"https://avatars.githubusercontent.com/u/153510?v=4"},"body":"Thomas Rast wrote:\n> git-pull has historically accepted full fetchspecs, meaning that you\n> could do\n> \n> git pull $repo A:B\n> \n> which would simultaneously fetch the remote branch A into the local\n> branch B and merge B into HEAD. This got especially confusing if B\n> was checked out. New users variously mistook pull for fetch or read\n> that command as \"merge the remote A into my B\", neither of which is\n> correct.\n\nIt gets worse. *Much* worse.\n\nYesterday on IRC I helped 'thrope' with the github pull requests\nguide. This is a wiki page, but placed at a sufficiently prominent\nURL to make it look like an authoritative guide to a new user.\n\n http://github.com/guides/pull-requests\n\nI have since replaced the part in question with one that is more in\nline with what the tools actually do, but the bottom line of the old\nversion was basically\n\n # You got a request to pull git://github.com/defunkt/grit.git master\n\n # mojombo can add the defunkt repository as a remote source, create\n # a new branch, and pull the defunkt repository contents into it\n # like this:\n\n $ git remote add -f defunkt git://github.com/defunkt/grit.git\n $ git checkout -b defunkt/master # (1)\n $ git pull defunkt master:4f0ea0c # (2)\n # [...]\n $ git commit # (3)\n $ git checkout master\n $ git merge defunkt/master # (4)\n $ git push\n\nNote that all but the first line and the numbers is literally\ncut&pasted from the old version, which is still available at\n\n http://github.com/guides/pull-requests/24\n\nso you can see for yourself. Note that the lines (1) and (2) were\nthere even in version 3.\n\nAnd as you can see, there are just so many things wrong with it:\n\n(1) will actually create a new branch defunkt/master based on whatever\nyou happened to be on, making (4) merge something entirely different\nthan what the pull request was for.\n\n(2) will pull defunkt's master into a local *branch* called 4f0ea0c\n(in the guide this is actually the sha1 of defunkt's master, but who\nknows), and then merge that into the local defunkt/master branch from\n(1).\n\n(3) shouldn't do anything at that point, but hell if I know how he got\nthe idea to commit there.\n\nSo this suggests several safety measures:\n\n* Perhaps branch/checkout -b can refuse to create branches that\n already exist with this exact name under remotes if that's the only\n argument. I.e., in the above situation (1),\n\n # refuse: remotes/defunkt/master exists\n git checkout -b defunkt/master\n git branch defunkt/master\n\n # accept: obviously you're asking for trouble explicitly\n git checkout -b defunkt/master defunkt/master \n git branch defunkt/master defunkt/master\n\n* Perhaps all branch-creating code could refuse to create branches\n that have a name that is also a valid sha1 prefix of an existing\n object? This would be fairly drastic if a user's language has many\n words consisting only of [a-f], but on the other hand, the user can\n hardly be helped by having something that looks like a sha1 resolve\n to some *other* sha1.\n\n* I ask you to reconsider this patch. For some reason, people read\n things into pull with fetchspecs that are far from correct.\n\nI haven't thought much about backwards compatibility yet, though, so\nsome of it may not be possible.\n\n-- \nThomas Rast\ntrast@{inf,student}.ethz.ch\n"},{"id":"127631","messageId":"7vhbsvk07i.fsf@alter.siamese.dyndns.org","threadId":"21299","inReplyTo":"200911151324.05109.trast@student.ethz.ch","subject":"Re: [PATCH] pull: refuse complete src:dst fetchspec arguments","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-11-15T20:22:09Z","receivedAt":"2009-11-15T20:22:09Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Thomas Rast writes:\n\n> Yesterday on IRC I helped 'thrope' with the github pull requests\n> guide. This is a wiki page, but placed at a sufficiently prominent\n> URL to make it look like an authoritative guide to a new user.\n>\n> http://github.com/guides/pull-requests\n>\n> I have since replaced the part in question ...\n\nThanks.\n\nIt is hard to control the quality of random third-party documents, and\nsuch a help as yours is greatly appreciated.\n\nA document with gross misinformation is much worse than not having it.\n"},{"id":"130444","messageId":"20091229200513.6117@nanako3.lavabit.com","threadId":"21299","inReplyTo":"d561e70f0aa802ceb96eba16d3bb2316134d69c8.1256062808.git.trast@student.ethz.ch","subject":"Re: [PATCH] pull: refuse complete src:dst fetchspec arguments","fromName":"Nanako Shiraishi","fromEmail":"nanako3@lavabit.com","sentAt":"2009-12-29T11:05:13Z","receivedAt":"2009-12-29T11:05:13Z","isPatch":true,"sender":{"key":"nanako3@lavabit.com","avatar":"https://gravatar.com/avatar/3777b9e201c5883a62b1a6fdf7c53f2d712d1d80989146063ea861e33aad72a8?d=mp&s=160"},"body":"Junio, could you tell us what happened to this thread?\n\nThe patch rejects \"git pull repo A:B\" because it is almost always a mistake;\nI think it makes sense.\n"},{"id":"130470","messageId":"7vk4w5g1j0.fsf@alter.siamese.dyndns.org","threadId":"21299","inReplyTo":"20091229200513.6117@nanako3.lavabit.com","subject":"Re: [PATCH] pull: refuse complete src:dst fetchspec arguments","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-12-29T16:58:43Z","receivedAt":"2009-12-29T16:58:43Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Nanako Shiraishi writes:\n\n> Junio, could you tell us what happened to this thread?\n>\n> The patch rejects \"git pull repo A:B\" because it is almost always a mistake;\n> I think it makes sense.\n\nIt seems that we got sidetracked into a long thread on different (but\ninteresting) side topics. I think what the patch attempts to do is quite\nsane, and the implementation is very straightforward. Perhaps we should\nresurrect it, but with a proper \"declare deprecation now, first warn then\nrefuse in two releases\" steps. I.e. the actual refusal would happen in\n1.7.1 or later.\n\nOne side topic was Daniel wondering if we should restrict the value of B\nfor \"git fetch repo A:B\" so that \"fetch\" is not used to update the refs\noutside refs/remotes namespace. I personally think it is an unwarranted\nrestriction, and also it is more or less an unrelated issue anyway.\n\nAnother side topic that distracted us was about the difference between the\nautogenerated commit log messages for \"git pull origin topic\" and \"git\nfetch && git merge origin/topic\". I personally think it is very good that\nthe latter already says \"Merge remote branch 'origin/topic'\" and there is\nno need to turn that into \"Merge 'topic' of ...\" (actually I'd prefer it\nthe way it is).\n"}]}