Re: [PATCH v2 0/5] Create branch_checked_out() helper

[Phillip Wood <phillip.wood123@gmail.com>]

Hi Stolee

On 14/06/2022 20:27, Derrick Stolee via GitGitGadget wrote:
> This is a replacement for some patches from v2 of my 'git rebase
> --update-refs' topic [1]. After some feedback from Philip, I've decided to
> pull that topic while I rework how I track the refs to rewrite [2]. This
> series moves forward with the branch_checked_out() helper that was a bit
> more complicated than expected at first glance. This series is a culmination
> of the discussion started by Junio at [3].
> 
> [1]
> https://lore.kernel.org/git/pull.1247.v2.git.1654634569.gitgitgadget@gmail.com
> [2]
> https://lore.kernel.org/git/34264915-8a37-62db-f954-0b5297639b34@github.com/
> [3] https://lore.kernel.org/git/xmqqr140t9am.fsf@gitster.g/
> 
> Here is the patch outline:
> 
>   1. Add a basic form of branch_checked_out() that only looks at the HEAD of
>      each worktree. Since this doesn't look at BISECT_HEAD or REBASE_HEAD,
>      the helper isn't linked to any consumer in this patch. A test script is
>      added that verifies the current behavior checks that are implemented,
>      even if they are not connected yet.
>   2. Make branch_checked_out() actually look at BISECT_HEAD and REBASE_HEAD.
>      Add tests for those cases, which was previously untested in the Git test
>      suite. Use branch_checked_out() in 'git branch -f' to demonstrate this
>      helper works as expected.
>   3. Use branch_checked_out() in 'git fetch' when checking refs that would be
>      updated by the refspec. Add tests for that scenario, too.
>   4. Use branch_checked_out() in 'git branch -D' to prevent deleting refs
>      from under an existing checkout. The commit message here describes some
>      barriers to removing other uses of find_shared_symref() that could be
>      good investigations for later.
>   5. Be careful when constructing the strmap to free old values, even though
>      this should only happen in error scenarios. Add tests that verify this
>      behavior.
> 
> 
> Updates in v2
> =============
> 
>   * branch_checked_out() has a new prototype where it returns a 'const char
>     *' instead of copying the path.
>   * The test script is marked with TEST_PASSES_SANITIZE_LEAK and test are
>     careful to avoid using 'git rebase' or 'git bisect' when possible.
>   * Tests that cannot avoid memory-loss from 'git fetch' are marked with the
>     "!SANITIZE_LEAK" prereq.
>   * A previous replacement of 'wt->current' with 'path' is removed. This
>     changes an error message, but it is a very rare scenario where this error
>     would be output.

The changes look good to me (modulo Peff's fixup), thanks for working on 
this

Best Wishes

Phillip