* [PATCH 00/12] rebase -r: support merge strategies other than recursive @ 2019-07-25 10:11 Johannes Schindelin via GitGitGadget 2019-07-25 10:11 ` [PATCH 01/12] t3427: add a clarifying comment Johannes Schindelin via GitGitGadget ` (14 more replies) 0 siblings, 15 replies; 39+ messages in thread From: Johannes Schindelin via GitGitGadget @ 2019-07-25 10:11 UTC (permalink / raw) To: git; +Cc: Junio C Hamano 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. Johannes Schindelin (12): 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 t3427: mark two test cases as requiring support for `git rebase -p` 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` Documentation/git-rebase.txt | 2 - builtin/rebase.c | 16 +-- sequencer.c | 18 ++- sequencer.h | 6 + t/lib-rebase.sh | 8 +- t/t3418-rebase-continue.sh | 14 +++ t/t3422-rebase-incompatible-options.sh | 10 -- t/t3427-rebase-subtree.sh | 150 ++++++++++++------------- t/t3430-rebase-merges.sh | 21 ++++ 9 files changed, 134 insertions(+), 111 deletions(-) base-commit: 082ef75b7bfc90ac236afbb857a9552a026832b8 Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-294%2Fdscho%2Frebase-r-with-strategies-v1 Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-294/dscho/rebase-r-with-strategies-v1 Pull-Request: https://github.com/gitgitgadget/git/pull/294 -- gitgitgadget ^ permalink raw reply [flat|nested] 39+ messages in thread
* [PATCH 01/12] t3427: add a clarifying comment 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 ` Johannes Schindelin via GitGitGadget 2019-07-25 10:11 ` [PATCH 02/12] t3427: simplify the `setup` test case significantly Johannes Schindelin via GitGitGadget ` (13 subsequent siblings) 14 siblings, 0 replies; 39+ messages in thread From: Johannes Schindelin via GitGitGadget @ 2019-07-25 10:11 UTC (permalink / raw) To: git; +Cc: Junio C Hamano, Johannes Schindelin From: Johannes Schindelin <johannes.schindelin@gmx.de> The flow of this test script is outright confusing, and to start the endeavor to address that, let's describe what this test is all about, and how it tries to do it. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> --- t/t3427-rebase-subtree.sh | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/t/t3427-rebase-subtree.sh b/t/t3427-rebase-subtree.sh index 3780877e4e..a9b93c39d0 100755 --- a/t/t3427-rebase-subtree.sh +++ b/t/t3427-rebase-subtree.sh @@ -11,6 +11,34 @@ commit_message() { git log --pretty=format:%s -1 "$1" } +# There are a few bugs in the rebase with regards to the subtree strategy, and +# this test script tries to document them. First, the following commit history +# is generated (the onelines are shown, time flows from left to right): +# +# master1 - master2 - master3 +# \ +# README ---------------------- Add subproject master - master4 - files_subtree/master5 +# +# Where the merge moves the files master[123].t into the subdirectory +# files_subtree/ and master4 as well as files_subtree/master5 add files to that +# directory directly. +# +# Then, in subsequent test cases, `git filter-branch` is used to distill just +# the commits that touch files_subtree/. To give it a final pre-rebase touch, +# an empty commit is added on top. The pre-rebase commit history looks like +# this: +# +# Add subproject master - master4 - files_subtree/master5 - Empty commit +# +# where the root commit adds three files: master1.t, master2.t and master3.t. +# +# This commit history is then rebased onto `master3` with the +# `-Xsubtree=files_subtree` option in three different ways: +# +# 1. using `--preserve-merges` +# 2. using `--preserve-merges` and --keep-empty +# 3. without specifying a rebase backend + test_expect_success 'setup' ' test_commit README && mkdir files && -- gitgitgadget ^ permalink raw reply related [flat|nested] 39+ messages in thread
* [PATCH 02/12] t3427: simplify the `setup` test case significantly 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 ` 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 ` (12 subsequent siblings) 14 siblings, 0 replies; 39+ messages in thread From: Johannes Schindelin via GitGitGadget @ 2019-07-25 10:11 UTC (permalink / raw) To: git; +Cc: Junio C Hamano, Johannes Schindelin From: Johannes Schindelin <johannes.schindelin@gmx.de> It still does the very same thing as before, but expresses it in a much more succinct (and still quite readable) manner. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> --- t/t3427-rebase-subtree.sh | 36 +++++++++++++++--------------------- 1 file changed, 15 insertions(+), 21 deletions(-) diff --git a/t/t3427-rebase-subtree.sh b/t/t3427-rebase-subtree.sh index a9b93c39d0..f41a08e436 100755 --- a/t/t3427-rebase-subtree.sh +++ b/t/t3427-rebase-subtree.sh @@ -41,27 +41,21 @@ commit_message() { test_expect_success 'setup' ' test_commit README && - mkdir files && - ( - cd files && - git init && - test_commit master1 && - test_commit master2 && - test_commit master3 - ) && - git fetch files master && - git branch files-master FETCH_HEAD && - git read-tree --prefix=files_subtree files-master && - git checkout -- files_subtree && - tree=$(git write-tree) && - head=$(git rev-parse HEAD) && - rev=$(git rev-parse --verify files-master^0) && - commit=$(git commit-tree -p $head -p $rev -m "Add subproject master" $tree) && - git update-ref HEAD $commit && - ( - cd files_subtree && - test_commit master4 - ) && + + git init files && + test_commit -C files master1 && + test_commit -C files master2 && + test_commit -C files master3 && + + : perform subtree merge into files_subtree/ && + git fetch files refs/heads/master:refs/heads/files-master && + git merge -s ours --no-commit --allow-unrelated-histories \ + files-master && + git read-tree --prefix=files_subtree -u files-master && + git commit -m "Add subproject master" && + + : add two extra commits to rebase && + test_commit -C files_subtree master4 && test_commit files_subtree/master5 ' -- gitgitgadget ^ permalink raw reply related [flat|nested] 39+ messages in thread
* [PATCH 03/12] t3427: move the `filter-branch` invocation into the `setup` case 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 ` 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 ` (11 subsequent siblings) 14 siblings, 0 replies; 39+ messages in thread From: Johannes Schindelin via GitGitGadget @ 2019-07-25 10:11 UTC (permalink / raw) To: git; +Cc: Junio C Hamano, Johannes Schindelin From: Johannes Schindelin <johannes.schindelin@gmx.de> The step to prepare a pre-rebase commit history is _identical_ in _all_ of the test cases (except of course the `setup` case). It should therefore clearly a part of the `setup` test case instead. As the `git filter-branch` command is quite costly on platforms where Unix shell scripting is simply slow (meaning: on Windows), this shaves off a noticeable part of the runtime: in this developer's setup, the time was reduced from ~1m25s to ~1m. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> --- t/t3427-rebase-subtree.sh | 38 +++++++++++++------------------------- 1 file changed, 13 insertions(+), 25 deletions(-) diff --git a/t/t3427-rebase-subtree.sh b/t/t3427-rebase-subtree.sh index f41a08e436..c0e6a49b61 100755 --- a/t/t3427-rebase-subtree.sh +++ b/t/t3427-rebase-subtree.sh @@ -56,15 +56,17 @@ test_expect_success 'setup' ' : add two extra commits to rebase && test_commit -C files_subtree master4 && - test_commit files_subtree/master5 + test_commit files_subtree/master5 && + + git checkout -b to-rebase && + git filter-branch --prune-empty -f --subdirectory-filter files_subtree && + git commit -m "Empty commit" --allow-empty ' # FAILURE: Does not preserve master4. test_expect_failure '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 && - git commit -m "Empty commit" --allow-empty && + git checkout -b rebase-preserve-merges-4 to-rebase && git rebase -Xsubtree=files_subtree --preserve-merges --onto files-master master && verbose test "$(commit_message HEAD~)" = "files_subtree/master4" ' @@ -72,9 +74,7 @@ test_expect_failure 'Rebase -Xsubtree --preserve-merges --onto commit 4' ' # FAILURE: Does not preserve master5. test_expect_failure '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 && - git commit -m "Empty commit" --allow-empty && + git checkout -b rebase-preserve-merges-5 to-rebase && git rebase -Xsubtree=files_subtree --preserve-merges --onto files-master master && verbose test "$(commit_message HEAD)" = "files_subtree/master5" ' @@ -82,9 +82,7 @@ test_expect_failure 'Rebase -Xsubtree --preserve-merges --onto commit 5' ' # FAILURE: Does not preserve master4. test_expect_failure '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 && - git commit -m "Empty commit" --allow-empty && + git checkout -b rebase-keep-empty-4 to-rebase && git rebase -Xsubtree=files_subtree --keep-empty --preserve-merges --onto files-master master && verbose test "$(commit_message HEAD~2)" = "files_subtree/master4" ' @@ -92,9 +90,7 @@ test_expect_failure 'Rebase -Xsubtree --keep-empty --preserve-merges --onto comm # FAILURE: Does not preserve master5. test_expect_failure '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 && - git commit -m "Empty commit" --allow-empty && + git checkout -b rebase-keep-empty-5 to-rebase && git rebase -Xsubtree=files_subtree --keep-empty --preserve-merges --onto files-master master && verbose test "$(commit_message HEAD~)" = "files_subtree/master5" ' @@ -102,9 +98,7 @@ test_expect_failure 'Rebase -Xsubtree --keep-empty --preserve-merges --onto comm # FAILURE: Does not preserve Empty. test_expect_failure '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 && - git commit -m "Empty commit" --allow-empty && + git checkout -b rebase-keep-empty-empty to-rebase && git rebase -Xsubtree=files_subtree --keep-empty --preserve-merges --onto files-master master && verbose test "$(commit_message HEAD)" = "Empty commit" ' @@ -112,9 +106,7 @@ test_expect_failure 'Rebase -Xsubtree --keep-empty --preserve-merges --onto empt # FAILURE: fatal: Could not parse object test_expect_failure 'Rebase -Xsubtree --onto commit 4' ' reset_rebase && - git checkout -b rebase-onto-4 master && - git filter-branch --prune-empty -f --subdirectory-filter files_subtree && - git commit -m "Empty commit" --allow-empty && + git checkout -b rebase-onto-4 to-rebase && git rebase -Xsubtree=files_subtree --onto files-master master && verbose test "$(commit_message HEAD~2)" = "files_subtree/master4" ' @@ -122,18 +114,14 @@ test_expect_failure 'Rebase -Xsubtree --onto commit 4' ' # FAILURE: fatal: Could not parse object test_expect_failure 'Rebase -Xsubtree --onto commit 5' ' reset_rebase && - git checkout -b rebase-onto-5 master && - git filter-branch --prune-empty -f --subdirectory-filter files_subtree && - git commit -m "Empty commit" --allow-empty && + git checkout -b rebase-onto-5 to-rebase && git rebase -Xsubtree=files_subtree --onto files-master master && verbose test "$(commit_message HEAD~)" = "files_subtree/master5" ' # FAILURE: fatal: Could not parse object test_expect_failure 'Rebase -Xsubtree --onto empty commit' ' reset_rebase && - git checkout -b rebase-onto-empty master && - git filter-branch --prune-empty -f --subdirectory-filter files_subtree && - git commit -m "Empty commit" --allow-empty && + git checkout -b rebase-onto-empty to-rebase && git rebase -Xsubtree=files_subtree --onto files-master master && verbose test "$(commit_message HEAD)" = "Empty commit" ' -- gitgitgadget ^ permalink raw reply related [flat|nested] 39+ messages in thread
* [PATCH 04/12] t3427: condense the unnecessarily repetitive test cases into three 2019-07-25 10:11 [PATCH 00/12] rebase -r: support merge strategies other than recursive Johannes Schindelin via GitGitGadget ` (2 preceding siblings ...) 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 ` Johannes Schindelin via GitGitGadget 2019-07-25 10:11 ` [PATCH 05/12] t3427: fix erroneous assumption Johannes Schindelin via GitGitGadget ` (10 subsequent siblings) 14 siblings, 0 replies; 39+ messages in thread From: Johannes Schindelin via GitGitGadget @ 2019-07-25 10:11 UTC (permalink / raw) To: git; +Cc: Junio C Hamano, Johannes Schindelin From: Johannes Schindelin <johannes.schindelin@gmx.de> Previously, this test script performed essentially three rebases and verified breakages by testing the post-rebase commits' messages. To do so, the rebases were performed multiple times, though, once per commit message to test. This wastes electricity (and CO2) and time. Let's condense the test cases to the essential number: the number of different rebases to validate. On Windows, where the scripted nature of the `--preserve-merges` backend hurts performance rather badly, this reduces the overall runtime in this developer's setup from ~1m to ~28s while still performing the exact same testing as before. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> --- t/t3427-rebase-subtree.sh | 56 ++++++++------------------------------- 1 file changed, 11 insertions(+), 45 deletions(-) diff --git a/t/t3427-rebase-subtree.sh b/t/t3427-rebase-subtree.sh index c0e6a49b61..7d3ba766de 100755 --- a/t/t3427-rebase-subtree.sh +++ b/t/t3427-rebase-subtree.sh @@ -64,65 +64,31 @@ test_expect_success 'setup' ' ' # FAILURE: Does not preserve master4. -test_expect_failure 'Rebase -Xsubtree --preserve-merges --onto commit 4' ' +test_expect_failure '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 && - verbose test "$(commit_message HEAD~)" = "files_subtree/master4" -' - -# FAILURE: Does not preserve master5. -test_expect_failure '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 && git rebase -Xsubtree=files_subtree --preserve-merges --onto files-master master && + verbose test "$(commit_message HEAD~)" = "files_subtree/master4" && 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 -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 && - 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' ' - reset_rebase && - git checkout -b rebase-keep-empty-5 to-rebase && - git rebase -Xsubtree=files_subtree --keep-empty --preserve-merges --onto files-master master && - 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' ' - reset_rebase && - git checkout -b rebase-keep-empty-empty to-rebase && + git checkout -b rebase-keep-empty to-rebase && git rebase -Xsubtree=files_subtree --keep-empty --preserve-merges --onto files-master master && + verbose test "$(commit_message HEAD~2)" = "files_subtree/master4" && + verbose test "$(commit_message HEAD~)" = "files_subtree/master5" && verbose test "$(commit_message HEAD)" = "Empty commit" ' # FAILURE: fatal: Could not parse object -test_expect_failure 'Rebase -Xsubtree --onto commit 4' ' - reset_rebase && - git checkout -b rebase-onto-4 to-rebase && - git rebase -Xsubtree=files_subtree --onto files-master master && - verbose test "$(commit_message HEAD~2)" = "files_subtree/master4" -' - -# FAILURE: fatal: Could not parse object -test_expect_failure 'Rebase -Xsubtree --onto commit 5' ' - reset_rebase && - git checkout -b rebase-onto-5 to-rebase && - git rebase -Xsubtree=files_subtree --onto files-master master && - verbose test "$(commit_message HEAD~)" = "files_subtree/master5" -' -# FAILURE: fatal: Could not parse object -test_expect_failure 'Rebase -Xsubtree --onto empty commit' ' +test_expect_failure 'Rebase -Xsubtree --onto commit' ' reset_rebase && - git checkout -b rebase-onto-empty to-rebase && + git checkout -b rebase-onto to-rebase && git rebase -Xsubtree=files_subtree --onto files-master master && + verbose test "$(commit_message HEAD~2)" = "files_subtree/master4" && + verbose test "$(commit_message HEAD~)" = "files_subtree/master5" && verbose test "$(commit_message HEAD)" = "Empty commit" ' -- gitgitgadget ^ permalink raw reply related [flat|nested] 39+ messages in thread
* [PATCH 05/12] t3427: fix erroneous assumption 2019-07-25 10:11 [PATCH 00/12] rebase -r: support merge strategies other than recursive Johannes Schindelin via GitGitGadget ` (3 preceding siblings ...) 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 ` 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 ` (9 subsequent siblings) 14 siblings, 0 replies; 39+ messages in thread From: Johannes Schindelin via GitGitGadget @ 2019-07-25 10:11 UTC (permalink / raw) To: git; +Cc: Junio C Hamano, Johannes Schindelin From: Johannes Schindelin <johannes.schindelin@gmx.de> Apart from the `setup` test case, `t3427-rebase-subtree.sh` is made up exclusively of demonstrations of breakages. The tricky thing about such demonstrations is that they are often buggy themselves. In this instance, somewhere over the course of the six iterations of the patch that eventually made it into Git's `master` as 5f35900849e (contrib/subtree: Add a test for subtree rebase that loses commits, 2016-06-28), the commit message "files_subtree/master4" was changed to just "master4", but the test cases still expected the old commit message. Let's fix this, at long last. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> --- t/t3427-rebase-subtree.sh | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/t/t3427-rebase-subtree.sh b/t/t3427-rebase-subtree.sh index 7d3ba766de..8c4ddd3408 100755 --- a/t/t3427-rebase-subtree.sh +++ b/t/t3427-rebase-subtree.sh @@ -68,7 +68,7 @@ test_expect_failure 'Rebase -Xsubtree --preserve-merges --onto commit' ' reset_rebase && git checkout -b rebase-preserve-merges to-rebase && git rebase -Xsubtree=files_subtree --preserve-merges --onto files-master master && - verbose test "$(commit_message HEAD~)" = "files_subtree/master4" && + verbose test "$(commit_message HEAD~)" = "master4" && verbose test "$(commit_message HEAD)" = "files_subtree/master5" ' @@ -77,7 +77,7 @@ test_expect_failure 'Rebase -Xsubtree --keep-empty --preserve-merges --onto comm reset_rebase && git checkout -b rebase-keep-empty to-rebase && git rebase -Xsubtree=files_subtree --keep-empty --preserve-merges --onto files-master master && - verbose test "$(commit_message HEAD~2)" = "files_subtree/master4" && + verbose test "$(commit_message HEAD~2)" = "master4" && verbose test "$(commit_message HEAD~)" = "files_subtree/master5" && verbose test "$(commit_message HEAD)" = "Empty commit" ' @@ -87,7 +87,7 @@ test_expect_failure 'Rebase -Xsubtree --onto commit' ' reset_rebase && git checkout -b rebase-onto to-rebase && git rebase -Xsubtree=files_subtree --onto files-master master && - verbose test "$(commit_message HEAD~2)" = "files_subtree/master4" && + verbose test "$(commit_message HEAD~2)" = "master4" && verbose test "$(commit_message HEAD~)" = "files_subtree/master5" && verbose test "$(commit_message HEAD)" = "Empty commit" ' -- gitgitgadget ^ permalink raw reply related [flat|nested] 39+ messages in thread
* [PATCH 06/12] t3427: accommodate for the `rebase --merge` backend having been replaced 2019-07-25 10:11 [PATCH 00/12] rebase -r: support merge strategies other than recursive Johannes Schindelin via GitGitGadget ` (4 preceding siblings ...) 2019-07-25 10:11 ` [PATCH 05/12] t3427: fix erroneous assumption Johannes Schindelin via GitGitGadget @ 2019-07-25 10:11 ` 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 ` (8 subsequent siblings) 14 siblings, 0 replies; 39+ messages in thread From: Johannes Schindelin via GitGitGadget @ 2019-07-25 10:11 UTC (permalink / raw) To: git; +Cc: Junio C Hamano, Johannes Schindelin From: Johannes Schindelin <johannes.schindelin@gmx.de> Since 68aa495b590 (rebase: implement --merge via the interactive machinery, 2018-12-11), the job of the old `--merge` backend is now performed by the `--interactive` backend, too. One consequence is that empty commits are no longer rebased by default. Meaning that the test case that calls `git rebase -Xsubtree` (which used to be handled by the `--merge` backend) now needs to ask explicitly for the empty commit to be rebased. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> --- t/t3427-rebase-subtree.sh | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/t/t3427-rebase-subtree.sh b/t/t3427-rebase-subtree.sh index 8c4ddd3408..b490919c60 100755 --- a/t/t3427-rebase-subtree.sh +++ b/t/t3427-rebase-subtree.sh @@ -83,10 +83,10 @@ test_expect_failure 'Rebase -Xsubtree --keep-empty --preserve-merges --onto comm ' # FAILURE: fatal: Could not parse object -test_expect_failure 'Rebase -Xsubtree --onto commit' ' +test_expect_failure 'Rebase -Xsubtree --keep-empty --onto commit' ' reset_rebase && git checkout -b rebase-onto to-rebase && - git rebase -Xsubtree=files_subtree --onto files-master master && + git rebase -Xsubtree=files_subtree --keep-empty --onto files-master master && verbose test "$(commit_message HEAD~2)" = "master4" && verbose test "$(commit_message HEAD~)" = "files_subtree/master5" && verbose test "$(commit_message HEAD)" = "Empty commit" -- gitgitgadget ^ permalink raw reply related [flat|nested] 39+ messages in thread
* [PATCH 08/12] t3427: mark two test cases as requiring support for `git rebase -p` 2019-07-25 10:11 [PATCH 00/12] rebase -r: support merge strategies other than recursive Johannes Schindelin via GitGitGadget ` (5 preceding siblings ...) 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 ` Johannes Schindelin via GitGitGadget 2019-07-25 10:11 ` [PATCH 07/12] t3427: fix another incorrect assumption Johannes Schindelin via GitGitGadget ` (7 subsequent siblings) 14 siblings, 0 replies; 39+ messages in thread From: Johannes Schindelin via GitGitGadget @ 2019-07-25 10:11 UTC (permalink / raw) To: git; +Cc: Junio C Hamano, Johannes Schindelin From: Johannes Schindelin <johannes.schindelin@gmx.de> To prepare for the long game, where the `--preserve-merges` backend will be dropped eventually, we already introduced the `REBASE_P` prerequisite to allow saving time by skipping the now-almost-pointless test cases that verify that that backend works as expected. Due to the nature of the tests in t3427 (`test_expect_failure` is happy as long as the scriptlet fails, whether it is for the intended reason or because `git-rebase--preserve-merges.sh` was deleted), these two test cases were missed. When running with GIT_TEST_SKIP_REBASE_P=OhYesPlease, this drops the overall run time of t3427 on this developer's machine from ~28s to ~8.5s. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> --- t/t3427-rebase-subtree.sh | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/t/t3427-rebase-subtree.sh b/t/t3427-rebase-subtree.sh index c0ff3241e4..7a37235768 100755 --- a/t/t3427-rebase-subtree.sh +++ b/t/t3427-rebase-subtree.sh @@ -64,7 +64,7 @@ test_expect_success 'setup' ' ' # FAILURE: Does not preserve master4. -test_expect_failure 'Rebase -Xsubtree --preserve-merges --onto commit' ' +test_expect_failure REBASE_P 'Rebase -Xsubtree --preserve-merges --onto commit' ' reset_rebase && git checkout -b rebase-preserve-merges to-rebase && git rebase -Xsubtree=files_subtree --preserve-merges --onto files-master master && @@ -73,7 +73,7 @@ test_expect_failure 'Rebase -Xsubtree --preserve-merges --onto commit' ' ' # FAILURE: Does not preserve master4. -test_expect_failure 'Rebase -Xsubtree --keep-empty --preserve-merges --onto commit' ' +test_expect_failure REBASE_P 'Rebase -Xsubtree --keep-empty --preserve-merges --onto commit' ' reset_rebase && git checkout -b rebase-keep-empty to-rebase && git rebase -Xsubtree=files_subtree --keep-empty --preserve-merges --onto files-master master && -- gitgitgadget ^ permalink raw reply related [flat|nested] 39+ messages in thread
* [PATCH 07/12] t3427: fix another incorrect assumption 2019-07-25 10:11 [PATCH 00/12] rebase -r: support merge strategies other than recursive Johannes Schindelin via GitGitGadget ` (6 preceding siblings ...) 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 ` Johannes Schindelin via GitGitGadget 2019-07-25 10:11 ` [PATCH 09/12] rebase -r: support merge strategies other than `recursive` Johannes Schindelin via GitGitGadget ` (6 subsequent siblings) 14 siblings, 0 replies; 39+ messages in thread From: Johannes Schindelin via GitGitGadget @ 2019-07-25 10:11 UTC (permalink / raw) To: git; +Cc: Junio C Hamano, Johannes Schindelin From: Johannes Schindelin <johannes.schindelin@gmx.de> The test case that concerns `git rebase -Xsubtree` (with the default rebase backend, not with `--preserve-merges`) starts out with a pre-rebase commit history that begins with a commit that introduces three files: master1.t, master2.t and master3.t. This commit was generated by passing a subtree merge commit through `git filter-branch --subdirectory-filter`, so it looks as if this commit really introduces all those files. The commit history onto which this commit is then rebased, however, introduced those files in individual commits. For that reason, the rebase will fail, it _must_ fail, because the first `pick` results in no changes to be committed. Let's fix the test case to expect exactly this situation. With this change, we can mark the original bug that this test case tried to demonstrate as fixed. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> --- t/t3427-rebase-subtree.sh | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/t/t3427-rebase-subtree.sh b/t/t3427-rebase-subtree.sh index b490919c60..c0ff3241e4 100755 --- a/t/t3427-rebase-subtree.sh +++ b/t/t3427-rebase-subtree.sh @@ -82,11 +82,12 @@ test_expect_failure 'Rebase -Xsubtree --keep-empty --preserve-merges --onto comm verbose test "$(commit_message HEAD)" = "Empty commit" ' -# FAILURE: fatal: Could not parse object -test_expect_failure 'Rebase -Xsubtree --keep-empty --onto commit' ' +test_expect_success 'Rebase -Xsubtree --keep-empty --onto commit' ' reset_rebase && git checkout -b rebase-onto to-rebase && - git rebase -Xsubtree=files_subtree --keep-empty --onto files-master master && + test_must_fail git rebase -Xsubtree=files_subtree --keep-empty --onto files-master master && + : first pick results in no changes && + git rebase --continue && verbose test "$(commit_message HEAD~2)" = "master4" && verbose test "$(commit_message HEAD~)" = "files_subtree/master5" && verbose test "$(commit_message HEAD)" = "Empty commit" -- gitgitgadget ^ permalink raw reply related [flat|nested] 39+ messages in thread
* [PATCH 09/12] rebase -r: support merge strategies other than `recursive` 2019-07-25 10:11 [PATCH 00/12] rebase -r: support merge strategies other than recursive Johannes Schindelin via GitGitGadget ` (7 preceding siblings ...) 2019-07-25 10:11 ` [PATCH 07/12] t3427: fix another incorrect assumption Johannes Schindelin via GitGitGadget @ 2019-07-25 10:11 ` 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 ` (5 subsequent siblings) 14 siblings, 0 replies; 39+ messages in thread From: Johannes Schindelin via GitGitGadget @ 2019-07-25 10:11 UTC (permalink / raw) To: git; +Cc: Junio C Hamano, Johannes Schindelin From: Johannes Schindelin <johannes.schindelin@gmx.de> We already support merge strategies in the sequencer, but only for `pick` commands. With this commit, we now also support them in `merge` commands. The approach is simple: if any merge strategy option is specified, or if any merge strategy other than `recursive` is specified, we simply spawn the `git merge` command. Otherwise, we handle the merge in-process just as before. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> --- Documentation/git-rebase.txt | 2 -- builtin/rebase.c | 9 --------- sequencer.c | 14 ++++++++++++-- t/t3422-rebase-incompatible-options.sh | 10 ---------- t/t3430-rebase-merges.sh | 21 +++++++++++++++++++++ 5 files changed, 33 insertions(+), 23 deletions(-) diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt index f5e6ae3907..f67f96425c 100644 --- a/Documentation/git-rebase.txt +++ b/Documentation/git-rebase.txt @@ -543,8 +543,6 @@ In addition, the following pairs of options are incompatible: * --preserve-merges and --interactive * --preserve-merges and --signoff * --preserve-merges and --rebase-merges - * --rebase-merges and --strategy - * --rebase-merges and --strategy-option BEHAVIORAL DIFFERENCES ----------------------- diff --git a/builtin/rebase.c b/builtin/rebase.c index 9c52144fc4..c1ea617125 100644 --- a/builtin/rebase.c +++ b/builtin/rebase.c @@ -1815,15 +1815,6 @@ int cmd_rebase(int argc, const char **argv, const char *prefix) "'--reschedule-failed-exec'")); } - if (options.rebase_merges) { - if (strategy_options.nr) - die(_("cannot combine '--rebase-merges' with " - "'--strategy-option'")); - if (options.strategy) - die(_("cannot combine '--rebase-merges' with " - "'--strategy'")); - } - if (!options.root) { if (argc < 1) { struct branch *branch; diff --git a/sequencer.c b/sequencer.c index 334de14542..d228448cd8 100644 --- a/sequencer.c +++ b/sequencer.c @@ -3256,6 +3256,9 @@ static int do_merge(struct repository *r, struct commit *head_commit, *merge_commit, *i; struct commit_list *bases, *j, *reversed = NULL; struct commit_list *to_merge = NULL, **tail = &to_merge; + const char *strategy = !opts->xopts_nr && + (!opts->strategy || !strcmp(opts->strategy, "recursive")) ? + NULL : opts->strategy; struct merge_options o; int merge_arg_len, oneline_offset, can_fast_forward, ret, k; static struct lock_file lock; @@ -3404,7 +3407,7 @@ static int do_merge(struct repository *r, goto leave_merge; } - if (to_merge->next) { + if (strategy || to_merge->next) { /* Octopus merge */ struct child_process cmd = CHILD_PROCESS_INIT; @@ -3418,7 +3421,14 @@ static int do_merge(struct repository *r, cmd.git_cmd = 1; argv_array_push(&cmd.args, "merge"); argv_array_push(&cmd.args, "-s"); - argv_array_push(&cmd.args, "octopus"); + if (!strategy) + argv_array_push(&cmd.args, "octopus"); + else { + argv_array_push(&cmd.args, strategy); + for (k = 0; k < opts->xopts_nr; k++) + argv_array_pushf(&cmd.args, + "-X%s", opts->xopts[k]); + } argv_array_push(&cmd.args, "--no-edit"); argv_array_push(&cmd.args, "--no-ff"); argv_array_push(&cmd.args, "--no-log"); diff --git a/t/t3422-rebase-incompatible-options.sh b/t/t3422-rebase-incompatible-options.sh index bb78a6ec86..596caf168a 100755 --- a/t/t3422-rebase-incompatible-options.sh +++ b/t/t3422-rebase-incompatible-options.sh @@ -75,14 +75,4 @@ test_expect_success '--preserve-merges incompatible with --rebase-merges' ' test_must_fail git rebase --preserve-merges --rebase-merges A ' -test_expect_success '--rebase-merges incompatible with --strategy' ' - git checkout B^0 && - test_must_fail git rebase --rebase-merges -s resolve A -' - -test_expect_success '--rebase-merges incompatible with --strategy-option' ' - git checkout B^0 && - test_must_fail git rebase --rebase-merges -Xignore-space-change A -' - test_done diff --git a/t/t3430-rebase-merges.sh b/t/t3430-rebase-merges.sh index 42ba5b9f09..8ea6ff3548 100755 --- a/t/t3430-rebase-merges.sh +++ b/t/t3430-rebase-merges.sh @@ -412,4 +412,25 @@ test_expect_success '--continue after resolving conflicts after a merge' ' test_path_is_missing .git/MERGE_HEAD ' +test_expect_success '--rebase-merges with strategies' ' + git checkout -b with-a-strategy F && + test_tick && + git merge -m "Merge conflicting-G" conflicting-G && + + : first, test with a merge strategy option && + git rebase -ir -Xtheirs G && + echo conflicting-G >expect && + test_cmp expect G.t && + + : now, try with a merge strategy other than recursive && + git reset --hard @{1} && + write_script git-merge-override <<-\EOF && + echo overridden$1 >>G.t + git add G.t + EOF + PATH="$PWD:$PATH" git rebase -ir -s override -Xxopt G && + test_write_lines G overridden--xopt >expect && + test_cmp expect G.t +' + test_done -- gitgitgadget ^ permalink raw reply related [flat|nested] 39+ messages in thread
* [PATCH 10/12] t/lib-rebase: prepare for testing `git rebase --rebase-merges` 2019-07-25 10:11 [PATCH 00/12] rebase -r: support merge strategies other than recursive Johannes Schindelin via GitGitGadget ` (8 preceding siblings ...) 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 ` Johannes Schindelin via GitGitGadget 2019-07-26 7:43 ` brian m. carlson 2019-07-25 10:11 ` [PATCH 12/12] rebase -r: do not (re-)generate root commits with `--root` *and* `--onto` Johannes Schindelin via GitGitGadget ` (4 subsequent siblings) 14 siblings, 1 reply; 39+ messages in thread From: Johannes Schindelin via GitGitGadget @ 2019-07-25 10:11 UTC (permalink / raw) To: git; +Cc: Junio C Hamano, Johannes Schindelin From: Johannes Schindelin <johannes.schindelin@gmx.de> The format of the todo list is quite a bit different in the `--rebase-merges` mode; Let's prepare the fake editor to handle those todo lists properly, too. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> --- t/lib-rebase.sh | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/t/lib-rebase.sh b/t/lib-rebase.sh index 7ea30e5006..662a958575 100644 --- a/t/lib-rebase.sh +++ b/t/lib-rebase.sh @@ -44,10 +44,10 @@ set_fake_editor () { rm -f "$1" echo 'rebase -i script before editing:' cat "$1".tmp - action=pick + action=\& for line in $FAKE_LINES; do case $line in - pick|p|squash|s|fixup|f|edit|e|reword|r|drop|d) + pick|p|squash|s|fixup|f|edit|e|reword|r|drop|d|label|l|reset|r|merge|m) action="$line";; exec_*|x_*|break|b) echo "$line" | sed 's/_/ /g' >> "$1";; @@ -61,8 +61,8 @@ set_fake_editor () { echo "$action XXXXXXX False commit" >> "$1" action=pick;; *) - sed -n "${line}s/^pick/$action/p" < "$1".tmp >> "$1" - action=pick;; + sed -n "${line}s/^[a-z][a-z]*/$action/p" < "$1".tmp >> "$1" + action=\&;; esac done echo 'rebase -i script after editing:' -- gitgitgadget ^ permalink raw reply related [flat|nested] 39+ messages in thread
* Re: [PATCH 10/12] t/lib-rebase: prepare for testing `git rebase --rebase-merges` 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 0 siblings, 1 reply; 39+ messages in thread From: brian m. carlson @ 2019-07-26 7:43 UTC (permalink / raw) To: Johannes Schindelin via GitGitGadget Cc: git, Junio C Hamano, Johannes Schindelin [-- Attachment #1: Type: text/plain, Size: 1436 bytes --] On 2019-07-25 at 10:11:22, Johannes Schindelin via GitGitGadget wrote: > From: Johannes Schindelin <johannes.schindelin@gmx.de> > > The format of the todo list is quite a bit different in the > `--rebase-merges` mode; Let's prepare the fake editor to handle those > todo lists properly, too. > > Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> > --- > t/lib-rebase.sh | 8 ++++---- > 1 file changed, 4 insertions(+), 4 deletions(-) > > diff --git a/t/lib-rebase.sh b/t/lib-rebase.sh > index 7ea30e5006..662a958575 100644 > --- a/t/lib-rebase.sh > +++ b/t/lib-rebase.sh > @@ -44,10 +44,10 @@ set_fake_editor () { > rm -f "$1" > echo 'rebase -i script before editing:' > cat "$1".tmp > - action=pick > + action=\& So we set action to "&" so we can use it as the result in the sed expression below… > for line in $FAKE_LINES; do > case $line in > - pick|p|squash|s|fixup|f|edit|e|reword|r|drop|d) > + pick|p|squash|s|fixup|f|edit|e|reword|r|drop|d|label|l|reset|r|merge|m) > action="$line";; > exec_*|x_*|break|b) > echo "$line" | sed 's/_/ /g' >> "$1";; > @@ -61,8 +61,8 @@ set_fake_editor () { > echo "$action XXXXXXX False commit" >> "$1" but then here it doesn't look like "&" is a thing we'd want to use. Is there something I'm missing about this particular case? -- brian m. carlson: Houston, Texas, US OpenPGP: https://keybase.io/bk2204 [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 868 bytes --] ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH 10/12] t/lib-rebase: prepare for testing `git rebase --rebase-merges` 2019-07-26 7:43 ` brian m. carlson @ 2019-07-26 14:01 ` Johannes Schindelin 2019-07-26 21:08 ` brian m. carlson 0 siblings, 1 reply; 39+ messages in thread From: Johannes Schindelin @ 2019-07-26 14:01 UTC (permalink / raw) To: brian m. carlson Cc: Johannes Schindelin via GitGitGadget, git, Junio C Hamano [-- Attachment #1: Type: text/plain, Size: 3694 bytes --] Hi brian, On Fri, 26 Jul 2019, brian m. carlson wrote: > On 2019-07-25 at 10:11:22, Johannes Schindelin via GitGitGadget wrote: > > From: Johannes Schindelin <johannes.schindelin@gmx.de> > > > > The format of the todo list is quite a bit different in the > > `--rebase-merges` mode; Let's prepare the fake editor to handle those > > todo lists properly, too. > > > > Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> > > --- > > t/lib-rebase.sh | 8 ++++---- > > 1 file changed, 4 insertions(+), 4 deletions(-) > > > > diff --git a/t/lib-rebase.sh b/t/lib-rebase.sh > > index 7ea30e5006..662a958575 100644 > > --- a/t/lib-rebase.sh > > +++ b/t/lib-rebase.sh > > @@ -44,10 +44,10 @@ set_fake_editor () { > > rm -f "$1" > > echo 'rebase -i script before editing:' > > cat "$1".tmp > > - action=pick > > + action=\& > > So we set action to "&" so we can use it as the result in the sed > expression below… > > > for line in $FAKE_LINES; do > > case $line in > > - pick|p|squash|s|fixup|f|edit|e|reword|r|drop|d) > > + pick|p|squash|s|fixup|f|edit|e|reword|r|drop|d|label|l|reset|r|merge|m) > > action="$line";; > > exec_*|x_*|break|b) > > echo "$line" | sed 's/_/ /g' >> "$1";; > > @@ -61,8 +61,8 @@ set_fake_editor () { > > echo "$action XXXXXXX False commit" >> "$1" > > but then here it doesn't look like "&" is a thing we'd want to use. Is > there something I'm missing about this particular case? Actually, the part that uses it is not shown in the patch (one of the many, many reasons why I try to discourage patch review and encourage code review instead). The relevant section currently looks somewhat like this: -- snip -- set_fake_editor () { write_script fake-editor.sh <<-\EOF case "$1" in */COMMIT_EDITMSG) test -z "$EXPECT_HEADER_COUNT" || test "$EXPECT_HEADER_COUNT" = "$(sed -n '1s/^# This is a combination of \(.*\) commits\./\1/p' < "$1")" || test "# # GETTEXT POISON #" = "$(sed -n '1p' < "$1")" || exit test -z "$FAKE_COMMIT_MESSAGE" || echo "$FAKE_COMMIT_MESSAGE" > "$1" test -z "$FAKE_COMMIT_AMEND" || echo "$FAKE_COMMIT_AMEND" >> "$1" exit ;; esac test -z "$EXPECT_COUNT" || test "$EXPECT_COUNT" = $(sed -e '/^#/d' -e '/^$/d' < "$1" | wc -l) || exit test -z "$FAKE_LINES" && exit grep -v '^#' < "$1" > "$1".tmp rm -f "$1" echo 'rebase -i script before editing:' cat "$1".tmp action=pick for line in $FAKE_LINES; do case $line in pick|p|squash|s|fixup|f|edit|e|reword|r|drop|d) action="$line";; exec_*|x_*|break|b) echo "$line" | sed 's/_/ /g' >> "$1";; "#") echo '# comment' >> "$1";; ">") echo >> "$1";; bad) action="badcmd";; fakesha) echo "$action XXXXXXX False commit" >> "$1" action=pick;; *) sed -n "${line}s/^pick/$action/p" < "$1".tmp >> "$1" action=pick;; esac done echo 'rebase -i script after editing:' cat "$1" EOF test_set_editor "$(pwd)/fake-editor.sh" } -- snap -- Most importantly, `action` is used here: sed -n "${line}s/^pick/$action/p" < "$1".tmp >> "$1" and I changed it to sed -n "${line}s/^[a-z][a-z]*/$action/p" < "$1".tmp >> "$1" In other words, rather than expecting the lines that are used by the fake editor to start with `pick`, after this patch, the tests expect the todo lists to start with a command consisting of lower-case ASCII letters (which catches `pick`, of course, but also `label`, `reset` and `merge`). After this patch, the fake editor also does not try to replace whatever command it finds by `pick`, but it keeps it as-is instead. Ciao, Dscho ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH 10/12] t/lib-rebase: prepare for testing `git rebase --rebase-merges` 2019-07-26 14:01 ` Johannes Schindelin @ 2019-07-26 21:08 ` brian m. carlson 2019-07-31 11:25 ` Johannes Schindelin 0 siblings, 1 reply; 39+ messages in thread From: brian m. carlson @ 2019-07-26 21:08 UTC (permalink / raw) To: Johannes Schindelin Cc: Johannes Schindelin via GitGitGadget, git, Junio C Hamano [-- Attachment #1: Type: text/plain, Size: 3419 bytes --] On 2019-07-26 at 14:01:03, Johannes Schindelin wrote: > Actually, the part that uses it is not shown in the patch (one of the > many, many reasons why I try to discourage patch review and encourage > code review instead). The relevant section currently looks somewhat like > this: I feel like I may have communicated poorly earlier, so let me retry asking this in a different way. > -- snip -- > set_fake_editor () { > write_script fake-editor.sh <<-\EOF > case "$1" in > */COMMIT_EDITMSG) > test -z "$EXPECT_HEADER_COUNT" || > test "$EXPECT_HEADER_COUNT" = "$(sed -n '1s/^# This is a combination of \(.*\) commits\./\1/p' < "$1")" || > test "# # GETTEXT POISON #" = "$(sed -n '1p' < "$1")" || > exit > test -z "$FAKE_COMMIT_MESSAGE" || echo "$FAKE_COMMIT_MESSAGE" > "$1" > test -z "$FAKE_COMMIT_AMEND" || echo "$FAKE_COMMIT_AMEND" >> "$1" > exit > ;; > esac > test -z "$EXPECT_COUNT" || > test "$EXPECT_COUNT" = $(sed -e '/^#/d' -e '/^$/d' < "$1" | wc -l) || > exit > test -z "$FAKE_LINES" && exit > grep -v '^#' < "$1" > "$1".tmp > rm -f "$1" > echo 'rebase -i script before editing:' > cat "$1".tmp > action=pick I believe you changed this line to "action=\&". > for line in $FAKE_LINES; do > case $line in > pick|p|squash|s|fixup|f|edit|e|reword|r|drop|d) > action="$line";; > exec_*|x_*|break|b) > echo "$line" | sed 's/_/ /g' >> "$1";; > "#") > echo '# comment' >> "$1";; > ">") > echo >> "$1";; > bad) > action="badcmd";; > fakesha) > echo "$action XXXXXXX False commit" >> "$1" And my question was about this line. > action=pick;; > *) > sed -n "${line}s/^pick/$action/p" < "$1".tmp >> "$1" > action=pick;; > esac > done > echo 'rebase -i script after editing:' > cat "$1" > EOF > > test_set_editor "$(pwd)/fake-editor.sh" > } > -- snap -- > > Most importantly, `action` is used here: > > sed -n "${line}s/^pick/$action/p" < "$1".tmp >> "$1" > > and I changed it to > > sed -n "${line}s/^[a-z][a-z]*/$action/p" < "$1".tmp >> "$1" > > In other words, rather than expecting the lines that are used by the > fake editor to start with `pick`, after this patch, the tests expect the > todo lists to start with a command consisting of lower-case ASCII > letters (which catches `pick`, of course, but also `label`, `reset` and > `merge`). > > After this patch, the fake editor also does not try to replace whatever > command it finds by `pick`, but it keeps it as-is instead. Right, that's how I read it, and that part I agree with. I think my question is this: in what case do we execute the "fakesha" case? Are we guaranteed that when we do, action isn't "&"? "&" seems fine for the right-hand side of a sed s-statement, but not as the beginning of a typical line in a sequencer file. I ask because if we're testing a failure case, we want it to fail for the right reason (e.g., the commit doesn't exist), and not because we're producing invalid data. If the answer in this case is, "Well, we'll always have something else before it which will set $action properly," then that's fine. This is test code, so it need not be bulletproof, but I did want to ask. If I'm still misunderstanding something, I apologize. -- brian m. carlson: Houston, Texas, US OpenPGP: https://keybase.io/bk2204 [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 868 bytes --] ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH 10/12] t/lib-rebase: prepare for testing `git rebase --rebase-merges` 2019-07-26 21:08 ` brian m. carlson @ 2019-07-31 11:25 ` Johannes Schindelin 0 siblings, 0 replies; 39+ messages in thread From: Johannes Schindelin @ 2019-07-31 11:25 UTC (permalink / raw) To: brian m. carlson Cc: Johannes Schindelin via GitGitGadget, git, Junio C Hamano Hi brian, On Fri, 26 Jul 2019, brian m. carlson wrote: > On 2019-07-26 at 14:01:03, Johannes Schindelin wrote: > > Actually, the part that uses it is not shown in the patch (one of the > > many, many reasons why I try to discourage patch review and encourage > > code review instead). The relevant section currently looks somewhat like > > this: > > I feel like I may have communicated poorly earlier, so let me retry > asking this in a different way. Actually, your communication was just fine, the misunderstanding was entirely on my side. My apologies. > > -- snip -- > > set_fake_editor () { > > write_script fake-editor.sh <<-\EOF > > case "$1" in > > */COMMIT_EDITMSG) > > test -z "$EXPECT_HEADER_COUNT" || > > test "$EXPECT_HEADER_COUNT" = "$(sed -n '1s/^# This is a combination of \(.*\) commits\./\1/p' < "$1")" || > > test "# # GETTEXT POISON #" = "$(sed -n '1p' < "$1")" || > > exit > > test -z "$FAKE_COMMIT_MESSAGE" || echo "$FAKE_COMMIT_MESSAGE" > "$1" > > test -z "$FAKE_COMMIT_AMEND" || echo "$FAKE_COMMIT_AMEND" >> "$1" > > exit > > ;; > > esac > > test -z "$EXPECT_COUNT" || > > test "$EXPECT_COUNT" = $(sed -e '/^#/d' -e '/^$/d' < "$1" | wc -l) || > > exit > > test -z "$FAKE_LINES" && exit > > grep -v '^#' < "$1" > "$1".tmp > > rm -f "$1" > > echo 'rebase -i script before editing:' > > cat "$1".tmp > > action=pick > > I believe you changed this line to "action=\&". > > > for line in $FAKE_LINES; do > > case $line in > > pick|p|squash|s|fixup|f|edit|e|reword|r|drop|d) > > action="$line";; > > exec_*|x_*|break|b) > > echo "$line" | sed 's/_/ /g' >> "$1";; > > "#") > > echo '# comment' >> "$1";; > > ">") > > echo >> "$1";; > > bad) > > action="badcmd";; > > fakesha) > > echo "$action XXXXXXX False commit" >> "$1" > > And my question was about this line. Right. It would append `& XXXXXXX False commit`, which is not a valid todo command. So something like - echo "$action XXXXXXX False commit" >> "$1" + test \& = "$action" && c=pick || c=$action + echo "$c XXXXXXX False commit" >>"$1" would be needed. However, what makes me really worried now is that our test suite did not have a fit about this. The CI build passes the test suite on Windows, macOS and Linux: https://github.com/gitgitgadget/git/runs/176651523. > > > action=pick;; > > *) > > sed -n "${line}s/^pick/$action/p" < "$1".tmp >> "$1" > > action=pick;; > > esac > > done > > echo 'rebase -i script after editing:' > > cat "$1" > > EOF > > > > test_set_editor "$(pwd)/fake-editor.sh" > > } > > -- snap -- > > > > Most importantly, `action` is used here: > > > > sed -n "${line}s/^pick/$action/p" < "$1".tmp >> "$1" > > > > and I changed it to > > > > sed -n "${line}s/^[a-z][a-z]*/$action/p" < "$1".tmp >> "$1" > > > > In other words, rather than expecting the lines that are used by the > > fake editor to start with `pick`, after this patch, the tests expect the > > todo lists to start with a command consisting of lower-case ASCII > > letters (which catches `pick`, of course, but also `label`, `reset` and > > `merge`). > > > > After this patch, the fake editor also does not try to replace whatever > > command it finds by `pick`, but it keeps it as-is instead. > > Right, that's how I read it, and that part I agree with. I think my > question is this: in what case do we execute the "fakesha" case? Are we > guaranteed that when we do, action isn't "&"? "&" seems fine for the > right-hand side of a sed s-statement, but not as the beginning of a > typical line in a sequencer file. Indeed, the sequencer should throw a real tantrum about this and not even bother to start. But then, the same would hold true for an obviously invalid commit hash. > I ask because if we're testing a failure case, we want it to fail for > the right reason (e.g., the commit doesn't exist), and not because we're > producing invalid data. Indeed. I have even come to the conclusion that our `test_expect_failure` command encourages exactly this type of problem: in the beginning, those test cases might actually be correct, but over time they are prone to fail for all the wrong reasons, because we do not actually test for a specific failure more, we just say that we expect this test case to fail (and that this indicates a bug). > If the answer in this case is, "Well, we'll always have something else > before it which will set $action properly," then that's fine. This is > test code, so it need not be bulletproof, but I did want to ask. I think you are perfectly sane to question this, and to expect me to double check. So, double check I did. Turns out there is a single user of the `fakesha` thing, and it is hidden deep in t3404, prefixed by `test_must_fail`: -- snip -- test_expect_success 'static check of bad SHA-1' ' rebase_setup_and_clean bad-sha && set_fake_editor && test_must_fail env FAKE_LINES="1 2 edit fakesha 3 4 5 #" \ git rebase -i --root 2>actual && test_i18ngrep "edit XXXXXXX False commit" actual && test_i18ngrep "You can fix this with .git rebase --edit-todo.." actual && FAKE_LINES="1 2 4 5 6" git rebase --edit-todo && git rebase --continue && test E = $(git cat-file commit HEAD | sed -ne \$p) ' -- snap -- As you can see, contrary to my expectations it does verify the output. It *also* changes the action to `edit`, which is the reason why there is no `&` ;-) But I think you are correct, I should make sure that the fake editor is still correct with respect to the `pick` command. > If I'm still misunderstanding something, I apologize. I am really impressed and inspired by your gentle language. Thank you for this. Ciao, Dscho ^ permalink raw reply [flat|nested] 39+ messages in thread
* [PATCH 12/12] rebase -r: do not (re-)generate root commits with `--root` *and* `--onto` 2019-07-25 10:11 [PATCH 00/12] rebase -r: support merge strategies other than recursive Johannes Schindelin via GitGitGadget ` (9 preceding siblings ...) 2019-07-25 10:11 ` [PATCH 10/12] t/lib-rebase: prepare for testing `git rebase --rebase-merges` Johannes Schindelin via GitGitGadget @ 2019-07-25 10:11 ` Johannes Schindelin via GitGitGadget 2019-07-25 10:11 ` [PATCH 11/12] t3418: test `rebase -r` with merge strategies Johannes Schindelin via GitGitGadget ` (3 subsequent siblings) 14 siblings, 0 replies; 39+ messages in thread From: Johannes Schindelin via GitGitGadget @ 2019-07-25 10:11 UTC (permalink / raw) To: git; +Cc: Junio C Hamano, Johannes Schindelin From: Johannes Schindelin <johannes.schindelin@gmx.de> When rebasing a complete commit history onto a given commit, it is pretty obvious that the root commits should be rebased on top of said given commit. To test this, let's kill two birds with one stone and add a test case to t3427-rebase-subtree.sh that not only demonstrates that this works, but also that `git rebase -r` works with merge strategies now. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> --- builtin/rebase.c | 7 +++++-- sequencer.c | 4 +++- sequencer.h | 6 ++++++ t/t3427-rebase-subtree.sh | 11 +++++++++++ 4 files changed, 25 insertions(+), 3 deletions(-) diff --git a/builtin/rebase.c b/builtin/rebase.c index c1ea617125..6a789c4421 100644 --- a/builtin/rebase.c +++ b/builtin/rebase.c @@ -62,7 +62,7 @@ struct rebase_options { const char *onto_name; const char *revisions; const char *switch_to; - int root; + int root, root_with_onto; struct object_id *squash_onto; struct commit *restrict_revision; int dont_finish_rebase; @@ -374,6 +374,7 @@ static int run_rebase_interactive(struct rebase_options *opts, flags |= abbreviate_commands ? TODO_LIST_ABBREVIATE_CMDS : 0; flags |= opts->rebase_merges ? TODO_LIST_REBASE_MERGES : 0; flags |= opts->rebase_cousins > 0 ? TODO_LIST_REBASE_COUSINS : 0; + flags |= opts->root_with_onto ? TODO_LIST_ROOT_WITH_ONTO : 0; flags |= command == ACTION_SHORTEN_OIDS ? TODO_LIST_SHORTEN_IDS : 0; switch (command) { @@ -1845,7 +1846,9 @@ int cmd_rebase(int argc, const char **argv, const char *prefix) options.squash_onto = &squash_onto; options.onto_name = squash_onto_name = xstrdup(oid_to_hex(&squash_onto)); - } + } else + options.root_with_onto = 1; + options.upstream_name = NULL; options.upstream = NULL; if (argc > 1) diff --git a/sequencer.c b/sequencer.c index d228448cd8..ca119c84e5 100644 --- a/sequencer.c +++ b/sequencer.c @@ -4440,6 +4440,7 @@ static int make_script_with_merges(struct pretty_print_context *pp, { int keep_empty = flags & TODO_LIST_KEEP_EMPTY; int rebase_cousins = flags & TODO_LIST_REBASE_COUSINS; + int root_with_onto = flags & TODO_LIST_ROOT_WITH_ONTO; struct strbuf buf = STRBUF_INIT, oneline = STRBUF_INIT; struct strbuf label = STRBUF_INIT; struct commit_list *commits = NULL, **tail = &commits, *iter; @@ -4606,7 +4607,8 @@ static int make_script_with_merges(struct pretty_print_context *pp, if (!commit) strbuf_addf(out, "%s %s\n", cmd_reset, - rebase_cousins ? "onto" : "[new root]"); + rebase_cousins || root_with_onto ? + "onto" : "[new root]"); else { const char *to = NULL; diff --git a/sequencer.h b/sequencer.h index 0c494b83d4..d506081d3c 100644 --- a/sequencer.h +++ b/sequencer.h @@ -142,6 +142,12 @@ int sequencer_remove_state(struct replay_opts *opts); */ #define TODO_LIST_REBASE_COUSINS (1U << 4) #define TODO_LIST_APPEND_TODO_HELP (1U << 5) +/* + * When generating a script that rebases merges with `--root` *and* with + * `--onto`, we do not want to re-generate the root commits. + */ +#define TODO_LIST_ROOT_WITH_ONTO (1U << 6) + int sequencer_make_script(struct repository *r, struct strbuf *out, int argc, const char **argv, unsigned flags); diff --git a/t/t3427-rebase-subtree.sh b/t/t3427-rebase-subtree.sh index 7a37235768..39e348de16 100755 --- a/t/t3427-rebase-subtree.sh +++ b/t/t3427-rebase-subtree.sh @@ -93,4 +93,15 @@ test_expect_success 'Rebase -Xsubtree --keep-empty --onto commit' ' verbose test "$(commit_message HEAD)" = "Empty commit" ' +test_expect_success 'Rebase -Xsubtree --keep-empty --rebase-merges --onto commit' ' + reset_rebase && + git checkout -b rebase-merges-onto to-rebase && + test_must_fail git rebase -Xsubtree=files_subtree --keep-empty --rebase-merges --onto files-master --root && + : first pick results in no changes && + git rebase --continue && + verbose test "$(commit_message HEAD~2)" = "master4" && + verbose test "$(commit_message HEAD~)" = "files_subtree/master5" && + verbose test "$(commit_message HEAD)" = "Empty commit" +' + test_done -- gitgitgadget ^ permalink raw reply related [flat|nested] 39+ messages in thread
* [PATCH 11/12] t3418: test `rebase -r` with merge strategies 2019-07-25 10:11 [PATCH 00/12] rebase -r: support merge strategies other than recursive Johannes Schindelin via GitGitGadget ` (10 preceding siblings ...) 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 ` Johannes Schindelin via GitGitGadget 2019-07-25 17:10 ` [PATCH 00/12] rebase -r: support merge strategies other than recursive Junio C Hamano ` (2 subsequent siblings) 14 siblings, 0 replies; 39+ messages in thread From: Johannes Schindelin via GitGitGadget @ 2019-07-25 10:11 UTC (permalink / raw) To: git; +Cc: Junio C Hamano, Johannes Schindelin From: Johannes Schindelin <johannes.schindelin@gmx.de> There is a test case in this script that verifies that `git rebase --preserve-merges` works all right with non-default merge strategies or non-default merge strategy options. Now that `git rebase --rebase-merges` learned about merge strategies, let's copy-edit this test case to verify that that works as intended, too. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> --- t/t3418-rebase-continue.sh | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/t/t3418-rebase-continue.sh b/t/t3418-rebase-continue.sh index bdaa511bb0..fbf9addfd1 100755 --- a/t/t3418-rebase-continue.sh +++ b/t/t3418-rebase-continue.sh @@ -120,6 +120,20 @@ test_expect_success REBASE_P 'rebase passes merge strategy options correctly' ' git rebase --continue ' +test_expect_success 'rebase -r passes merge strategy options correctly' ' + rm -fr .git/rebase-* && + git reset --hard commit-new-file-F3-on-topic-branch && + test_commit merge-theirs && + git reset --hard HEAD^ && + test_commit some-other-commit && + test_tick && + git merge --no-ff merge-theirs && + FAKE_LINES="1 3 edit 4 5 7 8 9" git rebase -i -f -r -m \ + -s recursive --strategy-option=theirs HEAD~2 && + test_commit force-change-ours && + git rebase --continue +' + test_expect_success '--skip after failed fixup cleans commit message' ' test_when_finished "test_might_fail git rebase --abort" && git checkout -b with-conflicting-fixup && -- gitgitgadget ^ permalink raw reply related [flat|nested] 39+ messages in thread
* Re: [PATCH 00/12] rebase -r: support merge strategies other than recursive 2019-07-25 10:11 [PATCH 00/12] rebase -r: support merge strategies other than recursive Johannes Schindelin via GitGitGadget ` (11 preceding siblings ...) 2019-07-25 10:11 ` [PATCH 11/12] t3418: test `rebase -r` with merge strategies Johannes Schindelin via GitGitGadget @ 2019-07-25 17:10 ` Junio C Hamano 2019-07-26 7:52 ` brian m. carlson 2019-07-31 15:18 ` [PATCH v2 00/16] " Johannes Schindelin via GitGitGadget 14 siblings, 0 replies; 39+ messages in thread From: Junio C Hamano @ 2019-07-25 17:10 UTC (permalink / raw) To: Johannes Schindelin via GitGitGadget; +Cc: git "Johannes Schindelin via GitGitGadget" <gitgitgadget@gmail.com> writes: > ... 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. Very nice. > base-commit: 082ef75b7bfc90ac236afbb857a9552a026832b8 > Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-294%2Fdscho%2Frebase-r-with-strategies-v1 > Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-294/dscho/rebase-r-with-strategies-v1 > Pull-Request: https://github.com/gitgitgadget/git/pull/294 ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH 00/12] rebase -r: support merge strategies other than recursive 2019-07-25 10:11 [PATCH 00/12] rebase -r: support merge strategies other than recursive Johannes Schindelin via GitGitGadget ` (12 preceding siblings ...) 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 ` [PATCH v2 00/16] " Johannes Schindelin via GitGitGadget 14 siblings, 0 replies; 39+ messages in thread From: brian m. carlson @ 2019-07-26 7:52 UTC (permalink / raw) To: Johannes Schindelin via GitGitGadget; +Cc: git, Junio C Hamano [-- Attachment #1: Type: text/plain, Size: 504 bytes --] On 2019-07-25 at 10:11:14, Johannes Schindelin via GitGitGadget wrote: > 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. This looks like a great improvement. I'm glad to see --rebase-merges gaining feature parity with --preserve-merges. -- brian m. carlson: Houston, Texas, US OpenPGP: https://keybase.io/bk2204 [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 868 bytes --] ^ permalink raw reply [flat|nested] 39+ messages in thread
* [PATCH v2 00/16] rebase -r: support merge strategies other than recursive 2019-07-25 10:11 [PATCH 00/12] rebase -r: support merge strategies other than recursive Johannes Schindelin via GitGitGadget ` (13 preceding siblings ...) 2019-07-26 7:52 ` brian m. carlson @ 2019-07-31 15:18 ` Johannes Schindelin via GitGitGadget 2019-07-31 15:18 ` [PATCH v2 01/16] Drop unused git-rebase--am.sh Johannes Schindelin via GitGitGadget ` (16 more replies) 14 siblings, 17 replies; 39+ messages in thread From: Johannes Schindelin via GitGitGadget @ 2019-07-31 15:18 UTC (permalink / raw) To: git; +Cc: brian m. carlson, Junio C Hamano 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 ^ permalink raw reply [flat|nested] 39+ messages in thread
* [PATCH v2 01/16] Drop unused git-rebase--am.sh 2019-07-31 15:18 ` [PATCH v2 00/16] " Johannes Schindelin via GitGitGadget @ 2019-07-31 15:18 ` Johannes Schindelin via GitGitGadget 2019-07-31 15:18 ` [PATCH v2 02/16] t3400: stop referring to the scripted rebase Johannes Schindelin via GitGitGadget ` (15 subsequent siblings) 16 siblings, 0 replies; 39+ messages in thread From: Johannes Schindelin via GitGitGadget @ 2019-07-31 15:18 UTC (permalink / raw) To: git; +Cc: brian m. carlson, Junio C Hamano, Johannes Schindelin From: Johannes Schindelin <johannes.schindelin@gmx.de> Since 21853626ea (built-in rebase: call `git am` directly, 2019-01-18), the built-in rebase already uses the built-in `git am` directly. Now that d03ebd411c (rebase: remove the rebase.useBuiltin setting, 2019-03-18) even removed the scripted rebase, there is no longer any user of `git-rebase--am.sh`, so let's just remove it. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com> --- .gitignore | 1 - Makefile | 1 - builtin/rebase.c | 4 --- git-rebase--am.sh | 85 ----------------------------------------------- 4 files changed, 91 deletions(-) delete mode 100644 git-rebase--am.sh diff --git a/.gitignore b/.gitignore index 2374f77a1a..875f3fc6e8 100644 --- a/.gitignore +++ b/.gitignore @@ -122,7 +122,6 @@ /git-range-diff /git-read-tree /git-rebase -/git-rebase--am /git-rebase--common /git-rebase--interactive /git-rebase--preserve-merges diff --git a/Makefile b/Makefile index 8a7e235352..63e1973333 100644 --- a/Makefile +++ b/Makefile @@ -624,7 +624,6 @@ SCRIPT_SH += git-web--browse.sh SCRIPT_LIB += git-mergetool--lib SCRIPT_LIB += git-parse-remote -SCRIPT_LIB += git-rebase--am SCRIPT_LIB += git-rebase--common SCRIPT_LIB += git-rebase--preserve-merges SCRIPT_LIB += git-sh-setup diff --git a/builtin/rebase.c b/builtin/rebase.c index 21681a551b..4dd76ee576 100644 --- a/builtin/rebase.c +++ b/builtin/rebase.c @@ -1153,10 +1153,6 @@ static int run_specific_rebase(struct rebase_options *opts, enum action action) } switch (opts->type) { - case REBASE_AM: - backend = "git-rebase--am"; - backend_func = "git_rebase__am"; - break; case REBASE_PRESERVE_MERGES: backend = "git-rebase--preserve-merges"; backend_func = "git_rebase__preserve_merges"; diff --git a/git-rebase--am.sh b/git-rebase--am.sh deleted file mode 100644 index 6416716ee6..0000000000 --- a/git-rebase--am.sh +++ /dev/null @@ -1,85 +0,0 @@ -# This shell script fragment is sourced by git-rebase to implement -# its default, fast, patch-based, non-interactive mode. -# -# Copyright (c) 2010 Junio C Hamano. -# - -git_rebase__am () { - -case "$action" in -continue) - git am --resolved --resolvemsg="$resolvemsg" \ - ${gpg_sign_opt:+"$gpg_sign_opt"} && - move_to_original_branch - return - ;; -skip) - git am --skip --resolvemsg="$resolvemsg" && - move_to_original_branch - return - ;; -show-current-patch) - exec git am --show-current-patch - ;; -esac - -if test -z "$rebase_root" - # this is now equivalent to ! -z "$upstream" -then - revisions=$upstream...$orig_head -else - revisions=$onto...$orig_head -fi - -ret=0 -rm -f "$GIT_DIR/rebased-patches" - -git format-patch -k --stdout --full-index --cherry-pick --right-only \ - --src-prefix=a/ --dst-prefix=b/ --no-renames --no-cover-letter \ - --pretty=mboxrd --topo-order \ - $git_format_patch_opt \ - "$revisions" ${restrict_revision+^$restrict_revision} \ - >"$GIT_DIR/rebased-patches" -ret=$? - -if test 0 != $ret -then - rm -f "$GIT_DIR/rebased-patches" - case "$head_name" in - refs/heads/*) - git checkout -q "$head_name" - ;; - *) - git checkout -q "$orig_head" - ;; - esac - - cat >&2 <<-EOF - - git encountered an error while preparing the patches to replay - these revisions: - - $revisions - - As a result, git cannot rebase them. - EOF - return $ret -fi - -git am $git_am_opt --rebasing --resolvemsg="$resolvemsg" \ - --patch-format=mboxrd \ - $allow_rerere_autoupdate \ - ${gpg_sign_opt:+"$gpg_sign_opt"} <"$GIT_DIR/rebased-patches" -ret=$? - -rm -f "$GIT_DIR/rebased-patches" - -if test 0 != $ret -then - test -d "$state_dir" && write_basic_state - return $ret -fi - -move_to_original_branch - -} -- gitgitgadget ^ permalink raw reply related [flat|nested] 39+ messages in thread
* [PATCH v2 02/16] t3400: stop referring to the scripted rebase 2019-07-31 15:18 ` [PATCH v2 00/16] " Johannes Schindelin via GitGitGadget 2019-07-31 15:18 ` [PATCH v2 01/16] Drop unused git-rebase--am.sh Johannes Schindelin via GitGitGadget @ 2019-07-31 15:18 ` 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 ` (14 subsequent siblings) 16 siblings, 0 replies; 39+ messages in thread From: Johannes Schindelin via GitGitGadget @ 2019-07-31 15:18 UTC (permalink / raw) To: git; +Cc: brian m. carlson, Junio C Hamano, Johannes Schindelin From: Johannes Schindelin <johannes.schindelin@gmx.de> One test case's title mentioned the then-current implementation detail that the `--am` backend was implemented in `git-rebase--am.sh`. This is no longer the case, so let's update the title to reflect the current reality. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com> --- t/t3400-rebase.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/t/t3400-rebase.sh b/t/t3400-rebase.sh index 42f147858d..80b23fd326 100755 --- a/t/t3400-rebase.sh +++ b/t/t3400-rebase.sh @@ -285,7 +285,7 @@ EOF test_cmp From_.msg out ' -test_expect_success 'rebase--am.sh and --show-current-patch' ' +test_expect_success 'rebase --am and --show-current-patch' ' test_create_repo conflict-apply && ( cd conflict-apply && -- gitgitgadget ^ permalink raw reply related [flat|nested] 39+ messages in thread
* [PATCH v2 03/16] .gitignore: there is no longer a built-in `git-rebase--interactive` 2019-07-31 15:18 ` [PATCH v2 00/16] " Johannes Schindelin via GitGitGadget 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 ` 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 ` (13 subsequent siblings) 16 siblings, 0 replies; 39+ messages in thread From: Johannes Schindelin via GitGitGadget @ 2019-07-31 15:18 UTC (permalink / raw) To: git; +Cc: brian m. carlson, Junio C Hamano, Johannes Schindelin From: Johannes Schindelin <johannes.schindelin@gmx.de> This went away in 0609b741a4 (rebase -i: combine rebase--interactive.c with rebase.c, 2019-04-17). Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com> --- .gitignore | 1 - 1 file changed, 1 deletion(-) diff --git a/.gitignore b/.gitignore index 875f3fc6e8..bcee4fda81 100644 --- a/.gitignore +++ b/.gitignore @@ -123,7 +123,6 @@ /git-read-tree /git-rebase /git-rebase--common -/git-rebase--interactive /git-rebase--preserve-merges /git-receive-pack /git-reflog -- gitgitgadget ^ permalink raw reply related [flat|nested] 39+ messages in thread
* [PATCH v2 04/16] sequencer: the `am` and `rebase--interactive` scripts are gone 2019-07-31 15:18 ` [PATCH v2 00/16] " Johannes Schindelin via GitGitGadget ` (2 preceding siblings ...) 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 ` 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 ` (12 subsequent siblings) 16 siblings, 0 replies; 39+ messages in thread From: Johannes Schindelin via GitGitGadget @ 2019-07-31 15:18 UTC (permalink / raw) To: git; +Cc: brian m. carlson, Junio C Hamano, Johannes Schindelin From: Johannes Schindelin <johannes.schindelin@gmx.de> Update a code comment that referred to those files as if they were still there. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com> --- sequencer.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sequencer.c b/sequencer.c index f88a97fb10..334de14542 100644 --- a/sequencer.c +++ b/sequencer.c @@ -767,7 +767,7 @@ static int parse_key_value_squoted(char *buf, struct string_list *list) * GIT_AUTHOR_DATE='$author_date' * * where $author_name, $author_email and $author_date are quoted. We are strict - * with our parsing, as the file was meant to be eval'd in the old + * with our parsing, as the file was meant to be eval'd in the now-removed * git-am.sh/git-rebase--interactive.sh scripts, and thus if the file differs * from what this function expects, it is better to bail out than to do * something that the user does not expect. -- gitgitgadget ^ permalink raw reply related [flat|nested] 39+ messages in thread
* [PATCH v2 05/16] rebase: fold git-rebase--common into the -p backend 2019-07-31 15:18 ` [PATCH v2 00/16] " Johannes Schindelin via GitGitGadget ` (3 preceding siblings ...) 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 ` Johannes Schindelin via GitGitGadget 2019-07-31 15:18 ` [PATCH v2 06/16] t3427: add a clarifying comment Johannes Schindelin via GitGitGadget ` (11 subsequent siblings) 16 siblings, 0 replies; 39+ messages in thread From: Johannes Schindelin via GitGitGadget @ 2019-07-31 15:18 UTC (permalink / raw) To: git; +Cc: brian m. carlson, Junio C Hamano, Johannes Schindelin From: Johannes Schindelin <johannes.schindelin@gmx.de> The only remaining scripted part of `git rebase` is the `--preserve-merges` backend. Meaning: there is little reason to keep the "library of common rebase functions" as a separate file. While moving the functions to `git-rebase--preserve-merges.sh`, we also drop the `move_to_original_branch` function that is no longer used. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com> --- .gitignore | 1 - Makefile | 1 - builtin/rebase.c | 3 +- git-rebase--common.sh | 69 ---------------------------------- git-rebase--preserve-merges.sh | 55 +++++++++++++++++++++++++++ 5 files changed, 56 insertions(+), 73 deletions(-) delete mode 100644 git-rebase--common.sh diff --git a/.gitignore b/.gitignore index bcee4fda81..4470d7cfc0 100644 --- a/.gitignore +++ b/.gitignore @@ -122,7 +122,6 @@ /git-range-diff /git-read-tree /git-rebase -/git-rebase--common /git-rebase--preserve-merges /git-receive-pack /git-reflog diff --git a/Makefile b/Makefile index 63e1973333..6c3bfb1733 100644 --- a/Makefile +++ b/Makefile @@ -624,7 +624,6 @@ SCRIPT_SH += git-web--browse.sh SCRIPT_LIB += git-mergetool--lib SCRIPT_LIB += git-parse-remote -SCRIPT_LIB += git-rebase--common SCRIPT_LIB += git-rebase--preserve-merges SCRIPT_LIB += git-sh-setup SCRIPT_LIB += git-sh-i18n diff --git a/builtin/rebase.c b/builtin/rebase.c index 4dd76ee576..74a60e8c83 100644 --- a/builtin/rebase.c +++ b/builtin/rebase.c @@ -1163,8 +1163,7 @@ static int run_specific_rebase(struct rebase_options *opts, enum action action) } strbuf_addf(&script_snippet, - ". git-sh-setup && . git-rebase--common &&" - " . %s && %s", backend, backend_func); + ". git-sh-setup && . %s && %s", backend, backend_func); argv[0] = script_snippet.buf; status = run_command_v_opt(argv, RUN_USING_SHELL); diff --git a/git-rebase--common.sh b/git-rebase--common.sh deleted file mode 100644 index f00e13e5d0..0000000000 --- a/git-rebase--common.sh +++ /dev/null @@ -1,69 +0,0 @@ - -resolvemsg=" -$(gettext 'Resolve all conflicts manually, mark them as resolved with -"git add/rm <conflicted_files>", then run "git rebase --continue". -You can instead skip this commit: run "git rebase --skip". -To abort and get back to the state before "git rebase", run "git rebase --abort".') -" - -write_basic_state () { - echo "$head_name" > "$state_dir"/head-name && - echo "$onto" > "$state_dir"/onto && - echo "$orig_head" > "$state_dir"/orig-head && - test t = "$GIT_QUIET" && : > "$state_dir"/quiet - test t = "$verbose" && : > "$state_dir"/verbose - test -n "$strategy" && echo "$strategy" > "$state_dir"/strategy - test -n "$strategy_opts" && echo "$strategy_opts" > \ - "$state_dir"/strategy_opts - test -n "$allow_rerere_autoupdate" && echo "$allow_rerere_autoupdate" > \ - "$state_dir"/allow_rerere_autoupdate - test -n "$gpg_sign_opt" && echo "$gpg_sign_opt" > "$state_dir"/gpg_sign_opt - test -n "$signoff" && echo "$signoff" >"$state_dir"/signoff - test -n "$reschedule_failed_exec" && : > "$state_dir"/reschedule-failed-exec -} - -apply_autostash () { - if test -f "$state_dir/autostash" - then - stash_sha1=$(cat "$state_dir/autostash") - if git stash apply $stash_sha1 >/dev/null 2>&1 - then - echo "$(gettext 'Applied autostash.')" >&2 - else - git stash store -m "autostash" -q $stash_sha1 || - die "$(eval_gettext "Cannot store \$stash_sha1")" - gettext 'Applying autostash resulted in conflicts. -Your changes are safe in the stash. -You can run "git stash pop" or "git stash drop" at any time. -' >&2 - fi - fi -} - -move_to_original_branch () { - case "$head_name" in - refs/*) - message="rebase finished: $head_name onto $onto" - git update-ref -m "$message" \ - $head_name $(git rev-parse HEAD) $orig_head && - git symbolic-ref \ - -m "rebase finished: returning to $head_name" \ - HEAD $head_name || - die "$(eval_gettext "Could not move back to \$head_name")" - ;; - esac -} - -output () { - case "$verbose" in - '') - output=$("$@" 2>&1 ) - status=$? - test $status != 0 && printf "%s\n" "$output" - return $status - ;; - *) - "$@" - ;; - esac -} diff --git a/git-rebase--preserve-merges.sh b/git-rebase--preserve-merges.sh index afbb65765d..dec90e9af6 100644 --- a/git-rebase--preserve-merges.sh +++ b/git-rebase--preserve-merges.sh @@ -77,6 +77,61 @@ rewritten_pending="$state_dir"/rewritten-pending # and leaves CR at the end instead. cr=$(printf "\015") +resolvemsg=" +$(gettext 'Resolve all conflicts manually, mark them as resolved with +"git add/rm <conflicted_files>", then run "git rebase --continue". +You can instead skip this commit: run "git rebase --skip". +To abort and get back to the state before "git rebase", run "git rebase --abort".') +" + +write_basic_state () { + echo "$head_name" > "$state_dir"/head-name && + echo "$onto" > "$state_dir"/onto && + echo "$orig_head" > "$state_dir"/orig-head && + test t = "$GIT_QUIET" && : > "$state_dir"/quiet + test t = "$verbose" && : > "$state_dir"/verbose + test -n "$strategy" && echo "$strategy" > "$state_dir"/strategy + test -n "$strategy_opts" && echo "$strategy_opts" > \ + "$state_dir"/strategy_opts + test -n "$allow_rerere_autoupdate" && echo "$allow_rerere_autoupdate" > \ + "$state_dir"/allow_rerere_autoupdate + test -n "$gpg_sign_opt" && echo "$gpg_sign_opt" > "$state_dir"/gpg_sign_opt + test -n "$signoff" && echo "$signoff" >"$state_dir"/signoff + test -n "$reschedule_failed_exec" && : > "$state_dir"/reschedule-failed-exec +} + +apply_autostash () { + if test -f "$state_dir/autostash" + then + stash_sha1=$(cat "$state_dir/autostash") + if git stash apply $stash_sha1 >/dev/null 2>&1 + then + echo "$(gettext 'Applied autostash.')" >&2 + else + git stash store -m "autostash" -q $stash_sha1 || + die "$(eval_gettext "Cannot store \$stash_sha1")" + gettext 'Applying autostash resulted in conflicts. +Your changes are safe in the stash. +You can run "git stash pop" or "git stash drop" at any time. +' >&2 + fi + fi +} + +output () { + case "$verbose" in + '') + output=$("$@" 2>&1 ) + status=$? + test $status != 0 && printf "%s\n" "$output" + return $status + ;; + *) + "$@" + ;; + esac +} + strategy_args=${strategy:+--strategy=$strategy} test -n "$strategy_opts" && eval ' -- gitgitgadget ^ permalink raw reply related [flat|nested] 39+ messages in thread
* [PATCH v2 06/16] t3427: add a clarifying comment 2019-07-31 15:18 ` [PATCH v2 00/16] " Johannes Schindelin via GitGitGadget ` (4 preceding siblings ...) 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 ` Johannes Schindelin via GitGitGadget 2019-07-31 15:18 ` [PATCH v2 07/16] t3427: simplify the `setup` test case significantly Johannes Schindelin via GitGitGadget ` (10 subsequent siblings) 16 siblings, 0 replies; 39+ messages in thread From: Johannes Schindelin via GitGitGadget @ 2019-07-31 15:18 UTC (permalink / raw) To: git; +Cc: brian m. carlson, Junio C Hamano, Johannes Schindelin From: Johannes Schindelin <johannes.schindelin@gmx.de> The flow of this test script is outright confusing, and to start the endeavor to address that, let's describe what this test is all about, and how it tries to do it. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> --- t/t3427-rebase-subtree.sh | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/t/t3427-rebase-subtree.sh b/t/t3427-rebase-subtree.sh index d8640522a0..3a2ae7b55d 100755 --- a/t/t3427-rebase-subtree.sh +++ b/t/t3427-rebase-subtree.sh @@ -11,6 +11,34 @@ commit_message() { git log --pretty=format:%s -1 "$1" } +# There are a few bugs in the rebase with regards to the subtree strategy, and +# this test script tries to document them. First, the following commit history +# is generated (the onelines are shown, time flows from left to right): +# +# master1 - master2 - master3 +# \ +# README ---------------------- Add subproject master - master4 - files_subtree/master5 +# +# Where the merge moves the files master[123].t into the subdirectory +# files_subtree/ and master4 as well as files_subtree/master5 add files to that +# directory directly. +# +# Then, in subsequent test cases, `git filter-branch` is used to distill just +# the commits that touch files_subtree/. To give it a final pre-rebase touch, +# an empty commit is added on top. The pre-rebase commit history looks like +# this: +# +# Add subproject master - master4 - files_subtree/master5 - Empty commit +# +# where the root commit adds three files: master1.t, master2.t and master3.t. +# +# This commit history is then rebased onto `master3` with the +# `-Xsubtree=files_subtree` option in three different ways: +# +# 1. using `--preserve-merges` +# 2. using `--preserve-merges` and --keep-empty +# 3. without specifying a rebase backend + test_expect_success 'setup' ' test_commit README && mkdir files && -- gitgitgadget ^ permalink raw reply related [flat|nested] 39+ messages in thread
* [PATCH v2 07/16] t3427: simplify the `setup` test case significantly 2019-07-31 15:18 ` [PATCH v2 00/16] " Johannes Schindelin via GitGitGadget ` (5 preceding siblings ...) 2019-07-31 15:18 ` [PATCH v2 06/16] t3427: add a clarifying comment Johannes Schindelin via GitGitGadget @ 2019-07-31 15:18 ` 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 ` (9 subsequent siblings) 16 siblings, 0 replies; 39+ messages in thread From: Johannes Schindelin via GitGitGadget @ 2019-07-31 15:18 UTC (permalink / raw) To: git; +Cc: brian m. carlson, Junio C Hamano, Johannes Schindelin From: Johannes Schindelin <johannes.schindelin@gmx.de> It still does the very same thing as before, but expresses it in a much more succinct (and still quite readable) manner. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> --- t/t3427-rebase-subtree.sh | 36 +++++++++++++++--------------------- 1 file changed, 15 insertions(+), 21 deletions(-) diff --git a/t/t3427-rebase-subtree.sh b/t/t3427-rebase-subtree.sh index 3a2ae7b55d..36c4f92e06 100755 --- a/t/t3427-rebase-subtree.sh +++ b/t/t3427-rebase-subtree.sh @@ -41,27 +41,21 @@ commit_message() { test_expect_success 'setup' ' test_commit README && - mkdir files && - ( - cd files && - git init && - test_commit master1 && - test_commit master2 && - test_commit master3 - ) && - git fetch files master && - git branch files-master FETCH_HEAD && - git read-tree --prefix=files_subtree files-master && - git checkout -- files_subtree && - tree=$(git write-tree) && - head=$(git rev-parse HEAD) && - rev=$(git rev-parse --verify files-master^0) && - commit=$(git commit-tree -p $head -p $rev -m "Add subproject master" $tree) && - git update-ref HEAD $commit && - ( - cd files_subtree && - test_commit master4 - ) && + + git init files && + test_commit -C files master1 && + test_commit -C files master2 && + test_commit -C files master3 && + + : perform subtree merge into files_subtree/ && + git fetch files refs/heads/master:refs/heads/files-master && + git merge -s ours --no-commit --allow-unrelated-histories \ + files-master && + git read-tree --prefix=files_subtree -u files-master && + git commit -m "Add subproject master" && + + : add two extra commits to rebase && + test_commit -C files_subtree master4 && test_commit files_subtree/master5 ' -- gitgitgadget ^ permalink raw reply related [flat|nested] 39+ messages in thread
* [PATCH v2 08/16] t3427: move the `filter-branch` invocation into the `setup` case 2019-07-31 15:18 ` [PATCH v2 00/16] " Johannes Schindelin via GitGitGadget ` (6 preceding siblings ...) 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 ` 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 ` (8 subsequent siblings) 16 siblings, 0 replies; 39+ messages in thread From: Johannes Schindelin via GitGitGadget @ 2019-07-31 15:18 UTC (permalink / raw) To: git; +Cc: brian m. carlson, Junio C Hamano, Johannes Schindelin From: Johannes Schindelin <johannes.schindelin@gmx.de> The step to prepare a pre-rebase commit history is _identical_ in _all_ of the test cases (except of course the `setup` case). It should therefore clearly a part of the `setup` test case instead. As the `git filter-branch` command is quite costly on platforms where Unix shell scripting is simply slow (meaning: on Windows), this shaves off a noticeable part of the runtime: in this developer's setup, the time was reduced from ~1m25s to ~1m. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> --- t/t3427-rebase-subtree.sh | 38 +++++++++++++------------------------- 1 file changed, 13 insertions(+), 25 deletions(-) diff --git a/t/t3427-rebase-subtree.sh b/t/t3427-rebase-subtree.sh index 36c4f92e06..64ba95f3e0 100755 --- a/t/t3427-rebase-subtree.sh +++ b/t/t3427-rebase-subtree.sh @@ -56,16 +56,18 @@ test_expect_success 'setup' ' : add two extra commits to rebase && test_commit -C files_subtree master4 && - test_commit files_subtree/master5 + test_commit files_subtree/master5 && + + git checkout -b to-rebase && + git filter-branch --prune-empty -f --subdirectory-filter files_subtree && + git commit -m "Empty commit" --allow-empty ' # FAILURE: Does not preserve master4. 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 && - git commit -m "Empty commit" --allow-empty && + git checkout -b rebase-preserve-merges-4 to-rebase && git rebase -Xsubtree=files_subtree --preserve-merges --onto files-master master && verbose test "$(commit_message HEAD~)" = "files_subtree/master4" ' @@ -74,9 +76,7 @@ test_expect_failure REBASE_P \ 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 && - git commit -m "Empty commit" --allow-empty && + git checkout -b rebase-preserve-merges-5 to-rebase && git rebase -Xsubtree=files_subtree --preserve-merges --onto files-master master && verbose test "$(commit_message HEAD)" = "files_subtree/master5" ' @@ -85,9 +85,7 @@ test_expect_failure REBASE_P \ 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 && - git commit -m "Empty commit" --allow-empty && + git checkout -b rebase-keep-empty-4 to-rebase && git rebase -Xsubtree=files_subtree --keep-empty --preserve-merges --onto files-master master && verbose test "$(commit_message HEAD~2)" = "files_subtree/master4" ' @@ -96,9 +94,7 @@ test_expect_failure REBASE_P \ 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 && - git commit -m "Empty commit" --allow-empty && + git checkout -b rebase-keep-empty-5 to-rebase && git rebase -Xsubtree=files_subtree --keep-empty --preserve-merges --onto files-master master && verbose test "$(commit_message HEAD~)" = "files_subtree/master5" ' @@ -107,9 +103,7 @@ test_expect_failure REBASE_P \ 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 && - git commit -m "Empty commit" --allow-empty && + git checkout -b rebase-keep-empty-empty to-rebase && git rebase -Xsubtree=files_subtree --keep-empty --preserve-merges --onto files-master master && verbose test "$(commit_message HEAD)" = "Empty commit" ' @@ -117,9 +111,7 @@ test_expect_failure REBASE_P \ # FAILURE: fatal: Could not parse object test_expect_failure 'Rebase -Xsubtree --onto commit 4' ' reset_rebase && - git checkout -b rebase-onto-4 master && - git filter-branch --prune-empty -f --subdirectory-filter files_subtree && - git commit -m "Empty commit" --allow-empty && + git checkout -b rebase-onto-4 to-rebase && git rebase -Xsubtree=files_subtree --onto files-master master && verbose test "$(commit_message HEAD~2)" = "files_subtree/master4" ' @@ -127,18 +119,14 @@ test_expect_failure 'Rebase -Xsubtree --onto commit 4' ' # FAILURE: fatal: Could not parse object test_expect_failure 'Rebase -Xsubtree --onto commit 5' ' reset_rebase && - git checkout -b rebase-onto-5 master && - git filter-branch --prune-empty -f --subdirectory-filter files_subtree && - git commit -m "Empty commit" --allow-empty && + git checkout -b rebase-onto-5 to-rebase && git rebase -Xsubtree=files_subtree --onto files-master master && verbose test "$(commit_message HEAD~)" = "files_subtree/master5" ' # FAILURE: fatal: Could not parse object test_expect_failure 'Rebase -Xsubtree --onto empty commit' ' reset_rebase && - git checkout -b rebase-onto-empty master && - git filter-branch --prune-empty -f --subdirectory-filter files_subtree && - git commit -m "Empty commit" --allow-empty && + git checkout -b rebase-onto-empty to-rebase && git rebase -Xsubtree=files_subtree --onto files-master master && verbose test "$(commit_message HEAD)" = "Empty commit" ' -- gitgitgadget ^ permalink raw reply related [flat|nested] 39+ messages in thread
* [PATCH v2 09/16] t3427: condense the unnecessarily repetitive test cases into three 2019-07-31 15:18 ` [PATCH v2 00/16] " Johannes Schindelin via GitGitGadget ` (7 preceding siblings ...) 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 ` Johannes Schindelin via GitGitGadget 2019-07-31 15:18 ` [PATCH v2 10/16] t3427: fix erroneous assumption Johannes Schindelin via GitGitGadget ` (7 subsequent siblings) 16 siblings, 0 replies; 39+ messages in thread From: Johannes Schindelin via GitGitGadget @ 2019-07-31 15:18 UTC (permalink / raw) To: git; +Cc: brian m. carlson, Junio C Hamano, Johannes Schindelin From: Johannes Schindelin <johannes.schindelin@gmx.de> Previously, this test script performed essentially three rebases and verified breakages by testing the post-rebase commits' messages. To do so, the rebases were performed multiple times, though, once per commit message to test. This wastes electricity (and CO2) and time. Let's condense the test cases to the essential number: the number of different rebases to validate. On Windows, where the scripted nature of the `--preserve-merges` backend hurts performance rather badly, this reduces the overall runtime in this developer's setup from ~1m to ~28s while still performing the exact same testing as before. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> --- t/t3427-rebase-subtree.sh | 61 +++++++-------------------------------- 1 file changed, 11 insertions(+), 50 deletions(-) diff --git a/t/t3427-rebase-subtree.sh b/t/t3427-rebase-subtree.sh index 64ba95f3e0..b21925d279 100755 --- a/t/t3427-rebase-subtree.sh +++ b/t/t3427-rebase-subtree.sh @@ -64,70 +64,31 @@ test_expect_success 'setup' ' ' # FAILURE: Does not preserve master4. -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 && - verbose test "$(commit_message HEAD~)" = "files_subtree/master4" -' - -# FAILURE: Does not preserve master5. -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 && git rebase -Xsubtree=files_subtree --preserve-merges --onto files-master master && + verbose test "$(commit_message HEAD~)" = "files_subtree/master4" && verbose test "$(commit_message HEAD)" = "files_subtree/master5" ' # FAILURE: Does not preserve master4. -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 && - verbose test "$(commit_message HEAD~2)" = "files_subtree/master4" -' - -# FAILURE: Does not preserve master5. -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 && - verbose test "$(commit_message HEAD~)" = "files_subtree/master5" -' - -# FAILURE: Does not preserve Empty. -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 && git rebase -Xsubtree=files_subtree --keep-empty --preserve-merges --onto files-master master && + verbose test "$(commit_message HEAD~2)" = "files_subtree/master4" && + verbose test "$(commit_message HEAD~)" = "files_subtree/master5" && verbose test "$(commit_message HEAD)" = "Empty commit" ' # FAILURE: fatal: Could not parse object -test_expect_failure 'Rebase -Xsubtree --onto commit 4' ' - reset_rebase && - git checkout -b rebase-onto-4 to-rebase && - git rebase -Xsubtree=files_subtree --onto files-master master && - verbose test "$(commit_message HEAD~2)" = "files_subtree/master4" -' - -# FAILURE: fatal: Could not parse object -test_expect_failure 'Rebase -Xsubtree --onto commit 5' ' - reset_rebase && - git checkout -b rebase-onto-5 to-rebase && - git rebase -Xsubtree=files_subtree --onto files-master master && - verbose test "$(commit_message HEAD~)" = "files_subtree/master5" -' -# FAILURE: fatal: Could not parse object -test_expect_failure 'Rebase -Xsubtree --onto empty commit' ' +test_expect_failure 'Rebase -Xsubtree --onto commit' ' reset_rebase && - git checkout -b rebase-onto-empty to-rebase && + git checkout -b rebase-onto to-rebase && git rebase -Xsubtree=files_subtree --onto files-master master && + verbose test "$(commit_message HEAD~2)" = "files_subtree/master4" && + verbose test "$(commit_message HEAD~)" = "files_subtree/master5" && verbose test "$(commit_message HEAD)" = "Empty commit" ' -- gitgitgadget ^ permalink raw reply related [flat|nested] 39+ messages in thread
* [PATCH v2 10/16] t3427: fix erroneous assumption 2019-07-31 15:18 ` [PATCH v2 00/16] " Johannes Schindelin via GitGitGadget ` (8 preceding siblings ...) 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 ` 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 ` (6 subsequent siblings) 16 siblings, 0 replies; 39+ messages in thread From: Johannes Schindelin via GitGitGadget @ 2019-07-31 15:18 UTC (permalink / raw) To: git; +Cc: brian m. carlson, Junio C Hamano, Johannes Schindelin From: Johannes Schindelin <johannes.schindelin@gmx.de> Apart from the `setup` test case, `t3427-rebase-subtree.sh` is made up exclusively of demonstrations of breakages. The tricky thing about such demonstrations is that they are often buggy themselves. In this instance, somewhere over the course of the six iterations of the patch that eventually made it into Git's `master` as 5f35900849e (contrib/subtree: Add a test for subtree rebase that loses commits, 2016-06-28), the commit message "files_subtree/master4" was changed to just "master4", but the test cases still expected the old commit message. Let's fix this, at long last. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> --- t/t3427-rebase-subtree.sh | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/t/t3427-rebase-subtree.sh b/t/t3427-rebase-subtree.sh index b21925d279..94cc532e10 100755 --- a/t/t3427-rebase-subtree.sh +++ b/t/t3427-rebase-subtree.sh @@ -68,7 +68,7 @@ test_expect_failure REBASE_P 'Rebase -Xsubtree --preserve-merges --onto commit' reset_rebase && git checkout -b rebase-preserve-merges to-rebase && git rebase -Xsubtree=files_subtree --preserve-merges --onto files-master master && - verbose test "$(commit_message HEAD~)" = "files_subtree/master4" && + verbose test "$(commit_message HEAD~)" = "master4" && verbose test "$(commit_message HEAD)" = "files_subtree/master5" ' @@ -77,7 +77,7 @@ test_expect_failure REBASE_P 'Rebase -Xsubtree --keep-empty --preserve-merges -- reset_rebase && git checkout -b rebase-keep-empty to-rebase && git rebase -Xsubtree=files_subtree --keep-empty --preserve-merges --onto files-master master && - verbose test "$(commit_message HEAD~2)" = "files_subtree/master4" && + verbose test "$(commit_message HEAD~2)" = "master4" && verbose test "$(commit_message HEAD~)" = "files_subtree/master5" && verbose test "$(commit_message HEAD)" = "Empty commit" ' @@ -87,7 +87,7 @@ test_expect_failure 'Rebase -Xsubtree --onto commit' ' reset_rebase && git checkout -b rebase-onto to-rebase && git rebase -Xsubtree=files_subtree --onto files-master master && - verbose test "$(commit_message HEAD~2)" = "files_subtree/master4" && + verbose test "$(commit_message HEAD~2)" = "master4" && verbose test "$(commit_message HEAD~)" = "files_subtree/master5" && verbose test "$(commit_message HEAD)" = "Empty commit" ' -- gitgitgadget ^ permalink raw reply related [flat|nested] 39+ messages in thread
* [PATCH v2 11/16] t3427: accommodate for the `rebase --merge` backend having been replaced 2019-07-31 15:18 ` [PATCH v2 00/16] " Johannes Schindelin via GitGitGadget ` (9 preceding siblings ...) 2019-07-31 15:18 ` [PATCH v2 10/16] t3427: fix erroneous assumption Johannes Schindelin via GitGitGadget @ 2019-07-31 15:18 ` Johannes Schindelin via GitGitGadget 2019-07-31 15:18 ` [PATCH v2 12/16] t3427: fix another incorrect assumption Johannes Schindelin via GitGitGadget ` (5 subsequent siblings) 16 siblings, 0 replies; 39+ messages in thread From: Johannes Schindelin via GitGitGadget @ 2019-07-31 15:18 UTC (permalink / raw) To: git; +Cc: brian m. carlson, Junio C Hamano, Johannes Schindelin From: Johannes Schindelin <johannes.schindelin@gmx.de> Since 68aa495b590 (rebase: implement --merge via the interactive machinery, 2018-12-11), the job of the old `--merge` backend is now performed by the `--interactive` backend, too. One consequence is that empty commits are no longer rebased by default. Meaning that the test case that calls `git rebase -Xsubtree` (which used to be handled by the `--merge` backend) now needs to ask explicitly for the empty commit to be rebased. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> --- t/t3427-rebase-subtree.sh | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/t/t3427-rebase-subtree.sh b/t/t3427-rebase-subtree.sh index 94cc532e10..a734716ea3 100755 --- a/t/t3427-rebase-subtree.sh +++ b/t/t3427-rebase-subtree.sh @@ -83,10 +83,10 @@ test_expect_failure REBASE_P 'Rebase -Xsubtree --keep-empty --preserve-merges -- ' # FAILURE: fatal: Could not parse object -test_expect_failure 'Rebase -Xsubtree --onto commit' ' +test_expect_failure 'Rebase -Xsubtree --keep-empty --onto commit' ' reset_rebase && git checkout -b rebase-onto to-rebase && - git rebase -Xsubtree=files_subtree --onto files-master master && + git rebase -Xsubtree=files_subtree --keep-empty --onto files-master master && verbose test "$(commit_message HEAD~2)" = "master4" && verbose test "$(commit_message HEAD~)" = "files_subtree/master5" && verbose test "$(commit_message HEAD)" = "Empty commit" -- gitgitgadget ^ permalink raw reply related [flat|nested] 39+ messages in thread
* [PATCH v2 12/16] t3427: fix another incorrect assumption 2019-07-31 15:18 ` [PATCH v2 00/16] " Johannes Schindelin via GitGitGadget ` (10 preceding siblings ...) 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 ` 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 ` (4 subsequent siblings) 16 siblings, 0 replies; 39+ messages in thread From: Johannes Schindelin via GitGitGadget @ 2019-07-31 15:18 UTC (permalink / raw) To: git; +Cc: brian m. carlson, Junio C Hamano, Johannes Schindelin From: Johannes Schindelin <johannes.schindelin@gmx.de> The test case that concerns `git rebase -Xsubtree` (with the default rebase backend, not with `--preserve-merges`) starts out with a pre-rebase commit history that begins with a commit that introduces three files: master1.t, master2.t and master3.t. This commit was generated by passing a subtree merge commit through `git filter-branch --subdirectory-filter`, so it looks as if this commit really introduces all those files. The commit history onto which this commit is then rebased, however, introduced those files in individual commits. For that reason, the rebase will fail, it _must_ fail, because the first `pick` results in no changes to be committed. Let's fix the test case to expect exactly this situation. With this change, we can mark the original bug that this test case tried to demonstrate as fixed. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> --- t/t3427-rebase-subtree.sh | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/t/t3427-rebase-subtree.sh b/t/t3427-rebase-subtree.sh index a734716ea3..7a37235768 100755 --- a/t/t3427-rebase-subtree.sh +++ b/t/t3427-rebase-subtree.sh @@ -82,11 +82,12 @@ test_expect_failure REBASE_P 'Rebase -Xsubtree --keep-empty --preserve-merges -- verbose test "$(commit_message HEAD)" = "Empty commit" ' -# FAILURE: fatal: Could not parse object -test_expect_failure 'Rebase -Xsubtree --keep-empty --onto commit' ' +test_expect_success 'Rebase -Xsubtree --keep-empty --onto commit' ' reset_rebase && git checkout -b rebase-onto to-rebase && - git rebase -Xsubtree=files_subtree --keep-empty --onto files-master master && + test_must_fail git rebase -Xsubtree=files_subtree --keep-empty --onto files-master master && + : first pick results in no changes && + git rebase --continue && verbose test "$(commit_message HEAD~2)" = "master4" && verbose test "$(commit_message HEAD~)" = "files_subtree/master5" && verbose test "$(commit_message HEAD)" = "Empty commit" -- gitgitgadget ^ permalink raw reply related [flat|nested] 39+ messages in thread
* [PATCH v2 13/16] rebase -r: support merge strategies other than `recursive` 2019-07-31 15:18 ` [PATCH v2 00/16] " Johannes Schindelin via GitGitGadget ` (11 preceding siblings ...) 2019-07-31 15:18 ` [PATCH v2 12/16] t3427: fix another incorrect assumption Johannes Schindelin via GitGitGadget @ 2019-07-31 15:18 ` 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 ` (3 subsequent siblings) 16 siblings, 0 replies; 39+ messages in thread From: Johannes Schindelin via GitGitGadget @ 2019-07-31 15:18 UTC (permalink / raw) To: git; +Cc: brian m. carlson, Junio C Hamano, Johannes Schindelin From: Johannes Schindelin <johannes.schindelin@gmx.de> We already support merge strategies in the sequencer, but only for `pick` commands. With this commit, we now also support them in `merge` commands. The approach is simple: if any merge strategy option is specified, or if any merge strategy other than `recursive` is specified, we simply spawn the `git merge` command. Otherwise, we handle the merge in-process just as before. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> --- Documentation/git-rebase.txt | 2 -- builtin/rebase.c | 9 --------- sequencer.c | 14 ++++++++++++-- t/t3422-rebase-incompatible-options.sh | 10 ---------- t/t3430-rebase-merges.sh | 21 +++++++++++++++++++++ 5 files changed, 33 insertions(+), 23 deletions(-) diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt index 5e4e927647..bc620c44e9 100644 --- a/Documentation/git-rebase.txt +++ b/Documentation/git-rebase.txt @@ -543,8 +543,6 @@ In addition, the following pairs of options are incompatible: * --preserve-merges and --interactive * --preserve-merges and --signoff * --preserve-merges and --rebase-merges - * --rebase-merges and --strategy - * --rebase-merges and --strategy-option BEHAVIORAL DIFFERENCES ----------------------- diff --git a/builtin/rebase.c b/builtin/rebase.c index 74a60e8c83..625f50c637 100644 --- a/builtin/rebase.c +++ b/builtin/rebase.c @@ -1811,15 +1811,6 @@ int cmd_rebase(int argc, const char **argv, const char *prefix) "'--reschedule-failed-exec'")); } - if (options.rebase_merges) { - if (strategy_options.nr) - die(_("cannot combine '--rebase-merges' with " - "'--strategy-option'")); - if (options.strategy) - die(_("cannot combine '--rebase-merges' with " - "'--strategy'")); - } - if (!options.root) { if (argc < 1) { struct branch *branch; diff --git a/sequencer.c b/sequencer.c index 334de14542..d228448cd8 100644 --- a/sequencer.c +++ b/sequencer.c @@ -3256,6 +3256,9 @@ static int do_merge(struct repository *r, struct commit *head_commit, *merge_commit, *i; struct commit_list *bases, *j, *reversed = NULL; struct commit_list *to_merge = NULL, **tail = &to_merge; + const char *strategy = !opts->xopts_nr && + (!opts->strategy || !strcmp(opts->strategy, "recursive")) ? + NULL : opts->strategy; struct merge_options o; int merge_arg_len, oneline_offset, can_fast_forward, ret, k; static struct lock_file lock; @@ -3404,7 +3407,7 @@ static int do_merge(struct repository *r, goto leave_merge; } - if (to_merge->next) { + if (strategy || to_merge->next) { /* Octopus merge */ struct child_process cmd = CHILD_PROCESS_INIT; @@ -3418,7 +3421,14 @@ static int do_merge(struct repository *r, cmd.git_cmd = 1; argv_array_push(&cmd.args, "merge"); argv_array_push(&cmd.args, "-s"); - argv_array_push(&cmd.args, "octopus"); + if (!strategy) + argv_array_push(&cmd.args, "octopus"); + else { + argv_array_push(&cmd.args, strategy); + for (k = 0; k < opts->xopts_nr; k++) + argv_array_pushf(&cmd.args, + "-X%s", opts->xopts[k]); + } argv_array_push(&cmd.args, "--no-edit"); argv_array_push(&cmd.args, "--no-ff"); argv_array_push(&cmd.args, "--no-log"); diff --git a/t/t3422-rebase-incompatible-options.sh b/t/t3422-rebase-incompatible-options.sh index a5868ea152..50e7960702 100755 --- a/t/t3422-rebase-incompatible-options.sh +++ b/t/t3422-rebase-incompatible-options.sh @@ -76,14 +76,4 @@ test_expect_success REBASE_P \ test_must_fail git rebase --preserve-merges --rebase-merges A ' -test_expect_success '--rebase-merges incompatible with --strategy' ' - git checkout B^0 && - test_must_fail git rebase --rebase-merges -s resolve A -' - -test_expect_success '--rebase-merges incompatible with --strategy-option' ' - git checkout B^0 && - test_must_fail git rebase --rebase-merges -Xignore-space-change A -' - test_done diff --git a/t/t3430-rebase-merges.sh b/t/t3430-rebase-merges.sh index 42ba5b9f09..8ea6ff3548 100755 --- a/t/t3430-rebase-merges.sh +++ b/t/t3430-rebase-merges.sh @@ -412,4 +412,25 @@ test_expect_success '--continue after resolving conflicts after a merge' ' test_path_is_missing .git/MERGE_HEAD ' +test_expect_success '--rebase-merges with strategies' ' + git checkout -b with-a-strategy F && + test_tick && + git merge -m "Merge conflicting-G" conflicting-G && + + : first, test with a merge strategy option && + git rebase -ir -Xtheirs G && + echo conflicting-G >expect && + test_cmp expect G.t && + + : now, try with a merge strategy other than recursive && + git reset --hard @{1} && + write_script git-merge-override <<-\EOF && + echo overridden$1 >>G.t + git add G.t + EOF + PATH="$PWD:$PATH" git rebase -ir -s override -Xxopt G && + test_write_lines G overridden--xopt >expect && + test_cmp expect G.t +' + test_done -- gitgitgadget ^ permalink raw reply related [flat|nested] 39+ messages in thread
* [PATCH v2 14/16] t/lib-rebase: prepare for testing `git rebase --rebase-merges` 2019-07-31 15:18 ` [PATCH v2 00/16] " Johannes Schindelin via GitGitGadget ` (12 preceding siblings ...) 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 ` Johannes Schindelin via GitGitGadget 2019-07-31 15:18 ` [PATCH v2 15/16] t3418: test `rebase -r` with merge strategies Johannes Schindelin via GitGitGadget ` (2 subsequent siblings) 16 siblings, 0 replies; 39+ messages in thread From: Johannes Schindelin via GitGitGadget @ 2019-07-31 15:18 UTC (permalink / raw) To: git; +Cc: brian m. carlson, Junio C Hamano, Johannes Schindelin From: Johannes Schindelin <johannes.schindelin@gmx.de> The format of the todo list is quite a bit different in the `--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> --- t/lib-rebase.sh | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/t/lib-rebase.sh b/t/lib-rebase.sh index 7ea30e5006..6d87961e41 100644 --- a/t/lib-rebase.sh +++ b/t/lib-rebase.sh @@ -44,10 +44,10 @@ set_fake_editor () { rm -f "$1" echo 'rebase -i script before editing:' cat "$1".tmp - action=pick + action=\& for line in $FAKE_LINES; do case $line in - pick|p|squash|s|fixup|f|edit|e|reword|r|drop|d) + pick|p|squash|s|fixup|f|edit|e|reword|r|drop|d|label|l|reset|r|merge|m) action="$line";; exec_*|x_*|break|b) echo "$line" | sed 's/_/ /g' >> "$1";; @@ -58,11 +58,12 @@ set_fake_editor () { bad) action="badcmd";; fakesha) + test \& != "$action" || action=pick echo "$action XXXXXXX False commit" >> "$1" action=pick;; *) - sed -n "${line}s/^pick/$action/p" < "$1".tmp >> "$1" - action=pick;; + sed -n "${line}s/^[a-z][a-z]*/$action/p" < "$1".tmp >> "$1" + action=\&;; esac done echo 'rebase -i script after editing:' -- gitgitgadget ^ permalink raw reply related [flat|nested] 39+ messages in thread
* [PATCH v2 15/16] t3418: test `rebase -r` with merge strategies 2019-07-31 15:18 ` [PATCH v2 00/16] " Johannes Schindelin via GitGitGadget ` (13 preceding siblings ...) 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 ` 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 16 siblings, 0 replies; 39+ messages in thread From: Johannes Schindelin via GitGitGadget @ 2019-07-31 15:18 UTC (permalink / raw) To: git; +Cc: brian m. carlson, Junio C Hamano, Johannes Schindelin From: Johannes Schindelin <johannes.schindelin@gmx.de> There is a test case in this script that verifies that `git rebase --preserve-merges` works all right with non-default merge strategies or non-default merge strategy options. Now that `git rebase --rebase-merges` learned about merge strategies, let's copy-edit this test case to verify that that works as intended, too. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> --- t/t3418-rebase-continue.sh | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/t/t3418-rebase-continue.sh b/t/t3418-rebase-continue.sh index bdaa511bb0..fbf9addfd1 100755 --- a/t/t3418-rebase-continue.sh +++ b/t/t3418-rebase-continue.sh @@ -120,6 +120,20 @@ test_expect_success REBASE_P 'rebase passes merge strategy options correctly' ' git rebase --continue ' +test_expect_success 'rebase -r passes merge strategy options correctly' ' + rm -fr .git/rebase-* && + git reset --hard commit-new-file-F3-on-topic-branch && + test_commit merge-theirs && + git reset --hard HEAD^ && + test_commit some-other-commit && + test_tick && + git merge --no-ff merge-theirs && + FAKE_LINES="1 3 edit 4 5 7 8 9" git rebase -i -f -r -m \ + -s recursive --strategy-option=theirs HEAD~2 && + test_commit force-change-ours && + git rebase --continue +' + test_expect_success '--skip after failed fixup cleans commit message' ' test_when_finished "test_might_fail git rebase --abort" && git checkout -b with-conflicting-fixup && -- gitgitgadget ^ permalink raw reply related [flat|nested] 39+ messages in thread
* [PATCH v2 16/16] rebase -r: do not (re-)generate root commits with `--root` *and* `--onto` 2019-07-31 15:18 ` [PATCH v2 00/16] " Johannes Schindelin via GitGitGadget ` (14 preceding siblings ...) 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 ` 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 16 siblings, 0 replies; 39+ messages in thread From: Johannes Schindelin via GitGitGadget @ 2019-07-31 15:18 UTC (permalink / raw) To: git; +Cc: brian m. carlson, Junio C Hamano, Johannes Schindelin From: Johannes Schindelin <johannes.schindelin@gmx.de> When rebasing a complete commit history onto a given commit, it is pretty obvious that the root commits should be rebased on top of said given commit. To test this, let's kill two birds with one stone and add a test case to t3427-rebase-subtree.sh that not only demonstrates that this works, but also that `git rebase -r` works with merge strategies now. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> --- builtin/rebase.c | 7 +++++-- sequencer.c | 4 +++- sequencer.h | 6 ++++++ t/t3427-rebase-subtree.sh | 11 +++++++++++ 4 files changed, 25 insertions(+), 3 deletions(-) diff --git a/builtin/rebase.c b/builtin/rebase.c index 625f50c637..ee2bc8b032 100644 --- a/builtin/rebase.c +++ b/builtin/rebase.c @@ -62,7 +62,7 @@ struct rebase_options { const char *onto_name; const char *revisions; const char *switch_to; - int root; + int root, root_with_onto; struct object_id *squash_onto; struct commit *restrict_revision; int dont_finish_rebase; @@ -374,6 +374,7 @@ static int run_rebase_interactive(struct rebase_options *opts, flags |= abbreviate_commands ? TODO_LIST_ABBREVIATE_CMDS : 0; flags |= opts->rebase_merges ? TODO_LIST_REBASE_MERGES : 0; flags |= opts->rebase_cousins > 0 ? TODO_LIST_REBASE_COUSINS : 0; + flags |= opts->root_with_onto ? TODO_LIST_ROOT_WITH_ONTO : 0; flags |= command == ACTION_SHORTEN_OIDS ? TODO_LIST_SHORTEN_IDS : 0; switch (command) { @@ -1841,7 +1842,9 @@ int cmd_rebase(int argc, const char **argv, const char *prefix) options.squash_onto = &squash_onto; options.onto_name = squash_onto_name = xstrdup(oid_to_hex(&squash_onto)); - } + } else + options.root_with_onto = 1; + options.upstream_name = NULL; options.upstream = NULL; if (argc > 1) diff --git a/sequencer.c b/sequencer.c index d228448cd8..ca119c84e5 100644 --- a/sequencer.c +++ b/sequencer.c @@ -4440,6 +4440,7 @@ static int make_script_with_merges(struct pretty_print_context *pp, { int keep_empty = flags & TODO_LIST_KEEP_EMPTY; int rebase_cousins = flags & TODO_LIST_REBASE_COUSINS; + int root_with_onto = flags & TODO_LIST_ROOT_WITH_ONTO; struct strbuf buf = STRBUF_INIT, oneline = STRBUF_INIT; struct strbuf label = STRBUF_INIT; struct commit_list *commits = NULL, **tail = &commits, *iter; @@ -4606,7 +4607,8 @@ static int make_script_with_merges(struct pretty_print_context *pp, if (!commit) strbuf_addf(out, "%s %s\n", cmd_reset, - rebase_cousins ? "onto" : "[new root]"); + rebase_cousins || root_with_onto ? + "onto" : "[new root]"); else { const char *to = NULL; diff --git a/sequencer.h b/sequencer.h index 0c494b83d4..d506081d3c 100644 --- a/sequencer.h +++ b/sequencer.h @@ -142,6 +142,12 @@ int sequencer_remove_state(struct replay_opts *opts); */ #define TODO_LIST_REBASE_COUSINS (1U << 4) #define TODO_LIST_APPEND_TODO_HELP (1U << 5) +/* + * When generating a script that rebases merges with `--root` *and* with + * `--onto`, we do not want to re-generate the root commits. + */ +#define TODO_LIST_ROOT_WITH_ONTO (1U << 6) + int sequencer_make_script(struct repository *r, struct strbuf *out, int argc, const char **argv, unsigned flags); diff --git a/t/t3427-rebase-subtree.sh b/t/t3427-rebase-subtree.sh index 7a37235768..39e348de16 100755 --- a/t/t3427-rebase-subtree.sh +++ b/t/t3427-rebase-subtree.sh @@ -93,4 +93,15 @@ test_expect_success 'Rebase -Xsubtree --keep-empty --onto commit' ' verbose test "$(commit_message HEAD)" = "Empty commit" ' +test_expect_success 'Rebase -Xsubtree --keep-empty --rebase-merges --onto commit' ' + reset_rebase && + git checkout -b rebase-merges-onto to-rebase && + test_must_fail git rebase -Xsubtree=files_subtree --keep-empty --rebase-merges --onto files-master --root && + : first pick results in no changes && + git rebase --continue && + verbose test "$(commit_message HEAD~2)" = "master4" && + verbose test "$(commit_message HEAD~)" = "files_subtree/master5" && + verbose test "$(commit_message HEAD)" = "Empty commit" +' + test_done -- gitgitgadget ^ permalink raw reply related [flat|nested] 39+ messages in thread
* [PATCH v2 17/16] t3427: accelerate this test by using fast-export and fast-import 2019-07-31 15:18 ` [PATCH v2 00/16] " Johannes Schindelin via GitGitGadget ` (15 preceding siblings ...) 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 ` Elijah Newren 2019-09-09 20:29 ` Johannes Schindelin 16 siblings, 1 reply; 39+ messages in thread From: Elijah Newren @ 2019-09-04 21:40 UTC (permalink / raw) To: Junio C Hamano; +Cc: git, Johannes Schindelin, Elijah Newren fast-export and fast-import can easily handle the simple rewrite that was being done by filter-branch, and should be faster on systems with a slow fork. Measuring the overall time taken for all of t3427 (not just the difference between filter-branch and fast-export/fast-import) shows a speedup of about 5% on Linux and 11% on Mac. Signed-off-by: Elijah Newren <newren@gmail.com> --- This patch is meant to be added onto the end of js/rebase-r-strategy; an earlier version of this patch conflicted js/rebase-r-strategy so now I'm basing on top of that series. The speedup is also less impressive now that there is only one filter-branch invocation being replaced instead of a handful. Still a nice speedup, though. t/t3427-rebase-subtree.sh | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/t/t3427-rebase-subtree.sh b/t/t3427-rebase-subtree.sh index 39e348de16..bec48e6a1f 100755 --- a/t/t3427-rebase-subtree.sh +++ b/t/t3427-rebase-subtree.sh @@ -59,7 +59,10 @@ test_expect_success 'setup' ' test_commit files_subtree/master5 && git checkout -b to-rebase && - git filter-branch --prune-empty -f --subdirectory-filter files_subtree && + git fast-export --no-data HEAD -- files_subtree/ | + sed -e "s%\([0-9a-f]\{40\} \)files_subtree/%\1%" | + git fast-import --force --quiet && + git reset --hard && git commit -m "Empty commit" --allow-empty ' -- 2.22.0.19.ga495766805 ^ permalink raw reply related [flat|nested] 39+ messages in thread
* Re: [PATCH v2 17/16] t3427: accelerate this test by using fast-export and fast-import 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 0 siblings, 1 reply; 39+ messages in thread From: Johannes Schindelin @ 2019-09-09 20:29 UTC (permalink / raw) To: Elijah Newren; +Cc: Junio C Hamano, git Hi Elijah, On Wed, 4 Sep 2019, Elijah Newren wrote: > fast-export and fast-import can easily handle the simple rewrite that > was being done by filter-branch, and should be faster on systems with a > slow fork. Measuring the overall time taken for all of t3427 (not just > the difference between filter-branch and fast-export/fast-import) shows > a speedup of about 5% on Linux and 11% on Mac. > > Signed-off-by: Elijah Newren <newren@gmail.com> > --- > This patch is meant to be added onto the end of js/rebase-r-strategy; an > earlier version of this patch conflicted js/rebase-r-strategy so now I'm > basing on top of that series. The speedup is also less impressive now > that there is only one filter-branch invocation being replaced instead of > a handful. Still a nice speedup, though. ACK! Thanks, Dscho > > t/t3427-rebase-subtree.sh | 5 ++++- > 1 file changed, 4 insertions(+), 1 deletion(-) > > diff --git a/t/t3427-rebase-subtree.sh b/t/t3427-rebase-subtree.sh > index 39e348de16..bec48e6a1f 100755 > --- a/t/t3427-rebase-subtree.sh > +++ b/t/t3427-rebase-subtree.sh > @@ -59,7 +59,10 @@ test_expect_success 'setup' ' > test_commit files_subtree/master5 && > > git checkout -b to-rebase && > - git filter-branch --prune-empty -f --subdirectory-filter files_subtree && > + git fast-export --no-data HEAD -- files_subtree/ | > + sed -e "s%\([0-9a-f]\{40\} \)files_subtree/%\1%" | > + git fast-import --force --quiet && > + git reset --hard && > git commit -m "Empty commit" --allow-empty > ' > > -- > 2.22.0.19.ga495766805 > > ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH v2 17/16] t3427: accelerate this test by using fast-export and fast-import 2019-09-09 20:29 ` Johannes Schindelin @ 2019-09-09 21:06 ` Junio C Hamano 0 siblings, 0 replies; 39+ messages in thread From: Junio C Hamano @ 2019-09-09 21:06 UTC (permalink / raw) To: Johannes Schindelin; +Cc: Elijah Newren, git Johannes Schindelin <Johannes.Schindelin@gmx.de> writes: > Hi Elijah, > > On Wed, 4 Sep 2019, Elijah Newren wrote: > >> fast-export and fast-import can easily handle the simple rewrite that >> was being done by filter-branch, and should be faster on systems with a >> slow fork. Measuring the overall time taken for all of t3427 (not just >> the difference between filter-branch and fast-export/fast-import) shows >> a speedup of about 5% on Linux and 11% on Mac. >> >> Signed-off-by: Elijah Newren <newren@gmail.com> >> --- >> This patch is meant to be added onto the end of js/rebase-r-strategy; an >> earlier version of this patch conflicted js/rebase-r-strategy so now I'm >> basing on top of that series. The speedup is also less impressive now >> that there is only one filter-branch invocation being replaced instead of >> a handful. Still a nice speedup, though. > > ACK! > > Thanks, > Dscho Thanks, both. This indeed is a good update. > >> >> t/t3427-rebase-subtree.sh | 5 ++++- >> 1 file changed, 4 insertions(+), 1 deletion(-) >> >> diff --git a/t/t3427-rebase-subtree.sh b/t/t3427-rebase-subtree.sh >> index 39e348de16..bec48e6a1f 100755 >> --- a/t/t3427-rebase-subtree.sh >> +++ b/t/t3427-rebase-subtree.sh >> @@ -59,7 +59,10 @@ test_expect_success 'setup' ' >> test_commit files_subtree/master5 && >> >> git checkout -b to-rebase && >> - git filter-branch --prune-empty -f --subdirectory-filter files_subtree && >> + git fast-export --no-data HEAD -- files_subtree/ | >> + sed -e "s%\([0-9a-f]\{40\} \)files_subtree/%\1%" | >> + git fast-import --force --quiet && >> + git reset --hard && >> git commit -m "Empty commit" --allow-empty >> ' >> >> -- >> 2.22.0.19.ga495766805 >> >> ^ permalink raw reply [flat|nested] 39+ messages in thread
end of thread, other threads:[~2019-09-09 21:06 UTC | newest] Thread overview: 39+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 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 08/12] t3427: mark two test cases as requiring support for `git rebase -p` 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 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 ` [PATCH v2 00/16] " Johannes Schindelin via GitGitGadget 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
Code repositories for project(s) associated with this public inbox https://80x24.org/mirrors/git.git This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).