All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Johannes Schindelin via GitGitGadget" <gitgitgadget@gmail.com>
To: git@vger.kernel.org
Cc: "brian m. carlson" <sandals@crustytoothpaste.net>,
	Junio C Hamano <gitster@pobox.com>
Subject: [PATCH v2 00/16] rebase -r: support merge strategies other than recursive
Date: Wed, 31 Jul 2019 08:18:37 -0700 (PDT)	[thread overview]
Message-ID: <pull.294.v2.git.gitgitgadget@gmail.com> (raw)
In-Reply-To: <pull.294.git.gitgitgadget@gmail.com>

This is the most notable shortcoming that --rebase-merges has, still,
relative to --preserve-merges' capabilities: it does not support passing
custom merge strategies or custom merge strategy options.

Let's fix this.

While working on this patch series, of course I tried to copy-edit the test
cases we have, to cover --preserve-merges' support for merge strategies. Oh
my, did I regret this decision as soon as my eyes set sight on 
t3427-rebase-subtree.sh!

At first I tried my best to make heads or tails of t3427, for way too long.
In the end the only way to understand what the heck it tries to do was to
actually fix it. That's why this patch series looks as if it focuses on
t3427 rather than on adding support for custom merge strategies to the 
--rebase-merges mode.

As a consolation to myself, this work was actually worth it, surprising as
that may look. Not only is t3427 now really easy to understand, adding that
test case for --rebase-merges -Xsubtree tickled the sequencer enough to
reveal a long-standing bug: the --onto option was simply ignored when passed
together with --rebase-merges and --root. For good measure, this patch
series addresses this bug, too.

Changes since v1:

 * Rebased to the current js/rebase-cleanup: that branch itself was rebased,
   and as a consequence, one already-applied patch was dropped from this
   patch series.
 * Forward-fixed the fakesha handling, just in case that anybody wants to
   use it with a regular pick command in the future (thanks, brian).

Johannes Schindelin (16):
  Drop unused git-rebase--am.sh
  t3400: stop referring to the scripted rebase
  .gitignore: there is no longer a built-in `git-rebase--interactive`
  sequencer: the `am` and `rebase--interactive` scripts are gone
  rebase: fold git-rebase--common into the -p backend
  t3427: add a clarifying comment
  t3427: simplify the `setup` test case significantly
  t3427: move the `filter-branch` invocation into the `setup` case
  t3427: condense the unnecessarily repetitive test cases into three
  t3427: fix erroneous assumption
  t3427: accommodate for the `rebase --merge` backend having been
    replaced
  t3427: fix another incorrect assumption
  rebase -r: support merge strategies other than `recursive`
  t/lib-rebase: prepare for testing `git rebase --rebase-merges`
  t3418: test `rebase -r` with merge strategies
  rebase -r: do not (re-)generate root commits with `--root` *and*
    `--onto`

 .gitignore                             |   3 -
 Documentation/git-rebase.txt           |   2 -
 Makefile                               |   2 -
 builtin/rebase.c                       |  23 +---
 git-rebase--am.sh                      |  85 --------------
 git-rebase--common.sh                  |  69 -----------
 git-rebase--preserve-merges.sh         |  55 +++++++++
 sequencer.c                            |  20 +++-
 sequencer.h                            |   6 +
 t/lib-rebase.sh                        |   9 +-
 t/t3400-rebase.sh                      |   2 +-
 t/t3418-rebase-continue.sh             |  14 +++
 t/t3422-rebase-incompatible-options.sh |  10 --
 t/t3427-rebase-subtree.sh              | 155 +++++++++++--------------
 t/t3430-rebase-merges.sh               |  21 ++++
 15 files changed, 193 insertions(+), 283 deletions(-)
 delete mode 100644 git-rebase--am.sh
 delete mode 100644 git-rebase--common.sh


base-commit: 80dfc9242ebaba357ffedececd88641a1a752411
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-294%2Fdscho%2Frebase-r-with-strategies-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-294/dscho/rebase-r-with-strategies-v2
Pull-Request: https://github.com/gitgitgadget/git/pull/294

Range-diff vs v1:

  -:  ---------- >  1:  e0645b3ad7 Drop unused git-rebase--am.sh
  -:  ---------- >  2:  6e8be7d873 t3400: stop referring to the scripted rebase
  -:  ---------- >  3:  9e00e66dc7 .gitignore: there is no longer a built-in `git-rebase--interactive`
  -:  ---------- >  4:  db9ec248e1 sequencer: the `am` and `rebase--interactive` scripts are gone
  -:  ---------- >  5:  38c8e3e284 rebase: fold git-rebase--common into the -p backend
  1:  05be92d921 =  6:  b3daf078e8 t3427: add a clarifying comment
  2:  df096e054d =  7:  c168c4499b t3427: simplify the `setup` test case significantly
  3:  a3944c5480 !  8:  138ff362fb t3427: move the `filter-branch` invocation into the `setup` case
     @@ -29,7 +29,8 @@
       '
       
       # FAILURE: Does not preserve master4.
     - test_expect_failure 'Rebase -Xsubtree --preserve-merges --onto commit 4' '
     + test_expect_failure REBASE_P \
     + 	'Rebase -Xsubtree --preserve-merges --onto commit 4' '
       	reset_rebase &&
      -	git checkout -b rebase-preserve-merges-4 master &&
      -	git filter-branch --prune-empty -f --subdirectory-filter files_subtree &&
     @@ -39,8 +40,8 @@
       	verbose test "$(commit_message HEAD~)" = "files_subtree/master4"
       '
      @@
     - # FAILURE: Does not preserve master5.
     - test_expect_failure 'Rebase -Xsubtree --preserve-merges --onto commit 5' '
     + test_expect_failure REBASE_P \
     + 	'Rebase -Xsubtree --preserve-merges --onto commit 5' '
       	reset_rebase &&
      -	git checkout -b rebase-preserve-merges-5 master &&
      -	git filter-branch --prune-empty -f --subdirectory-filter files_subtree &&
     @@ -50,8 +51,8 @@
       	verbose test "$(commit_message HEAD)" = "files_subtree/master5"
       '
      @@
     - # FAILURE: Does not preserve master4.
     - test_expect_failure 'Rebase -Xsubtree --keep-empty --preserve-merges --onto commit 4' '
     + test_expect_failure REBASE_P \
     + 	'Rebase -Xsubtree --keep-empty --preserve-merges --onto commit 4' '
       	reset_rebase &&
      -	git checkout -b rebase-keep-empty-4 master &&
      -	git filter-branch --prune-empty -f --subdirectory-filter files_subtree &&
     @@ -61,8 +62,8 @@
       	verbose test "$(commit_message HEAD~2)" = "files_subtree/master4"
       '
      @@
     - # FAILURE: Does not preserve master5.
     - test_expect_failure 'Rebase -Xsubtree --keep-empty --preserve-merges --onto commit 5' '
     + test_expect_failure REBASE_P \
     + 	'Rebase -Xsubtree --keep-empty --preserve-merges --onto commit 5' '
       	reset_rebase &&
      -	git checkout -b rebase-keep-empty-5 master &&
      -	git filter-branch --prune-empty -f --subdirectory-filter files_subtree &&
     @@ -72,8 +73,8 @@
       	verbose test "$(commit_message HEAD~)" = "files_subtree/master5"
       '
      @@
     - # FAILURE: Does not preserve Empty.
     - test_expect_failure 'Rebase -Xsubtree --keep-empty --preserve-merges --onto empty commit' '
     + test_expect_failure REBASE_P \
     + 	'Rebase -Xsubtree --keep-empty --preserve-merges --onto empty commit' '
       	reset_rebase &&
      -	git checkout -b rebase-keep-empty-empty master &&
      -	git filter-branch --prune-empty -f --subdirectory-filter files_subtree &&
  4:  9aeb57fa8f !  9:  46ef005365 t3427: condense the unnecessarily repetitive test cases into three
     @@ -25,8 +25,9 @@
       '
       
       # FAILURE: Does not preserve master4.
     --test_expect_failure 'Rebase -Xsubtree --preserve-merges --onto commit 4' '
     -+test_expect_failure 'Rebase -Xsubtree --preserve-merges --onto commit' '
     +-test_expect_failure REBASE_P \
     +-	'Rebase -Xsubtree --preserve-merges --onto commit 4' '
     ++test_expect_failure REBASE_P 'Rebase -Xsubtree --preserve-merges --onto commit' '
       	reset_rebase &&
      -	git checkout -b rebase-preserve-merges-4 to-rebase &&
      -	git rebase -Xsubtree=files_subtree --preserve-merges --onto files-master master &&
     @@ -34,7 +35,8 @@
      -'
      -
      -# FAILURE: Does not preserve master5.
     --test_expect_failure 'Rebase -Xsubtree --preserve-merges --onto commit 5' '
     +-test_expect_failure REBASE_P \
     +-	'Rebase -Xsubtree --preserve-merges --onto commit 5' '
      -	reset_rebase &&
      -	git checkout -b rebase-preserve-merges-5 to-rebase &&
      +	git checkout -b rebase-preserve-merges to-rebase &&
     @@ -44,8 +46,9 @@
       '
       
       # FAILURE: Does not preserve master4.
     --test_expect_failure 'Rebase -Xsubtree --keep-empty --preserve-merges --onto commit 4' '
     -+test_expect_failure 'Rebase -Xsubtree --keep-empty --preserve-merges --onto commit' '
     +-test_expect_failure REBASE_P \
     +-	'Rebase -Xsubtree --keep-empty --preserve-merges --onto commit 4' '
     ++test_expect_failure REBASE_P 'Rebase -Xsubtree --keep-empty --preserve-merges --onto commit' '
       	reset_rebase &&
      -	git checkout -b rebase-keep-empty-4 to-rebase &&
      -	git rebase -Xsubtree=files_subtree --keep-empty --preserve-merges --onto files-master master &&
     @@ -53,7 +56,8 @@
      -'
      -
      -# FAILURE: Does not preserve master5.
     --test_expect_failure 'Rebase -Xsubtree --keep-empty --preserve-merges --onto commit 5' '
     +-test_expect_failure REBASE_P \
     +-	'Rebase -Xsubtree --keep-empty --preserve-merges --onto commit 5' '
      -	reset_rebase &&
      -	git checkout -b rebase-keep-empty-5 to-rebase &&
      -	git rebase -Xsubtree=files_subtree --keep-empty --preserve-merges --onto files-master master &&
     @@ -61,7 +65,8 @@
      -'
      -
      -# FAILURE: Does not preserve Empty.
     --test_expect_failure 'Rebase -Xsubtree --keep-empty --preserve-merges --onto empty commit' '
     +-test_expect_failure REBASE_P \
     +-	'Rebase -Xsubtree --keep-empty --preserve-merges --onto empty commit' '
      -	reset_rebase &&
      -	git checkout -b rebase-keep-empty-empty to-rebase &&
      +	git checkout -b rebase-keep-empty to-rebase &&
  5:  3196413c2a = 10:  8afec74cfc t3427: fix erroneous assumption
  6:  261825fe44 = 11:  d73b0a05d9 t3427: accommodate for the `rebase --merge` backend having been replaced
  7:  f7452d3740 = 12:  57c63309bf t3427: fix another incorrect assumption
  8:  7f60b8e745 <  -:  ---------- t3427: mark two test cases as requiring support for `git rebase -p`
  9:  30405df99f = 13:  75b1395dae rebase -r: support merge strategies other than `recursive`
 10:  ae9e72b73b ! 14:  8f74bfbc53 t/lib-rebase: prepare for testing `git rebase --rebase-merges`
     @@ -6,6 +6,26 @@
          `--rebase-merges` mode; Let's prepare the fake editor to handle those
          todo lists properly, too.
      
     +    The original idea was that we keep the original command unless
     +    overridden, and because the original todo lists only had `pick` lines
     +    anyway, we could be sloppy and "override" the command by the same
     +    command (i.e. use the sed replacement pattern "pick" instead of "&").
     +
     +    This actually would not have worked with `fixup` and `squash` commands,
     +    but it would appear that we never tried to use the fake editor with
     +    `--autosquash`.
     +
     +    However, in the next commit we want to use the fake editor in
     +    conjunction with `--rebase-merges`, so let's use the correct sed
     +    replacement pattern.
     +
     +    Technically, it is not necessary to take care of the `fakesha` thing
     +    (where we reuse the sed replacement pattern to craft a new todo
     +    command), at least for now, as the only user of that thing overrides the
     +    `action` anyway. Nevertheless, for completeness' sake, we do take care
     +    of it.
     +
     +    Helped-by: brian m. carlson <sandals@crustytoothpaste.net>
          Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
      
       diff --git a/t/lib-rebase.sh b/t/lib-rebase.sh
     @@ -25,6 +45,10 @@
       		exec_*|x_*|break|b)
       			echo "$line" | sed 's/_/ /g' >> "$1";;
      @@
     + 		bad)
     + 			action="badcmd";;
     + 		fakesha)
     ++			test \& != "$action" || action=pick
       			echo "$action XXXXXXX False commit" >> "$1"
       			action=pick;;
       		*)
 11:  a326be77aa = 15:  0c938d02b3 t3418: test `rebase -r` with merge strategies
 12:  e6713568dc = 16:  be62d267cb rebase -r: do not (re-)generate root commits with `--root` *and* `--onto`

-- 
gitgitgadget

  parent reply	other threads:[~2019-07-31 15:18 UTC|newest]

Thread overview: 39+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-07-25 10:11 [PATCH 00/12] rebase -r: support merge strategies other than recursive Johannes Schindelin via GitGitGadget
2019-07-25 10:11 ` [PATCH 01/12] t3427: add a clarifying comment Johannes Schindelin via GitGitGadget
2019-07-25 10:11 ` [PATCH 02/12] t3427: simplify the `setup` test case significantly Johannes Schindelin via GitGitGadget
2019-07-25 10:11 ` [PATCH 03/12] t3427: move the `filter-branch` invocation into the `setup` case Johannes Schindelin via GitGitGadget
2019-07-25 10:11 ` [PATCH 04/12] t3427: condense the unnecessarily repetitive test cases into three Johannes Schindelin via GitGitGadget
2019-07-25 10:11 ` [PATCH 05/12] t3427: fix erroneous assumption Johannes Schindelin via GitGitGadget
2019-07-25 10:11 ` [PATCH 06/12] t3427: accommodate for the `rebase --merge` backend having been replaced Johannes Schindelin via GitGitGadget
2019-07-25 10:11 ` [PATCH 07/12] t3427: fix another incorrect assumption Johannes Schindelin via GitGitGadget
2019-07-25 10:11 ` [PATCH 08/12] t3427: mark two test cases as requiring support for `git rebase -p` Johannes Schindelin via GitGitGadget
2019-07-25 10:11 ` [PATCH 09/12] rebase -r: support merge strategies other than `recursive` Johannes Schindelin via GitGitGadget
2019-07-25 10:11 ` [PATCH 10/12] t/lib-rebase: prepare for testing `git rebase --rebase-merges` Johannes Schindelin via GitGitGadget
2019-07-26  7:43   ` brian m. carlson
2019-07-26 14:01     ` Johannes Schindelin
2019-07-26 21:08       ` brian m. carlson
2019-07-31 11:25         ` Johannes Schindelin
2019-07-25 10:11 ` [PATCH 12/12] rebase -r: do not (re-)generate root commits with `--root` *and* `--onto` Johannes Schindelin via GitGitGadget
2019-07-25 10:11 ` [PATCH 11/12] t3418: test `rebase -r` with merge strategies Johannes Schindelin via GitGitGadget
2019-07-25 17:10 ` [PATCH 00/12] rebase -r: support merge strategies other than recursive Junio C Hamano
2019-07-26  7:52 ` brian m. carlson
2019-07-31 15:18 ` Johannes Schindelin via GitGitGadget [this message]
2019-07-31 15:18   ` [PATCH v2 01/16] Drop unused git-rebase--am.sh Johannes Schindelin via GitGitGadget
2019-07-31 15:18   ` [PATCH v2 02/16] t3400: stop referring to the scripted rebase Johannes Schindelin via GitGitGadget
2019-07-31 15:18   ` [PATCH v2 03/16] .gitignore: there is no longer a built-in `git-rebase--interactive` Johannes Schindelin via GitGitGadget
2019-07-31 15:18   ` [PATCH v2 04/16] sequencer: the `am` and `rebase--interactive` scripts are gone Johannes Schindelin via GitGitGadget
2019-07-31 15:18   ` [PATCH v2 05/16] rebase: fold git-rebase--common into the -p backend Johannes Schindelin via GitGitGadget
2019-07-31 15:18   ` [PATCH v2 06/16] t3427: add a clarifying comment Johannes Schindelin via GitGitGadget
2019-07-31 15:18   ` [PATCH v2 07/16] t3427: simplify the `setup` test case significantly Johannes Schindelin via GitGitGadget
2019-07-31 15:18   ` [PATCH v2 08/16] t3427: move the `filter-branch` invocation into the `setup` case Johannes Schindelin via GitGitGadget
2019-07-31 15:18   ` [PATCH v2 09/16] t3427: condense the unnecessarily repetitive test cases into three Johannes Schindelin via GitGitGadget
2019-07-31 15:18   ` [PATCH v2 10/16] t3427: fix erroneous assumption Johannes Schindelin via GitGitGadget
2019-07-31 15:18   ` [PATCH v2 11/16] t3427: accommodate for the `rebase --merge` backend having been replaced Johannes Schindelin via GitGitGadget
2019-07-31 15:18   ` [PATCH v2 12/16] t3427: fix another incorrect assumption Johannes Schindelin via GitGitGadget
2019-07-31 15:18   ` [PATCH v2 13/16] rebase -r: support merge strategies other than `recursive` Johannes Schindelin via GitGitGadget
2019-07-31 15:18   ` [PATCH v2 14/16] t/lib-rebase: prepare for testing `git rebase --rebase-merges` Johannes Schindelin via GitGitGadget
2019-07-31 15:18   ` [PATCH v2 15/16] t3418: test `rebase -r` with merge strategies Johannes Schindelin via GitGitGadget
2019-07-31 15:18   ` [PATCH v2 16/16] rebase -r: do not (re-)generate root commits with `--root` *and* `--onto` Johannes Schindelin via GitGitGadget
2019-09-04 21:40   ` [PATCH v2 17/16] t3427: accelerate this test by using fast-export and fast-import Elijah Newren
2019-09-09 20:29     ` Johannes Schindelin
2019-09-09 21:06       ` Junio C Hamano

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=pull.294.v2.git.gitgitgadget@gmail.com \
    --to=gitgitgadget@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=sandals@crustytoothpaste.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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.