sequencer: handle unborn branch with `--allow-empty`

When using git-cherry-pick(1) with `--allow-empty` while on an unborn
branch, an error is thrown. This is inconsistent with the same
cherry-pick when `--allow-empty` is not specified.

Detect unborn branches in `is_index_unchanged`. When on an unborn
branch, use the `empty_tree` as the tree to compare against.

Add a new test to cover this scenario. While modelled off of the
existing 'cherry-pick on unborn branch' test, some improvements can be
made:

- Use `git switch --orphan unborn` instead of `git checkout --orphan
  unborn` to avoid the need for a separate `rm -rf *` call
- Avoid using `--quiet` in the `git diff` call to make debugging easier
  in the event of a failure. Use simply `--exit-code` instead.

Make these improvements to the existing test as well as the new test.

Helped-by: Phillip Wood <phillip.wood@dunelm.org.uk>
Helped-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Brian Lyles <brianmlyles@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/sequencer.c b/sequencer.c
index 35dd7e561a60c56d2639068a6afde50099497d65..ff0b3f4f06bfbf3a8e0fe13ab81b5d6c023e98b0 100644 (file)
--- a/sequencer.c
+++ b/sequencer.c
@@ -772,29 +772,42 @@ static struct object_id *get_cache_tree_oid(struct index_state *istate)
 static int is_index_unchanged(struct repository *r)
 {
        struct object_id head_oid, *cache_tree_oid;
+       const struct object_id *head_tree_oid;
        struct commit *head_commit;
        struct index_state *istate = r->index;
+       const char *head_name;
+
+       if (!resolve_ref_unsafe("HEAD", RESOLVE_REF_READING, &head_oid, NULL)) {
+               /* Check to see if this is an unborn branch */
+               head_name = resolve_ref_unsafe("HEAD",
+                       RESOLVE_REF_READING | RESOLVE_REF_NO_RECURSE,
+                       &head_oid, NULL);
+               if (!head_name ||
+                       !starts_with(head_name, "refs/heads/") ||
+                       !is_null_oid(&head_oid))
+                       return error(_("could not resolve HEAD commit"));
+               head_tree_oid = the_hash_algo->empty_tree;
+       } else {
+               head_commit = lookup_commit(r, &head_oid);
 
-       if (!resolve_ref_unsafe("HEAD", RESOLVE_REF_READING, &head_oid, NULL))
-               return error(_("could not resolve HEAD commit"));
-
-       head_commit = lookup_commit(r, &head_oid);
+               /*
+                * If head_commit is NULL, check_commit, called from
+                * lookup_commit, would have indicated that head_commit is not
+                * a commit object already.  repo_parse_commit() will return failure
+                * without further complaints in such a case.  Otherwise, if
+                * the commit is invalid, repo_parse_commit() will complain.  So
+                * there is nothing for us to say here.  Just return failure.
+                */
+               if (repo_parse_commit(r, head_commit))
+                       return -1;
 
-       /*
-        * If head_commit is NULL, check_commit, called from
-        * lookup_commit, would have indicated that head_commit is not
-        * a commit object already.  repo_parse_commit() will return failure
-        * without further complaints in such a case.  Otherwise, if
-        * the commit is invalid, repo_parse_commit() will complain.  So
-        * there is nothing for us to say here.  Just return failure.
-        */
-       if (repo_parse_commit(r, head_commit))
-               return -1;
+               head_tree_oid = get_commit_tree_oid(head_commit);
+       }
 
        if (!(cache_tree_oid = get_cache_tree_oid(istate)))
                return -1;
 
-       return oideq(cache_tree_oid, get_commit_tree_oid(head_commit));
+       return oideq(cache_tree_oid, head_tree_oid);
 }
 
 static int write_author_script(const char *message)

diff --git a/t/t3501-revert-cherry-pick.sh b/t/t3501-revert-cherry-pick.sh
index aeab689a98d00919e0473380fc9bf40497b32c80..af732275120b0d9f749c982e66e340153d944579 100755 (executable)
--- a/t/t3501-revert-cherry-pick.sh
+++ b/t/t3501-revert-cherry-pick.sh
@@ -104,11 +104,19 @@ test_expect_success 'revert forbidden on dirty working tree' '
 '
 
 test_expect_success 'cherry-pick on unborn branch' '
-       git checkout --orphan unborn &&
+       git switch --orphan unborn &&
        git rm --cached -r . &&
-       rm -rf * &&
        git cherry-pick initial &&
-       git diff --quiet initial &&
+       git diff --exit-code initial &&
+       test_cmp_rev ! initial HEAD
+'
+
+test_expect_success 'cherry-pick on unborn branch with --allow-empty' '
+       git checkout --detach &&
+       git branch -D unborn &&
+       git switch --orphan unborn &&
+       git cherry-pick initial --allow-empty &&
+       git diff --exit-code initial &&
        test_cmp_rev ! initial HEAD
 '