{"thread":{"id":"39973","subject":"git pull --upload-pack reversion in git 2.5.0","startedAt":"2015-07-30T15:45:23Z","lastAt":"2015-07-31T17:54:16Z","messageCount":8,"participants":["Joey Hess","Matthieu Moy","Junio C Hamano","Paul Tan"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"267151","messageId":"20150730154523.GA17002@kitenet.net","threadId":"39973","inReplyTo":null,"subject":"git pull --upload-pack reversion in git 2.5.0","fromName":"Joey Hess","fromEmail":"id@joeyh.name","sentAt":"2015-07-30T15:45:23Z","receivedAt":"2015-07-30T15:45:23Z","isPatch":false,"sender":{"key":"id@joeyh.name","avatar":"https://avatars.githubusercontent.com/u/16392?v=4"},"body":"In git 2.1.4, I can run: git pull --upload-pack 'echo --foo'\n\nThis also seems to work in 2.4.6, but in 2.5.0, the option parser\ndoes something weird, apparently looking inside the quoted parameter\nand parsing parameters in there:\n\nerror: unknown option `foo'\nusage: git fetch [<options>] [<repository> [<refspec>...]]\n\nNeedless to say, this broke my use of --upload-pack.\n\n-- \nsee shy jo\n"},{"id":"267171","messageId":"vpqvbd1k1ke.fsf@anie.imag.fr","threadId":"39973","inReplyTo":"20150730154523.GA17002@kitenet.net","subject":"Re: git pull --upload-pack reversion in git 2.5.0","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2015-07-30T17:17:05Z","receivedAt":"2015-07-30T17:17:05Z","isPatch":false,"sender":{"key":"matthieu.moy@grenoble-inp.fr","avatar":"https://gravatar.com/avatar/72c8a2705971a25dfaff23cece15130d405685845d911aedd5667ace277f3fc5?d=mp&s=160"},"body":"Joey Hess <id@joeyh.name> writes:\n\n> In git 2.1.4, I can run: git pull --upload-pack 'echo --foo'\n>\n> This also seems to work in 2.4.6, but in 2.5.0, the option parser\n> does something weird, apparently looking inside the quoted parameter\n> and parsing parameters in there:\n>\n> error: unknown option `foo'\n> usage: git fetch [<options>] [<repository> [<refspec>...]]\n\nThis bisects down to:\n\ncommit e3b601da2af53cbb9a63e59113d524a8d946ea12 (junio/pt/pull-optparse)\nAuthor: Paul Tan <pyokagan@gmail.com>\nDate:   Tue Jun 2 22:22:53 2015 +0800\n\n    pull: use git-rev-parse --parseopt for option parsing\n    \n    To enable unambiguous parsing of abbreviated options, bundled short\n    options, separate form options and to provide consistent usage help, use\n    git-rev-parse --parseopt for option parsing. With this, simplify the\n    option parsing code.\n    \n    Signed-off-by: Paul Tan <pyokagan@gmail.com>\n    Signed-off-by: Junio C Hamano <gitster@pobox.com>\n\n=> Cc-ing Paul.\n\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"},{"id":"267174","messageId":"20150730183144.GA6360@kitenet.net","threadId":"39973","inReplyTo":"vpqvbd1k1ke.fsf@anie.imag.fr","subject":"Re: git pull --upload-pack reversion in git 2.5.0","fromName":"Joey Hess","fromEmail":"id@joeyh.name","sentAt":"2015-07-30T18:31:44Z","receivedAt":"2015-07-30T18:31:44Z","isPatch":false,"sender":{"key":"id@joeyh.name","avatar":"https://avatars.githubusercontent.com/u/16392?v=4"},"body":"I think this comes down to a lack of quoting where git-pull runs\ngit-fetch. Before eb2a8d9ed3fca2ba2f617b704992d483605f3bb6,\n\"$@\" was passed through to git-fetch, but now there is a $upload_pack\nwhich is passed without being quoted.\n\n-- \nsee shy jo\n"},{"id":"267175","messageId":"CAPc5daXPY7RQSM6oyFYJ2LjYjaT9dDG-+3=nbHONirGh50pdBA@mail.gmail.com","threadId":"39973","inReplyTo":"20150730183144.GA6360@kitenet.net","subject":"Re: git pull --upload-pack reversion in git 2.5.0","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-07-30T18:41:31Z","receivedAt":"2015-07-30T18:41:31Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"On Thu, Jul 30, 2015 at 11:31 AM, Joey Hess <id@joeyh.name> wrote:\n> I think this comes down to a lack of quoting where git-pull runs\n> git-fetch. Before eb2a8d9ed3fca2ba2f617b704992d483605f3bb6,\n> \"$@\" was passed through to git-fetch, but now there is a $upload_pack\n> which is passed without being quoted.\n\nYes, it is not just the matter of using \"$upload_pack\", though ;-)\n\n${upload_pack+\"$upload_pack\"} or something.\n"},{"id":"267177","messageId":"1438288803-17953-1-git-send-email-Matthieu.Moy@imag.fr","threadId":"39973","inReplyTo":"CAPc5daXPY7RQSM6oyFYJ2LjYjaT9dDG-+3=nbHONirGh50pdBA@mail.gmail.com","subject":"[PATCH] pull.sh: quote $upload_pack when passing it to git-fetch","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@imag.fr","sentAt":"2015-07-30T20:40:03Z","receivedAt":"2015-07-30T20:40:03Z","isPatch":true,"sender":{"key":"git@matthieu-moy.fr","avatar":"https://avatars.githubusercontent.com/u/14709?v=4"},"body":"The previous code broke for example\n\n  git pull --upload-pack 'echo --foo'\n\nReported-by: Joey Hess <id@joeyh.name>\nFix-suggested-by: Junio C Hamano <gitster@pobox.com>\nSigned-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>\n---\nJunio wrote:\n> ${upload_pack+\"$upload_pack\"} or something.\n\nIndeed, we need to pass nothing, not the empty string if $upload_pack\nis not defined.\n\nThis should fix it.\n\n git-pull.sh | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/git-pull.sh b/git-pull.sh\nindex a814bf6..26c5e9f 100755\n--- a/git-pull.sh\n+++ b/git-pull.sh\n@@ -295,7 +295,7 @@ test true = \"$rebase\" && {\n }\n orig_head=$(git rev-parse -q --verify HEAD)\n git fetch $verbosity $progress $dry_run $recurse_submodules $all $append \\\n-$upload_pack $force $tags $prune $keep $depth $unshallow $update_shallow \\\n+${upload_pack+\"$upload_pack\"} $force $tags $prune $keep $depth $unshallow $update_shallow \\\n $refmap --update-head-ok \"$@\" || exit 1\n test -z \"$dry_run\" || exit 0\n \n-- \n2.5.0.rc0.7.ge1edd74.dirty\n"},{"id":"267178","messageId":"xmqq8u9x73hv.fsf@gitster.dls.corp.google.com","threadId":"39973","inReplyTo":"1438288803-17953-1-git-send-email-Matthieu.Moy@imag.fr","subject":"Re: [PATCH] pull.sh: quote $upload_pack when passing it to git-fetch","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-07-30T21:13:48Z","receivedAt":"2015-07-30T21:13:48Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Matthieu Moy <Matthieu.Moy@imag.fr> writes:\n\n> The previous code broke for example\n>\n>   git pull --upload-pack 'echo --foo'\n>\n> Reported-by: Joey Hess <id@joeyh.name>\n> Fix-suggested-by: Junio C Hamano <gitster@pobox.com>\n> Signed-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>\n> ---\n> Junio wrote:\n>> ${upload_pack+\"$upload_pack\"} or something.\n>\n> Indeed, we need to pass nothing, not the empty string if $upload_pack\n> is not defined.\n>\n> This should fix it.\n\nThe problematic commit touches a lot more than upload-pack, but all\nothers that take user-supplied strings are meant for \"git merge\" or\n\"git rebase\" that are properly quoted and then eval'ed, so this\nshould be sufficient.\n\nThanks.\n\n>\n>  git-pull.sh | 2 +-\n>  1 file changed, 1 insertion(+), 1 deletion(-)\n>\n> diff --git a/git-pull.sh b/git-pull.sh\n> index a814bf6..26c5e9f 100755\n> --- a/git-pull.sh\n> +++ b/git-pull.sh\n> @@ -295,7 +295,7 @@ test true = \"$rebase\" && {\n>  }\n>  orig_head=$(git rev-parse -q --verify HEAD)\n>  git fetch $verbosity $progress $dry_run $recurse_submodules $all $append \\\n> -$upload_pack $force $tags $prune $keep $depth $unshallow $update_shallow \\\n> +${upload_pack+\"$upload_pack\"} $force $tags $prune $keep $depth\n> $unshallow $update_shallow \\\n>  $refmap --update-head-ok \"$@\" || exit 1\n>  test -z \"$dry_run\" || exit 0\n"},{"id":"267195","messageId":"CACRoPnR5z95xGNgcPThnGO1KfftcETBprs8HyeskroXGD4ZPgw@mail.gmail.com","threadId":"39973","inReplyTo":"1438288803-17953-1-git-send-email-Matthieu.Moy@imag.fr","subject":"Re: [PATCH] pull.sh: quote $upload_pack when passing it to git-fetch","fromName":"Paul Tan","fromEmail":"pyokagan@gmail.com","sentAt":"2015-07-31T11:01:03Z","receivedAt":"2015-07-31T11:01:03Z","isPatch":true,"sender":{"key":"pyokagan@gmail.com","avatar":"https://avatars.githubusercontent.com/u/109479?v=4"},"body":"On Fri, Jul 31, 2015 at 4:40 AM, Matthieu Moy <Matthieu.Moy@imag.fr> wrote:\n> The previous code broke for example\n>\n>   git pull --upload-pack 'echo --foo'\n>\n> Reported-by: Joey Hess <id@joeyh.name>\n> Fix-suggested-by: Junio C Hamano <gitster@pobox.com>\n> Signed-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>\n\nThanks for cleaning up my mess! ><\n\nRegards,\nPaul\n"},{"id":"267209","messageId":"xmqqk2tg5i2f.fsf@gitster.dls.corp.google.com","threadId":"39973","inReplyTo":"CAPc5daXPY7RQSM6oyFYJ2LjYjaT9dDG-+3=nbHONirGh50pdBA@mail.gmail.com","subject":"Re: git pull --upload-pack reversion in git 2.5.0","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-07-31T17:54:16Z","receivedAt":"2015-07-31T17:54:16Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano <gitster@pobox.com> writes:\n\n> On Thu, Jul 30, 2015 at 11:31 AM, Joey Hess <id@joeyh.name> wrote:\n>> I think this comes down to a lack of quoting where git-pull runs\n>> git-fetch. Before eb2a8d9ed3fca2ba2f617b704992d483605f3bb6,\n>> \"$@\" was passed through to git-fetch, but now there is a $upload_pack\n>> which is passed without being quoted.\n>\n> Yes, it is not just the matter of using \"$upload_pack\", though ;-)\n>\n> ${upload_pack+\"$upload_pack\"} or something.\n\nThanks for a bug report.\n\nThe next time, I'd appreciate if these regressions are caught before\nit hits the tagged release, during the pre-release -rc period at the\nlatest, but preferrably earlier.  I'm greedy and expect far more\nfrom those who are known to be competent heavy Git users than from\n\"I update every few releases\" casual folks ;-)\n\nThanks, anyway.\n"}]}