{"thread":{"id":"34677","subject":"[PATCH] pull: Allow pull to preserve merges when rebasing.","startedAt":"2013-08-11T21:26:27Z","lastAt":"2013-08-12T17:04:50Z","messageCount":8,"participants":["Stephen Haberman","Andres Perera","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"225061","messageId":"1376256387-30974-1-git-send-email-stephen@exigencecorp.com","threadId":"34677","inReplyTo":null,"subject":"[PATCH] pull: Allow pull to preserve merges when rebasing.","fromName":"Stephen Haberman","fromEmail":"stephen@exigencecorp.com","sentAt":"2013-08-11T21:26:27Z","receivedAt":"2013-08-11T21:26:27Z","isPatch":true,"sender":{"key":"stephen@exigencecorp.com","avatar":"https://gravatar.com/avatar/23b93ad70a06ce53505f17ddba65176edbcfb6588e7a4c1a2dca04aaf0a6aff1?d=mp&s=160"},"body":"If a user is working on master, and has merged in their feature branch, but now\nhas to \"git pull\" because master moved, with pull.rebase their feature branch\nwill be flattened into master.\n\nThis is because \"git pull\" currently does not know about rebase's preserve\nmerges flag, which would avoid this behavior, as it would instead replay just\nthe merge commit of the feature branch onto the new master, and not replay each\nindividual commit in the feature branch.\n\nAdd a --rebase=preserve option, which will pass along --preserve-merges to\nrebase.\n\nAlso add 'preserve' to the allowed values for the pull.rebase config setting.\n\nSigned-off-by: Stephen Haberman <stephen@exigencecorp.com>\n---\nHi,\n\nThis is v3 of my previous pull.rebase=preserve patch, previously discussed here:\n\nhttp://thread.gmane.org/gmane.comp.version-control.git/232061\nhttp://thread.gmane.org/gmane.comp.version-control.git/231909\n\nIn this version, I addressed all of Eric's excellent feedback.\n\nI believe the patch is much better now, but would still appreciate more\ndetailed feedback. In particular, I kind of made up how to handle and\ninvalid \"--rebase=invalid\" value, and the resulting error message.\n\nAlso, I changed git-pull's usage to include the -r parameter...not\nsure if that's okay or not. Let me know if not.\n\nThanks!\n\n Documentation/config.txt   |  8 +++++\n Documentation/git-pull.txt | 18 +++++++----\n git-pull.sh                | 42 ++++++++++++++++++++----\n t/t5520-pull.sh            | 81 ++++++++++++++++++++++++++++++++++++++++++++++\n 4 files changed, 137 insertions(+), 12 deletions(-)\n\ndiff --git a/Documentation/config.txt b/Documentation/config.txt\nindex ec57a15..4c22be2 100644\n--- a/Documentation/config.txt\n+++ b/Documentation/config.txt\n@@ -766,6 +766,10 @@ branch.<name>.rebase::\n \t\"git pull\" is run. See \"pull.rebase\" for doing this in a non\n \tbranch-specific manner.\n +\n+\tWhen preserve, also pass `--preserve-merges` along to 'git rebase'\n+\tso that locally committed merge commits will not be flattened\n+\tby running 'git pull'.\n++\n *NOTE*: this is a possibly dangerous operation; do *not* use\n it unless you understand the implications (see linkgit:git-rebase[1]\n for details).\n@@ -1826,6 +1830,10 @@ pull.rebase::\n \tpull\" is run. See \"branch.<name>.rebase\" for setting this on a\n \tper-branch basis.\n +\n+\tWhen preserve, also pass `--preserve-merges` along to 'git rebase'\n+\tso that locally committed merge commits will not be flattened\n+\tby running 'git pull'.\n++\n *NOTE*: this is a possibly dangerous operation; do *not* use\n it unless you understand the implications (see linkgit:git-rebase[1]\n for details).\ndiff --git a/Documentation/git-pull.txt b/Documentation/git-pull.txt\nindex 6ef8d59..beea10b 100644\n--- a/Documentation/git-pull.txt\n+++ b/Documentation/git-pull.txt\n@@ -102,12 +102,18 @@ include::merge-options.txt[]\n :git-pull: 1\n \n -r::\n---rebase::\n-\tRebase the current branch on top of the upstream branch after\n-\tfetching.  If there is a remote-tracking branch corresponding to\n-\tthe upstream branch and the upstream branch was rebased since last\n-\tfetched, the rebase uses that information to avoid rebasing\n-\tnon-local changes.\n+--rebase[=false|true|preserve]::\n+\tWhen true, rebase the current branch on top of the upstream\n+\tbranch after fetching. If there is a remote-tracking branch\n+\tcorresponding to the upstream branch and the upstream branch\n+\twas rebased since last fetched, the rebase uses that information\n+\tto avoid rebasing non-local changes.\n++\n+When preserve, also rebase the current branch on top of the upstream\n+branch, but pass `--preserve-merges` along to `git rebase` so that\n+locally created merge commits will not be flattened.\n++\n+When false, merge the current branch into the upstream branch.\n +\n See `pull.rebase`, `branch.<name>.rebase` and `branch.autosetuprebase` in\n linkgit:git-config[1] if you want to make `git pull` always use\ndiff --git a/git-pull.sh b/git-pull.sh\nindex f0df41c..78ad52d 100755\n--- a/git-pull.sh\n+++ b/git-pull.sh\n@@ -4,7 +4,7 @@\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] [-s strategy]... [<fetch-options>] <repo> <head>...'\n+USAGE='[-n | --no-stat] [--[no-]commit] [--[no-]squash] [--[no-]ff] [-r [true|false|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@@ -40,13 +40,13 @@ test -f \"$GIT_DIR/MERGE_HEAD\" && die_merge\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=\n+merge_args= edit= rebase_args=\n curr_branch=$(git symbolic-ref -q HEAD)\n curr_branch_short=\"${curr_branch#refs/heads/}\"\n-rebase=$(git config --bool branch.$curr_branch_short.rebase)\n+rebase=$(git config branch.$curr_branch_short.rebase)\n if test -z \"$rebase\"\n then\n-\trebase=$(git config --bool pull.rebase)\n+\trebase=$(git config pull.rebase)\n fi\n dry_run=\n while :\n@@ -110,8 +110,27 @@ do\n \t\tesac\n \t\tmerge_args=\"$merge_args$xx \"\n \t\t;;\n+\t-r=*|--r=*|--re=*|--reb=*|--reba=*|--rebas=*|--rebase=*|\\\n \t-r|--r|--re|--reb|--reba|--rebas|--rebase)\n-\t\trebase=true\n+\t\tcase \"$#,$1\" in\n+\t\t*,*=*)\n+\t\t\trebase=\"${1#*=}\"\n+\t\t\t;;\n+\t\t1,*)\n+\t\t\trebase=true\n+\t\t\t;;\n+\t\t*)\n+\t\t\t# if the user typed 'git pull -r . copy', don't treat '.'\n+\t\t\t# as an argument to -r\n+\t\t\tif test true = \"$2\" || test false = \"$2\" || test preserve = \"$3\"\n+\t\t\tthen\n+\t\t\t\trebase=\"$2\"\n+\t\t\t\tshift\n+\t\t\telse\n+\t\t\t\trebase=true\n+\t\t\tfi\n+\t\t\t;;\n+\t\tesac\n \t\t;;\n \t--no-r|--no-re|--no-reb|--no-reba|--no-rebas|--no-rebase)\n \t\trebase=false\n@@ -145,6 +164,17 @@ do\n \tshift\n done\n \n+if test preserve = \"$rebase\"\n+then\n+\trebase=true\n+\trebase_args=--preserve-merges\n+elif test ! -z \"$rebase\" && test false != \"$rebase\" && test true != \"$rebase\"\n+then\n+\techo \"Invalid value for --rebase, should be true, false, or preserve\"\n+\tusage\n+\texit 1\n+fi\n+\n error_on_no_merge_candidates () {\n \texec >&2\n \tfor opt\n@@ -292,7 +322,7 @@ fi\n merge_name=$(git fmt-merge-msg $log_arg <\"$GIT_DIR/FETCH_HEAD\") || exit\n case \"$rebase\" in\n true)\n-\teval=\"git-rebase $diffstat $strategy_args $merge_args $verbosity\"\n+\teval=\"git-rebase $diffstat $strategy_args $merge_args $rebase_args $verbosity\"\n \teval=\"$eval --onto $merge_head ${oldremoteref:-$merge_head}\"\n \t;;\n *)\ndiff --git a/t/t5520-pull.sh b/t/t5520-pull.sh\nindex ed4d9c8..8be0482 100755\n--- a/t/t5520-pull.sh\n+++ b/t/t5520-pull.sh\n@@ -148,6 +148,87 @@ test_expect_success 'branch.to-rebase.rebase should override pull.rebase' '\n \ttest new = $(git show HEAD:file2)\n '\n \n+# add a feature branch, keep-merge, that is merged into master, so the\n+# test can try preserving the merge commit (or not) with various\n+# --rebase flags/pull.rebase settings.\n+test_expect_success 'preserve merge setup' '\n+\tgit reset --hard before-rebase &&\n+\tgit checkout -b keep-merge second^ &&\n+\ttest_commit file3 &&\n+\tgit checkout to-rebase &&\n+\tgit merge keep-merge &&\n+\tgit tag before-preserve-rebase\n+'\n+\n+test_expect_success 'pull.rebase=false create a new merge commit' '\n+\tgit reset --hard before-preserve-rebase &&\n+\ttest_config pull.rebase false &&\n+\tgit pull . copy &&\n+\ttest $(git rev-parse HEAD^1) = $(git rev-parse before-preserve-rebase) &&\n+\ttest $(git rev-parse HEAD^2) = $(git rev-parse copy) &&\n+\ttest file3 = $(git show HEAD:file3.t)\n+'\n+\n+test_expect_success 'pull.rebase=true flattens keep-merge' '\n+\tgit reset --hard before-preserve-rebase &&\n+\ttest_config pull.rebase true &&\n+\tgit pull . copy &&\n+\ttest $(git rev-parse HEAD^^) = $(git rev-parse copy) &&\n+\ttest file3 = $(git show HEAD:file3.t)\n+'\n+\n+test_expect_success 'pull.rebase=preserve rebases and merges keep-merge' '\n+\tgit reset --hard before-preserve-rebase &&\n+\ttest_config pull.rebase preserve &&\n+\tgit pull . copy &&\n+\ttest $(git rev-parse HEAD^^) = $(git rev-parse copy) &&\n+\ttest $(git rev-parse HEAD^2) = $(git rev-parse keep-merge)\n+'\n+\n+test_expect_success 'pull.rebase=invalid fails' '\n+\tgit reset --hard before-preserve-rebase &&\n+\ttest_config pull.rebase invalid &&\n+\t! git pull . copy\n+'\n+\n+test_expect_success '--rebase=false create a new merge commit' '\n+\tgit reset --hard before-preserve-rebase &&\n+\ttest_config pull.rebase true &&\n+\tgit pull --rebase=false . copy &&\n+\ttest $(git rev-parse HEAD^1) = $(git rev-parse before-preserve-rebase) &&\n+\ttest $(git rev-parse HEAD^2) = $(git rev-parse copy) &&\n+\ttest file3 = $(git show HEAD:file3.t)\n+'\n+\n+test_expect_success '--rebase=true rebases and flattens keep-merge' '\n+\tgit reset --hard before-preserve-rebase &&\n+\ttest_config pull.rebase preserve &&\n+\tgit pull --rebase=true . copy &&\n+\ttest $(git rev-parse HEAD^^) = $(git rev-parse copy) &&\n+\ttest file3 = $(git show HEAD:file3.t)\n+'\n+\n+test_expect_success '--rebase=preserve rebases and merges keep-merge' '\n+\tgit reset --hard before-preserve-rebase &&\n+\ttest_config pull.rebase true &&\n+\tgit pull --rebase=preserve . copy &&\n+\ttest $(git rev-parse HEAD^^) = $(git rev-parse copy) &&\n+\ttest $(git rev-parse HEAD^2) = $(git rev-parse keep-merge)\n+'\n+\n+test_expect_success '--rebase=invalid fails' '\n+\tgit reset --hard before-preserve-rebase &&\n+\t! git pull --rebase=invalid . copy\n+'\n+\n+test_expect_success '--rebase overrides pull.rebase=preserve and flattens keep-merge' '\n+\tgit reset --hard before-preserve-rebase &&\n+\ttest_config pull.rebase preserve &&\n+\tgit pull --rebase . copy &&\n+\ttest $(git rev-parse HEAD^^) = $(git rev-parse copy) &&\n+\ttest file3 = $(git show HEAD:file3.t)\n+'\n+\n test_expect_success '--rebase with rebased upstream' '\n \n \tgit remote add -f me . &&\n-- \n1.8.1.2\n"},{"id":"225062","messageId":"CAPrKj1b=QTdqVH+JtukJrfEc=EqxWOEYE4YG7oSY7413uqdKfg@mail.gmail.com","threadId":"34677","inReplyTo":"1376256387-30974-1-git-send-email-stephen@exigencecorp.com","subject":"Re: [PATCH] pull: Allow pull to preserve merges when rebasing.","fromName":"Andres Perera","fromEmail":"andres.p@zoho.com","sentAt":"2013-08-11T23:03:43Z","receivedAt":"2013-08-11T23:03:43Z","isPatch":true,"sender":{"key":"andres.p@zoho.com","avatar":null},"body":"hello, comments inline\n\nOn Sun, Aug 11, 2013 at 4:56 PM, Stephen Haberman\n<stephen@exigencecorp.com> wrote:\n> If a user is working on master, and has merged in their feature branch, but now\n> has to \"git pull\" because master moved, with pull.rebase their feature branch\n> will be flattened into master.\n>\n> This is because \"git pull\" currently does not know about rebase's preserve\n> merges flag, which would avoid this behavior, as it would instead replay just\n> the merge commit of the feature branch onto the new master, and not replay each\n> individual commit in the feature branch.\n>\n> Add a --rebase=preserve option, which will pass along --preserve-merges to\n> rebase.\n>\n> Also add 'preserve' to the allowed values for the pull.rebase config setting.\n>\n> Signed-off-by: Stephen Haberman <stephen@exigencecorp.com>\n> ---\n> Hi,\n>\n> This is v3 of my previous pull.rebase=preserve patch, previously discussed here:\n>\n> http://thread.gmane.org/gmane.comp.version-control.git/232061\n> http://thread.gmane.org/gmane.comp.version-control.git/231909\n>\n> In this version, I addressed all of Eric's excellent feedback.\n>\n> I believe the patch is much better now, but would still appreciate more\n> detailed feedback. In particular, I kind of made up how to handle and\n> invalid \"--rebase=invalid\" value, and the resulting error message.\n>\n> Also, I changed git-pull's usage to include the -r parameter...not\n> sure if that's okay or not. Let me know if not.\n>\n> Thanks!\n>\n>  Documentation/config.txt   |  8 +++++\n>  Documentation/git-pull.txt | 18 +++++++----\n>  git-pull.sh                | 42 ++++++++++++++++++++----\n>  t/t5520-pull.sh            | 81 ++++++++++++++++++++++++++++++++++++++++++++++\n>  4 files changed, 137 insertions(+), 12 deletions(-)\n>\n> diff --git a/Documentation/config.txt b/Documentation/config.txt\n> index ec57a15..4c22be2 100644\n> --- a/Documentation/config.txt\n> +++ b/Documentation/config.txt\n> @@ -766,6 +766,10 @@ branch.<name>.rebase::\n>         \"git pull\" is run. See \"pull.rebase\" for doing this in a non\n>         branch-specific manner.\n>  +\n> +       When preserve, also pass `--preserve-merges` along to 'git rebase'\n> +       so that locally committed merge commits will not be flattened\n> +       by running 'git pull'.\n> ++\n>  *NOTE*: this is a possibly dangerous operation; do *not* use\n>  it unless you understand the implications (see linkgit:git-rebase[1]\n>  for details).\n> @@ -1826,6 +1830,10 @@ pull.rebase::\n>         pull\" is run. See \"branch.<name>.rebase\" for setting this on a\n>         per-branch basis.\n>  +\n> +       When preserve, also pass `--preserve-merges` along to 'git rebase'\n> +       so that locally committed merge commits will not be flattened\n> +       by running 'git pull'.\n> ++\n>  *NOTE*: this is a possibly dangerous operation; do *not* use\n>  it unless you understand the implications (see linkgit:git-rebase[1]\n>  for details).\n> diff --git a/Documentation/git-pull.txt b/Documentation/git-pull.txt\n> index 6ef8d59..beea10b 100644\n> --- a/Documentation/git-pull.txt\n> +++ b/Documentation/git-pull.txt\n> @@ -102,12 +102,18 @@ include::merge-options.txt[]\n>  :git-pull: 1\n>\n>  -r::\n> ---rebase::\n> -       Rebase the current branch on top of the upstream branch after\n> -       fetching.  If there is a remote-tracking branch corresponding to\n> -       the upstream branch and the upstream branch was rebased since last\n> -       fetched, the rebase uses that information to avoid rebasing\n> -       non-local changes.\n> +--rebase[=false|true|preserve]::\n> +       When true, rebase the current branch on top of the upstream\n> +       branch after fetching. If there is a remote-tracking branch\n> +       corresponding to the upstream branch and the upstream branch\n> +       was rebased since last fetched, the rebase uses that information\n> +       to avoid rebasing non-local changes.\n> ++\n> +When preserve, also rebase the current branch on top of the upstream\n> +branch, but pass `--preserve-merges` along to `git rebase` so that\n> +locally created merge commits will not be flattened.\n> ++\n> +When false, merge the current branch into the upstream branch.\n>  +\n>  See `pull.rebase`, `branch.<name>.rebase` and `branch.autosetuprebase` in\n>  linkgit:git-config[1] if you want to make `git pull` always use\n> diff --git a/git-pull.sh b/git-pull.sh\n> index f0df41c..78ad52d 100755\n> --- a/git-pull.sh\n> +++ b/git-pull.sh\n> @@ -4,7 +4,7 @@\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] [-s strategy]... [<fetch-options>] <repo> <head>...'\n> +USAGE='[-n | --no-stat] [--[no-]commit] [--[no-]squash] [--[no-]ff] [-r [true|false|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> @@ -40,13 +40,13 @@ test -f \"$GIT_DIR/MERGE_HEAD\" && die_merge\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=\n> +merge_args= edit= rebase_args=\n>  curr_branch=$(git symbolic-ref -q HEAD)\n>  curr_branch_short=\"${curr_branch#refs/heads/}\"\n> -rebase=$(git config --bool branch.$curr_branch_short.rebase)\n> +rebase=$(git config branch.$curr_branch_short.rebase)\n>  if test -z \"$rebase\"\n>  then\n> -       rebase=$(git config --bool pull.rebase)\n> +       rebase=$(git config pull.rebase)\n>  fi\n>  dry_run=\n>  while :\n> @@ -110,8 +110,27 @@ do\n>                 esac\n>                 merge_args=\"$merge_args$xx \"\n>                 ;;\n> +       -r=*|--r=*|--re=*|--reb=*|--reba=*|--rebas=*|--rebase=*|\\\n>         -r|--r|--re|--reb|--reba|--rebas|--rebase)\n> -               rebase=true\n> +               case \"$#,$1\" in\n> +               *,*=*)\n> +                       rebase=\"${1#*=}\"\n> +                       ;;\n> +               1,*)\n> +                       rebase=true\n> +                       ;;\n> +               *)\n\n> +                       # if the user typed 'git pull -r . copy', don't treat '.'\n> +                       # as an argument to -r\n> +                       if test true = \"$2\" || test false = \"$2\" || test preserve = \"$3\"\n> +                       then\n> +                               rebase=\"$2\"\n> +                               shift\n> +                       else\n> +                               rebase=true\n> +                       fi\n\n1. i'm not sure why you are testing $3 == preserve. it looks like a typo\n\n2. clearer than a string of yoda conditions:\n\ncase $2 in\ntrue|false|preserve)\n    rebase=$2\n    shift\n    ;;\n*)\n    rebase=true\n    ;;\nesac\n\n> +                       ;;\n> +               esac\n>                 ;;\n>         --no-r|--no-re|--no-reb|--no-reba|--no-rebas|--no-rebase)\n>                 rebase=false\n> @@ -145,6 +164,17 @@ do\n>         shift\n>  done\n>\n\n> +if test preserve = \"$rebase\"\n> +then\n> +       rebase=true\n> +       rebase_args=--preserve-merges\n> +elif test ! -z \"$rebase\" && test false != \"$rebase\" && test true != \"$rebase\"\n> +then\n> +       echo \"Invalid value for --rebase, should be true, false, or preserve\"\n> +       usage\n> +       exit 1\n> +fi\n> +\n\n1. in the error message you say that rebase should be a trystate of\ntrue, false, or preserve. why then do you allow $rebase == '' ?\n\n2. clearer than a string of yoda conditions:\n\ncase $rebase in\npreserve)\n    rebase_args=--preserve-merges\n    rebase=true\n    ;;\ntrue|false)\n    ;;\n*)\n    echo \"Invalid value for --rebase, should be true, false, or preserve\" >&2\n    usage\n    exit 1\nesac\n\n\n>  error_on_no_merge_candidates () {\n>         exec >&2\n>         for opt\n> @@ -292,7 +322,7 @@ fi\n>  merge_name=$(git fmt-merge-msg $log_arg <\"$GIT_DIR/FETCH_HEAD\") || exit\n>  case \"$rebase\" in\n>  true)\n> -       eval=\"git-rebase $diffstat $strategy_args $merge_args $verbosity\"\n> +       eval=\"git-rebase $diffstat $strategy_args $merge_args $rebase_args $verbosity\"\n>         eval=\"$eval --onto $merge_head ${oldremoteref:-$merge_head}\"\n>         ;;\n>  *)\n> diff --git a/t/t5520-pull.sh b/t/t5520-pull.sh\n> index ed4d9c8..8be0482 100755\n> --- a/t/t5520-pull.sh\n> +++ b/t/t5520-pull.sh\n> @@ -148,6 +148,87 @@ test_expect_success 'branch.to-rebase.rebase should override pull.rebase' '\n>         test new = $(git show HEAD:file2)\n>  '\n>\n> +# add a feature branch, keep-merge, that is merged into master, so the\n> +# test can try preserving the merge commit (or not) with various\n> +# --rebase flags/pull.rebase settings.\n> +test_expect_success 'preserve merge setup' '\n> +       git reset --hard before-rebase &&\n> +       git checkout -b keep-merge second^ &&\n> +       test_commit file3 &&\n> +       git checkout to-rebase &&\n> +       git merge keep-merge &&\n> +       git tag before-preserve-rebase\n> +'\n> +\n> +test_expect_success 'pull.rebase=false create a new merge commit' '\n> +       git reset --hard before-preserve-rebase &&\n> +       test_config pull.rebase false &&\n> +       git pull . copy &&\n> +       test $(git rev-parse HEAD^1) = $(git rev-parse before-preserve-rebase) &&\n> +       test $(git rev-parse HEAD^2) = $(git rev-parse copy) &&\n> +       test file3 = $(git show HEAD:file3.t)\n> +'\n> +\n> +test_expect_success 'pull.rebase=true flattens keep-merge' '\n> +       git reset --hard before-preserve-rebase &&\n> +       test_config pull.rebase true &&\n> +       git pull . copy &&\n> +       test $(git rev-parse HEAD^^) = $(git rev-parse copy) &&\n> +       test file3 = $(git show HEAD:file3.t)\n> +'\n> +\n> +test_expect_success 'pull.rebase=preserve rebases and merges keep-merge' '\n> +       git reset --hard before-preserve-rebase &&\n> +       test_config pull.rebase preserve &&\n> +       git pull . copy &&\n> +       test $(git rev-parse HEAD^^) = $(git rev-parse copy) &&\n> +       test $(git rev-parse HEAD^2) = $(git rev-parse keep-merge)\n> +'\n> +\n> +test_expect_success 'pull.rebase=invalid fails' '\n> +       git reset --hard before-preserve-rebase &&\n> +       test_config pull.rebase invalid &&\n> +       ! git pull . copy\n> +'\n> +\n> +test_expect_success '--rebase=false create a new merge commit' '\n> +       git reset --hard before-preserve-rebase &&\n> +       test_config pull.rebase true &&\n> +       git pull --rebase=false . copy &&\n> +       test $(git rev-parse HEAD^1) = $(git rev-parse before-preserve-rebase) &&\n> +       test $(git rev-parse HEAD^2) = $(git rev-parse copy) &&\n> +       test file3 = $(git show HEAD:file3.t)\n> +'\n> +\n> +test_expect_success '--rebase=true rebases and flattens keep-merge' '\n> +       git reset --hard before-preserve-rebase &&\n> +       test_config pull.rebase preserve &&\n> +       git pull --rebase=true . copy &&\n> +       test $(git rev-parse HEAD^^) = $(git rev-parse copy) &&\n> +       test file3 = $(git show HEAD:file3.t)\n> +'\n> +\n> +test_expect_success '--rebase=preserve rebases and merges keep-merge' '\n> +       git reset --hard before-preserve-rebase &&\n> +       test_config pull.rebase true &&\n> +       git pull --rebase=preserve . copy &&\n> +       test $(git rev-parse HEAD^^) = $(git rev-parse copy) &&\n> +       test $(git rev-parse HEAD^2) = $(git rev-parse keep-merge)\n> +'\n> +\n> +test_expect_success '--rebase=invalid fails' '\n> +       git reset --hard before-preserve-rebase &&\n> +       ! git pull --rebase=invalid . copy\n> +'\n> +\n> +test_expect_success '--rebase overrides pull.rebase=preserve and flattens keep-merge' '\n> +       git reset --hard before-preserve-rebase &&\n> +       test_config pull.rebase preserve &&\n> +       git pull --rebase . copy &&\n> +       test $(git rev-parse HEAD^^) = $(git rev-parse copy) &&\n> +       test file3 = $(git show HEAD:file3.t)\n> +'\n> +\n>  test_expect_success '--rebase with rebased upstream' '\n>\n>         git remote add -f me . &&\n> --\n> 1.8.1.2\n>\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>\n"},{"id":"225063","messageId":"20130811180915.390d660a@sh9","threadId":"34677","inReplyTo":"CAPrKj1b=QTdqVH+JtukJrfEc=EqxWOEYE4YG7oSY7413uqdKfg@mail.gmail.com","subject":"Re: [PATCH] pull: Allow pull to preserve merges when rebasing.","fromName":"Stephen Haberman","fromEmail":"stephen@exigencecorp.com","sentAt":"2013-08-11T23:09:15Z","receivedAt":"2013-08-11T23:09:15Z","isPatch":true,"sender":{"key":"stephen@exigencecorp.com","avatar":"https://gravatar.com/avatar/23b93ad70a06ce53505f17ddba65176edbcfb6588e7a4c1a2dca04aaf0a6aff1?d=mp&s=160"},"body":"\n> 1. i'm not sure why you are testing $3 == preserve. it looks like a\n> typo\n\nYes, good catch. I've added a test that fails, and will fix that.\n\n> 2. clearer than a string of yoda conditions:\n> \n> case $2 in\n> true|false|preserve)\n\nMakes sense, will change.\n\n> 1. in the error message you say that rebase should be a trystate of\n> true, false, or preserve. why then do you allow $rebase == '' ?\n\nBecause it may be unset, like if the user ran \"git pull . copy\" and\nthe pull.rebase setting was not set.\n\n> 2. clearer than a string of yoda conditions:\n\nWill change again.\n\nI'll wait to see if I get any more feedback and then will send out\nanother version.\n\nThanks!\n\n- Stephen\n"},{"id":"225064","messageId":"CAPrKj1aMURcVoaiJ+WS64ekafUZgSagKrYSknTUk3+TL6tCETQ@mail.gmail.com","threadId":"34677","inReplyTo":"20130811180915.390d660a@sh9","subject":"Re: [PATCH] pull: Allow pull to preserve merges when rebasing.","fromName":"Andres Perera","fromEmail":"andres.p@zoho.com","sentAt":"2013-08-11T23:31:07Z","receivedAt":"2013-08-11T23:31:07Z","isPatch":true,"sender":{"key":"andres.p@zoho.com","avatar":null},"body":"On Sun, Aug 11, 2013 at 6:39 PM, Stephen Haberman\n<stephen@exigencecorp.com> wrote:\n>\n>> 1. i'm not sure why you are testing $3 == preserve. it looks like a\n>> typo\n>\n> Yes, good catch. I've added a test that fails, and will fix that.\n>\n>> 2. clearer than a string of yoda conditions:\n>>\n>> case $2 in\n>> true|false|preserve)\n>\n> Makes sense, will change.\n>\n>> 1. in the error message you say that rebase should be a trystate of\n>> true, false, or preserve. why then do you allow $rebase == '' ?\n>\n> Because it may be unset, like if the user ran \"git pull . copy\" and\n> the pull.rebase setting was not set.\n>\n>> 2. clearer than a string of yoda conditions:\n>\n> Will change again.\n>\n> I'll wait to see if I get any more feedback and then will send out\n> another version.\n\ni just realized that there are ambiguities:\n\npull -r (true|false|preserve) foo\n\nthere are 2 ways to interpret this:\n\npull --rebase=(true|false|preserve) foo # pull from remote named foo\n\npull --rebase (true|false|preserve) foo # pull from remote named\n(true|false|preserve), branch foo\n\noptions with optional operands usually require that the operands be\nconcatenated with the option argument, so that\n\npull --rebase[=(true|false|preserve)] | -r[(true|false|preserve)]\n\navoids the ambiguity of\n\npull --rebase [(true|false|preserve)] | -r [(true|false|preserve)]\n\n1. you can make it a disambiguation by appending ? to the optspec\n(according to man git-rev-parse)\n\n2. you could also disambiguate by testing if the argument is a\nconfigured remote and warn the user, but this makes option parsing\ninconsistent, requires additional logic, and is overall inelegant\n\n>\n> Thanks!\n>\n> - Stephen\n>\n>\n>\n"},{"id":"225065","messageId":"20130811183845.18381b8c@sh9","threadId":"34677","inReplyTo":"CAPrKj1aMURcVoaiJ+WS64ekafUZgSagKrYSknTUk3+TL6tCETQ@mail.gmail.com","subject":"Re: [PATCH] pull: Allow pull to preserve merges when rebasing.","fromName":"Stephen Haberman","fromEmail":"stephen@exigencecorp.com","sentAt":"2013-08-11T23:38:45Z","receivedAt":"2013-08-11T23:38:45Z","isPatch":true,"sender":{"key":"stephen@exigencecorp.com","avatar":"https://gravatar.com/avatar/23b93ad70a06ce53505f17ddba65176edbcfb6588e7a4c1a2dca04aaf0a6aff1?d=mp&s=160"},"body":"Hi Andres,\n\n> i just realized that there are ambiguities:\n\n> pull --rebase (true|false|preserve) foo # pull from remote named\n> (true|false|preserve), branch foo\n\nYeah.\n\nRight now, I did the latter. Around line 125, when parsing \"--rebase\n<somearg>\", we accept <somearg> only if it's true, false, or preserve,\nand shift it off. Otherwise we leave it alone and assume it's a remote\nname.\n\nWithout this logic, t5520 fails because it uses \"git pull --rebase .\ncopy\", which, as you noted, is ambiguous, so \".\" was showing up as the\nrebase argument.\n\nSo, this is technically handled right now, but I'm fine removing the\nambiguous \"--rebase true|false|preserve\" option if that is what is\npreferred.\n\n- Stephen\n"},{"id":"225071","messageId":"7vr4dz1n6c.fsf@alter.siamese.dyndns.org","threadId":"34677","inReplyTo":"CAPrKj1aMURcVoaiJ+WS64ekafUZgSagKrYSknTUk3+TL6tCETQ@mail.gmail.com","subject":"Re: [PATCH] pull: Allow pull to preserve merges when rebasing.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-08-12T05:40:59Z","receivedAt":"2013-08-12T05:40:59Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Andres Perera <andres.p@zoho.com> writes:\n\n> i just realized that there are ambiguities:\n>\n> pull -r (true|false|preserve) foo\n>\n> there are 2 ways to interpret this:\n>\n> pull --rebase=(true|false|preserve) foo # pull from remote named foo\n>\n> pull --rebase (true|false|preserve) foo # pull from remote named\n> (true|false|preserve), branch foo\n>\n> options with optional operands usually require that the operands be\n> concatenated with the option argument.\n\nYes.  This command line option should be like this:\n\n - \"--rebase\" and \"--no-rebase\" are accepted as \"true\" and \"false\";\n\n - \"--rebase=preserve\" should be the _only_ way to spell the new\n   mode of operation (if we were to add \"--rebase=interactive\"\n   later, that should follow suit); and\n\n - \"--rebase=true\" and \"--rebase=false\" is nice to have for\n   consistency.\n\nThanks.\n"},{"id":"225080","messageId":"7viozbz950.fsf@alter.siamese.dyndns.org","threadId":"34677","inReplyTo":"7vr4dz1n6c.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] pull: Allow pull to preserve merges when rebasing.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-08-12T07:00:11Z","receivedAt":"2013-08-12T07:00:11Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano <gitster@pobox.com> writes:\n\n> Andres Perera <andres.p@zoho.com> writes:\n>\n>> i just realized that there are ambiguities:\n>>\n>> pull -r (true|false|preserve) foo\n>>\n>> there are 2 ways to interpret this:\n>>\n>> pull --rebase=(true|false|preserve) foo # pull from remote named foo\n>>\n>> pull --rebase (true|false|preserve) foo # pull from remote named\n>> (true|false|preserve), branch foo\n>>\n>> options with optional operands usually require that the operands be\n>> concatenated with the option argument.\n>\n> Yes.  This command line option should be like this:\n>\n>  - \"--rebase\" and \"--no-rebase\" are accepted as \"true\" and \"false\";\n>\n>  - \"--rebase=preserve\" should be the _only_ way to spell the new\n>    mode of operation (if we were to add \"--rebase=interactive\"\n>    later, that should follow suit); and\n>\n>  - \"--rebase=true\" and \"--rebase=false\" is nice to have for\n>    consistency.\n>\n> Thanks.\n\nOh, another thing.\n\nHow should this interact with 949e0d8e (pull: require choice between\nrebase/merge on non-fast-forward pull, 2013-06-27) which has been in\n'next' and will likely to be one of the earlier topics to graduate\nto 'master' after 1.8.4 is released?\n"},{"id":"225116","messageId":"20130812120450.47f785b2@sh9","threadId":"34677","inReplyTo":"7viozbz950.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] pull: Allow pull to preserve merges when rebasing.","fromName":"Stephen Haberman","fromEmail":"stephen@exigencecorp.com","sentAt":"2013-08-12T17:04:50Z","receivedAt":"2013-08-12T17:04:50Z","isPatch":true,"sender":{"key":"stephen@exigencecorp.com","avatar":"https://gravatar.com/avatar/23b93ad70a06ce53505f17ddba65176edbcfb6588e7a4c1a2dca04aaf0a6aff1?d=mp&s=160"},"body":"\n> How should this interact with 949e0d8e (pull: require choice between\n> rebase/merge on non-fast-forward pull, 2013-06-27)\n\nI believe there should not be any conflicts in functionality, other\nthan just tweaking the docs to mention --rebase=preserve as an option.\n\nPersonally, I would assert that, for people using a rebase workflow with\n\"git pull\", --prebase=preserve should be the default behavior, otherwise\nthey'll be surprised when their feature branches get flattened.\n\nUnfortunately, we can't change the behavior of the naked \"--rebase\"\nflag to really mean \"--rebase=preserve\", but I think that would be\nideal. I think it's what people mean they do \"git pull\". If you want a\nmore raw rebase, they would likely (I think/assume) be running \"git\nrebase\" directly.\n\nNonetheless, thanks for pointing out 949e0d8e, I did not know about it.\n\nPerhaps after that commit graduates to master, I can base this commit\non it, and tweak the new docs to suggest --rebase=preserve as the\nleast-surprising behavior.\n\n(Since I'm offering opinions, I think --rebase=preserve would be a great\ndefault for \"git pull\" in 2.0, but please ignore this statement if\nyou've already hashed out the future/2.0 behavior of git pull.)\n\n- Stephen\n"}]}