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@xxxxxxxxx [2] https://lore.kernel.org/git/34264915-8a37-62db-f954-0b5297639b34@xxxxxxxxxx/ [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
