On Thu, Feb 22, 2024 at 10:34 AM <phillip.wood123@xxxxxxxxx> wrote: >> 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. Makes sense -- I'll update this in v3. >> +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? Yes, the previous test leaves us on that branch. In v3, I will update this to instead just use `git checkout --detach`, as that does seem a little less confusing than switching to some other branch that is only relevant because it's not `unborn`. If there is a cleaner way to do this, I'd be happy to switch to 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. Thanks for pointing this out. Using git-switch(1) here instead of git-checkout(1) allows us to drop the `rm -rf *` call form both the existing 'cherry-pick on unborn branch' test as well as my new test. It appears that the `git rm --cached -r .` call is still necessary in the existing test. >> + 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. This makes sense. In v3, I will update this new test as well as the existing test to not use `--quiet`. Combining the above suggestions, here's the version of the existing and new tests that I intend to use in v3. Let me know if this isn't what you had in mind! test_expect_success 'cherry-pick on unborn branch' ' git switch --orphan unborn && git rm --cached -r . && git cherry-pick initial && git diff 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 initial && test_cmp_rev ! initial HEAD ' -- Thank you, Brian Lyles