Re: [PATCH v2 4/8] sequencer: treat error reading HEAD as unborn branch

[phillip.wood123@gmail.com]

Hi Brian

On 23/02/2024 05:28, Brian Lyles wrote:
> On Thu, Feb 22, 2024 at 10:34 AM <phillip.wood123@gmail.com> wrote:
>>> +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.

I think "git checkout --detach" is probably the best we can do. It would 
be nice to be able to do "git switch -C --orphan unborn" but "-C" does 
not work with "--orphan"

>>> +	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.

It looks like the previous test 'revert forbidden on dirty working tree' 
fails to clean up properly and so "git switch" is carrying the 
uncommitted changes across to the new orphan branch. I think that "git 
switch --discard-changes --orphan unborn" ought to clean the worktree 
and index but it doesn't seem to work.

>>> +	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
>      '

These look good

Thanks

Phillip