Skip to content

Commit 99f5b08

Browse files
gitstergitster
committed
Merge branch 'cc/cherry-pick-ff'
* cc/cherry-pick-ff: revert: fix tiny memory leak in cherry-pick --ff rebase -i: use new --ff cherry-pick option Documentation: describe new cherry-pick --ff option cherry-pick: add tests for new --ff option revert: add --ff option to allow fast forward when cherry-picking builtin/merge: make checkout_fast_forward() non static parse-options: add parse_options_concat() to concat options
2 parents 3b37d9c + 28db756 commit 99f5b08

File tree

8 files changed

+168
-18
lines changed

8 files changed

+168
-18
lines changed

‎Documentation/git-cherry-pick.txt‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ git-cherry-pick - Apply the change introduced by an existing commit
77

88
SYNOPSIS
99
--------
10-
'git cherry-pick' [--edit] [-n] [-m parent-number] [-s] [-x] <commit>
10+
'git cherry-pick' [--edit] [-n] [-m parent-number] [-s] [-x] [--ff] <commit>
1111

1212
DESCRIPTION
1313
-----------
@@ -70,6 +70,10 @@ effect to your index in a row.
7070
--signoff::
7171
Add Signed-off-by line at the end of the commit message.
7272

73+
--ff::
74+
If the current HEAD is the same as the parent of the
75+
cherry-pick'ed commit, then a fast forward to this commit will
76+
be performed.
7377

7478
Author
7579
------

‎builtin/merge.c‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -667,7 +667,7 @@ static int count_unmerged_entries(void)
667667
return ret;
668668
}
669669

670-
static int checkout_fast_forward(unsigned char *head, unsigned char *remote)
670+
int checkout_fast_forward(const unsigned char *head, const unsigned char *remote)
671671
{
672672
struct tree *trees[MAX_UNPACK_TREES];
673673
struct unpack_trees_options opts;

‎builtin/revert.c‎

Lines changed: 42 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
#include "revision.h"
1414
#include "rerere.h"
1515
#include "merge-recursive.h"
16+
#include "refs.h"
1617

1718
/*
1819
* This implements the builtins revert and cherry-pick.
@@ -35,7 +36,7 @@ static const char * const cherry_pick_usage[] = {
3536
NULL
3637
};
3738

38-
static int edit, no_replay, no_commit, mainline, signoff;
39+
static int edit, no_replay, no_commit, mainline, signoff, allow_ff;
3940
static enum { REVERT, CHERRY_PICK } action;
4041
static struct commit *commit;
4142
static const char *commit_name;
@@ -60,8 +61,19 @@ static void parse_args(int argc, const char **argv)
6061
OPT_INTEGER('m', "mainline", &mainline, "parent number"),
6162
OPT_RERERE_AUTOUPDATE(&allow_rerere_auto),
6263
OPT_END(),
64+
OPT_END(),
65+
OPT_END(),
6366
};
6467

68+
if (action == CHERRY_PICK) {
69+
struct option cp_extra[] = {
70+
OPT_BOOLEAN(0, "ff", &allow_ff, "allow fast-forward"),
71+
OPT_END(),
72+
};
73+
if (parse_options_concat(options, ARRAY_SIZE(options), cp_extra))
74+
die("program error");
75+
}
76+
6577
if (parse_options(argc, argv, NULL, options, usage_str, 0) != 1)
6678
usage_with_options(usage_str, options);
6779

@@ -244,14 +256,25 @@ static NORETURN void die_dirty_index(const char *me)
244256
}
245257
}
246258

259+
static int fast_forward_to(const unsigned char *to, const unsigned char *from)
260+
{
261+
struct ref_lock *ref_lock;
262+
263+
read_cache();
264+
if (checkout_fast_forward(from, to))
265+
exit(1); /* the callee should have complained already */
266+
ref_lock = lock_any_ref_for_update("HEAD", from, 0);
267+
return write_ref_sha1(ref_lock, to, "cherry-pick");
268+
}
269+
247270
static int revert_or_cherry_pick(int argc, const char **argv)
248271
{
249272
unsigned char head[20];
250273
struct commit *base, *next, *parent;
251274
int i, index_fd, clean;
252275
char *oneline, *reencoded_message = NULL;
253276
const char *message, *encoding;
254-
char *defmsg = git_pathdup("MERGE_MSG");
277+
char *defmsg = NULL;
255278
struct merge_options o;
256279
struct tree *result, *next_tree, *base_tree, *head_tree;
257280
static struct lock_file index_lock;
@@ -265,6 +288,17 @@ static int revert_or_cherry_pick(int argc, const char **argv)
265288
if (action == REVERT && !no_replay)
266289
die("revert is incompatible with replay");
267290

291+
if (allow_ff) {
292+
if (signoff)
293+
die("cherry-pick --ff cannot be used with --signoff");
294+
if (no_commit)
295+
die("cherry-pick --ff cannot be used with --no-commit");
296+
if (no_replay)
297+
die("cherry-pick --ff cannot be used with -x");
298+
if (edit)
299+
die("cherry-pick --ff cannot be used with --edit");
300+
}
301+
268302
if (read_cache() < 0)
269303
die("git %s: failed to read the index", me);
270304
if (no_commit) {
@@ -284,8 +318,6 @@ static int revert_or_cherry_pick(int argc, const char **argv)
284318
}
285319
discard_cache();
286320

287-
index_fd = hold_locked_index(&index_lock, 1);
288-
289321
if (!commit->parents) {
290322
if (action == REVERT)
291323
die ("Cannot revert a root commit");
@@ -314,6 +346,9 @@ static int revert_or_cherry_pick(int argc, const char **argv)
314346
else
315347
parent = commit->parents->item;
316348

349+
if (allow_ff && !hashcmp(parent->object.sha1, head))
350+
return fast_forward_to(commit->object.sha1, head);
351+
317352
if (!(message = commit->buffer))
318353
die ("Cannot get commit message for %s",
319354
sha1_to_hex(commit->object.sha1));
@@ -329,6 +364,7 @@ static int revert_or_cherry_pick(int argc, const char **argv)
329364
* reverse of it if we are revert.
330365
*/
331366

367+
defmsg = git_pathdup("MERGE_MSG");
332368
msg_fd = hold_lock_file_for_update(&msg_file, defmsg,
333369
LOCK_DIE_ON_ERROR);
334370

@@ -343,6 +379,8 @@ static int revert_or_cherry_pick(int argc, const char **argv)
343379

344380
oneline = get_oneline(message);
345381

382+
index_fd = hold_locked_index(&index_lock, 1);
383+
346384
if (action == REVERT) {
347385
char *oneline_body = strchr(oneline, ' ');
348386

‎cache.h‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1058,4 +1058,7 @@ void overlay_tree_on_cache(const char *tree_name, const char *prefix);
10581058
char *alias_lookup(const char *alias);
10591059
int split_cmdline(char *cmdline, const char ***argv);
10601060

1061+
/* builtin/merge.c */
1062+
int checkout_fast_forward(const unsigned char *from, const unsigned char *to);
1063+
10611064
#endif /* CACHE_H */

‎git-rebase--interactive.sh‎

Lines changed: 3 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -230,8 +230,8 @@ do_with_author () {
230230
}
231231

232232
pick_one () {
233-
no_ff=
234-
case "$1" in -n) sha1=$2; no_ff=t ;; *) sha1=$1 ;; esac
233+
ff=--ff
234+
case "$1" in -n) sha1=$2; ff= ;; *) sha1=$1 ;; esac
235235
output git rev-parse --verify $sha1 || die "Invalid commit name: $sha1"
236236
test -d "$REWRITTEN" &&
237237
pick_one_preserving_merges "$@" && return
@@ -240,16 +240,7 @@ pick_one () {
240240
output git cherry-pick "$@"
241241
return
242242
fi
243-
parent_sha1=$(git rev-parse --verify $sha1^) ||
244-
die "Could not get the parent of $sha1"
245-
current_sha1=$(git rev-parse --verify HEAD)
246-
if test -z "$no_ff" && test "$current_sha1" = "$parent_sha1"
247-
then
248-
output git reset --hard $sha1
249-
output warn Fast-forward to $(git rev-parse --short $sha1)
250-
else
251-
output git cherry-pick "$@"
252-
fi
243+
output git cherry-pick $ff "$@"
253244
}
254245

255246
pick_one_preserving_merges () {

‎parse-options.c‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -659,3 +659,18 @@ int parse_opt_tertiary(const struct option *opt, const char *arg, int unset)
659659
*target = unset ? 2 : 1;
660660
return 0;
661661
}
662+
663+
int parse_options_concat(struct option *dst, size_t dst_size, struct option *src)
664+
{
665+
int i, j;
666+
667+
for (i = 0; i < dst_size; i++)
668+
if (dst[i].type == OPTION_END)
669+
break;
670+
for (j = 0; i < dst_size; i++, j++) {
671+
dst[i] = src[j];
672+
if (src[j].type == OPTION_END)
673+
return 0;
674+
}
675+
return -1;
676+
}

‎parse-options.h‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -187,6 +187,7 @@ extern int parse_options_step(struct parse_opt_ctx_t *ctx,
187187

188188
extern int parse_options_end(struct parse_opt_ctx_t *ctx);
189189

190+
extern int parse_options_concat(struct option *dst, size_t, struct option *src);
190191

191192
/*----- some often used options -----*/
192193
extern int parse_opt_abbrev_cb(const struct option *, const char *, int);

‎t/t3506-cherry-pick-ff.sh‎

Lines changed: 98 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,98 @@
1+
#!/bin/sh
2+
3+
test_description='test cherry-picking with --ff option'
4+
5+
. ./test-lib.sh
6+
7+
test_expect_success setup '
8+
echo first > file1 &&
9+
git add file1 &&
10+
test_tick &&
11+
git commit -m "first" &&
12+
git tag first &&
13+
14+
git checkout -b other &&
15+
echo second >> file1 &&
16+
git add file1 &&
17+
test_tick &&
18+
git commit -m "second" &&
19+
git tag second
20+
'
21+
22+
test_expect_success 'cherry-pick using --ff fast forwards' '
23+
git checkout master &&
24+
git reset --hard first &&
25+
test_tick &&
26+
git cherry-pick --ff second &&
27+
test "$(git rev-parse --verify HEAD)" = "$(git rev-parse --verify second)"
28+
'
29+
30+
test_expect_success 'cherry-pick not using --ff does not fast forwards' '
31+
git checkout master &&
32+
git reset --hard first &&
33+
test_tick &&
34+
git cherry-pick second &&
35+
test "$(git rev-parse --verify HEAD)" != "$(git rev-parse --verify second)"
36+
'
37+
38+
#
39+
# We setup the following graph:
40+
#
41+
# B---C
42+
# / /
43+
# first---A
44+
#
45+
# (This has been taken from t3502-cherry-pick-merge.sh)
46+
#
47+
test_expect_success 'merge setup' '
48+
git checkout master &&
49+
git reset --hard first &&
50+
echo new line >A &&
51+
git add A &&
52+
test_tick &&
53+
git commit -m "add line to A" A &&
54+
git tag A &&
55+
git checkout -b side first &&
56+
echo new line >B &&
57+
git add B &&
58+
test_tick &&
59+
git commit -m "add line to B" B &&
60+
git tag B &&
61+
git checkout master &&
62+
git merge side &&
63+
git tag C &&
64+
git checkout -b new A
65+
'
66+
67+
test_expect_success 'cherry-pick a non-merge with --ff and -m should fail' '
68+
git reset --hard A -- &&
69+
test_must_fail git cherry-pick --ff -m 1 B &&
70+
git diff --exit-code A --
71+
'
72+
73+
test_expect_success 'cherry pick a merge with --ff but without -m should fail' '
74+
git reset --hard A -- &&
75+
test_must_fail git cherry-pick --ff C &&
76+
git diff --exit-code A --
77+
'
78+
79+
test_expect_success 'cherry pick with --ff a merge (1)' '
80+
git reset --hard A -- &&
81+
git cherry-pick --ff -m 1 C &&
82+
git diff --exit-code C &&
83+
test "$(git rev-parse --verify HEAD)" = "$(git rev-parse --verify C)"
84+
'
85+
86+
test_expect_success 'cherry pick with --ff a merge (2)' '
87+
git reset --hard B -- &&
88+
git cherry-pick --ff -m 2 C &&
89+
git diff --exit-code C &&
90+
test "$(git rev-parse --verify HEAD)" = "$(git rev-parse --verify C)"
91+
'
92+
93+
test_expect_success 'cherry pick a merge relative to nonexistent parent with --ff should fail' '
94+
git reset --hard B -- &&
95+
test_must_fail git cherry-pick --ff -m 3 C
96+
'
97+
98+
test_done

0 commit comments

Comments
 (0)