* [PATCH 0/2] Improve git-pull's option parsing
@ 2015-05-18 13:54 Paul Tan
2015-05-18 13:54 ` [PATCH 1/2] pull: handle git-fetch's options as well Paul Tan
2015-05-18 13:54 ` [PATCH 2/2] pull: use git-rev-parse --parseopt for option parsing Paul Tan
0 siblings, 2 replies; 5+ messages in thread
From: Paul Tan @ 2015-05-18 13:54 UTC (permalink / raw)
To: git; +Cc: Stefan Beller, Johannes Schindelin, Paul Tan
While parsing the command-line arguments, git-pull stops parsing at the
first unrecognized option, assuming that any subsequent options are for
git-fetch, and can thus be kept in the shell's positional parameters
list, so that it can be passed to git-fetch via the expansion of "$@".
However, certain functions in git-pull assume that the positional
parameters do not contain any options. Fix this by making git-pull handle
git-fetch's options as well at the option parsing stage.
With this change in place, we can move on to migrate git-pull to use
git-rev-parse --parseopt such that its option parsing is consistent with the
other git commands.
Paul Tan (2):
pull: handle git-fetch's options as well
pull: use git-rev-parse --parseopt for option parsing
git-pull.sh | 137 ++++++++++++++++++++++++++++++++++--------------
t/t5520-pull.sh | 20 +++++++
t/t5521-pull-options.sh | 14 +++++
3 files changed, 132 insertions(+), 39 deletions(-)
--
2.1.4
^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH 1/2] pull: handle git-fetch's options as well
2015-05-18 13:54 [PATCH 0/2] Improve git-pull's option parsing Paul Tan
@ 2015-05-18 13:54 ` Paul Tan
2015-05-18 13:54 ` [PATCH 2/2] pull: use git-rev-parse --parseopt for option parsing Paul Tan
1 sibling, 0 replies; 5+ messages in thread
From: Paul Tan @ 2015-05-18 13:54 UTC (permalink / raw)
To: git; +Cc: Stefan Beller, Johannes Schindelin, Paul Tan
While parsing the command-line arguments, git-pull stops parsing at the
first unrecognized option, assuming that any subsequent options are for
git-fetch, and can thus be kept in the shell's positional parameters
list, so that it can be passed to git-fetch via the expansion of "$@".
However, certain functions in git-pull assume that the positional
parameters do not contain any options:
* error_on_no_merge_candidates() uses the number of positional
parameters to determine which error message to print out, and will
thus print the wrong message if git-fetch's options are passed in as
well.
* the call to get_remote_merge_branch() assumes that the positional
parameters only contains the optional repo and refspecs, and will
thus silently fail if git-fetch's options are passed in as well.
* --dry-run is a valid git-fetch option, but if provided after any
git-fetch options, it is not recognized by git-pull and thus git-pull
will continue to run the merge or rebase.
Fix these bugs by teaching git-pull to parse git-fetch's options as
well. Add tests to prevent regressions.
Signed-off-by: Paul Tan <pyokagan@gmail.com>
---
git-pull.sh | 34 ++++++++++++++++++++++++++++++++--
t/t5520-pull.sh | 20 ++++++++++++++++++++
t/t5521-pull-options.sh | 14 ++++++++++++++
3 files changed, 66 insertions(+), 2 deletions(-)
diff --git a/git-pull.sh b/git-pull.sh
index 5ff4545..633c385 100755
--- a/git-pull.sh
+++ b/git-pull.sh
@@ -163,11 +163,39 @@ do
--d|--dr|--dry|--dry-|--dry-r|--dry-ru|--dry-run)
dry_run=--dry-run
;;
+ --all|--no-all)
+ all=$1 ;;
+ -a|--append|--no-append)
+ append=$1 ;;
+ --upload-pack=*|--no-upload-pack)
+ upload_pack=$1 ;;
+ -f|--force|--no-force)
+ force="$force $1" ;;
+ -t|--tags|--no-tags)
+ tags=$1 ;;
+ -p|--prune|--no-prune)
+ prune=$1 ;;
+ -k|--keep|--no-keep)
+ keep=$1 ;;
+ --depth=*|--no-depth)
+ depth=$1 ;;
+ --unshallow|--no-unshallow)
+ unshallow=$1 ;;
+ --update-shallow|--no-update-shallow)
+ update_shallow=$1 ;;
+ --refmap=*|--no-refmap)
+ refmap=$1 ;;
-h|--help-all)
usage
;;
+ --)
+ shift
+ break
+ ;;
+ -*)
+ usage
+ ;;
*)
- # Pass thru anything that may be meant for fetch.
break
;;
esac
@@ -254,7 +282,9 @@ test true = "$rebase" && {
oldremoteref=$(git merge-base --fork-point "$remoteref" $curr_branch 2>/dev/null)
}
orig_head=$(git rev-parse -q --verify HEAD)
-git fetch $verbosity $progress $dry_run $recurse_submodules --update-head-ok "$@" || exit 1
+git fetch $verbosity $progress $dry_run $recurse_submodules $all $append \
+$upload_pack $force $tags $prune $keep $depth $unshallow $update_shallow \
+$refmap --update-head-ok "$@" || exit 1
test -z "$dry_run" || exit 0
curr_head=$(git rev-parse -q --verify HEAD)
diff --git a/t/t5520-pull.sh b/t/t5520-pull.sh
index 62dbfb5..ea73f2f 100755
--- a/t/t5520-pull.sh
+++ b/t/t5520-pull.sh
@@ -160,6 +160,18 @@ test_expect_success 'fail if no configuration for current branch' '
test "$(cat file)" = file
'
+test_expect_success 'pull --all: fail if no configuration for current branch' '
+ git remote add test_remote . &&
+ test_when_finished "git remote remove test_remote" &&
+ git checkout -b test copy^ &&
+ test_when_finished "git checkout -f copy && git branch -D test" &&
+ test_config branch.test.remote test_remote &&
+ test "$(cat file)" = file &&
+ test_must_fail git pull --all 2>err &&
+ test_i18ngrep "There is no tracking information" err &&
+ test "$(cat file)" = file
+'
+
test_expect_success 'fail if upstream branch does not exist' '
git checkout -b test copy^ &&
test_when_finished "git checkout -f copy && git branch -D test" &&
@@ -366,6 +378,14 @@ test_expect_success '--rebase with rebased upstream' '
'
+test_expect_success '--rebase -f with rebased upstream' '
+ test_when_finished "test_might_fail git rebase --abort" &&
+ git reset --hard to-rebase-orig &&
+ git pull --rebase -f me copy &&
+ test "conflicting modification" = "$(cat file)" &&
+ test file = "$(cat file2)"
+'
+
test_expect_success '--rebase with rebased default upstream' '
git update-ref refs/remotes/me/copy copy-orig &&
diff --git a/t/t5521-pull-options.sh b/t/t5521-pull-options.sh
index 56e7377..d61af3d 100755
--- a/t/t5521-pull-options.sh
+++ b/t/t5521-pull-options.sh
@@ -130,4 +130,18 @@ test_expect_success 'git pull --dry-run' '
)
'
+test_expect_success 'git pull --all --dry-run' '
+ test_when_finished "rm -rf clonedmulti" &&
+ git init clonedry &&
+ (
+ cd clonedry &&
+ git remote add origin ../parent &&
+ git pull --all --dry-run &&
+ test_path_is_missing .git/FETCH_HEAD &&
+ test_path_is_missing .git/refs/remotes/origin/master &&
+ test_path_is_missing .git/index &&
+ test_path_is_missing file
+ )
+'
+
test_done
--
2.1.4
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH 2/2] pull: use git-rev-parse --parseopt for option parsing
2015-05-18 13:54 [PATCH 0/2] Improve git-pull's option parsing Paul Tan
2015-05-18 13:54 ` [PATCH 1/2] pull: handle git-fetch's options as well Paul Tan
@ 2015-05-18 13:54 ` Paul Tan
2015-05-18 14:43 ` Johannes Schindelin
1 sibling, 1 reply; 5+ messages in thread
From: Paul Tan @ 2015-05-18 13:54 UTC (permalink / raw)
To: git; +Cc: Stefan Beller, Johannes Schindelin, Paul Tan
To enable unambiguous parsing of abbreviated options, bundled short
options, separate form options and to provide consistent usage help, use
git-rev-parse --parseopt for option parsing. This allows the option parsing
code to be simplified.
Signed-off-by: Paul Tan <pyokagan@gmail.com>
---
git-pull.sh | 109 ++++++++++++++++++++++++++++++++++++++----------------------
1 file changed, 69 insertions(+), 40 deletions(-)
diff --git a/git-pull.sh b/git-pull.sh
index 633c385..67f825c 100755
--- a/git-pull.sh
+++ b/git-pull.sh
@@ -4,13 +4,53 @@
#
# Fetch one or more remote refs and merge it/them into the current HEAD.
-USAGE='[-n | --no-stat] [--[no-]commit] [--[no-]squash] [--[no-]ff|--ff-only] [--[no-]rebase|--rebase=preserve] [-s strategy]... [<fetch-options>] <repo> <head>...'
-LONG_USAGE='Fetch one or more remote refs and integrate it/them with the current HEAD.'
SUBDIRECTORY_OK=Yes
-OPTIONS_SPEC=
+OPTIONS_KEEPDASHDASH=Yes
+OPTIONS_STUCKLONG=Yes
+OPTIONS_SPEC="\
+git pull [options] [<repo> [<refspec>...]]
+
+Fetch one or more remote refs and integrate it/them with the current HEAD.
+--
+v,verbose be more verbose
+q,quiet be more quiet
+progress force progress reporting
+
+ Options related to merging
+r,rebase?false|true|preserve incorporate changes by rebasing rather than merging
+n! do not show a diffstat at the end of the merge
+stat show a diffstat at the end of the merge
+summary (synonym to --stat)
+log?n add (at most <n>) entries from shortlog to merge commit message
+squash create a single commit instead of doing a merge
+commit perform a commit if the merge succeeds (default)
+e,edit edit message before committing
+ff allow fast-forward
+ff-only! abort if fast-forward is not possible
+verify-signatures verify that the named commit has a valid GPG signature
+s,strategy=strategy merge strategy to use
+X,strategy-option=option option for selected merge strategy
+S,gpg-sign?key-id GPG sign commit
+
+ Options related to fetching
+all fetch from all remotes
+a,append append to .git/FETCH_HEAD instead of overwriting
+upload-pack=path path to upload pack on remote end
+f,force force overwrite of local branch
+t,tags fetch all tags and associated objects
+p,prune prune remote-tracking branches no longer on remote
+recurse-submodules?on-demand control recursive fetching of submodules
+dry-run dry run
+k,keep keep downloaded pack
+depth=depth deepen history of shallow clone
+unshallow convert to a complete repository
+update-shallow accept refs that update .git/shallow
+refmap=refmap specify fetch refmap
+"
+test $# -gt 0 && args="$*"
. git-sh-setup
. git-sh-i18n
-set_reflog_action "pull${1+ $*}"
+set_reflog_action "pull${args+ $args}"
require_work_tree_exists
cd_to_toplevel
@@ -83,17 +123,17 @@ do
diffstat=--stat ;;
--log|--log=*|--no-log)
log_arg="$1" ;;
- --no-c|--no-co|--no-com|--no-comm|--no-commi|--no-commit)
+ --no-commit)
no_commit=--no-commit ;;
- --c|--co|--com|--comm|--commi|--commit)
+ --commit)
no_commit=--commit ;;
-e|--edit)
edit=--edit ;;
--no-edit)
edit=--no-edit ;;
- --sq|--squ|--squa|--squas|--squash)
+ --squash)
squash=--squash ;;
- --no-sq|--no-squ|--no-squa|--no-squas|--no-squash)
+ --no-squash)
squash=--no-squash ;;
--ff)
no_ff=--ff ;;
@@ -101,39 +141,25 @@ do
no_ff=--no-ff ;;
--ff-only)
ff_only=--ff-only ;;
- -s=*|--s=*|--st=*|--str=*|--stra=*|--strat=*|--strate=*|\
- --strateg=*|--strategy=*|\
- -s|--s|--st|--str|--stra|--strat|--strate|--strateg|--strategy)
- case "$#,$1" in
- *,*=*)
- strategy=$(expr "z$1" : 'z-[^=]*=\(.*\)') ;;
- 1,*)
- usage ;;
- *)
- strategy="$2"
- shift ;;
- esac
- strategy_args="${strategy_args}-s $strategy "
+ -s*|--strategy=*)
+ strategy_args="$strategy_args $1"
;;
- -X*)
- case "$#,$1" in
- 1,-X)
- usage ;;
- *,-X)
- xx="-X $(git rev-parse --sq-quote "$2")"
- shift ;;
- *,*)
- xx=$(git rev-parse --sq-quote "$1") ;;
- esac
- merge_args="$merge_args$xx "
+ --no-strategy)
+ strategy_args=
+ ;;
+ -X*|--strategy-option=*)
+ merge_args="$merge_args $(git rev-parse --sq-quote "$1")"
+ ;;
+ --no-strategy-option)
+ merge_args=
;;
- -r=*|--r=*|--re=*|--reb=*|--reba=*|--rebas=*|--rebase=*)
+ -r*|--rebase=*)
rebase="${1#*=}"
;;
- -r|--r|--re|--reb|--reba|--rebas|--rebase)
+ --rebase)
rebase=true
;;
- --no-r|--no-re|--no-reb|--no-reba|--no-rebas|--no-rebase)
+ --no-rebase)
rebase=false
;;
--recurse-submodules)
@@ -160,9 +186,15 @@ do
-S*)
gpg_sign_args=$(git rev-parse --sq-quote "$1")
;;
- --d|--dr|--dry|--dry-|--dry-r|--dry-ru|--dry-run)
+ --no-gpg-sign)
+ gpg_sign_args=--no-gpg-sign
+ ;;
+ --dry-run)
dry_run=--dry-run
;;
+ --no-dry-run)
+ dry_run=
+ ;;
--all|--no-all)
all=$1 ;;
-a|--append|--no-append)
@@ -192,11 +224,8 @@ do
shift
break
;;
- -*)
- usage
- ;;
*)
- break
+ usage
;;
esac
shift
--
2.1.4
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH 2/2] pull: use git-rev-parse --parseopt for option parsing
2015-05-18 13:54 ` [PATCH 2/2] pull: use git-rev-parse --parseopt for option parsing Paul Tan
@ 2015-05-18 14:43 ` Johannes Schindelin
2015-05-21 8:41 ` Paul Tan
0 siblings, 1 reply; 5+ messages in thread
From: Johannes Schindelin @ 2015-05-18 14:43 UTC (permalink / raw)
To: Paul Tan; +Cc: git, Stefan Beller
Hi Paul,
On 2015-05-18 15:54, Paul Tan wrote:
> diff --git a/git-pull.sh b/git-pull.sh
> index 633c385..67f825c 100755
> --- a/git-pull.sh
> +++ b/git-pull.sh
> @@ -4,13 +4,53 @@
> #
> # Fetch one or more remote refs and merge it/them into the current HEAD.
>
> -USAGE='[-n | --no-stat] [--[no-]commit] [--[no-]squash]
> [--[no-]ff|--ff-only] [--[no-]rebase|--rebase=preserve] [-s
> strategy]... [<fetch-options>] <repo> <head>...'
> -LONG_USAGE='Fetch one or more remote refs and integrate it/them with
> the current HEAD.'
> SUBDIRECTORY_OK=Yes
> -OPTIONS_SPEC=
> +OPTIONS_KEEPDASHDASH=Yes
I have to admit that I was puzzled by this at first. But it seems that the intention is to handle a dashdash argument as an error, therefore we have to keep it. Is my understanding correct?
Ciao,
Dscho
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 2/2] pull: use git-rev-parse --parseopt for option parsing
2015-05-18 14:43 ` Johannes Schindelin
@ 2015-05-21 8:41 ` Paul Tan
0 siblings, 0 replies; 5+ messages in thread
From: Paul Tan @ 2015-05-21 8:41 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: Git List, Stefan Beller
Hi,
On Mon, May 18, 2015 at 10:43 PM, Johannes Schindelin
<johannes.schindelin@gmx.de> wrote:
> On 2015-05-18 15:54, Paul Tan wrote:
>
>> diff --git a/git-pull.sh b/git-pull.sh
>> index 633c385..67f825c 100755
>> --- a/git-pull.sh
>> +++ b/git-pull.sh
>> @@ -4,13 +4,53 @@
>> #
>> # Fetch one or more remote refs and merge it/them into the current HEAD.
>>
>> -USAGE='[-n | --no-stat] [--[no-]commit] [--[no-]squash]
>> [--[no-]ff|--ff-only] [--[no-]rebase|--rebase=preserve] [-s
>> strategy]... [<fetch-options>] <repo> <head>...'
>> -LONG_USAGE='Fetch one or more remote refs and integrate it/them with
>> the current HEAD.'
>> SUBDIRECTORY_OK=Yes
>> -OPTIONS_SPEC=
>> +OPTIONS_KEEPDASHDASH=Yes
>
> I have to admit that I was puzzled by this at first. But it seems that the intention is to handle a dashdash argument as an error, therefore we have to keep it. Is my understanding correct?
Ugh, I thought we had to keep the dashdash so that we could
disambiguate the arguments from the options, but it turns out that
rev-parse would add its own dashdash. So, no, we don't need to keep
the dashdash.
Thanks for catching this. Fixed.
Regards,
Paul
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2015-05-21 8:41 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-05-18 13:54 [PATCH 0/2] Improve git-pull's option parsing Paul Tan
2015-05-18 13:54 ` [PATCH 1/2] pull: handle git-fetch's options as well Paul Tan
2015-05-18 13:54 ` [PATCH 2/2] pull: use git-rev-parse --parseopt for option parsing Paul Tan
2015-05-18 14:43 ` Johannes Schindelin
2015-05-21 8:41 ` Paul Tan
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.