{"thread":{"id":"39496","subject":"[PATCH v2 0/2] Improve git-pull's option parsing","startedAt":"2015-06-02T14:22:51Z","lastAt":"2015-06-02T20:26:36Z","messageCount":4,"participants":["Paul Tan","Junio C Hamano"],"isPatch":true,"patchVersion":2,"patchTotal":2},"messages":[{"id":"262723","messageId":"1433254973-13797-1-git-send-email-pyokagan@gmail.com","threadId":"39496","inReplyTo":null,"subject":"[PATCH v2 0/2] Improve git-pull's option parsing","fromName":"Paul Tan","fromEmail":"pyokagan@gmail.com","sentAt":"2015-06-02T14:22:51Z","receivedAt":"2015-06-02T14:22:51Z","isPatch":true,"sender":{"key":"pyokagan@gmail.com","avatar":"https://avatars.githubusercontent.com/u/109479?v=4"},"body":"This is a re-roll of [v1]. Thanks Johannes for the reviews last round.\n\nPrevious versions:\n\n[v1] http://thread.gmane.org/gmane.comp.version-control.git/269249\n\nThis patch series is based on pt/pull-tests.\n\nWhile parsing the command-line arguments, git-pull stops parsing at the\nfirst unrecognized option, assuming that any subsequent options are for\ngit-fetch, and can thus be kept in the shell's positional parameters\nlist, so that it can be passed to git-fetch via the expansion of \"$@\".\n\nHowever, certain functions in git-pull assume that the positional\nparameters do not contain any options. Fix this by making git-pull\nhandle git-fetch's options as well at the option parsing stage.\n\nWith this change in place, we can move on to migrate git-pull to use\ngit-rev-parse --parseopt such that its option parsing is consistent with\nthe other git commands.\n\nI believe this is the last required behavior change for my rewrite of\ngit-pull.sh to C.\n\nPaul Tan (2):\n  pull: handle git-fetch's options as well\n  pull: use git-rev-parse --parseopt for option parsing\n\n Documentation/git-pull.txt |   3 --\n git-pull.sh                | 128 +++++++++++++++++++++++++++++++--------------\n t/t5520-pull.sh            |  20 +++++++\n t/t5521-pull-options.sh    |  14 +++++\n 4 files changed, 122 insertions(+), 43 deletions(-)\n\n-- \n2.1.4\n"},{"id":"262724","messageId":"1433254973-13797-2-git-send-email-pyokagan@gmail.com","threadId":"39496","inReplyTo":"1433254973-13797-1-git-send-email-pyokagan@gmail.com","subject":"[PATCH v2 1/2] pull: handle git-fetch's options as well","fromName":"Paul Tan","fromEmail":"pyokagan@gmail.com","sentAt":"2015-06-02T14:22:52Z","receivedAt":"2015-06-02T14:22:52Z","isPatch":true,"sender":{"key":"pyokagan@gmail.com","avatar":"https://avatars.githubusercontent.com/u/109479?v=4"},"body":"While parsing the command-line arguments, git-pull stops parsing at the\nfirst unrecognized option, assuming that any subsequent options are for\ngit-fetch, and can thus be kept in the shell's positional parameters\nlist, so that it can be passed to git-fetch via the expansion of \"$@\".\n\nHowever, certain functions in git-pull assume that the positional\nparameters do not contain any options:\n\n* error_on_no_merge_candidates() uses the number of positional\n  parameters to determine which error message to print out, and will\n  thus print the wrong message if git-fetch's options are passed in as\n  well.\n\n* the call to get_remote_merge_branch() assumes that the positional\n  parameters only contains the optional repo and refspecs, and will\n  thus silently fail if git-fetch's options are passed in as well.\n\n* --dry-run is a valid git-fetch option, but if provided after any\n  git-fetch options, it is not recognized by git-pull and thus git-pull\n  will continue to run the merge or rebase.\n\nFix these bugs by teaching git-pull to parse git-fetch's options as\nwell. Add tests to prevent regressions.\n\nThis removes the limitation where git-fetch's options have to come after\ngit-merge's and git-rebase's options on the command line. Update the\ndocumentation to reflect this.\n\nSigned-off-by: Paul Tan <pyokagan@gmail.com>\n---\n\nNotes:\n    Improve git-pull's option parsing\n    \n    Previous versions:\n    \n    [v1] http://thread.gmane.org/gmane.comp.version-control.git/269249\n    \n    This patch series is based on pt/pull-tests.\n    \n    While parsing the command-line arguments, git-pull stops parsing at the\n    first unrecognized option, assuming that any subsequent options are for\n    git-fetch, and can thus be kept in the shell's positional parameters\n    list, so that it can be passed to git-fetch via the expansion of \"$ <at>\n    \".\n    \n    However, certain functions in git-pull assume that the positional\n    parameters do not contain any options. Fix this by making git-pull\n    handle git-fetch's options as well at the option parsing stage.\n    \n    With this change in place, we can move on to migrate git-pull to use\n    git-rev-parse --parseopt such that its option parsing is consistent with\n    the other git commands.\n    \n    I believe this is the last required behavior change for my rewrite of\n    git-pull.sh to C.\n    \n    v2\n    \n    * Initialize variables to prevent them from being set in the command\n      line.\n    \n    * Update the documentation as well.\n\n Documentation/git-pull.txt |  3 ---\n git-pull.sh                | 37 ++++++++++++++++++++++++++++++++++---\n t/t5520-pull.sh            | 20 ++++++++++++++++++++\n t/t5521-pull-options.sh    | 14 ++++++++++++++\n 4 files changed, 68 insertions(+), 6 deletions(-)\n\ndiff --git a/Documentation/git-pull.txt b/Documentation/git-pull.txt\nindex 712ab4b..93c72a2 100644\n--- a/Documentation/git-pull.txt\n+++ b/Documentation/git-pull.txt\n@@ -74,9 +74,6 @@ pulling or stash them away with linkgit:git-stash[1].\n OPTIONS\n -------\n \n-Options meant for 'git pull' itself and the underlying 'git merge'\n-must be given before the options meant for 'git fetch'.\n-\n -q::\n --quiet::\n \tThis is passed to both underlying git-fetch to squelch reporting of\ndiff --git a/git-pull.sh b/git-pull.sh\nindex 0917d0d..623ba7a 100755\n--- a/git-pull.sh\n+++ b/git-pull.sh\n@@ -44,7 +44,8 @@ bool_or_string_config () {\n \n strategy_args= diffstat= no_commit= squash= no_ff= ff_only=\n log_arg= verbosity= progress= recurse_submodules= verify_signatures=\n-merge_args= edit= rebase_args=\n+merge_args= edit= rebase_args= all= append= upload_pack= force= tags= prune=\n+keep= depth= unshallow= update_shallow= refmap=\n curr_branch=$(git symbolic-ref -q HEAD)\n curr_branch_short=\"${curr_branch#refs/heads/}\"\n rebase=$(bool_or_string_config branch.$curr_branch_short.rebase)\n@@ -166,11 +167,39 @@ do\n \t--d|--dr|--dry|--dry-|--dry-r|--dry-ru|--dry-run)\n \t\tdry_run=--dry-run\n \t\t;;\n+\t--all|--no-all)\n+\t\tall=$1 ;;\n+\t-a|--append|--no-append)\n+\t\tappend=$1 ;;\n+\t--upload-pack=*|--no-upload-pack)\n+\t\tupload_pack=$1 ;;\n+\t-f|--force|--no-force)\n+\t\tforce=\"$force $1\" ;;\n+\t-t|--tags|--no-tags)\n+\t\ttags=$1 ;;\n+\t-p|--prune|--no-prune)\n+\t\tprune=$1 ;;\n+\t-k|--keep|--no-keep)\n+\t\tkeep=$1 ;;\n+\t--depth=*|--no-depth)\n+\t\tdepth=$1 ;;\n+\t--unshallow|--no-unshallow)\n+\t\tunshallow=$1 ;;\n+\t--update-shallow|--no-update-shallow)\n+\t\tupdate_shallow=$1 ;;\n+\t--refmap=*|--no-refmap)\n+\t\trefmap=$1 ;;\n \t-h|--help-all)\n \t\tusage\n \t\t;;\n+\t--)\n+\t\tshift\n+\t\tbreak\n+\t\t;;\n+\t-*)\n+\t\tusage\n+\t\t;;\n \t*)\n-\t\t# Pass thru anything that may be meant for fetch.\n \t\tbreak\n \t\t;;\n \tesac\n@@ -248,7 +277,9 @@ test true = \"$rebase\" && {\n \toldremoteref=$(git merge-base --fork-point \"$remoteref\" $curr_branch 2>/dev/null)\n }\n orig_head=$(git rev-parse -q --verify HEAD)\n-git fetch $verbosity $progress $dry_run $recurse_submodules --update-head-ok \"$@\" || exit 1\n+git fetch $verbosity $progress $dry_run $recurse_submodules $all $append \\\n+$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 curr_head=$(git rev-parse -q --verify HEAD)\ndiff --git a/t/t5520-pull.sh b/t/t5520-pull.sh\nindex af31f04..f4a7193 100755\n--- a/t/t5520-pull.sh\n+++ b/t/t5520-pull.sh\n@@ -160,6 +160,18 @@ test_expect_success 'fail if no configuration for current branch' '\n \ttest \"$(cat file)\" = file\n '\n \n+test_expect_success 'pull --all: fail if no configuration for current branch' '\n+\tgit remote add test_remote . &&\n+\ttest_when_finished \"git remote remove test_remote\" &&\n+\tgit checkout -b test copy^ &&\n+\ttest_when_finished \"git checkout -f copy && git branch -D test\" &&\n+\ttest_config branch.test.remote test_remote &&\n+\ttest \"$(cat file)\" = file &&\n+\ttest_must_fail git pull --all 2>err &&\n+\ttest_i18ngrep \"There is no tracking information\" err &&\n+\ttest \"$(cat file)\" = file\n+'\n+\n test_expect_success 'fail if upstream branch does not exist' '\n \tgit checkout -b test copy^ &&\n \ttest_when_finished \"git checkout -f copy && git branch -D test\" &&\n@@ -365,6 +377,14 @@ test_expect_success '--rebase with rebased upstream' '\n \n '\n \n+test_expect_success '--rebase -f with rebased upstream' '\n+\ttest_when_finished \"test_might_fail git rebase --abort\" &&\n+\tgit reset --hard to-rebase-orig &&\n+\tgit pull --rebase -f me copy &&\n+\ttest \"conflicting modification\" = \"$(cat file)\" &&\n+\ttest file = \"$(cat file2)\"\n+'\n+\n test_expect_success '--rebase with rebased default upstream' '\n \n \tgit update-ref refs/remotes/me/copy copy-orig &&\ndiff --git a/t/t5521-pull-options.sh b/t/t5521-pull-options.sh\nindex 56e7377..18372ca 100755\n--- a/t/t5521-pull-options.sh\n+++ b/t/t5521-pull-options.sh\n@@ -130,4 +130,18 @@ test_expect_success 'git pull --dry-run' '\n \t)\n '\n \n+test_expect_success 'git pull --all --dry-run' '\n+\ttest_when_finished \"rm -rf cloneddry\" &&\n+\tgit init clonedry &&\n+\t(\n+\t\tcd clonedry &&\n+\t\tgit remote add origin ../parent &&\n+\t\tgit pull --all --dry-run &&\n+\t\ttest_path_is_missing .git/FETCH_HEAD &&\n+\t\ttest_path_is_missing .git/refs/remotes/origin/master &&\n+\t\ttest_path_is_missing .git/index &&\n+\t\ttest_path_is_missing file\n+\t)\n+'\n+\n test_done\n-- \n2.1.4\n"},{"id":"262725","messageId":"1433254973-13797-3-git-send-email-pyokagan@gmail.com","threadId":"39496","inReplyTo":"1433254973-13797-1-git-send-email-pyokagan@gmail.com","subject":"[PATCH v2 2/2] pull: use git-rev-parse --parseopt for option parsing","fromName":"Paul Tan","fromEmail":"pyokagan@gmail.com","sentAt":"2015-06-02T14:22:53Z","receivedAt":"2015-06-02T14:22:53Z","isPatch":true,"sender":{"key":"pyokagan@gmail.com","avatar":"https://avatars.githubusercontent.com/u/109479?v=4"},"body":"To enable unambiguous parsing of abbreviated options, bundled short\noptions, separate form options and to provide consistent usage help, use\ngit-rev-parse --parseopt for option parsing. With this, simplify the\noption parsing code.\n\nSigned-off-by: Paul Tan <pyokagan@gmail.com>\n---\n\nNotes:\n    v2\n    \n    * Don't use OPTIONS_KEEPDASHDASH\n\n git-pull.sh | 97 ++++++++++++++++++++++++++++++++++++-------------------------\n 1 file changed, 57 insertions(+), 40 deletions(-)\n\ndiff --git a/git-pull.sh b/git-pull.sh\nindex 623ba7a..a814bf6 100755\n--- a/git-pull.sh\n+++ b/git-pull.sh\n@@ -4,13 +4,53 @@\n #\n # Fetch one or more remote refs and merge it/them into the current HEAD.\n \n-USAGE='[-n | --no-stat] [--[no-]commit] [--[no-]squash] [--[no-]ff|--ff-only] [--[no-]rebase|--rebase=preserve] [-s strategy]... [<fetch-options>] <repo> <head>...'\n-LONG_USAGE='Fetch one or more remote refs and integrate it/them with the current HEAD.'\n SUBDIRECTORY_OK=Yes\n-OPTIONS_SPEC=\n+OPTIONS_KEEPDASHDASH=\n+OPTIONS_STUCKLONG=Yes\n+OPTIONS_SPEC=\"\\\n+git pull [options] [<repository> [<refspec>...]]\n+\n+Fetch one or more remote refs and integrate it/them with the current HEAD.\n+--\n+v,verbose                  be more verbose\n+q,quiet                    be more quiet\n+progress                   force progress reporting\n+\n+  Options related to merging\n+r,rebase?false|true|preserve incorporate changes by rebasing rather than merging\n+n!                         do not show a diffstat at the end of the merge\n+stat                       show a diffstat at the end of the merge\n+summary                    (synonym to --stat)\n+log?n                      add (at most <n>) entries from shortlog to merge commit message\n+squash                     create a single commit instead of doing a merge\n+commit                     perform a commit if the merge succeeds (default)\n+e,edit                       edit message before committing\n+ff                         allow fast-forward\n+ff-only!                   abort if fast-forward is not possible\n+verify-signatures          verify that the named commit has a valid GPG signature\n+s,strategy=strategy        merge strategy to use\n+X,strategy-option=option   option for selected merge strategy\n+S,gpg-sign?key-id          GPG sign commit\n+\n+  Options related to fetching\n+all                        fetch from all remotes\n+a,append                   append to .git/FETCH_HEAD instead of overwriting\n+upload-pack=path           path to upload pack on remote end\n+f,force                    force overwrite of local branch\n+t,tags                     fetch all tags and associated objects\n+p,prune                    prune remote-tracking branches no longer on remote\n+recurse-submodules?on-demand control recursive fetching of submodules\n+dry-run                    dry run\n+k,keep                     keep downloaded pack\n+depth=depth                deepen history of shallow clone\n+unshallow                  convert to a complete repository\n+update-shallow             accept refs that update .git/shallow\n+refmap=refmap              specify fetch refmap\n+\"\n+test $# -gt 0 && args=\"$*\"\n . git-sh-setup\n . git-sh-i18n\n-set_reflog_action \"pull${1+ $*}\"\n+set_reflog_action \"pull${args+ $args}\"\n require_work_tree_exists\n cd_to_toplevel\n \n@@ -87,17 +127,17 @@ do\n \t\tdiffstat=--stat ;;\n \t--log|--log=*|--no-log)\n \t\tlog_arg=\"$1\" ;;\n-\t--no-c|--no-co|--no-com|--no-comm|--no-commi|--no-commit)\n+\t--no-commit)\n \t\tno_commit=--no-commit ;;\n-\t--c|--co|--com|--comm|--commi|--commit)\n+\t--commit)\n \t\tno_commit=--commit ;;\n \t-e|--edit)\n \t\tedit=--edit ;;\n \t--no-edit)\n \t\tedit=--no-edit ;;\n-\t--sq|--squ|--squa|--squas|--squash)\n+\t--squash)\n \t\tsquash=--squash ;;\n-\t--no-sq|--no-squ|--no-squa|--no-squas|--no-squash)\n+\t--no-squash)\n \t\tsquash=--no-squash ;;\n \t--ff)\n \t\tno_ff=--ff ;;\n@@ -105,39 +145,19 @@ do\n \t\tno_ff=--no-ff ;;\n \t--ff-only)\n \t\tff_only=--ff-only ;;\n-\t-s=*|--s=*|--st=*|--str=*|--stra=*|--strat=*|--strate=*|\\\n-\t\t--strateg=*|--strategy=*|\\\n-\t-s|--s|--st|--str|--stra|--strat|--strate|--strateg|--strategy)\n-\t\tcase \"$#,$1\" in\n-\t\t*,*=*)\n-\t\t\tstrategy=$(expr \"z$1\" : 'z-[^=]*=\\(.*\\)') ;;\n-\t\t1,*)\n-\t\t\tusage ;;\n-\t\t*)\n-\t\t\tstrategy=\"$2\"\n-\t\t\tshift ;;\n-\t\tesac\n-\t\tstrategy_args=\"${strategy_args}-s $strategy \"\n+\t-s*|--strategy=*)\n+\t\tstrategy_args=\"$strategy_args $1\"\n \t\t;;\n-\t-X*)\n-\t\tcase \"$#,$1\" in\n-\t\t1,-X)\n-\t\t\tusage ;;\n-\t\t*,-X)\n-\t\t\txx=\"-X $(git rev-parse --sq-quote \"$2\")\"\n-\t\t\tshift ;;\n-\t\t*,*)\n-\t\t\txx=$(git rev-parse --sq-quote \"$1\") ;;\n-\t\tesac\n-\t\tmerge_args=\"$merge_args$xx \"\n+\t-X*|--strategy-option=*)\n+\t\tmerge_args=\"$merge_args $(git rev-parse --sq-quote \"$1\")\"\n \t\t;;\n-\t-r=*|--r=*|--re=*|--reb=*|--reba=*|--rebas=*|--rebase=*)\n+\t-r*|--rebase=*)\n \t\trebase=\"${1#*=}\"\n \t\t;;\n-\t-r|--r|--re|--reb|--reba|--rebas|--rebase)\n+\t--rebase)\n \t\trebase=true\n \t\t;;\n-\t--no-r|--no-re|--no-reb|--no-reba|--no-rebas|--no-rebase)\n+\t--no-rebase)\n \t\trebase=false\n \t\t;;\n \t--recurse-submodules)\n@@ -164,7 +184,7 @@ do\n \t-S*)\n \t\tgpg_sign_args=$(git rev-parse --sq-quote \"$1\")\n \t\t;;\n-\t--d|--dr|--dry|--dry-|--dry-r|--dry-ru|--dry-run)\n+\t--dry-run)\n \t\tdry_run=--dry-run\n \t\t;;\n \t--all|--no-all)\n@@ -196,11 +216,8 @@ do\n \t\tshift\n \t\tbreak\n \t\t;;\n-\t-*)\n-\t\tusage\n-\t\t;;\n \t*)\n-\t\tbreak\n+\t\tusage\n \t\t;;\n \tesac\n \tshift\n-- \n2.1.4\n"},{"id":"262777","messageId":"xmqqpp5d26wz.fsf@gitster.dls.corp.google.com","threadId":"39496","inReplyTo":"1433254973-13797-1-git-send-email-pyokagan@gmail.com","subject":"Re: [PATCH v2 0/2] Improve git-pull's option parsing","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-06-02T20:26:36Z","receivedAt":"2015-06-02T20:26:36Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Paul Tan <pyokagan@gmail.com> writes:\n\n> However, certain functions in git-pull assume that the positional\n> parameters do not contain any options. Fix this by making git-pull\n> handle git-fetch's options as well at the option parsing stage.\n\nGood.\n"}]}