Re: [PATCH RESEND] branch: allow deleting dangling branches with --force
- From
Ævar Arnfjörð Bjarmason <avarab@gmail.com>
- Date
- Aug 25, 2021, 23:30 UTC
- Message-ID
- <87eeahcd7f.fsf@evledraar.gmail.com>
- In-Reply-To
- <7894f736-4681-7656-e2d4-5945d2c71d31@web.de>
On Wed, Aug 25 2021, René Scharfe wrote:
Show 57 quoted lines
> git branch only allows deleting branches that point to valid commits.
> Skip that check if --force is given, as the caller is indicating with
> it that they know what they are doing and accept the consequences.
> This allows deleting dangling branches, which previously had to be
> reset to a valid start-point using --force first.
>
> Signed-off-by: René Scharfe <l.s.r@web.de>
> ---
> Original submission:
> http://public-inbox.org/git/52847a99-db7c-9634-b3b1-fd9b1342bc32@web.de/
>
> Documentation/git-branch.txt | 3 ++-
> builtin/branch.c | 2 +-
> t/t3200-branch.sh | 7 +++++++
> 3 files changed, 10 insertions(+), 2 deletions(-)
>
> diff --git a/Documentation/git-branch.txt b/Documentation/git-branch.txt
> index 94dc9a54f2..5449767121 100644
> --- a/Documentation/git-branch.txt
> +++ b/Documentation/git-branch.txt
> @@ -118,7 +118,8 @@ OPTIONS
> Reset <branchname> to <startpoint>, even if <branchname> exists
> already. Without `-f`, 'git branch' refuses to change an existing branch.
> In combination with `-d` (or `--delete`), allow deleting the
> - branch irrespective of its merged status. In combination with
> + branch irrespective of its merged status, or whether it even
> + points to a valid commit. In combination with
> `-m` (or `--move`), allow renaming the branch even if the new
> branch name already exists, the same applies for `-c` (or `--copy`).
>
> diff --git a/builtin/branch.c b/builtin/branch.c
> index b23b1d1752..03c7b7253a 100644
> --- a/builtin/branch.c
> +++ b/builtin/branch.c
> @@ -168,7 +168,7 @@ static int check_branch_commit(const char *branchname, const char *refname,
> int kinds, int force)
> {
> struct commit *rev = lookup_commit_reference(the_repository, oid);
> - if (!rev) {
> + if (!force && !rev) {
> error(_("Couldn't look up commit object for '%s'"), refname);
> return -1;
> }
> diff --git a/t/t3200-branch.sh b/t/t3200-branch.sh
> index cc4b10236e..ec61a10c29 100755
> --- a/t/t3200-branch.sh
> +++ b/t/t3200-branch.sh
> @@ -1272,6 +1272,13 @@ test_expect_success 'attempt to delete a branch merged to its base' '
> test_must_fail git branch -d my10
> '
>
> +test_expect_success 'branch --delete --force removes dangling branch' '
> + test_when_finished "rm -f .git/refs/heads/dangling" &&
> + echo $ZERO_OID >.git/refs/heads/dangling &&
> + git branch --delete --force dangling &&
> + test_path_is_missing .git/refs/heads/dangling
> +'Isn't a more meaningful test here to use a "real" SHA, instead of the $ZERO_OID? You can use $(test_oid deadbeef) to get one of those.
That way we know that this this test & logic is really testing that we can delete a branch that's been racily GC'd away or whatever, and not one in the already-broken state of referring to the $ZERO_OID.
Also: How does "git tag -d" handle this scenario if the same sort of data were added to .git/refs/tags/* ?