{"thread":{"id":"32484","subject":"git filter-branch doesn't dereference annotated tags","startedAt":"2012-12-31T16:24:11Z","lastAt":"2013-01-03T20:52:31Z","messageCount":13,"participants":["Grégory Pakosz","Junio C Hamano","Johannes Sixt","Brandon Casey"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"205696","messageId":"CAC_01E174m_6tDwPKZ5P0BUxnLNWUf9p+VkECFosPTzip0sYsA@mail.gmail.com","threadId":"32484","inReplyTo":null,"subject":"git filter-branch doesn't dereference annotated tags","fromName":"Grégory Pakosz","fromEmail":"gpakosz@visionobjects.com","sentAt":"2012-12-31T16:24:11Z","receivedAt":"2012-12-31T16:24:11Z","isPatch":false,"sender":{"key":"gpakosz@visionobjects.com","avatar":"https://gravatar.com/avatar/4346628bc87f6417d0efe6a1a66d941e8f3afed0a8621ca1cec4cb768058204c?d=mp&s=160"},"body":"Please disregard the previous email that contains an incorrect fix\nsuggestion. I wish my first contribution was flawless.\n\nHere is what's happening.\ngit-filter-branch let git-update-ref -d verify that the value for $ref\nmatches $sha1.\nHowever, when $ref points to an annotated tag that is being deleted,\nthat verification fails because $sha1 is the commit underneath.\n\nI think there are two possible fixes:\n  1) either make git-filter-branch dereference annotated tags and do\nthe verification itself then use the two arguments version of git\nupdate-ref\n  2) in the case of an annotated tag, pass another <old value> to git update-ref\n\nPlease find below a patch that implements solution 1). Please note the\npatch doesn't contain a unit test for this situation as I wasn't sure\nhow to provide one. Yet I tested it on the repository I'm working on.\n\nGregory\n\n>From 9d21960088a61bfbac1ffdb4b13e3038f88ab4d6 Mon Sep 17 00:00:00 2001\nFrom: Gregory Pakosz <gpakosz@visionobjects.com>\nDate: Mon, 31 Dec 2012 15:30:36 +0100\nSubject: [PATCH] git-filter-branch: support annotated tags deletion\n\ngit-filter-branch let git-update-ref -d verify that the value for $ref matches\n$sha1. However, when $ref is an annotated tag being deleted that verfication\nfails because $sha1 corresponds to a commit object.\n\nInstead of asking git-update-ref to verify values actually match, dereference\n$ref ourselves and test against $sha1 first. Then invoke git-update-ref with two\narguments.\n\nSigned-off-by: Gregory Pakosz <gpakosz@visionobjects.com>\n---\n git-filter-branch.sh | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/git-filter-branch.sh b/git-filter-branch.sh\nindex 5314249..bbee6d0 100755\n--- a/git-filter-branch.sh\n+++ b/git-filter-branch.sh\n@@ -383,7 +383,7 @@ do\n \tcase \"$rewritten\" in\n \t'')\n \t\techo \"Ref '$ref' was deleted\"\n-\t\tgit update-ref -m \"filter-branch: delete\" -d \"$ref\" $sha1 ||\n+\t\ttest $(git rev-parse --verify \"$ref^{commit}\") = $sha1 && git\nupdate-ref -m \"filter-branch: delete\" -d \"$ref\" ||\n \t\t\tdie \"Could not delete $ref\"\n \t;;\n \t$_x40)\n-- \n1.8.0.1\n\n\nFrom 9d21960088a61bfbac1ffdb4b13e3038f88ab4d6 Mon Sep 17 00:00:00 2001\nFrom: Gregory Pakosz <gpakosz@visionobjects.com>\nDate: Mon, 31 Dec 2012 15:30:36 +0100\nSubject: [PATCH] git-filter-branch: support annotated tags deletion\n\ngit-filter-branch let git-update-ref -d verify that the value for $ref matches\n$sha1. However, when $ref is an annotated tag being deleted that verfication\nfails because $sha1 corresponds to a commit object.\n\nInstead of asking git-update-ref to verify values actually match, dereference\n$ref ourselves and test against $sha1 first. Then invoke git-update-ref with two\narguments.\n\nSigned-off-by: Gregory Pakosz <gpakosz@visionobjects.com>\n---\n git-filter-branch.sh | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/git-filter-branch.sh b/git-filter-branch.sh\nindex 5314249..bbee6d0 100755\n--- a/git-filter-branch.sh\n+++ b/git-filter-branch.sh\n@@ -383,7 +383,7 @@ do\n \tcase \"$rewritten\" in\n \t'')\n \t\techo \"Ref '$ref' was deleted\"\n-\t\tgit update-ref -m \"filter-branch: delete\" -d \"$ref\" $sha1 ||\n+\t\ttest $(git rev-parse --verify \"$ref^{commit}\") = $sha1 && git update-ref -m \"filter-branch: delete\" -d \"$ref\" ||\n \t\t\tdie \"Could not delete $ref\"\n \t;;\n \t$_x40)\n-- \n1.8.0.1\n\n"},{"id":"205700","messageId":"7vsj6mdqeo.fsf@alter.siamese.dyndns.org","threadId":"32484","inReplyTo":"CAC_01E174m_6tDwPKZ5P0BUxnLNWUf9p+VkECFosPTzip0sYsA@mail.gmail.com","subject":"Re: git filter-branch doesn't dereference annotated tags","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-12-31T18:31:43Z","receivedAt":"2012-12-31T18:31:43Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Grégory Pakosz <gpakosz@visionobjects.com> writes:\n\n>   1) either make git-filter-branch dereference annotated tags and do\n> the verification itself then use the two arguments version of git\n> update-ref\n>   2) in the case of an annotated tag, pass another <old value> to git update-ref\n>\n> Please find below a patch that implements solution 1).\n> ...\n>  \t\techo \"Ref '$ref' was deleted\"\n> -\t\tgit update-ref -m \"filter-branch: delete\" -d \"$ref\" $sha1 ||\n> +\t\ttest $(git rev-parse --verify \"$ref^{commit}\") = $sha1 && git\n> update-ref -m \"filter-branch: delete\" -d \"$ref\" ||\n\nThanks.  A few comments.\n\nAt the design level.  Where does this $sha1 come from in the first\nplace?  If a ref that named the annotated tag was deleted, shouldn't\nwe arrange things so this part of the code receives the $sha1 of the\ntag that corresponds to the $ref so that \"update-ref -d\" can check\nthat nobody tampered with the repository while the script was\nworking?\n\nAt the implementation level.  When the ref being deleted pointed at\na tree or a blob, the original would have correctly removed it, but\nwill the updated one?\n"},{"id":"205765","messageId":"50E2D6E1.5030102@kdbg.org","threadId":"32484","inReplyTo":"CAC_01E174m_6tDwPKZ5P0BUxnLNWUf9p+VkECFosPTzip0sYsA@mail.gmail.com","subject":"Re: git filter-branch doesn't dereference annotated tags","fromName":"Johannes Sixt","fromEmail":"j6t@kdbg.org","sentAt":"2013-01-01T12:30:25Z","receivedAt":"2013-01-01T12:30:25Z","isPatch":false,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Am 31.12.2012 17:24, schrieb Grégory Pakosz:\n> Please disregard the previous email that contains an incorrect fix\n> suggestion. I wish my first contribution was flawless.\n> \n> Here is what's happening.\n> git-filter-branch let git-update-ref -d verify that the value for $ref\n> matches $sha1.\n> However, when $ref points to an annotated tag that is being deleted,\n> that verification fails because $sha1 is the commit underneath.\n> \n> I think there are two possible fixes:\n>   1) either make git-filter-branch dereference annotated tags and do\n> the verification itself then use the two arguments version of git\n> update-ref\n>   2) in the case of an annotated tag, pass another <old value> to git update-ref\n> \n> Please find below a patch that implements solution 1). Please note the\n> patch doesn't contain a unit test for this situation as I wasn't sure\n> how to provide one. Yet I tested it on the repository I'm working on.\n> \n> Gregory\n\nWe write material like this below the three-dash line of the patch.\n\n> \n> From 9d21960088a61bfbac1ffdb4b13e3038f88ab4d6 Mon Sep 17 00:00:00 2001\n> From: Gregory Pakosz <gpakosz@visionobjects.com>\n> Date: Mon, 31 Dec 2012 15:30:36 +0100\n\nThen you can remove these three lines because they are inferred from the\nemail message. And you should not attach the patch, only place it\ninline, but make sure that it is not line-wrapped and space-mangled on\nthe mail route.\n\n> Subject: [PATCH] git-filter-branch: support annotated tags deletion\n> \n> git-filter-branch let git-update-ref -d verify that the value for $ref matches\n> $sha1. However, when $ref is an annotated tag being deleted that verfication\n> fails because $sha1 corresponds to a commit object.\n> \n> Instead of asking git-update-ref to verify values actually match, dereference\n> $ref ourselves and test against $sha1 first. Then invoke git-update-ref with two\n> arguments.\n\nIt would have been very helpful if you could summarize the conditions\nunder which the unexpected behavior happens in the commit message. A\ntest case would also be great; it should be a matter of inserting\nanother case in t/t7003.\n\nWithout that information, I can't decide whether it is a good thing that\na tag (annotated or not) is deleted, because we have --tag-name-filter\nto treat tags (even though you can't delete tags with this feature).\n\n> Signed-off-by: Gregory Pakosz <gpakosz@visionobjects.com>\n> ---\n>  git-filter-branch.sh | 2 +-\n>  1 file changed, 1 insertion(+), 1 deletion(-)\n> \n> diff --git a/git-filter-branch.sh b/git-filter-branch.sh\n> index 5314249..bbee6d0 100755\n> --- a/git-filter-branch.sh\n> +++ b/git-filter-branch.sh\n> @@ -383,7 +383,7 @@ do\n>  \tcase \"$rewritten\" in\n>  \t'')\n>  \t\techo \"Ref '$ref' was deleted\"\n> -\t\tgit update-ref -m \"filter-branch: delete\" -d \"$ref\" $sha1 ||\n> +\t\ttest $(git rev-parse --verify \"$ref^{commit}\") = $sha1 && git\n> update-ref -m \"filter-branch: delete\" -d \"$ref\" ||\n>  \t\t\tdie \"Could not delete $ref\"\n\nAs written, it counts as error \"Could not delete $ref\" if the test...\ncommand fails. Is this intended?\n\n-- Hannes\n"},{"id":"205766","messageId":"CAC_01E3twtNq8YXQ8=SU5oXxmnAQS43L-46NF=7RyT4tFQvU5g@mail.gmail.com","threadId":"32484","inReplyTo":"7vsj6mdqeo.fsf@alter.siamese.dyndns.org","subject":"Re: git filter-branch doesn't dereference annotated tags","fromName":"Grégory Pakosz","fromEmail":"gpakosz@visionobjects.com","sentAt":"2013-01-01T13:11:42Z","receivedAt":"2013-01-01T13:11:42Z","isPatch":false,"sender":{"key":"gpakosz@visionobjects.com","avatar":"https://gravatar.com/avatar/4346628bc87f6417d0efe6a1a66d941e8f3afed0a8621ca1cec4cb768058204c?d=mp&s=160"},"body":"> Thanks.  A few comments.\n>\n> At the design level.  Where does this $sha1 come from in the first\n> place?\n>\nactually, sha1=$(git rev-parse \"$ref\"^0)\n(please remember that I'm discovering git internals while figuring out\nhow to make git filter-branch work in my use case)\n\nin my use case, $ref is \"refs/tags/4.0\" which is an annotated tag\n\n$ git rev-parse \"refs/tags/4.0\"\ne941b1999906c17b59320f776d58b71fc2fcdb72\n\n$ git cat-file -t e941b1999906c17b59320f776d58b71fc2fcdb72\ntag\n\n$ git rev-parse e941b1999906c17b59320f776d58b71fc2fcdb72^0\ndcd7cdc18240dd9a54b30d757dd2347f52040490\n\n$ git cat-file -t dcd7cdc18240dd9a54b30d757dd2347f52040490\ncommit\n\nso $sha1 is dcd7cdc18240dd9a54b30d757dd2347f52040490\n\nand then git-filter-branch calls git update-ref -m \"filter-branch:\ndelete\" -d \"refs/tags/4.0\" dcd7cdc18240dd9a54b30d757dd2347f52040490\nwhich makes git update-index complains\ne941b1999906c17b59320f776d58b71fc2fcdb72 !=\ndcd7cdc18240dd9a54b30d757dd2347f52040490\n\nso hmm, adding test $(git rev-parse --verify \"$ref^{commit}\") = $sha1\nas I did in my patch is always true since sha1=$(git rev-parse\n\"$ref\"^0)\n\n>  If a ref that named the annotated tag was deleted, shouldn't\n> we arrange things so this part of the code receives the $sha1 of the\n> tag that corresponds to the $ref\n>\nI'm not sure what you mean by \"a ref that named the annotated tag was deleted\"\nWhat's happening in my situation is that the commit the tag points to\ngets rewritten to nothing as the result of my filtering:\n\"refs/tags/4.0\" points to e941b1999906c17b59320f776d58b71fc2fcdb72\n(tag) which points to dcd7cdc18240dd9a54b30d757dd2347f52040490\n(commit) which gets rewritten to nothing so the tag must be deleted.\n\n> so that \"update-ref -d\" can check\n> that nobody tampered with the repository while the script was\n> working?\n>\nI'm not quite sure what could possibly go well if somebody tampers\nwith the repository while it's being filtered with git filter-branch\nanyways???\n\nIf we want to address \"did somebody tamper with the repository while\nthe script was working?\", then test $(git rev-parse --verify\n\"$ref^{commit}\") = $sha1 verifies somebody didn't tamper with $ref\nsince we got $sha1 from it.\nBut that doesn't ensure tampering didn't take place in between\n  test $(git rev-parse --verify \"$ref^{commit}\") = $sha1\nand\n  git update-ref -m \"filter-branch: delete\" -d \"$ref\".\n\nHow defensive should git filter-branch really be?\n\n> At the implementation level.  When the ref being deleted pointed at\n> a tree or a blob, the original would have correctly removed it, but\n> will the updated one?\n>\nYes.\n\nNow that you made me think about it even more, the title of that\nthread isn't \"git filter-branch doesn't dereference annotated tags\".\nIt in fact does as per sha1=$(git rev-parse \"$ref\"^0).\n\nMaybe the suggested fix should be: in case the tag points to a commit\nthat has been rewritten to nothing, get $sha1 again without\ndereferencing recursively with sha1=$(git rev-parse \"$ref\") then use\nthe 3 arguments versions of git update-ref as before.\n\nThanks for reading\nGregory\n\n>From 59f86c9c07715734d59009c15816220f996b75be Mon Sep 17 00:00:00 2001\nFrom: Gregory Pakosz <gpakosz@visionobjects.com>\nDate: Mon, 31 Dec 2012 15:30:36 +0100\nSubject: [PATCH] git-filter-branch: support annotated tags deletion\n\ngit-filter-branch let git-update-ref -d verify that the value for $ref matches\n$sha1. $sha1 is obtained form dereferencing $ref recursively. In case $sha1 gets\nrewritten to nothing as per result of the filtering, the tag should be deleted.\nHowever, in case of an annotated tag, git-update-ref -d fails because $ref\ndoesn't directly point to $sha1.\n\nTo make git-filter-branch properly delete an annotated tag, obtain $sha1 again\nwithouth dereferencing the tag before asking git-update-ref to verify $ref and\n$sha1 match.\n\nSigned-off-by: Gregory Pakosz <gpakosz@visionobjects.com>\n---\n git-filter-branch.sh | 1 +\n 1 file changed, 1 insertion(+)\n\ndiff --git a/git-filter-branch.sh b/git-filter-branch.sh\nindex 5314249..7ae9912 100755\n--- a/git-filter-branch.sh\n+++ b/git-filter-branch.sh\n@@ -383,6 +383,7 @@ do\n \tcase \"$rewritten\" in\n \t'')\n \t\techo \"Ref '$ref' was deleted\"\n+\t\ttest $(git cat-file -t \"$ref\") = 'tag' && sha1=$(git rev-parse \"$ref\")\n \t\tgit update-ref -m \"filter-branch: delete\" -d \"$ref\" $sha1 ||\n \t\t\tdie \"Could not delete $ref\"\n \t;;\n-- \n1.8.0.1\n\n\nFrom 59f86c9c07715734d59009c15816220f996b75be Mon Sep 17 00:00:00 2001\nFrom: Gregory Pakosz <gpakosz@visionobjects.com>\nDate: Mon, 31 Dec 2012 15:30:36 +0100\nSubject: [PATCH] git-filter-branch: support annotated tags deletion\n\ngit-filter-branch let git-update-ref -d verify that the value for $ref matches\n$sha1. $sha1 is obtained form dereferencing $ref recursively. In case $sha1 gets\nrewritten to nothing as per result of the filtering, the tag should be deleted.\nHowever, in case of an annotated tag, git-update-ref -d fails because $ref\ndoesn't directly point to $sha1.\n\nTo make git-filter-branch properly delete an annotated tag, obtain $sha1 again\nwithouth dereferencing the tag before asking git-update-ref to verify $ref and\n$sha1 match.\n\nSigned-off-by: Gregory Pakosz <gpakosz@visionobjects.com>\n---\n git-filter-branch.sh | 1 +\n 1 file changed, 1 insertion(+)\n\ndiff --git a/git-filter-branch.sh b/git-filter-branch.sh\nindex 5314249..7ae9912 100755\n--- a/git-filter-branch.sh\n+++ b/git-filter-branch.sh\n@@ -383,6 +383,7 @@ do\n \tcase \"$rewritten\" in\n \t'')\n \t\techo \"Ref '$ref' was deleted\"\n+\t\ttest $(git cat-file -t \"$ref\") = 'tag' && sha1=$(git rev-parse \"$ref\")\n \t\tgit update-ref -m \"filter-branch: delete\" -d \"$ref\" $sha1 ||\n \t\t\tdie \"Could not delete $ref\"\n \t;;\n-- \n1.8.0.1\n\n"},{"id":"205774","messageId":"7vfw2kbs4h.fsf@alter.siamese.dyndns.org","threadId":"32484","inReplyTo":"CAC_01E3twtNq8YXQ8=SU5oXxmnAQS43L-46NF=7RyT4tFQvU5g@mail.gmail.com","subject":"Re: git filter-branch doesn't dereference annotated tags","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-01-01T19:49:50Z","receivedAt":"2013-01-01T19:49:50Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Grégory Pakosz <gpakosz@visionobjects.com> writes:\n\n> in my use case, $ref is \"refs/tags/4.0\" which is an annotated tag\n>\n> $ git rev-parse \"refs/tags/4.0\"\n> e941b1999906c17b59320f776d58b71fc2fcdb72\n\nYeah, but in that case it appears to me that you told the command to\nrewrite the tag itself and the history behind the commit the tag\nrefers to, but the end result did not rewrite the tag itself and\nleft the tag pointing at the original history.\n\nThe \"$rewritten\" variable being empty in this codepath tells me that\nthe command decided that it *does* want to delete the tag, but as\nJ6t mentioned in his review, it is unclear to me what is expected by\nthe user.\n\nIf the command and the filters you gave the command decided the tag\nshould be removed, then not being able to delete it is a problem and\nthe code you patched that compares the object name of the tag and\nthe object name of the commit the tag refers to is certainly doing a\nwrong comparison. \n\nBut I have this suspicion that you did not want to remove the tag in\nthe first place.  The application of \"map\" shell function to obtain\n$rewritten is done on the unwrapped object name by passing \"$ref^0\"\nto rev-parse, but perhaps that \"^0\" is the real source of the\nproblem instead, so that it checks what new object the original\nannotated tag was rewritten to?  If the tag object was rewritten,\neither to empty to signal its removal or to a new object name, then\nwe do want to update-ref it, but the decision should be made on its\nobject name, not the object name of the commit it points at?\n"},{"id":"205777","messageId":"CAC_01E3VWtsFd8ww+7W8DMhRAs4WgHf=bm+xoh9wszCkb-DfUA@mail.gmail.com","threadId":"32484","inReplyTo":"7vfw2kbs4h.fsf@alter.siamese.dyndns.org","subject":"Re: git filter-branch doesn't dereference annotated tags","fromName":"Grégory Pakosz","fromEmail":"gpakosz@visionobjects.com","sentAt":"2013-01-01T20:20:14Z","receivedAt":"2013-01-01T20:20:14Z","isPatch":false,"sender":{"key":"gpakosz@visionobjects.com","avatar":"https://gravatar.com/avatar/4346628bc87f6417d0efe6a1a66d941e8f3afed0a8621ca1cec4cb768058204c?d=mp&s=160"},"body":"> Yeah, but in that case it appears to me that you told the command to\n> rewrite the tag itself and the history behind the commit the tag\n> refers to, but the end result did not rewrite the tag itself and\n> left the tag pointing at the original history.\n>\nThe problem exhibits at the point git filter-branch updates the references.\nI indeed asked to rewrite tagged commits by passing --tag-name-filter cat\n\n> The \"$rewritten\" variable being empty in this codepath tells me that\n> the command decided that it *does* want to delete the tag, but as\n> J6t mentioned in his review, it is unclear to me what is expected by\n> the user.\n>\nThe history behind the commit the tag refers to is empty after\nfiltering has been applied.\nAs a user, I expected the tag to be removed: it's not illogical, all\ntags that pointed to history that has been entirely filtered out\nshould go away imho.\n--tag-name-filter doesn't allow to deleted tags as J6t mentioned and\nI'm not surre what the new contract would be: empty tag name to delete\na tag so if $(map $sha1) yields '' I can decided to delete the tag?\n\nIf the tag wasn't an annotated one, I guess it would have been\nsuccessfully deleted which exhibits a different behavior between\nannotated and lightweight tags.\n\n> If the command and the filters you gave the command decided the tag\n> should be removed, then not being able to delete it is a problem and\n> the code you patched that compares the object name of the tag and\n> the object name of the commit the tag refers to is certainly doing a\n> wrong comparison.\n>\nThat's what I believe.\nThe index and commit filters are made to keep only a couple of\ndirectories and get rid of the rest.\nThose directories didn't exist early in the history. Commits in that\nearly part of the history were tagged with annotated tags.\n\n> But I have this suspicion that you did not want to remove the tag in\n> the first place.\n>\nThe tag pointed to something the filters decided to throw out. Since I\nwant tags to be updated, it doesn't make much sense to keep it and I'm\nexpecting its deletion.\n\nIt appears to me that the suggested fix doing test $(git cat-file -t\n\"$ref\") = 'tag' && sha1=$(git rev-parse \"$ref\") so that $sha1 is\nobtained again from the tag ref but without dereferencing recursively\nshould only apply if --tag-name-filter has been provided. What do you\nthink?\n\n> The application of \"map\" shell function to obtain\n> $rewritten is done on the unwrapped object name by passing \"$ref^0\"\n> to rev-parse, but perhaps that \"^0\" is the real source of the\n> problem instead, so that it checks what new object the original\n> annotated tag was rewritten to?  If the tag object was rewritten,\n> either to empty to signal its removal or to a new object name, then\n> we do want to update-ref it, but the decision should be made on its\n> object name, not the object name of the commit it points at?\n>\nAre you suggesting $sha1 should be obtained differently before\nentering case \"$rewritten\" ?\nThat would mean changing sha1=$(git rev-parse \"$ref\"^0) at line 376 to\nsomething like $(git cat-file -t \"$ref\") = 'tag' && sha1=$(git\nrev-parse \"$ref\") || sha1=$(git rev-parse \"$ref^0\") ?\n\nGregory\n"},{"id":"205781","messageId":"7vk3rwaa3r.fsf@alter.siamese.dyndns.org","threadId":"32484","inReplyTo":"CAC_01E3VWtsFd8ww+7W8DMhRAs4WgHf=bm+xoh9wszCkb-DfUA@mail.gmail.com","subject":"Re: git filter-branch doesn't dereference annotated tags","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-01-01T21:04:24Z","receivedAt":"2013-01-01T21:04:24Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Grégory Pakosz <gpakosz@visionobjects.com> writes:\n\n> Are you suggesting $sha1 should be obtained differently before\n> entering case \"$rewritten\" ?\n> That would mean changing sha1=$(git rev-parse \"$ref\"^0) at line 376 to\n> something like $(git cat-file -t \"$ref\") = 'tag' && sha1=$(git\n> rev-parse \"$ref\") || sha1=$(git rev-parse \"$ref^0\") ?\n\nI was wondering if it should be\n\n\tsha1=$(git rev-parse --verify \"$ref\")\n\nor something that does not dereference a tag at all.\n\nThe way I read what that loop seems to want to do is:\n\n\tRead each refname that was given originally from the file\n\t$tempdir/heads, find out the object it used to refer to and\n\thave it in $sha1, find out what new object the object was\n\trewritten to and have it in $rewritten, and:\n\n\t(1) if the rewrite left the object unchanged, do nothing but\n\t    warn users just in case this was a mistake;\n\t(2) if the rewrite told us to remove it, then delete the\n\t    ref; or\n        (3) if the rewrite gave us a new object, replace the ref to \n\t    point to that new one.\n\n\tAnd in the latter two cases, save the original one in\n\t$orig_namespace so that the user can choose to recover if\n\tthis filter-branch was done by mistake.\n\nSo I do not think unwraping the ref at that point makes any sense,\nunless it is not prepared to handle annotated tags at all by\nunwrapping tags too early.\n\nWhat am I missing?\n"},{"id":"205887","messageId":"CAC_01E2iHgNvh5PnBh3TcNKr2pLazZwRojVK9ksaE3x0a1QHmQ@mail.gmail.com","threadId":"32484","inReplyTo":"7vk3rwaa3r.fsf@alter.siamese.dyndns.org","subject":"Re: git filter-branch doesn't dereference annotated tags","fromName":"Grégory Pakosz","fromEmail":"gpakosz@visionobjects.com","sentAt":"2013-01-02T22:03:16Z","receivedAt":"2013-01-02T22:03:16Z","isPatch":false,"sender":{"key":"gpakosz@visionobjects.com","avatar":"https://gravatar.com/avatar/4346628bc87f6417d0efe6a1a66d941e8f3afed0a8621ca1cec4cb768058204c?d=mp&s=160"},"body":"> I was wondering if it should be\n>\n>         sha1=$(git rev-parse --verify \"$ref\")\n>\n> or something that does not dereference a tag at all.\n>\n> The way I read what that loop seems to want to do is:\n>\n>         Read each refname that was given originally from the file\n>         $tempdir/heads, find out the object it used to refer to and\n>         have it in $sha1, find out what new object the object was\n>         rewritten to and have it in $rewritten, and:\n>\n>         (1) if the rewrite left the object unchanged, do nothing but\n>             warn users just in case this was a mistake;\n>         (2) if the rewrite told us to remove it, then delete the\n>             ref; or\n>         (3) if the rewrite gave us a new object, replace the ref to\n>             point to that new one.\n>\n>         And in the latter two cases, save the original one in\n>         $orig_namespace so that the user can choose to recover if\n>         this filter-branch was done by mistake.\n>\n> So I do not think unwraping the ref at that point makes any sense,\n> unless it is not prepared to handle annotated tags at all by\n> unwrapping tags too early.\n>\n> What am I missing?\n>\nSo we have an annotated tag that points to a commit that is rewritten\nto nothing as the result of the filtering. What should happen?\n\nMy initial questions and patching suggestions are based on git\n1.7.10.4 behavior.\nHowever, playing with git HEAD exhibits a slightly different behavior:\nit breaks when invoking git mktag line 459 (introduced by\n1bf6551e42c79a594689a356a9b14759d55f3cf5):\n  error: char7: could not get SHA1 hash\n  fatal: invalid tag signature file\n  Could not create new tag object for tag-a\n\nIt's basically the same problem. In my opinion, lines 447-466 should\ntake into account $new_sha1 is empty.\n\nPlease forgive me again for not having configured my mailer yet :(\nWhen I'm ready to provide a patch that implements a solution we all\nagree with I'll use git send-email.\nIn the mean time, I would like to pursue the discussion in this mail\nthread so please find attached a patch that deletes a tag instead of\ninvoking the tag-name-filter when it detects $new_sha1 is empty.\n\nI tested the patch doesn't break t7003. What do you think?\n\nGregory\n\n\nFrom 2cb9d5bc605cc2f2d8a6603b6a06657516959aa6 Mon Sep 17 00:00:00 2001\nFrom: Gregory Pakosz <gpakosz@visionobjects.com>\nDate: Wed, 2 Jan 2013 23:02:03 +0100\nSubject: [PATCH] git-filter-branch: allow deletion of tags when referenced\n commit gets rewritten to nothing\n\n---\n git-filter-branch.sh | 61 ++++++++++++++++++++++++++++------------------------\n 1 file changed, 33 insertions(+), 28 deletions(-)\n\ndiff --git a/git-filter-branch.sh b/git-filter-branch.sh\nindex 5314249..2e8569c 100755\n--- a/git-filter-branch.sh\n+++ b/git-filter-branch.sh\n@@ -436,36 +436,41 @@ if [ \"$filter_tag_name\" ]; then\n \n \t\t[ -f \"../map/$sha1\" ] || continue\n \t\tnew_sha1=\"$(cat \"../map/$sha1\")\"\n-\t\tGIT_COMMIT=\"$sha1\"\n-\t\texport GIT_COMMIT\n-\t\tnew_ref=\"$(echo \"$ref\" | eval \"$filter_tag_name\")\" ||\n-\t\t\tdie \"tag name filter failed: $filter_tag_name\"\n-\n-\t\techo \"$ref -> $new_ref ($sha1 -> $new_sha1)\"\n-\n-\t\tif [ \"$type\" = \"tag\" ]; then\n-\t\t\tnew_sha1=$( ( printf 'object %s\\ntype commit\\ntag %s\\n' \\\n-\t\t\t\t\t\t\"$new_sha1\" \"$new_ref\"\n-\t\t\t\tgit cat-file tag \"$ref\" |\n-\t\t\t\tsed -n \\\n-\t\t\t\t    -e '1,/^$/{\n-\t\t\t\t\t  /^object /d\n-\t\t\t\t\t  /^type /d\n-\t\t\t\t\t  /^tag /d\n-\t\t\t\t\t}' \\\n-\t\t\t\t    -e '/^-----BEGIN PGP SIGNATURE-----/q' \\\n-\t\t\t\t    -e 'p' ) |\n-\t\t\t\tgit mktag) ||\n-\t\t\t\tdie \"Could not create new tag object for $ref\"\n-\t\t\tif git cat-file tag \"$ref\" | \\\n-\t\t\t   sane_grep '^-----BEGIN PGP SIGNATURE-----' >/dev/null 2>&1\n-\t\t\tthen\n-\t\t\t\twarn \"gpg signature stripped from tag object $sha1t\"\n+\t\tif [ -n \"$new_sha1\" ]; then\n+\t\t\tGIT_COMMIT=\"$sha1\"\n+\t\t\texport GIT_COMMIT\n+\t\t\tnew_ref=\"$(echo \"$ref\" | eval \"$filter_tag_name\")\" ||\n+\t\t\t\tdie \"tag name filter failed: $filter_tag_name\"\n+\n+\t\t\techo \"$ref -> $new_ref ($sha1 -> $new_sha1)\"\n+\n+\t\t\tif [ \"$type\" = \"tag\" ]; then\n+\t\t\t\tnew_sha1=$( ( printf 'object %s\\ntype commit\\ntag %s\\n' \\\n+\t\t\t\t\t\t\t\"$new_sha1\" \"$new_ref\"\n+\t\t\t\t\tgit cat-file tag \"$ref\" |\n+\t\t\t\t\tsed -n \\\n+\t\t\t\t\t\t\t-e '1,/^$/{\n+\t\t\t\t\t\t\t/^object /d\n+\t\t\t\t\t\t\t/^type /d\n+\t\t\t\t\t\t\t/^tag /d\n+\t\t\t\t\t\t}' \\\n+\t\t\t\t\t\t\t-e '/^-----BEGIN PGP SIGNATURE-----/q' \\\n+\t\t\t\t\t\t\t-e 'p' ) |\n+\t\t\t\t\tgit mktag) ||\n+\t\t\t\t\tdie \"Could not create new tag object for $ref\"\n+\t\t\t\tif git cat-file tag \"$ref\" | \\\n+\t\t\t\t\tsane_grep '^-----BEGIN PGP SIGNATURE-----' >/dev/null 2>&1\n+\t\t\t\tthen\n+\t\t\t\t\twarn \"gpg signature stripped from tag object $sha1t\"\n+\t\t\t\tfi\n \t\t\tfi\n-\t\tfi\n \n-\t\tgit update-ref \"refs/tags/$new_ref\" \"$new_sha1\" ||\n-\t\t\tdie \"Could not write tag $new_ref\"\n+\t\t\tgit update-ref \"refs/tags/$new_ref\" \"$new_sha1\" ||\n+\t\t\t\tdie \"Could not write tag $new_ref\"\n+\t\telse\n+\t\t\tgit update-ref -d \"refs/tags/$ref\" \"$sha1t\" ||\n+\t\t\t\tdie \"Could not delete tag $ref\"\n+\t\tfi\n \tdone\n fi\n \n-- \n1.8.0.1\n\n"},{"id":"205896","messageId":"7v623f18ci.fsf@alter.siamese.dyndns.org","threadId":"32484","inReplyTo":"CAC_01E2iHgNvh5PnBh3TcNKr2pLazZwRojVK9ksaE3x0a1QHmQ@mail.gmail.com","subject":"Re: git filter-branch doesn't dereference annotated tags","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-01-02T23:19:25Z","receivedAt":"2013-01-02T23:19:25Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Grégory Pakosz <gpakosz@visionobjects.com> writes:\n\n> So we have an annotated tag that points to a commit that is rewritten\n> to nothing as the result of the filtering. What should happen?\n\nIf the user asked to filter that tag itself, it may make sense to\nremove it, rather than keeping it pointing at the original commit,\nbecause the commit it used to point at no longer exists in the\nalternate history being created by filter-branch.\n\n> It's basically the same problem. In my opinion, lines 447-466 should\n> take into account $new_sha1 is empty.\n\nYeah, I think that is a sensible observation.\n\nHaving said that, I welcome comments from others, of course.  My\ninvolvement in this script has been very limited.\n"},{"id":"205917","messageId":"50E55198.5080202@kdbg.org","threadId":"32484","inReplyTo":"7v623f18ci.fsf@alter.siamese.dyndns.org","subject":"Re: git filter-branch doesn't dereference annotated tags","fromName":"Johannes Sixt","fromEmail":"j6t@kdbg.org","sentAt":"2013-01-03T09:38:32Z","receivedAt":"2013-01-03T09:38:32Z","isPatch":false,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Am 03.01.2013 00:19, schrieb Junio C Hamano:\n> Grégory Pakosz <gpakosz@visionobjects.com> writes:\n> \n>> So we have an annotated tag that points to a commit that is rewritten\n>> to nothing as the result of the filtering. What should happen?\n> \n> If the user asked to filter that tag itself, it may make sense to\n> remove it, rather than keeping it pointing at the original commit,\n> because the commit it used to point at no longer exists in the\n> alternate history being created by filter-branch.\n\nIOW, if the command was something like\n\n  git filter-branch ...filter options... -- v1.0 master ...\n\nand v1.0 is an annotated tag, then it is reasonable to expect v1.0 to be\ndeleted if the commit it points to goes away. But if the commit did not\ngo away, but was rewritten, then it is equally reasonable to expect that\nthe tag is also rewritten. But I don't think that we currently do the\nlatter.\n\nTherefore, IMO, a change that implements the former behavior should also\nimplement the latter behavior.\n\n-- Hannes\n"},{"id":"205918","messageId":"CAC_01E2HXSnXBgDm=Cbwgi5PbiuHp_qPpoaqT_=pdDWDMnC5jA@mail.gmail.com","threadId":"32484","inReplyTo":"50E55198.5080202@kdbg.org","subject":"Re: git filter-branch doesn't dereference annotated tags","fromName":"Grégory Pakosz","fromEmail":"gpakosz@visionobjects.com","sentAt":"2013-01-03T09:50:27Z","receivedAt":"2013-01-03T09:50:27Z","isPatch":false,"sender":{"key":"gpakosz@visionobjects.com","avatar":"https://gravatar.com/avatar/4346628bc87f6417d0efe6a1a66d941e8f3afed0a8621ca1cec4cb768058204c?d=mp&s=160"},"body":">\n> IOW, if the command was something like\n>\n>   git filter-branch ...filter options... -- v1.0 master ...\n>\n> and v1.0 is an annotated tag, then it is reasonable to expect v1.0 to be\n> deleted if the commit it points to goes away. But if the commit did not\n> go away, but was rewritten, then it is equally reasonable to expect that\n> the tag is also rewritten. But I don't think that we currently do the\n> latter.\n>\nWhen the commit doesn't go away, the tag is currently being rewritten properly.\n\n> Therefore, IMO, a change that implements the former behavior should also\n> implement the latter behavior.\n>\nThe patch in my latest email does both. (yet lacks unit tests for now)\n\nGregory\n"},{"id":"205921","messageId":"50E55E94.2090401@kdbg.org","threadId":"32484","inReplyTo":"CAC_01E2HXSnXBgDm=Cbwgi5PbiuHp_qPpoaqT_=pdDWDMnC5jA@mail.gmail.com","subject":"Re: git filter-branch doesn't dereference annotated tags","fromName":"Johannes Sixt","fromEmail":"j6t@kdbg.org","sentAt":"2013-01-03T10:33:56Z","receivedAt":"2013-01-03T10:33:56Z","isPatch":false,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Am 03.01.2013 10:50, schrieb Grégory Pakosz:\n>>\n>> IOW, if the command was something like\n>>\n>>   git filter-branch ...filter options... -- v1.0 master ...\n>>\n>> and v1.0 is an annotated tag, then it is reasonable to expect v1.0 to be\n>> deleted if the commit it points to goes away. But if the commit did not\n>> go away, but was rewritten, then it is equally reasonable to expect that\n>> the tag is also rewritten. But I don't think that we currently do the\n>> latter.\n>>\n> When the commit doesn't go away, the tag is currently being rewritten properly.\n\nIndeed, but only if a --tag-name-filter was specified.\n\n>> Therefore, IMO, a change that implements the former behavior should also\n>> implement the latter behavior.\n>>\n> The patch in my latest email does both. (yet lacks unit tests for now)\n\nIf it deletes a tag only when --tag-name-filter was specified, than that\nshould be fine.\n\n-- Hannes\n"},{"id":"205952","messageId":"CA+sFfMcRsD-4QwPTw6edJdUOJxdqR71oQaT9vz3QL5oCnfLTsQ@mail.gmail.com","threadId":"32484","inReplyTo":"50E55E94.2090401@kdbg.org","subject":"Re: git filter-branch doesn't dereference annotated tags","fromName":"Brandon Casey","fromEmail":"drafnel@gmail.com","sentAt":"2013-01-03T20:52:31Z","receivedAt":"2013-01-03T20:52:31Z","isPatch":false,"sender":{"key":"drafnel@gmail.com","avatar":"https://avatars.githubusercontent.com/u/921167?v=4"},"body":"On Thu, Jan 3, 2013 at 2:33 AM, Johannes Sixt <j6t@kdbg.org> wrote:\n> Am 03.01.2013 10:50, schrieb Grégory Pakosz:\n>>>\n>>> IOW, if the command was something like\n>>>\n>>>   git filter-branch ...filter options... -- v1.0 master ...\n>>>\n>>> and v1.0 is an annotated tag, then it is reasonable to expect v1.0 to be\n>>> deleted if the commit it points to goes away. But if the commit did not\n>>> go away, but was rewritten, then it is equally reasonable to expect that\n>>> the tag is also rewritten. But I don't think that we currently do the\n>>> latter.\n>>>\n>> When the commit doesn't go away, the tag is currently being rewritten properly.\n>\n> Indeed, but only if a --tag-name-filter was specified.\n>\n>>> Therefore, IMO, a change that implements the former behavior should also\n>>> implement the latter behavior.\n>>>\n>> The patch in my latest email does both. (yet lacks unit tests for now)\n>\n> If it deletes a tag only when --tag-name-filter was specified, than that\n> should be fine.\n\nHmm, if a tag name filter _other_ than 'cat' is supplied, I think a\nuser will expect that the original tags will _not_ be touched, and\nespecially not deleted.\n\nRather than blindly deleting the original tag ref, maybe we should\nstill call the user's tag name filter, and then attempt to delete the\n\"new\" name provided by the filter, if it exists.  If the filter was\n'cat', then the new and old names will be the same.\n\n-Brandon\n"}]}