Skip to content

Commit

Permalink
Merge branch 'so/cherry-pick-always-allow-m1'
Browse files Browse the repository at this point in the history
"git cherry-pick -m1" was forbidden when picking a non-merge
commit, even though there _is_ parent number 1 for such a commit.
This was done to avoid mistakes back when "cherry-pick" was about
picking a single commit, but is no longer useful with "cherry-pick"
that can pick a range of commits.  Now the "-m$num" option is
allowed when picking any commit, as long as $num names an existing
parent of the commit.

Technically this is a backward incompatible change; hopefully
nobody is relying on the error-checking behaviour.

* so/cherry-pick-always-allow-m1:
  t3506: validate '-m 1 -ff' is now accepted for non-merge commits
  t3502: validate '-m 1' argument is now accepted for non-merge commits
  cherry-pick: do not error on non-merge commits when '-m 1' is specified
  t3510: stop using '-m 1' to force failure mid-sequence of cherry-picks
  • Loading branch information
gitster committed Jan 18, 2019
2 parents 726f89c + 1c32013 commit 77fbd96
Show file tree
Hide file tree
Showing 4 changed files with 22 additions and 14 deletions.
10 changes: 7 additions & 3 deletions sequencer.c
Original file line number Diff line number Diff line change
Expand Up @@ -1790,9 +1790,13 @@ static int do_pick_commit(struct repository *r,
return error(_("commit %s does not have parent %d"),
oid_to_hex(&commit->object.oid), opts->mainline);
parent = p->item;
} else if (0 < opts->mainline)
return error(_("mainline was specified but commit %s is not a merge."),
oid_to_hex(&commit->object.oid));
} else if (1 < opts->mainline)
/*
* Non-first parent explicitly specified as mainline for
* non-merge commit
*/
return error(_("commit %s does not have parent %d"),
oid_to_hex(&commit->object.oid), opts->mainline);
else
parent = commit->parents->item;

Expand Down
12 changes: 6 additions & 6 deletions t/t3502-cherry-pick-merge.sh
Original file line number Diff line number Diff line change
Expand Up @@ -40,12 +40,12 @@ test_expect_success 'cherry-pick -m complains of bogus numbers' '
test_expect_code 129 git cherry-pick -m 0 b
'

test_expect_success 'cherry-pick a non-merge with -m should fail' '
test_expect_success 'cherry-pick explicit first parent of a non-merge' '
git reset --hard &&
git checkout a^0 &&
test_expect_code 128 git cherry-pick -m 1 b &&
git diff --exit-code a --
git cherry-pick -m 1 b &&
git diff --exit-code c --
'

Expand Down Expand Up @@ -84,12 +84,12 @@ test_expect_success 'cherry pick a merge relative to nonexistent parent should f
'

test_expect_success 'revert a non-merge with -m should fail' '
test_expect_success 'revert explicit first parent of a non-merge' '
git reset --hard &&
git checkout c^0 &&
test_must_fail git revert -m 1 b &&
git diff --exit-code c
git revert -m 1 b &&
git diff --exit-code a --
'

Expand Down
6 changes: 3 additions & 3 deletions t/t3506-cherry-pick-ff.sh
Original file line number Diff line number Diff line change
Expand Up @@ -64,10 +64,10 @@ test_expect_success 'merge setup' '
git checkout -b new A
'

test_expect_success 'cherry-pick a non-merge with --ff and -m should fail' '
test_expect_success 'cherry-pick explicit first parent of a non-merge with --ff' '
git reset --hard A -- &&
test_must_fail git cherry-pick --ff -m 1 B &&
git diff --exit-code A --
git cherry-pick --ff -m 1 B &&
git diff --exit-code C --
'

test_expect_success 'cherry pick a merge with --ff but without -m should fail' '
Expand Down
8 changes: 6 additions & 2 deletions t/t3510-cherry-pick-sequence.sh
Original file line number Diff line number Diff line change
Expand Up @@ -61,15 +61,19 @@ test_expect_success 'cherry-pick mid-cherry-pick-sequence' '

test_expect_success 'cherry-pick persists opts correctly' '
pristine_detach initial &&
test_expect_code 128 git cherry-pick -s -m 1 --strategy=recursive -X patience -X ours initial..anotherpick &&
# to make sure that the session to cherry-pick a sequence
# gets interrupted, use a high-enough number that is larger
# than the number of parents of any commit we have created
mainline=4 &&
test_expect_code 128 git cherry-pick -s -m $mainline --strategy=recursive -X patience -X ours initial..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 &&
echo "$mainline" >expect &&
git config --file=.git/sequencer/opts --get-all options.mainline >actual &&
test_cmp expect actual &&
echo "recursive" >expect &&
Expand Down

0 comments on commit 77fbd96

Please sign in to comment.