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
<commit>^{tree} and <commit>^^{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 <nhorman@tuxdriver.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/Documentation/git-cherry-pick.txt b/Documentation/git-cherry-pick.txt
index 730237ab27a5e0244c1708eb2546123cc711d233..3d25a20b67a0dbe9dbb463c6d7bdc53a16fd96e6 100644 (file)
--- 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=<strategy>::
        Use the given merge strategy.  Should only be used once.

diff --git a/builtin/revert.c b/builtin/revert.c
index 06b00e6f83d797f12c7dc261d35adb1c3de0a812..b0b9b1a4e6550806b4ab71f85a628aa87df62ab4 100644 (file)
--- 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 71929ba8cdeb45dc4a05a6513ac72b90dacf32be..4e3af82442b949a387bb630c77c2f3f4add0bb03 100644 (file)
--- 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 e2cd725401390939ed765b6cd2c4bd3393a99179..aa5f17cc3079f39000e9d7526d24354a027daabd 100644 (file)
--- 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;