Hi Brian
On 10/02/2024 07:43, Brian Lyles wrote:
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.
Treat a failure reading HEAD as an unborn branch in
`is_index_unchanged`. This is consistent with other sequencer logic such
as `do_pick_commit`. When on an unborn branch, use the `empty_tree` as
the tree to compare against.
Signed-off-by: Brian Lyles <brianmlyles@xxxxxxxxx>
Helped-by: Phillip Wood <phillip.wood@xxxxxxxxxxxxx>
---
This is another new commit that was not present in v1.
See this comment[1] from Phillip for context.
[1]: https://lore.kernel.org/git/b5213705-4cd6-40ef-8c5f-32b214534b8b@xxxxxxxxx/
Thanks for fixing this and adding a test, I've left a few small comments
below.
static int is_index_unchanged(struct repository *r)
{
- struct object_id head_oid, *cache_tree_oid;
+ struct object_id head_oid, *cache_tree_oid, head_tree_oid;
I think we can make `head_tree_oid` a pointer like cache_tree_oid and
avoid de-referencing `the_hash_algo->empty_tree` and the return value of
`get_commit_tree_oid()`. I think the only reason to copy it would be if
the underlying object had a shorter lifetime than `head_tree_oid` but I
don't think that's the case.
+test_expect_success 'cherry-pick on unborn branch with --allow-empty' '
+ git checkout main &&
I'm a bit confused by this - are we already on the branch "unborn" and
so need to move away from it to delete it?
+ git branch -D unborn &&
+ git checkout --orphan unborn &&
+ git rm --cached -r . &&
+ rm -rf * &&
"git switch --orphan" leaves us with an empty index and working copy
without having to remove the files ourselves.
+ git cherry-pick initial --allow-empty &&
+ git diff --quiet initial &&
I'd drop "--quiet" here as it makes debugging easier if we can see the
diff if the test fails.
+ test_cmp_rev ! initial HEAD
+'
Best Wishes
Phillip
