Re: git filter-branch doesn't dereference annotated tags

[Johannes Sixt <j6t@kdbg.org>]

Am 31.12.2012 17:24, schrieb Grégory Pakosz:
> Please disregard the previous email that contains an incorrect fix
> suggestion. I wish my first contribution was flawless.
> 
> Here is what's happening.
> git-filter-branch let git-update-ref -d verify that the value for $ref
> matches $sha1.
> However, when $ref points to an annotated tag that is being deleted,
> that verification fails because $sha1 is the commit underneath.
> 
> I think there are two possible fixes:
>   1) either make git-filter-branch dereference annotated tags and do
> the verification itself then use the two arguments version of git
> update-ref
>   2) in the case of an annotated tag, pass another <old value> to git update-ref
> 
> Please find below a patch that implements solution 1). Please note the
> patch doesn't contain a unit test for this situation as I wasn't sure
> how to provide one. Yet I tested it on the repository I'm working on.
> 
> Gregory

We write material like this below the three-dash line of the patch.

> 
> From 9d21960088a61bfbac1ffdb4b13e3038f88ab4d6 Mon Sep 17 00:00:00 2001
> From: Gregory Pakosz <gpakosz@visionobjects.com>
> Date: Mon, 31 Dec 2012 15:30:36 +0100

Then you can remove these three lines because they are inferred from the
email message. And you should not attach the patch, only place it
inline, but make sure that it is not line-wrapped and space-mangled on
the mail route.

> Subject: [PATCH] git-filter-branch: support annotated tags deletion
> 
> git-filter-branch let git-update-ref -d verify that the value for $ref matches
> $sha1. However, when $ref is an annotated tag being deleted that verfication
> fails because $sha1 corresponds to a commit object.
> 
> Instead of asking git-update-ref to verify values actually match, dereference
> $ref ourselves and test against $sha1 first. Then invoke git-update-ref with two
> arguments.

It would have been very helpful if you could summarize the conditions
under which the unexpected behavior happens in the commit message. A
test case would also be great; it should be a matter of inserting
another case in t/t7003.

Without that information, I can't decide whether it is a good thing that
a tag (annotated or not) is deleted, because we have --tag-name-filter
to treat tags (even though you can't delete tags with this feature).

> Signed-off-by: Gregory Pakosz <gpakosz@visionobjects.com>
> ---
>  git-filter-branch.sh | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/git-filter-branch.sh b/git-filter-branch.sh
> index 5314249..bbee6d0 100755
> --- a/git-filter-branch.sh
> +++ b/git-filter-branch.sh
> @@ -383,7 +383,7 @@ do
>  	case "$rewritten" in
>  	'')
>  		echo "Ref '$ref' was deleted"
> -		git update-ref -m "filter-branch: delete" -d "$ref" $sha1 ||
> +		test $(git rev-parse --verify "$ref^{commit}") = $sha1 && git
> update-ref -m "filter-branch: delete" -d "$ref" ||
>  			die "Could not delete $ref"

As written, it counts as error "Could not delete $ref" if the test...
command fails. Is this intended?

-- Hannes