From: Phillip Wood Date: Sun, 19 Jun 2022 13:58:28 GMT Subject: Re: [PATCH v2 0/5] Create branch_checked_out() helper Message-ID: In-Reply-To: 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