From 04d3d3cfc4a26f003c5ae2b5598cc975a31e4395 Mon Sep 17 00:00:00 2001 From: Ramkumar Ramachandra Date: Thu, 4 Aug 2011 16:09:08 +0530 Subject: revert: Save data for continuing after conflict resolution Ever since v1.7.2-rc1~4^2~7 (revert: allow cherry-picking more than one commit, 2010-06-02), a single invocation of "git cherry-pick" or "git revert" can perform picks of several individual commits. To implement features like "--continue" to continue the whole operation, we will need to store some information about the state and the plan at the beginning. Introduce a ".git/sequencer/head" file to store this state, and ".git/sequencer/todo" file to store the plan. The head file contains the SHA-1 of the HEAD before the start of the operation, and the todo file contains an instruction sheet whose format is inspired by the format of the "rebase -i" instruction sheet. As a result, a typical todo file looks like: pick 8537f0e submodule add: test failure when url is not configured pick 4d68932 submodule add: allow relative repository path pick f22a17e submodule add: clean up duplicated code pick 59a5775 make copy_ref globally available Since SHA-1 hex is abbreviated using an find_unique_abbrev(), it is unambiguous. This does not guarantee that there will be no ambiguity when more objects are added to the repository. These two files alone are not enough to implement a "--continue" that remembers the command-line options specified; later patches in the series save them too. These new files are unrelated to the existing .git/CHERRY_PICK_HEAD, which will still be useful while committing after a conflict resolution. Inspired-by: Christian Couder Helped-by: Junio C Hamano Helped-by: Jonathan Nieder Signed-off-by: Ramkumar Ramachandra Signed-off-by: Jonathan Nieder Signed-off-by: Junio C Hamano --- t/t3510-cherry-pick-sequence.sh | 48 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 48 insertions(+) create mode 100755 t/t3510-cherry-pick-sequence.sh (limited to 't') diff --git a/t/t3510-cherry-pick-sequence.sh b/t/t3510-cherry-pick-sequence.sh new file mode 100755 index 0000000000..a2c70ad4b8 --- /dev/null +++ b/t/t3510-cherry-pick-sequence.sh @@ -0,0 +1,48 @@ +#!/bin/sh + +test_description='Test cherry-pick continuation features + + + anotherpick: rewrites foo to d + + picked: rewrites foo to c + + unrelatedpick: rewrites unrelated to reallyunrelated + + base: rewrites foo to b + + initial: writes foo as a, unrelated as unrelated + +' + +. ./test-lib.sh + +pristine_detach () { + rm -rf .git/sequencer && + git checkout -f "$1^0" && + git read-tree -u --reset HEAD && + git clean -d -f -f -q -x +} + +test_expect_success setup ' + echo unrelated >unrelated && + git add unrelated && + test_commit initial foo a && + test_commit base foo b && + test_commit unrelatedpick unrelated reallyunrelated && + test_commit picked foo c && + test_commit anotherpick foo d && + git config advice.detachedhead false + +' + +test_expect_success 'cherry-pick persists data on failure' ' + pristine_detach initial && + test_must_fail git cherry-pick base..anotherpick && + test_path_is_dir .git/sequencer && + test_path_is_file .git/sequencer/head && + test_path_is_file .git/sequencer/todo +' + +test_expect_success 'cherry-pick cleans up sequencer state upon success' ' + pristine_detach initial && + git cherry-pick initial..picked && + test_path_is_missing .git/sequencer +' + +test_done -- cgit v1.2.3 From 6f0322633b2659d26d1c7b20d4af1fba33978690 Mon Sep 17 00:00:00 2001 From: Ramkumar Ramachandra Date: Thu, 4 Aug 2011 16:09:09 +0530 Subject: revert: Save command-line options for continuing operation In the same spirit as ".git/sequencer/head" and ".git/sequencer/todo", introduce ".git/sequencer/opts" to persist the replay_opts structure for continuing after a conflict resolution. Use the gitconfig format for this file so that it looks like: [options] signoff = true record-origin = true mainline = 1 strategy = recursive strategy-option = patience strategy-option = ours Helped-by: Jonathan Nieder Helped-by: Christian Couder Signed-off-by: Ramkumar Ramachandra Signed-off-by: Jonathan Nieder Signed-off-by: Junio C Hamano --- t/t3510-cherry-pick-sequence.sh | 29 +++++++++++++++++++++++++++-- 1 file changed, 27 insertions(+), 2 deletions(-) (limited to 't') diff --git a/t/t3510-cherry-pick-sequence.sh b/t/t3510-cherry-pick-sequence.sh index a2c70ad4b8..8ee000180a 100755 --- a/t/t3510-cherry-pick-sequence.sh +++ b/t/t3510-cherry-pick-sequence.sh @@ -33,10 +33,35 @@ test_expect_success setup ' test_expect_success 'cherry-pick persists data on failure' ' pristine_detach initial && - test_must_fail git cherry-pick base..anotherpick && + test_must_fail git cherry-pick -s base..anotherpick && test_path_is_dir .git/sequencer && test_path_is_file .git/sequencer/head && - test_path_is_file .git/sequencer/todo + test_path_is_file .git/sequencer/todo && + test_path_is_file .git/sequencer/opts +' + +test_expect_success 'cherry-pick persists opts correctly' ' + pristine_detach initial && + test_must_fail git cherry-pick -s -m 1 --strategy=recursive -X patience -X ours base..anotherpick && + test_path_is_dir .git/sequencer && + test_path_is_file .git/sequencer/head && + test_path_is_file .git/sequencer/todo && + test_path_is_file .git/sequencer/opts && + echo "true" >expect && + git config --file=.git/sequencer/opts --get-all options.signoff >actual && + test_cmp expect actual && + echo "1" >expect && + git config --file=.git/sequencer/opts --get-all options.mainline >actual && + test_cmp expect actual && + echo "recursive" >expect && + git config --file=.git/sequencer/opts --get-all options.strategy >actual && + test_cmp expect actual && + cat >expect <<-\EOF && + patience + ours + EOF + git config --file=.git/sequencer/opts --get-all options.strategy-option >actual && + test_cmp expect actual ' test_expect_success 'cherry-pick cleans up sequencer state upon success' ' -- cgit v1.2.3 From 26ae337be11e440420d8ec7ce415425daaabe573 Mon Sep 17 00:00:00 2001 From: Ramkumar Ramachandra Date: Thu, 4 Aug 2011 16:09:11 +0530 Subject: revert: Introduce --reset to remove sequencer state To explicitly remove the sequencer state for a fresh cherry-pick or revert invocation, introduce a new subcommand called "--reset" to remove the sequencer state. Take the opportunity to publicly expose the sequencer paths, and a generic function called "remove_sequencer_state" that various git programs can use to remove the sequencer state in a uniform manner; "git reset" uses it later in this series. Introducing this public API is also in line with our long-term goal of eventually factoring out functions from revert.c into a generic commit sequencer. Signed-off-by: Ramkumar Ramachandra Signed-off-by: Jonathan Nieder Signed-off-by: Junio C Hamano --- t/t3510-cherry-pick-sequence.sh | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) (limited to 't') diff --git a/t/t3510-cherry-pick-sequence.sh b/t/t3510-cherry-pick-sequence.sh index 8ee000180a..fd6986541a 100755 --- a/t/t3510-cherry-pick-sequence.sh +++ b/t/t3510-cherry-pick-sequence.sh @@ -13,7 +13,7 @@ test_description='Test cherry-pick continuation features . ./test-lib.sh pristine_detach () { - rm -rf .git/sequencer && + git cherry-pick --reset && git checkout -f "$1^0" && git read-tree -u --reset HEAD && git clean -d -f -f -q -x @@ -70,4 +70,16 @@ test_expect_success 'cherry-pick cleans up sequencer state upon success' ' test_path_is_missing .git/sequencer ' +test_expect_success '--reset does not complain when no cherry-pick is in progress' ' + pristine_detach initial && + git cherry-pick --reset +' + +test_expect_success '--reset cleans up sequencer state' ' + pristine_detach initial && + test_must_fail git cherry-pick base..picked && + git cherry-pick --reset && + test_path_is_missing .git/sequencer +' + test_done -- cgit v1.2.3 From 95eb88d8ee588d89b4f06d2753ed4d16ab13b39f Mon Sep 17 00:00:00 2001 From: Ramkumar Ramachandra Date: Thu, 4 Aug 2011 16:09:12 +0530 Subject: reset: Make reset remove the sequencer state Years of muscle memory have trained users to use "git reset --hard" to remove the branch state after any sort operation. Make it also remove the sequencer state to facilitate this established workflow: $ git cherry-pick foo..bar ... conflict encountered ... $ git reset --hard # Oops, I didn't mean that $ git cherry-pick quux..bar ... cherry-pick succeeded ... Guard against accidental removal of the sequencer state by providing one level of "undo". In the first "reset" invocation, ".git/sequencer" is moved to ".git/sequencer-old"; it is completely removed only in the second invocation. Signed-off-by: Ramkumar Ramachandra Signed-off-by: Jonathan Nieder Signed-off-by: Junio C Hamano --- t/t7106-reset-sequence.sh | 44 ++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 44 insertions(+) create mode 100755 t/t7106-reset-sequence.sh (limited to 't') diff --git a/t/t7106-reset-sequence.sh b/t/t7106-reset-sequence.sh new file mode 100755 index 0000000000..4956caaf82 --- /dev/null +++ b/t/t7106-reset-sequence.sh @@ -0,0 +1,44 @@ +#!/bin/sh + +test_description='Test interaction of reset --hard with sequencer + + + anotherpick: rewrites foo to d + + picked: rewrites foo to c + + unrelatedpick: rewrites unrelated to reallyunrelated + + base: rewrites foo to b + + initial: writes foo as a, unrelated as unrelated +' + +. ./test-lib.sh + +pristine_detach () { + git cherry-pick --reset && + git checkout -f "$1^0" && + git read-tree -u --reset HEAD && + git clean -d -f -f -q -x +} + +test_expect_success setup ' + echo unrelated >unrelated && + git add unrelated && + test_commit initial foo a && + test_commit base foo b && + test_commit unrelatedpick unrelated reallyunrelated && + test_commit picked foo c && + test_commit anotherpick foo d && + git config advice.detachedhead false + +' + +test_expect_success 'reset --hard cleans up sequencer state, providing one-level undo' ' + pristine_detach initial && + test_must_fail git cherry-pick base..anotherpick && + test_path_is_dir .git/sequencer && + git reset --hard && + test_path_is_missing .git/sequencer && + test_path_is_dir .git/sequencer-old && + git reset --hard && + test_path_is_missing .git/sequencer-old +' + +test_done -- cgit v1.2.3 From 2d27daa91da1d6376916533de2ec4b50e6acc925 Mon Sep 17 00:00:00 2001 From: Ramkumar Ramachandra Date: Thu, 4 Aug 2011 16:09:13 +0530 Subject: revert: Remove sequencer state when no commits are pending When cherry-pick or revert is called on a list of commits, and a conflict encountered somewhere in the middle, the data in ".git/sequencer" is required to continue the operation. However, when a conflict is encountered in the very last commit, the user will have to "continue" after resolving the conflict and committing just so that the sequencer state is removed. This is how the current "rebase -i" script works as well. $ git cherry-pick foo..bar ... conflict encountered while picking "bar" ... $ echo "resolved" >problematicfile $ git add problematicfile $ git commit $ git cherry-pick --continue # This would be a no-op Change this so that the sequencer state is cleared when a conflict is encountered in the last commit. Incidentally, this patch makes sure that some existing tests don't break when features like "--reset" and "--continue" are implemented later in the series. A better way to implement this feature is to get the last "git commit" to remove the sequencer state. However, that requires tighter coupling between "git commit" and the sequencer, a goal that can be pursued once the sequencer is made more general. Signed-off-by: Ramkumar Ramachandra Acked-by: Jonathan Nieder Signed-off-by: Jonathan Nieder Signed-off-by: Junio C Hamano --- t/t3510-cherry-pick-sequence.sh | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) (limited to 't') diff --git a/t/t3510-cherry-pick-sequence.sh b/t/t3510-cherry-pick-sequence.sh index fd6986541a..a414086578 100755 --- a/t/t3510-cherry-pick-sequence.sh +++ b/t/t3510-cherry-pick-sequence.sh @@ -82,4 +82,28 @@ test_expect_success '--reset cleans up sequencer state' ' test_path_is_missing .git/sequencer ' +test_expect_success 'cherry-pick cleans up sequencer state when one commit is left' ' + pristine_detach initial && + test_must_fail git cherry-pick base..picked && + test_path_is_missing .git/sequencer && + echo "resolved" >foo && + git add foo && + git commit && + { + git rev-list HEAD | + git diff-tree --root --stdin | + sed "s/$_x40/OBJID/g" + } >actual && + cat >expect <<-\EOF && + OBJID + :100644 100644 OBJID OBJID M foo + OBJID + :100644 100644 OBJID OBJID M unrelated + OBJID + :000000 100644 OBJID OBJID A foo + :000000 100644 OBJID OBJID A unrelated + EOF + test_cmp expect actual +' + test_done -- cgit v1.2.3 From 21afd0806205d4a41c7c6076bd049842779ec080 Mon Sep 17 00:00:00 2001 From: Ramkumar Ramachandra Date: Thu, 4 Aug 2011 16:09:14 +0530 Subject: revert: Don't implicitly stomp pending sequencer operation Protect the user from forgetting about a pending sequencer operation by immediately erroring out when an existing cherry-pick or revert operation is in progress like: $ git cherry-pick foo ... conflict ... $ git cherry-pick moo error: .git/sequencer already exists hint: A cherry-pick or revert is in progress hint: Use --reset to forget about it fatal: cherry-pick failed A naive version of this would break the following established ways of working: $ git cherry-pick foo ... conflict ... $ git reset --hard # I actually meant "moo" when I said "foo" $ git cherry-pick moo $ git cherry-pick foo ... conflict ... $ git commit # commit the resolution $ git cherry-pick moo # New operation However, the previous patches "reset: Make reset remove the sequencer state" and "revert: Remove sequencer state when no commits are pending" make sure that this does not happen. Signed-off-by: Ramkumar Ramachandra Signed-off-by: Jonathan Nieder Signed-off-by: Junio C Hamano --- t/t3510-cherry-pick-sequence.sh | 9 +++++++++ 1 file changed, 9 insertions(+) (limited to 't') diff --git a/t/t3510-cherry-pick-sequence.sh b/t/t3510-cherry-pick-sequence.sh index a414086578..566a15ed8a 100755 --- a/t/t3510-cherry-pick-sequence.sh +++ b/t/t3510-cherry-pick-sequence.sh @@ -106,4 +106,13 @@ test_expect_success 'cherry-pick cleans up sequencer state when one commit is le test_cmp expect actual ' +test_expect_success 'cherry-pick does not implicitly stomp an existing operation' ' + pristine_detach initial && + test_must_fail git cherry-pick base..anotherpick && + test-chmtime -v +0 .git/sequencer >expect && + test_must_fail git cherry-pick unrelatedpick && + test-chmtime -v +0 .git/sequencer >actual && + test_cmp expect actual +' + test_done -- cgit v1.2.3 From 5a5d80f4ca16fdb1f2577474ad94135839853a3e Mon Sep 17 00:00:00 2001 From: Ramkumar Ramachandra Date: Thu, 4 Aug 2011 16:09:15 +0530 Subject: revert: Introduce --continue to continue the operation Introduce a new "git cherry-pick --continue" command which uses the information in ".git/sequencer" to continue a cherry-pick that stopped because of a conflict or other error. It works by dropping the first instruction from .git/sequencer/todo and performing the remaining cherry-picks listed there, with options (think "-s" and "-X") from the initial command listed in ".git/sequencer/opts". So now you can do: $ git cherry-pick -Xpatience foo..bar ... description conflict in commit moo ... $ git cherry-pick --continue error: 'cherry-pick' is not possible because you have unmerged files. fatal: failed to resume cherry-pick $ echo resolved >conflictingfile $ git add conflictingfile && git commit $ git cherry-pick --continue; # resumes with the commit after "moo" During the "git commit" stage, CHERRY_PICK_HEAD will aid by providing the commit message from the conflicting "moo" commit. Note that the cherry-pick mechanism has no control at this stage, so the user is free to violate anything that was specified during the first cherry-pick invocation. For example, if "-x" was specified during the first cherry-pick invocation, the user is free to edit out the message during commit time. Note that the "--signoff" option specified at cherry-pick invocation time is not reflected in the commit message provided by CHERRY_PICK_HEAD; the user must take care to add "--signoff" during the "git commit" invocation. Helped-by: Christian Couder Signed-off-by: Ramkumar Ramachandra Signed-off-by: Jonathan Nieder Signed-off-by: Junio C Hamano --- t/t3510-cherry-pick-sequence.sh | 96 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 96 insertions(+) (limited to 't') diff --git a/t/t3510-cherry-pick-sequence.sh b/t/t3510-cherry-pick-sequence.sh index 566a15ed8a..3bca2b3dd5 100755 --- a/t/t3510-cherry-pick-sequence.sh +++ b/t/t3510-cherry-pick-sequence.sh @@ -115,4 +115,100 @@ test_expect_success 'cherry-pick does not implicitly stomp an existing operation test_cmp expect actual ' +test_expect_success '--continue complains when no cherry-pick is in progress' ' + pristine_detach initial && + test_must_fail git cherry-pick --continue +' + +test_expect_success '--continue complains when there are unresolved conflicts' ' + pristine_detach initial && + test_must_fail git cherry-pick base..anotherpick && + test_must_fail git cherry-pick --continue +' + +test_expect_success '--continue continues after conflicts are resolved' ' + pristine_detach initial && + test_must_fail git cherry-pick base..anotherpick && + echo "c" >foo && + git add foo && + git commit && + git cherry-pick --continue && + test_path_is_missing .git/sequencer && + { + git rev-list HEAD | + git diff-tree --root --stdin | + sed "s/$_x40/OBJID/g" + } >actual && + cat >expect <<-\EOF && + OBJID + :100644 100644 OBJID OBJID M foo + OBJID + :100644 100644 OBJID OBJID M foo + OBJID + :100644 100644 OBJID OBJID M unrelated + OBJID + :000000 100644 OBJID OBJID A foo + :000000 100644 OBJID OBJID A unrelated + EOF + test_cmp expect actual +' + +test_expect_success '--continue respects opts' ' + pristine_detach initial && + test_must_fail git cherry-pick -x base..anotherpick && + echo "c" >foo && + git add foo && + git commit && + git cherry-pick --continue && + test_path_is_missing .git/sequencer && + git cat-file commit HEAD >anotherpick_msg && + git cat-file commit HEAD~1 >picked_msg && + git cat-file commit HEAD~2 >unrelatedpick_msg && + git cat-file commit HEAD~3 >initial_msg && + test_must_fail grep "cherry picked from" initial_msg && + grep "cherry picked from" unrelatedpick_msg && + grep "cherry picked from" picked_msg && + grep "cherry picked from" anotherpick_msg +' + +test_expect_success '--signoff is not automatically propagated to resolved conflict' ' + pristine_detach initial && + test_must_fail git cherry-pick --signoff base..anotherpick && + echo "c" >foo && + git add foo && + git commit && + git cherry-pick --continue && + test_path_is_missing .git/sequencer && + git cat-file commit HEAD >anotherpick_msg && + git cat-file commit HEAD~1 >picked_msg && + git cat-file commit HEAD~2 >unrelatedpick_msg && + git cat-file commit HEAD~3 >initial_msg && + test_must_fail grep "Signed-off-by:" initial_msg && + grep "Signed-off-by:" unrelatedpick_msg && + test_must_fail grep "Signed-off-by:" picked_msg && + grep "Signed-off-by:" anotherpick_msg +' + +test_expect_success 'malformed instruction sheet 1' ' + pristine_detach initial && + test_must_fail git cherry-pick base..anotherpick && + echo "resolved" >foo && + git add foo && + git commit && + sed "s/pick /pick/" .git/sequencer/todo >new_sheet && + cp new_sheet .git/sequencer/todo && + test_must_fail git cherry-pick --continue +' + +test_expect_success 'malformed instruction sheet 2' ' + pristine_detach initial && + test_must_fail git cherry-pick base..anotherpick && + echo "resolved" >foo && + git add foo && + git commit && + sed "s/pick/revert/" .git/sequencer/todo >new_sheet && + cp new_sheet .git/sequencer/todo && + test_must_fail git cherry-pick --continue +' + test_done -- cgit v1.2.3