From df478b744cee9821eb2abbe2202d262b87a34289 Mon Sep 17 00:00:00 2001 From: Neil Horman Date: Wed, 11 Apr 2012 16:21:53 -0400 Subject: git-cherry-pick: add allow-empty option git cherry-pick fails when picking a non-ff commit that is empty. The advice given with the failure is that a git-commit --allow-empty should be issued to explicitly add the empty commit during the cherry pick. This option allows a user to specify before hand that they want to keep the empty commit. This eliminates the need to issue both a cherry pick and a commit operation. Signed-off-by: Neil Horman Signed-off-by: Junio C Hamano diff --git a/Documentation/git-cherry-pick.txt b/Documentation/git-cherry-pick.txt index fed5097..730237a 100644 --- a/Documentation/git-cherry-pick.txt +++ b/Documentation/git-cherry-pick.txt @@ -103,6 +103,15 @@ effect to your index in a row. cherry-pick'ed commit, then a fast forward to this commit will be performed. +--allow-empty:: + By default, cherry-picking an empty commit will fail, + indicating that an explicit invocation of `git commit + --allow-empty` is required. This option overrides that + behavior, allowing empty commits to be preserved automatically + in a cherry-pick. Note that when "--ff" is in effect, empty + commits that meet the "fast-forward" requirement will be kept + even without this option. + --strategy=:: Use the given merge strategy. Should only be used once. See the MERGE STRATEGIES section in linkgit:git-merge[1] diff --git a/builtin/revert.c b/builtin/revert.c index e6840f2..06b00e6 100644 --- a/builtin/revert.c +++ b/builtin/revert.c @@ -114,12 +114,14 @@ static void parse_args(int argc, const char **argv, struct replay_opts *opts) OPT_END(), OPT_END(), OPT_END(), + OPT_END(), }; if (opts->action == REPLAY_PICK) { struct option cp_extra[] = { OPT_BOOLEAN('x', NULL, &opts->record_origin, "append commit name"), OPT_BOOLEAN(0, "ff", &opts->allow_ff, "allow fast-forward"), + OPT_BOOLEAN(0, "allow-empty", &opts->allow_empty, "preserve empty commits"), OPT_END(), }; if (parse_options_concat(options, ARRAY_SIZE(options), cp_extra)) diff --git a/sequencer.c b/sequencer.c index a37846a..71929ba 100644 --- a/sequencer.c +++ b/sequencer.c @@ -260,8 +260,8 @@ static int do_recursive_merge(struct commit *base, struct commit *next, */ static int run_git_commit(const char *defmsg, struct replay_opts *opts) { - /* 6 is max possible length of our args array including NULL */ - const char *args[6]; + /* 7 is max possible length of our args array including NULL */ + const char *args[7]; int i = 0; args[i++] = "commit"; @@ -272,6 +272,9 @@ static int run_git_commit(const char *defmsg, struct replay_opts *opts) args[i++] = "-F"; args[i++] = defmsg; } + if (opts->allow_empty) + args[i++] = "--allow-empty"; + args[i] = NULL; return run_command_v_opt(args, RUN_GIT_CMD); diff --git a/sequencer.h b/sequencer.h index bb4b138..e2cd725 100644 --- a/sequencer.h +++ b/sequencer.h @@ -29,6 +29,7 @@ struct replay_opts { int signoff; int allow_ff; int allow_rerere_auto; + int allow_empty; int mainline; -- cgit v0.10.2-6-g49f6 From b27cfb0d8d4cbb6d079c70ffeadac9c0dcfff250 Mon Sep 17 00:00:00 2001 From: Neil Horman Date: Fri, 20 Apr 2012 10:36:15 -0400 Subject: git-cherry-pick: Add keep-redundant-commits option The git-cherry-pick --allow-empty command by default only preserves empty commits that were originally empty, i.e only those commits for which ^{tree} and ^^{tree} are equal. By default commits which are non-empty, but were made empty by the inclusion of a prior commit on the current history are filtered out. This option allows us to override that behavior and include redundant commits as empty commits in the change history. Note that this patch changes the default behavior of git cherry-pick slightly. Prior to this patch all commits in a cherry-pick sequence were applied and git commit was run. The implication here was that, if a commit was redundant, and the commit did not trigger the fast forward logic, the git commit operation, and therefore the git cherry-pick operation would fail, displaying the cherry pick advice (i.e. run git commit --allow-empty). With this patch however, such redundant commits are automatically skipped without stopping, unless --keep-redundant-commits is specified, in which case, they are automatically applied as empty commits. Signed-off-by: Neil Horman Signed-off-by: Junio C Hamano diff --git a/Documentation/git-cherry-pick.txt b/Documentation/git-cherry-pick.txt index 730237a..3d25a20 100644 --- a/Documentation/git-cherry-pick.txt +++ b/Documentation/git-cherry-pick.txt @@ -110,7 +110,17 @@ effect to your index in a row. behavior, allowing empty commits to be preserved automatically in a cherry-pick. Note that when "--ff" is in effect, empty commits that meet the "fast-forward" requirement will be kept - even without this option. + even without this option. Note also, that use of this option only + keeps commits that were initially empty (i.e. the commit recorded the + same tree as its parent). Commits which are made empty due to a + previous commit are dropped. To force the inclusion of those commits + use `--keep-redundant-commits`. + +--keep-redundant-commits:: + If a commit being cherry picked duplicates a commit already in the + current history, it will become empty. By default these + redundant commits are ignored. This option overrides that behavior and + creates an empty commit object. Implies `--allow-empty`. --strategy=:: Use the given merge strategy. Should only be used once. diff --git a/builtin/revert.c b/builtin/revert.c index 06b00e6..b0b9b1a 100644 --- a/builtin/revert.c +++ b/builtin/revert.c @@ -115,13 +115,15 @@ static void parse_args(int argc, const char **argv, struct replay_opts *opts) OPT_END(), OPT_END(), OPT_END(), + OPT_END(), }; if (opts->action == REPLAY_PICK) { struct option cp_extra[] = { OPT_BOOLEAN('x', NULL, &opts->record_origin, "append commit name"), OPT_BOOLEAN(0, "ff", &opts->allow_ff, "allow fast-forward"), - OPT_BOOLEAN(0, "allow-empty", &opts->allow_empty, "preserve empty commits"), + OPT_BOOLEAN(0, "allow-empty", &opts->allow_empty, "preserve initially empty commits"), + OPT_BOOLEAN(0, "keep-redundant-commits", &opts->keep_redundant_commits, "keep redundant, empty commits"), OPT_END(), }; if (parse_options_concat(options, ARRAY_SIZE(options), cp_extra)) @@ -139,6 +141,10 @@ static void parse_args(int argc, const char **argv, struct replay_opts *opts) "--abort", rollback, NULL); + /* implies allow_empty */ + if (opts->keep_redundant_commits) + opts->allow_empty = 1; + /* Set the subcommand */ if (remove_state) opts->subcommand = REPLAY_REMOVE_STATE; diff --git a/sequencer.c b/sequencer.c index 71929ba..4e3af82 100644 --- a/sequencer.c +++ b/sequencer.c @@ -13,6 +13,7 @@ #include "rerere.h" #include "merge-recursive.h" #include "refs.h" +#include "argv-array.h" #define GIT_REFLOG_ACTION "GIT_REFLOG_ACTION" @@ -251,6 +252,30 @@ static int do_recursive_merge(struct commit *base, struct commit *next, return !clean; } +static int is_index_unchanged(void) +{ + unsigned char head_sha1[20]; + struct commit *head_commit; + + if (!resolve_ref_unsafe("HEAD", head_sha1, 1, NULL)) + return error(_("Could not resolve HEAD commit\n")); + + head_commit = lookup_commit(head_sha1); + if (!head_commit || parse_commit(head_commit)) + return error(_("could not parse commit %s\n"), + sha1_to_hex(head_commit->object.sha1)); + + if (!active_cache_tree) + active_cache_tree = cache_tree(); + + if (!cache_tree_fully_valid(active_cache_tree)) + if (cache_tree_update(active_cache_tree, active_cache, + active_nr, 0)) + return error(_("Unable to update cache tree\n")); + + return !hashcmp(active_cache_tree->sha1, head_commit->tree->object.sha1); +} + /* * If we are cherry-pick, and if the merge did not result in * hand-editing, we will hit this commit and inherit the original @@ -260,24 +285,46 @@ static int do_recursive_merge(struct commit *base, struct commit *next, */ static int run_git_commit(const char *defmsg, struct replay_opts *opts) { - /* 7 is max possible length of our args array including NULL */ - const char *args[7]; - int i = 0; + struct argv_array array; + int rc; + + argv_array_init(&array); + argv_array_push(&array, "commit"); + argv_array_push(&array, "-n"); - args[i++] = "commit"; - args[i++] = "-n"; if (opts->signoff) - args[i++] = "-s"; + argv_array_push(&array, "-s"); if (!opts->edit) { - args[i++] = "-F"; - args[i++] = defmsg; + argv_array_push(&array, "-F"); + argv_array_push(&array, defmsg); } + if (opts->allow_empty) - args[i++] = "--allow-empty"; + argv_array_push(&array, "--allow-empty"); - args[i] = NULL; + rc = run_command_v_opt(array.argv, RUN_GIT_CMD); + argv_array_clear(&array); + return rc; +} + +static int is_original_commit_empty(struct commit *commit) +{ + const unsigned char *ptree_sha1; + + if (parse_commit(commit)) + return error(_("Could not parse commit %s\n"), + sha1_to_hex(commit->object.sha1)); + if (commit->parents) { + struct commit *parent = commit->parents->item; + if (parse_commit(parent)) + return error(_("Could not parse parent commit %s\n"), + sha1_to_hex(parent->object.sha1)); + ptree_sha1 = parent->tree->object.sha1; + } else { + ptree_sha1 = EMPTY_TREE_SHA1_BIN; /* commit is root */ + } - return run_command_v_opt(args, RUN_GIT_CMD); + return !hashcmp(ptree_sha1, commit->tree->object.sha1); } static int do_pick_commit(struct commit *commit, struct replay_opts *opts) @@ -289,6 +336,8 @@ static int do_pick_commit(struct commit *commit, struct replay_opts *opts) char *defmsg = NULL; struct strbuf msgbuf = STRBUF_INIT; int res; + int empty_commit; + int index_unchanged; if (opts->no_commit) { /* @@ -414,6 +463,10 @@ static int do_pick_commit(struct commit *commit, struct replay_opts *opts) free_commit_list(remotes); } + empty_commit = is_original_commit_empty(commit); + if (empty_commit < 0) + return empty_commit; + /* * If the merge was clean or if it failed due to conflict, we write * CHERRY_PICK_HEAD for the subsequent invocation of commit to use. @@ -434,6 +487,25 @@ static int do_pick_commit(struct commit *commit, struct replay_opts *opts) print_advice(res == 1, opts); rerere(opts->allow_rerere_auto); } else { + index_unchanged = is_index_unchanged(); + /* + * If index_unchanged is less than 0, that indicates we either + * couldn't parse HEAD or the index, so error out here. + */ + if (index_unchanged < 0) + return index_unchanged; + + if (!empty_commit && !opts->keep_redundant_commits && index_unchanged) + /* + * The head tree and the index match + * meaning the commit is empty. Since it wasn't created + * empty (based on the previous test), we can conclude + * the commit has been made redundant. Since we don't + * want to keep redundant commits, we can just return + * here, skipping this commit + */ + return 0; + if (!opts->no_commit) res = run_git_commit(defmsg, opts); } diff --git a/sequencer.h b/sequencer.h index e2cd725..aa5f17c 100644 --- a/sequencer.h +++ b/sequencer.h @@ -30,6 +30,7 @@ struct replay_opts { int allow_ff; int allow_rerere_auto; int allow_empty; + int keep_redundant_commits; int mainline; -- cgit v0.10.2-6-g49f6 From bedfe86ce6ccee90e2cbecbf8d72e06219a2768a Mon Sep 17 00:00:00 2001 From: Neil Horman Date: Fri, 20 Apr 2012 10:36:16 -0400 Subject: git-cherry-pick: Add test to validate new options Since we've added the --allow-empty and --keep-redundant-commits options to git cherry-pick we should also add a test to ensure that its working properly. Signed-off-by: Neil Horman Signed-off-by: Junio C Hamano diff --git a/t/t3505-cherry-pick-empty.sh b/t/t3505-cherry-pick-empty.sh index c10b28c..92f00cd 100755 --- a/t/t3505-cherry-pick-empty.sh +++ b/t/t3505-cherry-pick-empty.sh @@ -18,7 +18,12 @@ test_expect_success setup ' echo third >> file1 && git add file1 && test_tick && - git commit --allow-empty-message -m "" + git commit --allow-empty-message -m "" && + + git checkout master && + git checkout -b empty-branch2 && + test_tick && + git commit --allow-empty -m "empty" ' @@ -48,4 +53,22 @@ test_expect_success 'index lockfile was removed' ' ' +test_expect_success 'cherry pick an empty non-ff commit without --allow-empty' ' + git checkout master && + echo fourth >>file2 && + git add file2 && + git commit -m "fourth" && + test_must_fail git cherry-pick empty-branch2 +' + +test_expect_success 'cherry pick an empty non-ff commit with --allow-empty' ' + git checkout master && + git cherry-pick --allow-empty empty-branch2 +' + +test_expect_success 'cherry pick with --keep-redundant-commits' ' + git checkout master && + git cherry-pick --keep-redundant-commits HEAD^ +' + test_done -- cgit v0.10.2-6-g49f6 From 90e1818f9a06015159712e204dd90868e0a6c421 Mon Sep 17 00:00:00 2001 From: Neil Horman Date: Fri, 20 Apr 2012 10:36:17 -0400 Subject: git-rebase: add keep_empty flag Add a command line switch to git-rebase to allow a user the ability to specify that they want to keep any commits in a series that are empty. When git-rebase's type is am, then this option will automatically keep any commit that has a tree object identical to its parent. This patch changes the default behavior of interactive rebases as well. With this patch, git-rebase -i will produce a revision set passed to git-revision-editor, in which empty commits are commented out. Empty commits may be kept manually by uncommenting them. If the new --keep-empty option is used in an interactive rebase the empty commits will automatically all be uncommented in the editor. Signed-off-by: Neil Horman Signed-off-by: Junio C Hamano diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt index 520aaa9..841ebd6 100644 --- a/Documentation/git-rebase.txt +++ b/Documentation/git-rebase.txt @@ -238,6 +238,10 @@ leave out at most one of A and B, in which case it defaults to HEAD. will be reset to where it was when the rebase operation was started. +--keep-empty:: + Keep the commits that do not change anything from its + parents in the result. + --skip:: Restart the rebasing process by skipping the current patch. diff --git a/git-rebase--am.sh b/git-rebase--am.sh index c815a24..04d8941 100644 --- a/git-rebase--am.sh +++ b/git-rebase--am.sh @@ -20,11 +20,20 @@ esac test -n "$rebase_root" && root_flag=--root -git format-patch -k --stdout --full-index --ignore-if-in-upstream \ - --src-prefix=a/ --dst-prefix=b/ \ - --no-renames $root_flag "$revisions" | -git am $git_am_opt --rebasing --resolvemsg="$resolvemsg" && -move_to_original_branch +if test -n "$keep_empty" +then + # we have to do this the hard way. git format-patch completely squashes + # empty commits and even if it didn't the format doesn't really lend + # itself well to recording empty patches. fortunately, cherry-pick + # makes this easy + git cherry-pick --allow-empty "$revisions" +else + git format-patch -k --stdout --full-index --ignore-if-in-upstream \ + --src-prefix=a/ --dst-prefix=b/ \ + --no-renames $root_flag "$revisions" | + git am $git_am_opt --rebasing --resolvemsg="$resolvemsg" +fi && move_to_original_branch + ret=$? test 0 != $ret -a -d "$state_dir" && write_basic_state exit $ret diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh index 5812222..70538bb 100644 --- a/git-rebase--interactive.sh +++ b/git-rebase--interactive.sh @@ -167,6 +167,14 @@ has_action () { sane_grep '^[^#]' "$1" >/dev/null } +is_empty_commit() { + tree=$(git rev-parse -q --verify "$1"^{tree} 2>/dev/null || + die "$1: not a commit that can be picked") + ptree=$(git rev-parse -q --verify "$1"^^{tree} 2>/dev/null || + ptree=4b825dc642cb6eb9a060e54bf8d69288fbee4904) + test "$tree" = "$ptree" +} + # Run command with GIT_AUTHOR_NAME, GIT_AUTHOR_EMAIL, and # GIT_AUTHOR_DATE exported from the current environment. do_with_author () { @@ -191,12 +199,19 @@ git_sequence_editor () { pick_one () { ff=--ff + case "$1" in -n) sha1=$2; ff= ;; *) sha1=$1 ;; esac case "$force_rebase" in '') ;; ?*) ff= ;; esac output git rev-parse --verify $sha1 || die "Invalid commit name: $sha1" + + if is_empty_commit "$sha1" + then + empty_args="--allow-empty" + fi + test -d "$rewritten" && pick_one_preserving_merges "$@" && return - output git cherry-pick $ff "$@" + output git cherry-pick $empty_args $ff "$@" } pick_one_preserving_merges () { @@ -780,9 +795,17 @@ git rev-list $merges_option --pretty=oneline --abbrev-commit \ sed -n "s/^>//p" | while read -r shortsha1 rest do + + if test -z "$keep_empty" && is_empty_commit $shortsha1 + then + comment_out="# " + else + comment_out= + fi + if test t != "$preserve_merges" then - printf '%s\n' "pick $shortsha1 $rest" >> "$todo" + printf '%s\n' "${comment_out}pick $shortsha1 $rest" >>"$todo" else sha1=$(git rev-parse $shortsha1) if test -z "$rebase_root" @@ -801,7 +824,7 @@ do if test f = "$preserve" then touch "$rewritten"/$sha1 - printf '%s\n' "pick $shortsha1 $rest" >> "$todo" + printf '%s\n' "${comment_out}pick $shortsha1 $rest" >>"$todo" fi fi done @@ -851,6 +874,12 @@ cat >> "$todo" << EOF # EOF +if test -z "$keep_empty" +then + echo "# Note that empty commits are commented out" >>"$todo" +fi + + has_action "$todo" || die_abort "Nothing to do" diff --git a/git-rebase.sh b/git-rebase.sh index 69c1374..24a2840 100755 --- a/git-rebase.sh +++ b/git-rebase.sh @@ -43,6 +43,7 @@ s,strategy=! use the given merge strategy no-ff! cherry-pick all commits, even if unchanged m,merge! use merging strategies to rebase i,interactive! let the user edit the list of commits to rebase +k,keep-empty preserve empty commits during rebase f,force-rebase! force rebase even if branch is up to date X,strategy-option=! pass the argument through to the merge strategy stat! display a diffstat of what changed upstream @@ -97,6 +98,7 @@ state_dir= action= preserve_merges= autosquash= +keep_empty= test "$(git config --bool rebase.autosquash)" = "true" && autosquash=t read_basic_state () { @@ -220,6 +222,9 @@ do -i) interactive_rebase=explicit ;; + -k) + keep_empty=yes + ;; -p) preserve_merges=t test -z "$interactive_rebase" && interactive_rebase=implied -- cgit v0.10.2-6-g49f6