Merge branch 'nh/empty-rebase'

* nh/empty-rebase:
cherry-pick: regression fix for empty commits

diff --git a/sequencer.c b/sequencer.c
index 3c384b94d24e3099cb149c66347386d1430fadef..bf078f274bfe23997fcd1bc8c2a1c958f39ac9fe 100644 (file)
--- a/sequencer.c
+++ b/sequencer.c
@@ -291,7 +291,8 @@ static int is_index_unchanged(void)
  * If we are revert, or if our cherry-pick results in a hand merge,
  * we had better say that the current user is responsible for that.
  */
-static int run_git_commit(const char *defmsg, struct replay_opts *opts)
+static int run_git_commit(const char *defmsg, struct replay_opts *opts,
+                         int allow_empty)
 {
        struct argv_array array;
        int rc;
@@ -307,7 +308,7 @@ static int run_git_commit(const char *defmsg, struct replay_opts *opts)
                argv_array_push(&array, defmsg);
        }
 
-       if (opts->allow_empty)
+       if (allow_empty)
                argv_array_push(&array, "--allow-empty");
 
        rc = run_command_v_opt(array.argv, RUN_GIT_CMD);
@@ -335,6 +336,44 @@ static int is_original_commit_empty(struct commit *commit)
        return !hashcmp(ptree_sha1, commit->tree->object.sha1);
 }
 
+/*
+ * Do we run "git commit" with "--allow-empty"?
+ */
+static int allow_empty(struct replay_opts *opts, struct commit *commit)
+{
+       int index_unchanged, empty_commit;
+
+       /*
+        * Three cases:
+        *
+        * (1) we do not allow empty at all and error out.
+        *
+        * (2) we allow ones that were initially empty, but
+        * forbid the ones that become empty;
+        *
+        * (3) we allow both.
+        */
+       if (!opts->allow_empty)
+               return 0; /* let "git commit" barf as necessary */
+
+       index_unchanged = is_index_unchanged();
+       if (index_unchanged < 0)
+               return index_unchanged;
+       if (!index_unchanged)
+               return 0; /* we do not have to say --allow-empty */
+
+       if (opts->keep_redundant_commits)
+               return 1;
+
+       empty_commit = is_original_commit_empty(commit);
+       if (empty_commit < 0)
+               return empty_commit;
+       if (!empty_commit)
+               return 0;
+       else
+               return 1;
+}
+
 static int do_pick_commit(struct commit *commit, struct replay_opts *opts)
 {
        unsigned char head[20];
@@ -344,8 +383,6 @@ 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) {
                /*
@@ -471,10 +508,6 @@ 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.
@@ -495,27 +528,11 @@ 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;
-
+               int allow = allow_empty(opts, commit);
+               if (allow < 0)
+                       return allow;
                if (!opts->no_commit)
-                       res = run_git_commit(defmsg, opts);
+                       res = run_git_commit(defmsg, opts, allow);
        }
 
        free_message(&msg);

diff --git a/t/t3505-cherry-pick-empty.sh b/t/t3505-cherry-pick-empty.sh
index 92f00cdf84993da995f074efa12344c26b627d67..5a1340cee69f344613c09b7d4a3f3f7bebd6719e 100755 (executable)
--- a/t/t3505-cherry-pick-empty.sh
+++ b/t/t3505-cherry-pick-empty.sh
@@ -71,4 +71,34 @@ test_expect_success 'cherry pick with --keep-redundant-commits' '
        git cherry-pick --keep-redundant-commits HEAD^
 '
 
+test_expect_success 'cherry-pick a commit that becomes no-op (prep)' '
+       git checkout master &&
+       git branch fork &&
+       echo foo >file2 &&
+       git add file2 &&
+       test_tick &&
+       git commit -m "add file2 on master" &&
+
+       git checkout fork &&
+       echo foo >file2 &&
+       git add file2 &&
+       test_tick &&
+       git commit -m "add file2 on the side"
+'
+
+test_expect_success 'cherry-pick a no-op without --keep-redundant' '
+       git reset --hard &&
+       git checkout fork^0 &&
+       test_must_fail git cherry-pick master
+'
+
+test_expect_success 'cherry-pick a no-op with --keep-redundant' '
+       git reset --hard &&
+       git checkout fork^0 &&
+       git cherry-pick --keep-redundant-commits master &&
+       git show -s --format='%s' >actual &&
+       echo "add file2 on master" >expect &&
+       test_cmp expect actual
+'
+
 test_done