{"thread":{"id":"34668","subject":"[PATCH] pull: Allow pull to preserve merges when rebasing.","startedAt":"2013-08-10T04:58:56Z","lastAt":"2013-08-11T07:12:51Z","messageCount":3,"participants":["Stephen Haberman","Eric Sunshine"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"224983","messageId":"1376110736-11748-1-git-send-email-stephen@exigencecorp.com","threadId":"34668","inReplyTo":null,"subject":"[PATCH] pull: Allow pull to preserve merges when rebasing.","fromName":"Stephen Haberman","fromEmail":"stephen@exigencecorp.com","sentAt":"2013-08-10T04:58:56Z","receivedAt":"2013-08-10T04:58:56Z","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---\nHey,\n\nThis is version 2 of my previous patch--I changed over to the --rebase=preserve\nsyntax as suggested by Johannes and Junio.\n\nI also updated the documentation.\n\nI believe it is ready for serious consideration. Please let me know if I'm\nmissing anything subtle or obvious.\n\nThanks,\nStephen\n\n Documentation/config.txt   |  8 ++++++++\n Documentation/git-pull.txt | 18 ++++++++++++------\n git-pull.sh                | 23 ++++++++++++++++++-----\n t/t5520-pull.sh            | 28 ++++++++++++++++++++++++++++\n 4 files changed, 66 insertions(+), 11 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..87ff938 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 flatten.\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..d142b38 100755\n--- a/git-pull.sh\n+++ b/git-pull.sh\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@@ -111,7 +111,14 @@ do\n \t\tmerge_args=\"$merge_args$xx \"\n \t\t;;\n \t-r|--r|--re|--reb|--reba|--rebas|--rebase)\n-\t\trebase=true\n+\t\t# if the value is already non-false, like preserve, leave it alone\n+\t\tif test -z \"$rebase\" -o false = \"$rebase\"\n+\t\tthen\n+\t\t\trebase=true\n+\t\tfi\n+\t\t;;\n+\t--rebase=*)\n+\t\trebase=\"${1#*=}\"\n \t\t;;\n \t--no-r|--no-re|--no-reb|--no-reba|--no-rebas|--no-rebase)\n \t\trebase=false\n@@ -145,6 +152,12 @@ do\n \tshift\n done\n \n+if test preserve = \"$rebase\"\n+then\n+\trebase=true\n+\trebase_args=--preserve-merges\n+fi\n+\n error_on_no_merge_candidates () {\n \texec >&2\n \tfor opt\n@@ -292,7 +305,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..29cd45d 100755\n--- a/t/t5520-pull.sh\n+++ b/t/t5520-pull.sh\n@@ -148,6 +148,34 @@ test_expect_success 'branch.to-rebase.rebase should override pull.rebase' '\n \ttest new = $(git show HEAD:file2)\n '\n \n+test_expect_success 'pull.rebase=preserve' '\n+\tgit reset --hard before-rebase &&\n+\ttest_config pull.rebase preserve &&\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+\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 '--rebase=preserve' '\n+\tgit reset --hard before-preserve-rebase &&\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 respects pull.rebase=preserve' '\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 $(git rev-parse HEAD^2) = $(git rev-parse keep-merge)\n+'\n+\n test_expect_success '--rebase with rebased upstream' '\n \n \tgit remote add -f me . &&\n-- \n1.8.1.2\n"},{"id":"225044","messageId":"CAPig+cQ2NhMJGTBLHNH2zcfqKUv_ZNF4zGEiXjvHz-LS307kqw@mail.gmail.com","threadId":"34668","inReplyTo":"1376110736-11748-1-git-send-email-stephen@exigencecorp.com","subject":"Re: [PATCH] pull: Allow pull to preserve merges when rebasing.","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2013-08-11T06:16:56Z","receivedAt":"2013-08-11T06:16:56Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Sat, Aug 10, 2013 at 12:58 AM, 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> Hey,\n>\n> This is version 2 of my previous patch--I changed over to the --rebase=preserve\n> syntax as suggested by Johannes and Junio.\n\nIt is helpful to mention a link to the previous submission [1] in\norder to make it easier for people to refer to the associated\ndiscussion.\n\n[1]: http://thread.gmane.org/gmane.comp.version-control.git/231909\n\nMore comments below.\n\n> I also updated the documentation.\n>\n> I believe it is ready for serious consideration. Please let me know if I'm\n> missing anything subtle or obvious.\n>\n> Thanks,\n> Stephen\n>\n> diff --git a/Documentation/git-pull.txt b/Documentation/git-pull.txt\n> index 6ef8d59..87ff938 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 flatten.\n\ns/flatten/flattened/\n\nAlso, it's not clear from the documentation how one would override\npull.rebase=preserve in order to do a normal non-preserving rebase.\nFrom reading the code, one can see that --preserve=true (or\n--no-rebase followed by --rebase) will override pull.rebase=preserve,\nbut it would be hard for someone to guess this. One could imagine\npeople thinking that --rebase alone would intuitively override\npull.rebase=preserve, or that --preserve=no-preserve would do so, but\nthat's getting ugly.\n\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..d142b38 100755\n> --- a/git-pull.sh\n> +++ b/git-pull.sh\n> @@ -111,7 +111,14 @@ do\n>                 merge_args=\"$merge_args$xx \"\n>                 ;;\n>         -r|--r|--re|--reb|--reba|--rebas|--rebase)\n> -               rebase=true\n> +               # if the value is already non-false, like preserve, leave it alone\n> +               if test -z \"$rebase\" -o false = \"$rebase\"\n\nUnportable use of test -o [2]. Better to say 'test foo || test bar'.\n(There is already an -o in git-pull.sh, but no need to add more.)\n\n[2]: http://www.gnu.org/savannah-checkouts/gnu/autoconf/manual/autoconf-2.69/html_node/Limitations-of-Builtins.html#Limitations-of-Builtins\n\n> +               then\n> +                       rebase=true\n> +               fi\n> +               ;;\n> +       --rebase=*)\n> +               rebase=\"${1#*=}\"\n>                 ;;\n\nThere are a couple inconsistencies here.\n\nFirst, short-forms of --rebase are recognized (--r, --re, --reb,\netc.), however only long-form --rebase= is accepted.\n\nSecond, it is standard practice to allow the option's argument to\nbundled or not. For instance, in git-pull, both '--strategy=foo' and\n'--strategy foo' are accepted. --rebase should follow suit.\n\nAdditionally, shouldn't the code be checking for valid values of\n--rebase's argument (\"true\", \"false\", \"preserve\") and complain if\nsomething other is encountered?\n\n>         --no-r|--no-re|--no-reb|--no-reba|--no-rebas|--no-rebase)\n>                 rebase=false\n> diff --git a/t/t5520-pull.sh b/t/t5520-pull.sh\n> index ed4d9c8..29cd45d 100755\n> --- a/t/t5520-pull.sh\n> +++ b/t/t5520-pull.sh\n> @@ -148,6 +148,34 @@ test_expect_success 'branch.to-rebase.rebase should override pull.rebase' '\n>         test new = $(git show HEAD:file2)\n>  '\n>\n> +test_expect_success 'pull.rebase=preserve' '\n> +       git reset --hard before-rebase &&\n> +       test_config pull.rebase preserve &&\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> +       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 '--rebase=preserve' '\n> +       git reset --hard before-preserve-rebase &&\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 respects pull.rebase=preserve' '\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 $(git rev-parse HEAD^2) = $(git rev-parse keep-merge)\n> +'\n\nSince --rebase= now accepts an argument and since only three values\nare allowed for that argument, it makes sense to add tests for\n--rebase=preserve (already done), --rebase=true, --rebase=false,\n--rebase=bogus.\n\nMoreover, there are additional combinations of --rebase=foo and\npull.rebase which can be tested. For instance, --rebase=false\noverrides pull.rebase=preserve, --rebase=true overrides\npull.rebase=preserve, --rebase=preserve overrides pull.rebase=true,\netc.\n\n>  test_expect_success '--rebase with rebased upstream' '\n>\n>         git remote add -f me . &&\n> --\n> 1.8.1.2\n"},{"id":"225046","messageId":"CAPig+cTFSuorbayPWLLbFLtm69LQ2wme1f=V0tTO4FVVYZireQ@mail.gmail.com","threadId":"34668","inReplyTo":"CAPig+cQ2NhMJGTBLHNH2zcfqKUv_ZNF4zGEiXjvHz-LS307kqw@mail.gmail.com","subject":"Re: [PATCH] pull: Allow pull to preserve merges when rebasing.","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2013-08-11T07:12:51Z","receivedAt":"2013-08-11T07:12:51Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Sun, Aug 11, 2013 at 2:16 AM, Eric Sunshine <sunshine@sunshineco.com> wrote:\n> Also, it's not clear from the documentation how one would override\n> pull.rebase=preserve in order to do a normal non-preserving rebase.\n> From reading the code, one can see that --preserve=true (or\n\ns/--preserve=true/--rebase=true/\n\n> --no-rebase followed by --rebase) will override pull.rebase=preserve,\n> but it would be hard for someone to guess this. One could imagine\n> people thinking that --rebase alone would intuitively override\n> pull.rebase=preserve, or that --preserve=no-preserve would do so, but\n> that's getting ugly.\n"}]}