{"thread":{"id":"50796","subject":"Strange annotated tag issue","startedAt":"2019-03-21T16:59:59Z","lastAt":"2019-04-12T03:21:55Z","messageCount":48,"participants":["Robert Dailey","Bryan Turner","Jeff King","Ævar Arnfjörð Bjarmason","Denton Liu","Elijah Newren","Junio C Hamano","Johannes Sixt","Eckhard Maaß"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"372144","messageId":"CAHd499BM91tf7f8=phR4Az8vMsHAHUGYsSb1x9as=WukUVZHJw@mail.gmail.com","threadId":"50796","inReplyTo":null,"subject":"Strange annotated tag issue","fromName":"Robert Dailey","fromEmail":"rcdailey.lists@gmail.com","sentAt":"2019-03-21T16:59:42Z","receivedAt":"2019-03-21T16:59:59Z","isPatch":false,"sender":{"key":"rcdailey.lists@gmail.com","avatar":null},"body":"I have a particular tag in my repo that shows 2 annotated\ndescriptions, which is very confusing.\n\nThe command I ran:\n\n```\ngit show --format=fuller 4.2.0.1900\n```\n\nAnd the output:\n\n```\ntag 4.2.0/1900\nTagger: John Doe \nTaggerDate: Fri Jul 18 10:46:30 2014 -0500\n\nQA/Internal Release for 4.2.0.19\n\ntag 4.2.0/1900\nTagger: John Doe \nTaggerDate: Fri Jul 18 10:46:15 2014 -0500\n\nQA/Internal Release\n\ncommit 2fcfd00ef84572fb88852be55315914f37e91e11 (tag: 4.2.0.1900)\nAuthor: John Doe \nAuthorDate: Thu Jul 17 11:20:17 2014 -0500\nCommit: John Doe \nCommitDate: Thu Jul 17 11:20:17 2014 -0500\n\n Commit description\n```\n\nWhy does it show two entries? In my `packed-refs` file, it also shows\na strange revision for the tag (I expect to see just 1 SHA1). Not sure\nif it is related:\n\n```\n66c41d67da887025c4e22e9891f5cd261f82eb31 refs/tags/4.2.0.1900\n^2fcfd00ef84572fb88852be55315914f37e91e11\n```\n\nNote I'm checking all of this on a bare clone (used `git clone\n--mirror`). Can someone help me understand what is going on here? I\nfound this issue because I'm trying to do `git lfs migrate import`,\nand it isn't processing my tag because of this.\n"},{"id":"372148","messageId":"CAGyf7-F4vvzwVsdgtiog+xvwgHgYkNMKQ59bCxrZYtdn+eGAPw@mail.gmail.com","threadId":"50796","inReplyTo":"CAHd499BM91tf7f8=phR4Az8vMsHAHUGYsSb1x9as=WukUVZHJw@mail.gmail.com","subject":"Re: Strange annotated tag issue","fromName":"Bryan Turner","fromEmail":"bturner@atlassian.com","sentAt":"2019-03-21T19:04:22Z","receivedAt":"2019-03-21T19:04:35Z","isPatch":false,"sender":{"key":"bturner@atlassian.com","avatar":"https://gravatar.com/avatar/16bcf3167981c1ef7c804e502642366d888a35b0d0b0a4ca01fdc442aa1acb1e?d=mp&s=160"},"body":"On Thu, Mar 21, 2019 at 9:59 AM Robert Dailey wrote:\n>\n> I have a particular tag in my repo that shows 2 annotated\n> descriptions, which is very confusing.\n>\n> The command I ran:\n>\n> ```\n> git show --format=fuller 4.2.0.1900\n> ```\n>\n> And the output:\n>\n> ```\n> tag 4.2.0/1900\n> Tagger: John Doe \n> TaggerDate: Fri Jul 18 10:46:30 2014 -0500\n>\n> QA/Internal Release for 4.2.0.19\n>\n> tag 4.2.0/1900\n> Tagger: John Doe \n> TaggerDate: Fri Jul 18 10:46:15 2014 -0500\n>\n> QA/Internal Release\n\nNot sure about this part, though I notice that the two rows have\ndifferent \"TaggerDate\" values by ~15 seconds.\n\n>\n> commit 2fcfd00ef84572fb88852be55315914f37e91e11 (tag: 4.2.0.1900)\n> Author: John Doe \n> AuthorDate: Thu Jul 17 11:20:17 2014 -0500\n> Commit: John Doe \n> CommitDate: Thu Jul 17 11:20:17 2014 -0500\n>\n> Commit description\n> ```\n>\n> Why does it show two entries? In my `packed-refs` file, it also shows\n> a strange revision for the tag (I expect to see just 1 SHA1). Not sure\n> if it is related:\n>\n> ```\n> 66c41d67da887025c4e22e9891f5cd261f82eb31 refs/tags/4.2.0.1900\n> ^2fcfd00ef84572fb88852be55315914f37e91e11\n> ```\n\nThis part, though, is normal for \"packed-refs\". The first line shows\nthe annotated tag object's hash (\"66c41d67da8\") and the tagged\nobject's hash (\"2fcfd00ef8\"). You can see that \"2fcfd00ef8\" matches\nthe tagged commit output by \"git show\". The leading \"^\" on the second\nline is how Git knows the line identifies a peeled tag's target rather\nthan the start of a new ref. If your \"packed-refs\" starts with\n\"peeled\" (and maybe \"fully-peeled\") then every annotated (or signed)\ntag in the file should have a second line prefixed by \"^\".\n\nHopefully this at least resolves part of your question!\n\nBryan\n\n>\n> Note I'm checking all of this on a bare clone (used `git clone\n> --mirror`). Can someone help me understand what is going on here? I\n> found this issue because I'm trying to do `git lfs migrate import`,\n> and it isn't processing my tag because of this.\n"},{"id":"372151","messageId":"20190321192928.GA19427@sigill.intra.peff.net","threadId":"50796","inReplyTo":"CAHd499BM91tf7f8=phR4Az8vMsHAHUGYsSb1x9as=WukUVZHJw@mail.gmail.com","subject":"Re: Strange annotated tag issue","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2019-03-21T19:29:29Z","receivedAt":"2019-03-21T19:29:32Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Mar 21, 2019 at 11:59:42AM -0500, Robert Dailey wrote:\n\n> I have a particular tag in my repo that shows 2 annotated\n> descriptions, which is very confusing.\n> \n> The command I ran:\n> \n> ```\n> git show --format=fuller 4.2.0.1900\n> ```\n> \n> And the output:\n> \n> ```\n> tag 4.2.0/1900\n> Tagger: John Doe \n> TaggerDate: Fri Jul 18 10:46:30 2014 -0500\n> \n> QA/Internal Release for 4.2.0.19\n> \n> tag 4.2.0/1900\n> Tagger: John Doe \n> TaggerDate: Fri Jul 18 10:46:15 2014 -0500\n> \n> QA/Internal Release\n> \n> commit 2fcfd00ef84572fb88852be55315914f37e91e11 (tag: 4.2.0.1900)\n\nTags can point to any object, including another tag. It looks like\nsomebody made an annotated tag of an annotated tag (probably by\nmistake, given that they have the same tag-name).\n\nTry this:\n\n git init\n git commit -m commit --allow-empty\n git tag -m inner mytag HEAD\n git tag -f -m outer mytag mytag\n\n git show mytag\n\nwhich produces similar output. You can walk the chain yourself with \"git\nat-file tag 4.2.0.1900\". That will have a \"type\" and \"object\" field\nwhich presumably point to the second commit.\n\nMy guess is that somebody was trying to amend the tag commit message,\nbut used the tag name to create the second one, rather than the original\ncommit. I.e,. any of these would have worked for the second command to\nreplace the old tag:\n\n git tag -f -m 'new message' mytag HEAD\n\n git tag -f -m 'new message' 2fcfd00ef84572fb88852be55315914f37e91e11\n\n git tag -f -m 'new message' mytag mytag^{commit}\n\nIf the original tag isn't signed, you could rewrite it at this point\nusing one of the above commands, coupled with GIT_COMMITTER_* to munge\nthe author and date. But note that fetch doesn't update modified tags\nby default, so it may just cause confusion if you have a lot of people\nusing the repository.\n\n-Peff\n"},{"id":"372158","messageId":"20190321193912.GB19427@sigill.intra.peff.net","threadId":"50796","inReplyTo":"CAGyf7-F4vvzwVsdgtiog+xvwgHgYkNMKQ59bCxrZYtdn+eGAPw@mail.gmail.com","subject":"Re: Strange annotated tag issue","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2019-03-21T19:39:12Z","receivedAt":"2019-03-21T19:39:15Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Mar 21, 2019 at 12:04:22PM -0700, Bryan Turner wrote:\n\n> > Why does it show two entries? In my `packed-refs` file, it also shows\n> > a strange revision for the tag (I expect to see just 1 SHA1). Not sure\n> > if it is related:\n> >\n> > ```\n> > 66c41d67da887025c4e22e9891f5cd261f82eb31 refs/tags/4.2.0.1900\n> > ^2fcfd00ef84572fb88852be55315914f37e91e11\n> > ```\n> \n> This part, though, is normal for \"packed-refs\". The first line shows\n> the annotated tag object's hash (\"66c41d67da8\") and the tagged\n> object's hash (\"2fcfd00ef8\"). You can see that \"2fcfd00ef8\" matches\n> the tagged commit output by \"git show\". The leading \"^\" on the second\n> line is how Git knows the line identifies a peeled tag's target rather\n> than the start of a new ref. If your \"packed-refs\" starts with\n> \"peeled\" (and maybe \"fully-peeled\") then every annotated (or signed)\n> tag in the file should have a second line prefixed by \"^\".\n\nNicely explained.\n\nI think there's one other interesting bit of trivia, since we seem to be\ndealing with a tag-of-a-tag here. We store only a single peeled value\nfor each ref, whatever is at the bottom (which is always a non-tag). So\nin this case of a tag that points to a tag that points to a commit, the\npeeled value is the commit, and the tag in the middle isn't mentioned.\n\nLikewise for upload-pack output, which mentions the peeled objects (so\nthat clients can auto-fetch tags). It only gives the full peeling.\n\nIn theory we could store and advertise the intermediate objects, but I\ndon't think anybody has ever cared enough about this case to explore\nthat (and this is mostly an optimization; it should all work correctly,\nand I recall fixing some bugs over the years).\n\n-Peff\n"},{"id":"372425","messageId":"CAHd499BTACjf91Ohi34ozFQE_NOn-LVf-35t7h4CTtDFoMCpWw@mail.gmail.com","threadId":"50796","inReplyTo":"20190321192928.GA19427@sigill.intra.peff.net","subject":"Re: Strange annotated tag issue","fromName":"Robert Dailey","fromEmail":"rcdailey.lists@gmail.com","sentAt":"2019-03-25T13:50:14Z","receivedAt":"2019-03-25T13:50:32Z","isPatch":false,"sender":{"key":"rcdailey.lists@gmail.com","avatar":null},"body":"On Thu, Mar 21, 2019 at 2:29 PM Jeff King wrote:\n> Tags can point to any object, including another tag. It looks like\n> somebody made an annotated tag of an annotated tag (probably by\n> mistake, given that they have the same tag-name).\n>\n> Try this:\n>\n> git init\n> git commit -m commit --allow-empty\n> git tag -m inner mytag HEAD\n> git tag -f -m outer mytag mytag\n>\n> git show mytag\n>\n> which produces similar output. You can walk the chain yourself with \"git\n> at-file tag 4.2.0.1900\". That will have a \"type\" and \"object\" field\n> which presumably point to the second commit.\n>\n> My guess is that somebody was trying to amend the tag commit message,\n> but used the tag name to create the second one, rather than the original\n> commit. I.e,. any of these would have worked for the second command to\n> replace the old tag:\n>\n> git tag -f -m 'new message' mytag HEAD\n>\n> git tag -f -m 'new message' 2fcfd00ef84572fb88852be55315914f37e91e11\n>\n> git tag -f -m 'new message' mytag mytag^{commit}\n>\n> If the original tag isn't signed, you could rewrite it at this point\n> using one of the above commands, coupled with GIT_COMMITTER_* to munge\n> the author and date. But note that fetch doesn't update modified tags\n> by default, so it may just cause confusion if you have a lot of people\n> using the repository.\n\nThanks for explaining. This is very helpful. Am I naive to think that\nthis should be an error? I haven't seen a valid _pragmatic_ use for\ntags pointing to tags. In 100% of cases (including this one), it is\ndone out of error. As per your example, users try to \"correct\" an\nannotated tag pointing at a wrong tag or commit. What they expect is\nthe tag to point to the other tag's commit, but that's not what they\nget.\n\nFrom a high-level, pragmatic perspective, doesn't it make more sense\nto change the git behavior so that annotated tags may only point to\ncommit objects? And in the `git tag -f -m outer mytag mytag` case in\nyour example, this would automatically perform `mytag^{}` to ensure\nthat the behavior the user expects is the behavior they get?\n"},{"id":"372428","messageId":"20190325144930.GA19929@sigill.intra.peff.net","threadId":"50796","inReplyTo":"CAHd499BTACjf91Ohi34ozFQE_NOn-LVf-35t7h4CTtDFoMCpWw@mail.gmail.com","subject":"Re: Strange annotated tag issue","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2019-03-25T14:49:31Z","receivedAt":"2019-03-25T14:49:34Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Mar 25, 2019 at 08:50:14AM -0500, Robert Dailey wrote:\n\n> On Thu, Mar 21, 2019 at 2:29 PM Jeff King wrote:\n> > Tags can point to any object, including another tag. It looks like\n> > somebody made an annotated tag of an annotated tag (probably by\n> > mistake, given that they have the same tag-name).\n> [..]\n> Thanks for explaining. This is very helpful. Am I naive to think that\n> this should be an error? I haven't seen a valid _pragmatic_ use for\n> tags pointing to tags. In 100% of cases (including this one), it is\n> done out of error. As per your example, users try to \"correct\" an\n> annotated tag pointing at a wrong tag or commit. What they expect is\n> the tag to point to the other tag's commit, but that's not what they\n> get.\n\nI don't think I've ever seen a tag-to-a-tag in the wild, but I wouldn't\nbe surprised if somebody has found a use for it. For example, because\ntags can be signed, I can make a signature of your signature, showing a\ncryptographic chain of custody.\n\nAnd at any rate, it has been allowed in the data model for almost 15\nyears, so I think disallowing it now would be a bad idea. It might be\nacceptable to introduce a safety valve into the porcelain, though.\n\n> From a high-level, pragmatic perspective, doesn't it make more sense\n> to change the git behavior so that annotated tags may only point to\n> commit objects? And in the `git tag -f -m outer mytag mytag` case in\n> your example, this would automatically perform `mytag^{}` to ensure\n> that the behavior the user expects is the behavior they get?\n\nI think \"just commits\" is too restrictive. linux.git contains a tag of a\ntree, for example (we also have tags pointing to blobs in git.git, but\nthey are not annotated).\n\nHowever, I could see an argument for the git-tag porcelain to notice a\ntag-of-tag and complain. Probably peeling the tag automatically is a bad\nidea, just because it behaved differently for so long. But something\nlike might be OK:\n\n $ git tag -a mytag\n error: refusing to make a recursive tag\n hint: The object 'mytag' referred to by your new tag is already a tag.\n hint:\n hint: If you meant to create a tag of a tag, use:\n hint:\n hint: git tag -a -f mytag\n hint:\n hint: If you meant to tag the object that it points to, use:\n hint:\n hint: git tag -a mytag^{}\n\nIt would be a minor annoyance to somebody who frequently makes\ntags-of-tags, but it leaves them with an escape hatch.\n\n-Peff\n"},{"id":"372433","messageId":"CAHd499CimFqfa-k6pB3NuMmM6fBUeMCOjs0ZUuGEHCc8Q5eEBg@mail.gmail.com","threadId":"50796","inReplyTo":"20190325144930.GA19929@sigill.intra.peff.net","subject":"Re: Strange annotated tag issue","fromName":"Robert Dailey","fromEmail":"rcdailey.lists@gmail.com","sentAt":"2019-03-25T15:31:50Z","receivedAt":"2019-03-25T15:32:05Z","isPatch":false,"sender":{"key":"rcdailey.lists@gmail.com","avatar":null},"body":"On Mon, Mar 25, 2019 at 9:49 AM Jeff King wrote:\n> I think \"just commits\" is too restrictive. linux.git contains a tag of a\n> tree, for example (we also have tags pointing to blobs in git.git, but\n> they are not annotated).\n>\n> However, I could see an argument for the git-tag porcelain to notice a\n> tag-of-tag and complain. Probably peeling the tag automatically is a bad\n> idea, just because it behaved differently for so long. But something\n> like might be OK:\n>\n> $ git tag -a mytag\n> error: refusing to make a recursive tag\n> hint: The object 'mytag' referred to by your new tag is already a tag.\n> hint:\n> hint: If you meant to create a tag of a tag, use:\n> hint:\n> hint: git tag -a -f mytag\n> hint:\n> hint: If you meant to tag the object that it points to, use:\n> hint:\n> hint: git tag -a mytag^{}\n>\n> It would be a minor annoyance to somebody who frequently makes\n> tags-of-tags, but it leaves them with an escape hatch.\n\nI think a warning/error would be perfect. Again, if I had realized the\nconsequences of my actions years ago when I made this tag, it would\nhave changed a lot down the line for me. If I had seen that message\nbefore, I think it would have helped a lot. You might even add an\neducational bit in there and say \"Warning: You are creating an\nannotated tag that points to another annotated tag. Annotated tags may\npoint to more than just commits by design. Refspec tag^{} may be used\nto peel the source tag back to the commit it points to\". Then follow\nwith the instructional commands.\n\nI agree with your approach though. Thanks again.\n"},{"id":"372454","messageId":"CAGyf7-EOrCgPY19jgwRd5H-0ADMX9WtLUuACmTAANnNVW-K-_Q@mail.gmail.com","threadId":"50796","inReplyTo":"20190325144930.GA19929@sigill.intra.peff.net","subject":"Re: Strange annotated tag issue","fromName":"Bryan Turner","fromEmail":"bturner@atlassian.com","sentAt":"2019-03-25T18:43:52Z","receivedAt":"2019-03-25T18:44:05Z","isPatch":false,"sender":{"key":"bturner@atlassian.com","avatar":"https://gravatar.com/avatar/16bcf3167981c1ef7c804e502642366d888a35b0d0b0a4ca01fdc442aa1acb1e?d=mp&s=160"},"body":"On Mon, Mar 25, 2019 at 7:49 AM Jeff King wrote:\n>\n> On Mon, Mar 25, 2019 at 08:50:14AM -0500, Robert Dailey wrote:\n>\n> > On Thu, Mar 21, 2019 at 2:29 PM Jeff King wrote:\n> > > Tags can point to any object, including another tag. It looks like\n> > > somebody made an annotated tag of an annotated tag (probably by\n> > > mistake, given that they have the same tag-name).\n> > [..]\n> > Thanks for explaining. This is very helpful. Am I naive to think that\n> > this should be an error? I haven't seen a valid _pragmatic_ use for\n> > tags pointing to tags. In 100% of cases (including this one), it is\n> > done out of error. As per your example, users try to \"correct\" an\n> > annotated tag pointing at a wrong tag or commit. What they expect is\n> > the tag to point to the other tag's commit, but that's not what they\n> > get.\n>\n> I don't think I've ever seen a tag-to-a-tag in the wild, but I wouldn't\n> be surprised if somebody has found a use for it. For example, because\n> tags can be signed, I can make a signature of your signature, showing a\n> cryptographic chain of custody.\n\nFor a while the Atlassian Bamboo team followed a workflow where they\nwould do a build in CI, tag that build and then deploy it to a sandbox\nenvironment for smoke testing. If it passed the smoke tests, it would\nget \"promoted\" from the sandbox environment to internal instances used\nby the various teams to do their builds. When a sandbox build was\n\"promoted\", they'd create a tag of the sandbox build's tag to have\ntraceability between the two environments.\n\nI'm not advocating for or judging that workflow one way or another,\nand the Bamboo team has since moved on to a different workflow. I just\nthought I'd share it as a tag-of-tag workflow that I've seen a real\nteam using. (There was one place in Bitbucket Server's code where we\ndidn't handle recursive tags correctly, so their workflow caused some\nerrors that I needed to make some adjustments for. As a result,\nBitbucket Server's test suite now includes tests that cover tag-of-tag\nbehaviors.)\n\nBryan\n"},{"id":"372457","messageId":"87tvfqbw3m.fsf@evledraar.gmail.com","threadId":"50796","inReplyTo":"20190325144930.GA19929@sigill.intra.peff.net","subject":"Re: Strange annotated tag issue","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2019-03-25T19:19:25Z","receivedAt":"2019-03-25T19:19:30Z","isPatch":false,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Mon, Mar 25 2019, Jeff King wrote:\n\n> On Mon, Mar 25, 2019 at 08:50:14AM -0500, Robert Dailey wrote:\n>\n>> On Thu, Mar 21, 2019 at 2:29 PM Jeff King wrote:\n>> > Tags can point to any object, including another tag. It looks like\n>> > somebody made an annotated tag of an annotated tag (probably by\n>> > mistake, given that they have the same tag-name).\n>> [..]\n>> Thanks for explaining. This is very helpful. Am I naive to think that\n>> this should be an error? I haven't seen a valid _pragmatic_ use for\n>> tags pointing to tags. In 100% of cases (including this one), it is\n>> done out of error. As per your example, users try to \"correct\" an\n>> annotated tag pointing at a wrong tag or commit. What they expect is\n>> the tag to point to the other tag's commit, but that's not what they\n>> get.\n>\n> I don't think I've ever seen a tag-to-a-tag in the wild, but I wouldn't\n> be surprised if somebody has found a use for it. For example, because\n> tags can be signed, I can make a signature of your signature, showing a\n> cryptographic chain of custody.\n>\n> And at any rate, it has been allowed in the data model for almost 15\n> years, so I think disallowing it now would be a bad idea. It might be\n> acceptable to introduce a safety valve into the porcelain, though.\n>\n>> From a high-level, pragmatic perspective, doesn't it make more sense\n>> to change the git behavior so that annotated tags may only point to\n>> commit objects? And in the `git tag -f -m outer mytag mytag` case in\n>> your example, this would automatically perform `mytag^{}` to ensure\n>> that the behavior the user expects is the behavior they get?\n>\n> I think \"just commits\" is too restrictive. linux.git contains a tag of a\n> tree, for example (we also have tags pointing to blobs in git.git, but\n> they are not annotated).\n>\n> However, I could see an argument for the git-tag porcelain to notice a\n> tag-of-tag and complain. Probably peeling the tag automatically is a bad\n> idea, just because it behaved differently for so long. But something\n> like might be OK:\n\nSounds good!\n\n> $ git tag -a mytag\n> error: refusing to make a recursive tag\n> hint: The object 'mytag' referred to by your new tag is already a tag.\n> hint:\n> hint: If you meant to create a tag of a tag, use:\n> hint:\n> hint: git tag -a -f mytag\n> hint:\n> hint: If you meant to tag the object that it points to, use:\n> hint:\n> hint: git tag -a mytag^{}\n>\n> It would be a minor annoyance to somebody who frequently makes\n> tags-of-tags, but it leaves them with an escape hatch.\n\nLet's call that something like --allow-recursive-tag (inspired by\n'merge' --allow-unrelated-histories) so we don't confuse the desire to\ncreate such a tag with clobbering an existing tag (which -f is\ndocumented to do).\n\nI was going to say \"let's make the 'error:' part self-explanatory\nwithout the 'hint:'\" part, in case the advice was disabled. But looking\nwe only allow turning advice off on a per-variable basis, so I suspect\nif someone bothered to do that they know about this already. Our\n--allow-recursive-tag message is also fairly cryptic (and should, but\ndoesn't have, advice).\n"},{"id":"372488","messageId":"20190325233644.GC23728@sigill.intra.peff.net","threadId":"50796","inReplyTo":"CAGyf7-EOrCgPY19jgwRd5H-0ADMX9WtLUuACmTAANnNVW-K-_Q@mail.gmail.com","subject":"Re: Strange annotated tag issue","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2019-03-25T23:36:45Z","receivedAt":"2019-03-25T23:36:48Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Mar 25, 2019 at 11:43:52AM -0700, Bryan Turner wrote:\n\n> > I don't think I've ever seen a tag-to-a-tag in the wild, but I wouldn't\n> > be surprised if somebody has found a use for it. For example, because\n> > tags can be signed, I can make a signature of your signature, showing a\n> > cryptographic chain of custody.\n> \n> For a while the Atlassian Bamboo team followed a workflow where they\n> would do a build in CI, tag that build and then deploy it to a sandbox\n> environment for smoke testing. If it passed the smoke tests, it would\n> get \"promoted\" from the sandbox environment to internal instances used\n> by the various teams to do their builds. When a sandbox build was\n> \"promoted\", they'd create a tag of the sandbox build's tag to have\n> traceability between the two environments.\n> \n> I'm not advocating for or judging that workflow one way or another,\n> and the Bamboo team has since moved on to a different workflow. I just\n> thought I'd share it as a tag-of-tag workflow that I've seen a real\n> team using. (There was one place in Bitbucket Server's code where we\n> didn't handle recursive tags correctly, so their workflow caused some\n> errors that I needed to make some adjustments for. As a result,\n> Bitbucket Server's test suite now includes tests that cover tag-of-tag\n> behaviors.)\n\nThanks, I always like hearing these kinds of data points. If nothing\nelse, it's a good reminder that if Git has behaved some way for many\nyears, then _somebody_ is likely to have taken advantage of it, whether\nwe considered it a possibility or not. :)\n\n-Peff\n"},{"id":"372489","messageId":"20190325233723.GD23728@sigill.intra.peff.net","threadId":"50796","inReplyTo":"87tvfqbw3m.fsf@evledraar.gmail.com","subject":"Re: Strange annotated tag issue","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2019-03-25T23:37:24Z","receivedAt":"2019-03-25T23:37:27Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Mar 25, 2019 at 08:19:25PM +0100, Ævar Arnfjörð Bjarmason wrote:\n\n> > $ git tag -a mytag\n> > error: refusing to make a recursive tag\n> > hint: The object 'mytag' referred to by your new tag is already a tag.\n> > hint:\n> > hint: If you meant to create a tag of a tag, use:\n> > hint:\n> > hint: git tag -a -f mytag\n> > hint:\n> > hint: If you meant to tag the object that it points to, use:\n> > hint:\n> > hint: git tag -a mytag^{}\n> >\n> > It would be a minor annoyance to somebody who frequently makes\n> > tags-of-tags, but it leaves them with an escape hatch.\n> \n> Let's call that something like --allow-recursive-tag (inspired by\n> 'merge' --allow-unrelated-histories) so we don't confuse the desire to\n> create such a tag with clobbering an existing tag (which -f is\n> documented to do).\n\nYeah, that's probably a good idea. Now we just need somebody to write\nthe patch...\n\n-Peff\n"},{"id":"372501","messageId":"cover.1553586707.git.liu.denton@gmail.com","threadId":"50796","inReplyTo":"20190325233723.GD23728@sigill.intra.peff.net","subject":"[PATCH 0/3] tag: prevent recursive tags","fromName":"Denton Liu","fromEmail":"liu.denton@gmail.com","sentAt":"2019-03-26T07:53:14Z","receivedAt":"2019-03-26T07:53:20Z","isPatch":true,"sender":{"key":"liu.denton@gmail.com","avatar":"https://avatars.githubusercontent.com/u/9620836?v=4"},"body":"Peff said:\n> Yeah, that's probably a good idea. Now we just need somebody to write\n> the patch...\n> \n> -Peff\n\nHey would you look at that, somebody wrote the patch!\n\n---\n\nEarlier in the mailing list[1], Robert Dailey reported confusion over\nsome recursive tags.\n\nPeff noted that he hasn't seen a tag-to-a-tag in the wild so in most\ncases, it'd probably be a mistake on the part of a user. He also\nsuggested we error out on a recursive tag unless \"--allow-recursive-tag\"\nis provided.\n\nThis patchset implements those suggestions.\n\n[1]: https://public-inbox.org/git/CAHd499BM91tf7f8=phR4Az8vMsHAHUGYsSb1x9as=WukUVZHJw@mail.gmail.com/\n[2]: https://public-inbox.org/git/20190325144930.GA19929@sigill.intra.peff.net/\n\n\nDenton Liu (3):\n tag: prevent recursive tags\n t7004: ensure recursive tag behavior is working\n git-tag.txt: document --allow-recursive-tag option\n\n Documentation/git-tag.txt | 7 ++++++-\n advice.c | 2 ++\n advice.h | 1 +\n builtin/tag.c | 30 ++++++++++++++++++++++++++----\n t/annotate-tests.sh | 2 +-\n t/t0410-partial-clone.sh | 2 +-\n t/t4205-log-pretty-formats.sh | 2 +-\n t/t5305-include-tag.sh | 2 +-\n t/t5500-fetch-pack.sh | 2 +-\n t/t6302-for-each-ref-filter.sh | 4 ++--\n t/t7004-tag.sh | 12 ++++++++++--\n t/t9350-fast-export.sh | 4 ++--\n 12 files changed, 54 insertions(+), 16 deletions(-)\n\n-- \n2.21.0.512.g57bf1b23e1\n\n"},{"id":"372502","messageId":"c371a653b4049256f3427e467b144385ee47ef43.1553586707.git.liu.denton@gmail.com","threadId":"50796","inReplyTo":"cover.1553586707.git.liu.denton@gmail.com","subject":"[PATCH 1/3] tag: prevent recursive tags","fromName":"Denton Liu","fromEmail":"liu.denton@gmail.com","sentAt":"2019-03-26T07:53:18Z","receivedAt":"2019-03-26T07:53:25Z","isPatch":true,"sender":{"key":"liu.denton@gmail.com","avatar":"https://avatars.githubusercontent.com/u/9620836?v=4"},"body":"Robert Dailey reported confusion on the mailing list about a recursive\ntag which was most likely created by mistake. Jeff King noted that this\nisn't a very common case so, most likely, creating a tag-to-a-tag is a\nuser-error.\n\nPrevent mistakes by erroring and providing advice on recursive tags,\nunless \"--allow-recursive-tag\" is specified. Fix tests that fail as a\nresult of this change.\n\nReported-by: Robert Dailey \nHelped-by: Jeff King \nHelped-by: Ævar Arnfjörð Bjarmason \nSigned-off-by: Denton Liu \n---\n advice.c | 2 ++\n advice.h | 1 +\n builtin/tag.c | 30 ++++++++++++++++++++++++++----\n t/annotate-tests.sh | 2 +-\n t/t0410-partial-clone.sh | 2 +-\n t/t4205-log-pretty-formats.sh | 2 +-\n t/t5305-include-tag.sh | 2 +-\n t/t5500-fetch-pack.sh | 2 +-\n t/t6302-for-each-ref-filter.sh | 4 ++--\n t/t7004-tag.sh | 4 ++--\n t/t9350-fast-export.sh | 4 ++--\n 11 files changed, 40 insertions(+), 15 deletions(-)\n\ndiff --git a/advice.c b/advice.c\nindex 567209aa79..f31889e6de 100644\n--- a/advice.c\n+++ b/advice.c\n@@ -26,6 +26,7 @@ int advice_ignored_hook = 1;\n int advice_waiting_for_editor = 1;\n int advice_graft_file_deprecated = 1;\n int advice_checkout_ambiguous_remote_branch_name = 1;\n+int advice_recursive_tag = 1;\n \n static int advice_use_color = -1;\n static char advice_colors[][COLOR_MAXLEN] = {\n@@ -81,6 +82,7 @@ static struct {\n \t{ \"waitingForEditor\", &advice_waiting_for_editor },\n \t{ \"graftFileDeprecated\", &advice_graft_file_deprecated },\n \t{ \"checkoutAmbiguousRemoteBranchName\", &advice_checkout_ambiguous_remote_branch_name },\n+\t{ \"recursiveTag\", &advice_recursive_tag },\n \n \t/* make this an alias for backward compatibility */\n \t{ \"pushNonFastForward\", &advice_push_update_rejected }\ndiff --git a/advice.h b/advice.h\nindex f875f8cd8d..66aa39757c 100644\n--- a/advice.h\n+++ b/advice.h\n@@ -26,6 +26,7 @@ extern int advice_ignored_hook;\n extern int advice_waiting_for_editor;\n extern int advice_graft_file_deprecated;\n extern int advice_checkout_ambiguous_remote_branch_name;\n+extern int advice_recursive_tag;\n \n int git_default_advice_config(const char *var, const char *value);\n __attribute__((format (printf, 1, 2)))\ndiff --git a/builtin/tag.c b/builtin/tag.c\nindex 02f6bd1279..0b44a3cbc1 100644\n--- a/builtin/tag.c\n+++ b/builtin/tag.c\n@@ -22,10 +22,11 @@\n #include \"ref-filter.h\"\n \n static const char * const git_tag_usage[] = {\n-\tN_(\"git tag [-a | -s | -u ] [-f] [-m | -F ] []\"),\n+\tN_(\"git tag [-a | -s | -u ] [-f] [-m | -F ] [--allow-recursive-tag]\\n\"\n+\t\t\"\\t\\t []\"),\n \tN_(\"git tag -d ...\"),\n-\tN_(\"git tag -l [-n[]] [--contains ] [--no-contains ] [--points-at ]\"\n-\t\t\"\\n\\t\\t[--format=] [--[no-]merged []] [...]\"),\n+\tN_(\"git tag -l [-n[]] [--contains ] [--no-contains ] [--points-at ]\\n\"\n+\t\t\"\\t\\t[--format=] [--[no-]merged []] [...]\"),\n \tN_(\"git tag -v [--format=] ...\"),\n \tNULL\n };\n@@ -197,6 +198,7 @@ static int build_tag_object(struct strbuf *buf, int sign, struct object_id *resu\n struct create_tag_options {\n \tunsigned int message_given:1;\n \tunsigned int use_editor:1;\n+\tunsigned int allow_recursive_tag;\n \tunsigned int sign;\n \tenum {\n \t\tCLEANUP_NONE,\n@@ -205,6 +207,17 @@ struct create_tag_options {\n \t} cleanup_mode;\n };\n \n+static const char message_advice_recursive_tag[] =\n+\tN_(\"The object '%s' referred to by your new tag is already a tag.\\n\"\n+\t \"\\n\"\n+\t \"If you meant to create a tag of a tag, use:\\n\"\n+\t \"\\n\"\n+\t \"\\tgit tag --allow-recursive-tag %s\\n\"\n+\t \"\\n\"\n+\t \"If you meant to tag the object that it points to, use:\\n\"\n+\t \"\\n\"\n+\t \"\\tgit tag %s^{}\");\n+\n static void create_tag(const struct object_id *object, const char *tag,\n \t\t struct strbuf *buf, struct create_tag_options *opt,\n \t\t struct object_id *prev, struct object_id *result)\n@@ -215,7 +228,14 @@ static void create_tag(const struct object_id *object, const char *tag,\n \n \ttype = oid_object_info(the_repository, object, NULL);\n \tif (type <= OBJ_NONE)\n-\t die(_(\"bad object type.\"));\n+\t\tdie(_(\"bad object type.\"));\n+\n+\tif (type == OBJ_TAG && !opt->allow_recursive_tag) {\n+\t\terror(_(\"refusing to make a recursive tag\"));\n+\t\tif (advice_recursive_tag)\n+\t\t\tadvise(_(message_advice_recursive_tag), tag, tag, tag);\n+\t\texit(1);\n+\t}\n \n \tstrbuf_addf(&header,\n \t\t \"object %s\\n\"\n@@ -403,6 +423,8 @@ int cmd_tag(int argc, const char **argv, const char *prefix)\n \t\t\t\t\tN_(\"use another key to sign the tag\")),\n \t\tOPT__FORCE(&force, N_(\"replace the tag if exists\"), 0),\n \t\tOPT_BOOL(0, \"create-reflog\", &create_reflog, N_(\"create a reflog\")),\n+\t\tOPT_BOOL(0, \"allow-recursive-tag\", &opt.allow_recursive_tag,\n+\t\t\t\t\tN_(\"allow recursive tags to be made\")),\n \n \t\tOPT_GROUP(N_(\"Tag listing options\")),\n \t\tOPT_COLUMN(0, \"column\", &colopts, N_(\"show tag list in columns\")),\ndiff --git a/t/annotate-tests.sh b/t/annotate-tests.sh\nindex 6da48a2e0a..841f922e07 100644\n--- a/t/annotate-tests.sh\n+++ b/t/annotate-tests.sh\n@@ -70,7 +70,7 @@ test_expect_success 'blame 1 author' '\n \n test_expect_success 'blame by tag objects' '\n \tgit tag -m \"test tag\" testTag &&\n-\tgit tag -m \"test tag #2\" testTag2 testTag &&\n+\tgit tag -m \"test tag #2\" --allow-recursive-tag testTag2 testTag &&\n \tcheck_count -h testTag A 2 &&\n \tcheck_count -h testTag2 A 2\n '\ndiff --git a/t/t0410-partial-clone.sh b/t/t0410-partial-clone.sh\nindex bce02788e6..5f06c2d76f 100755\n--- a/t/t0410-partial-clone.sh\n+++ b/t/t0410-partial-clone.sh\n@@ -16,7 +16,7 @@ pack_as_from_promisor () {\n \n promise_and_delete () {\n \tHASH=$(git -C repo rev-parse \"$1\") &&\n-\tgit -C repo tag -a -m message my_annotated_tag \"$HASH\" &&\n+\tgit -C repo tag -a -m message my_annotated_tag --allow-recursive-tag \"$HASH\" &&\n \tgit -C repo rev-parse my_annotated_tag | pack_as_from_promisor &&\n \t# tag -d prints a message to stdout, so redirect it\n \tgit -C repo tag -d my_annotated_tag >/dev/null &&\ndiff --git a/t/t4205-log-pretty-formats.sh b/t/t4205-log-pretty-formats.sh\nindex f42a69faa2..018550f3b2 100755\n--- a/t/t4205-log-pretty-formats.sh\n+++ b/t/t4205-log-pretty-formats.sh\n@@ -511,7 +511,7 @@ test_expect_success 'set up log decoration tests' '\n \n test_expect_success 'log decoration properly follows tag chain' '\n \tgit tag -a tag1 -m tag1 &&\n-\tgit tag -a tag2 -m tag2 tag1 &&\n+\tgit tag -a tag2 -m tag2 --allow-recursive-tag tag1 &&\n \tgit tag -d tag1 &&\n \tgit commit --amend -m shorter &&\n \tgit log --no-walk --tags --pretty=\"%H %d\" --decorate=full >actual &&\ndiff --git a/t/t5305-include-tag.sh b/t/t5305-include-tag.sh\nindex a5eca210b8..c99850c1c0 100755\n--- a/t/t5305-include-tag.sh\n+++ b/t/t5305-include-tag.sh\n@@ -68,7 +68,7 @@ test_expect_success 'check unpacked result (have commit, have tag)' '\n test_expect_success 'create hidden inner tag' '\n \ttest_commit commit &&\n \tgit tag -m inner inner HEAD &&\n-\tgit tag -m outer outer inner &&\n+\tgit tag -m outer --allow-recursive-tag outer inner &&\n \tgit tag -d inner\n '\n \ndiff --git a/t/t5500-fetch-pack.sh b/t/t5500-fetch-pack.sh\nindex 49c540b1e1..c549b37aec 100755\n--- a/t/t5500-fetch-pack.sh\n+++ b/t/t5500-fetch-pack.sh\n@@ -562,7 +562,7 @@ test_expect_success 'test --all wrt tag to non-commits' '\n \t\thello tag\n \tEOF\n \t) &&\n-\tgit tag -a -m \"tag -> tag\" tag-to-tag $tag &&\n+\tgit tag -a -m \"tag -> tag\" --allow-recursive-tag tag-to-tag $tag &&\n \n \t# `fetch-pack --all` should succeed fetching all those objects.\n \tmkdir fetchall &&\ndiff --git a/t/t6302-for-each-ref-filter.sh b/t/t6302-for-each-ref-filter.sh\nindex fc067ed672..f7b56ae195 100755\n--- a/t/t6302-for-each-ref-filter.sh\n+++ b/t/t6302-for-each-ref-filter.sh\n@@ -12,7 +12,7 @@ test_expect_success 'setup some history and refs' '\n \tgit checkout -b side &&\n \ttest_commit four &&\n \tgit tag -m \"An annotated tag\" annotated-tag &&\n-\tgit tag -m \"Annonated doubly\" doubly-annotated-tag annotated-tag &&\n+\tgit tag -m \"Annonated doubly\" --allow-recursive-tag doubly-annotated-tag annotated-tag &&\n \n \t# Note that these \"signed\" tags might not actually be signed.\n \t# Tests which care about the distinction should be marked\n@@ -24,7 +24,7 @@ test_expect_success 'setup some history and refs' '\n \t\tsign=\n \tfi &&\n \tgit tag $sign -m \"A signed tag\" signed-tag &&\n-\tgit tag $sign -m \"Signed doubly\" doubly-signed-tag signed-tag &&\n+\tgit tag $sign -m \"Signed doubly\" --allow-recursive-tag doubly-signed-tag signed-tag &&\n \n \tgit checkout master &&\n \tgit update-ref refs/odd/spot master\ndiff --git a/t/t7004-tag.sh b/t/t7004-tag.sh\nindex 0b01862c23..7a7c0ccee9 100755\n--- a/t/t7004-tag.sh\n+++ b/t/t7004-tag.sh\n@@ -1265,7 +1265,7 @@ echo \"A message for another tag\" >>expect\n echo '-----BEGIN PGP SIGNATURE-----' >>expect\n test_expect_success GPG \\\n \t'creating a signed tag pointing to another tag should succeed' '\n-\tgit tag -s -m \"A message for another tag\" tag-signed-tag signed-tag &&\n+\tgit tag -s -m \"A message for another tag\" --allow-recursive-tag tag-signed-tag signed-tag &&\n \tget_tag_msg tag-signed-tag >actual &&\n \ttest_cmp expect actual\n '\n@@ -1690,7 +1690,7 @@ test_expect_success '--points-at finds annotated tags of commits' '\n '\n \n test_expect_success '--points-at finds annotated tags of tags' '\n-\tgit tag -m \"describing the v4.0 tag object\" \\\n+\tgit tag -m \"describing the v4.0 tag object\" --allow-recursive-tag \\\n \t\tannotated-again-v4.0 annotated-v4.0 &&\n \tcat >expect <<-\\EOF &&\n \tannotated-again-v4.0\ndiff --git a/t/t9350-fast-export.sh b/t/t9350-fast-export.sh\nindex 5690fe2810..b5ed7e119a 100755\n--- a/t/t9350-fast-export.sh\n+++ b/t/t9350-fast-export.sh\n@@ -441,8 +441,8 @@ test_expect_success 'set-up a few more tags for tag export tests' '\n \tHEAD_TREE=$(git show -s --pretty=raw HEAD | grep tree | sed \"s/tree //\") &&\n \tgit tag tree_tag -m \"tagging a tree\" $HEAD_TREE &&\n \tgit tag -a tree_tag-obj -m \"tagging a tree\" $HEAD_TREE &&\n-\tgit tag tag-obj_tag -m \"tagging a tag\" tree_tag-obj &&\n-\tgit tag -a tag-obj_tag-obj -m \"tagging a tag\" tree_tag-obj\n+\tgit tag tag-obj_tag -m \"tagging a tag\" --allow-recursive-tag tree_tag-obj &&\n+\tgit tag -a tag-obj_tag-obj -m \"tagging a tag\" --allow-recursive-tag tree_tag-obj\n '\n \n test_expect_success 'tree_tag' '\n-- \n2.21.0.512.g57bf1b23e1\n\n"},{"id":"372503","messageId":"d545bed9d3686b41ba67cfb25e00bb5a8b61c77b.1553586707.git.liu.denton@gmail.com","threadId":"50796","inReplyTo":"cover.1553586707.git.liu.denton@gmail.com","subject":"[PATCH 2/3] t7004: ensure recursive tag behavior is working","fromName":"Denton Liu","fromEmail":"liu.denton@gmail.com","sentAt":"2019-03-26T07:53:20Z","receivedAt":"2019-03-26T07:53:26Z","isPatch":true,"sender":{"key":"liu.denton@gmail.com","avatar":"https://avatars.githubusercontent.com/u/9620836?v=4"},"body":"Add tests to ensure that recursive tags are disallowed unless the\n\"--allow-recursive-tag\" option is provided.\n\nSigned-off-by: Denton Liu \n---\n t/t7004-tag.sh | 8 ++++++++\n 1 file changed, 8 insertions(+)\n\ndiff --git a/t/t7004-tag.sh b/t/t7004-tag.sh\nindex 7a7c0ccee9..5297da952d 100755\n--- a/t/t7004-tag.sh\n+++ b/t/t7004-tag.sh\n@@ -1700,6 +1700,14 @@ test_expect_success '--points-at finds annotated tags of tags' '\n \ttest_cmp expect actual\n '\n \n+test_expect_success 'recursive tagging should fail without --allow-recursive-tag' '\n+\ttest_must_fail git tag -m recursive recursive annotated-v4.0\n+'\n+\n+test_expect_success 'recursive tagging should pass with --allow-recursive-tag' '\n+\tgit tag --allow-recursive-tag -m recursive recursive annotated-v4.0\n+'\n+\n test_expect_success 'multiple --points-at are OR-ed together' '\n \tcat >expect <<-\\EOF &&\n \tv2.0\n-- \n2.21.0.512.g57bf1b23e1\n\n"},{"id":"372504","messageId":"fe503ddc8f01f3c0c598b00da22c201a7f16ba7f.1553586707.git.liu.denton@gmail.com","threadId":"50796","inReplyTo":"cover.1553586707.git.liu.denton@gmail.com","subject":"[PATCH 3/3] git-tag.txt: document --allow-recursive-tag option","fromName":"Denton Liu","fromEmail":"liu.denton@gmail.com","sentAt":"2019-03-26T07:53:23Z","receivedAt":"2019-03-26T07:53:33Z","isPatch":true,"sender":{"key":"liu.denton@gmail.com","avatar":"https://avatars.githubusercontent.com/u/9620836?v=4"},"body":"Signed-off-by: Denton Liu \n---\n Documentation/git-tag.txt | 7 ++++++-\n 1 file changed, 6 insertions(+), 1 deletion(-)\n\ndiff --git a/Documentation/git-tag.txt b/Documentation/git-tag.txt\nindex a74e7b926d..7e7eb9a7e9 100644\n--- a/Documentation/git-tag.txt\n+++ b/Documentation/git-tag.txt\n@@ -10,7 +10,7 @@ SYNOPSIS\n --------\n [verse]\n 'git tag' [-a | -s | -u ] [-f] [-m | -F ] [-e]\n-\t [ | ]\n+\t[--allow-recursive-tag] [ | ]\n 'git tag' -d ...\n 'git tag' [-n[]] -l [--contains ] [--no-contains ]\n \t[--points-at ] [--column[=] | --no-column]\n@@ -193,6 +193,11 @@ This option is only applicable when listing tags without annotation lines.\n \tthat of linkgit:git-for-each-ref[1]. When unspecified,\n \tdefaults to `%(refname:strip=2)`.\n \n+--allow-recursive-tag::\n+\tUsually recursively tagging a tag object is a mistake and the\n+\tcommand prevents you from making such a tag. This option\n+\tbypasses the safety and allows this to happen.\n+\n ::\n \tThe name of the tag to create, delete, or describe.\n \tThe new tag name must pass all checks defined by\n-- \n2.21.0.512.g57bf1b23e1\n\n"},{"id":"372505","messageId":"20190326085147.GA3837@archbookpro.localdomain","threadId":"50796","inReplyTo":"c371a653b4049256f3427e467b144385ee47ef43.1553586707.git.liu.denton@gmail.com","subject":"Re: [PATCH 1/3] tag: prevent recursive tags","fromName":"Denton Liu","fromEmail":"liu.denton@gmail.com","sentAt":"2019-03-26T08:51:47Z","receivedAt":"2019-03-26T08:51:53Z","isPatch":true,"sender":{"key":"liu.denton@gmail.com","avatar":"https://avatars.githubusercontent.com/u/9620836?v=4"},"body":"On Tue, Mar 26, 2019 at 12:53:18AM -0700, Denton Liu wrote:\n> Robert Dailey reported confusion on the mailing list about a recursive\n> tag which was most likely created by mistake. Jeff King noted that this\n> isn't a very common case so, most likely, creating a tag-to-a-tag is a\n> user-error.\n> \n> Prevent mistakes by erroring and providing advice on recursive tags,\n> unless \"--allow-recursive-tag\" is specified. Fix tests that fail as a\n> result of this change.\n> \n> Reported-by: Robert Dailey \n> Helped-by: Jeff King \n> Helped-by: Ævar Arnfjörð Bjarmason \n> Signed-off-by: Denton Liu \n> ---\n> advice.c | 2 ++\n> advice.h | 1 +\n> builtin/tag.c | 30 ++++++++++++++++++++++++++----\n> t/annotate-tests.sh | 2 +-\n> t/t0410-partial-clone.sh | 2 +-\n> t/t4205-log-pretty-formats.sh | 2 +-\n> t/t5305-include-tag.sh | 2 +-\n> t/t5500-fetch-pack.sh | 2 +-\n> t/t6302-for-each-ref-filter.sh | 4 ++--\n> t/t7004-tag.sh | 4 ++--\n> t/t9350-fast-export.sh | 4 ++--\n> 11 files changed, 40 insertions(+), 15 deletions(-)\n> \n> diff --git a/advice.c b/advice.c\n> index 567209aa79..f31889e6de 100644\n> --- a/advice.c\n> +++ b/advice.c\n> @@ -26,6 +26,7 @@ int advice_ignored_hook = 1;\n> int advice_waiting_for_editor = 1;\n> int advice_graft_file_deprecated = 1;\n> int advice_checkout_ambiguous_remote_branch_name = 1;\n> +int advice_recursive_tag = 1;\n> \n> static int advice_use_color = -1;\n> static char advice_colors[][COLOR_MAXLEN] = {\n> @@ -81,6 +82,7 @@ static struct {\n> \t{ \"waitingForEditor\", &advice_waiting_for_editor },\n> \t{ \"graftFileDeprecated\", &advice_graft_file_deprecated },\n> \t{ \"checkoutAmbiguousRemoteBranchName\", &advice_checkout_ambiguous_remote_branch_name },\n> +\t{ \"recursiveTag\", &advice_recursive_tag },\n> \n> \t/* make this an alias for backward compatibility */\n> \t{ \"pushNonFastForward\", &advice_push_update_rejected }\n> diff --git a/advice.h b/advice.h\n> index f875f8cd8d..66aa39757c 100644\n> --- a/advice.h\n> +++ b/advice.h\n> @@ -26,6 +26,7 @@ extern int advice_ignored_hook;\n> extern int advice_waiting_for_editor;\n> extern int advice_graft_file_deprecated;\n> extern int advice_checkout_ambiguous_remote_branch_name;\n> +extern int advice_recursive_tag;\n> \n> int git_default_advice_config(const char *var, const char *value);\n> __attribute__((format (printf, 1, 2)))\n> diff --git a/builtin/tag.c b/builtin/tag.c\n> index 02f6bd1279..0b44a3cbc1 100644\n> --- a/builtin/tag.c\n> +++ b/builtin/tag.c\n> @@ -22,10 +22,11 @@\n> #include \"ref-filter.h\"\n> \n> static const char * const git_tag_usage[] = {\n> -\tN_(\"git tag [-a | -s | -u ] [-f] [-m | -F ] []\"),\n> +\tN_(\"git tag [-a | -s | -u ] [-f] [-m | -F ] [--allow-recursive-tag]\\n\"\n> +\t\t\"\\t\\t []\"),\n> \tN_(\"git tag -d ...\"),\n> -\tN_(\"git tag -l [-n[]] [--contains ] [--no-contains ] [--points-at ]\"\n> -\t\t\"\\n\\t\\t[--format=] [--[no-]merged []] [...]\"),\n> +\tN_(\"git tag -l [-n[]] [--contains ] [--no-contains ] [--points-at ]\\n\"\n> +\t\t\"\\t\\t[--format=] [--[no-]merged []] [...]\"),\n> \tN_(\"git tag -v [--format=] ...\"),\n> \tNULL\n> };\n> @@ -197,6 +198,7 @@ static int build_tag_object(struct strbuf *buf, int sign, struct object_id *resu\n> struct create_tag_options {\n> \tunsigned int message_given:1;\n> \tunsigned int use_editor:1;\n> +\tunsigned int allow_recursive_tag;\n> \tunsigned int sign;\n> \tenum {\n> \t\tCLEANUP_NONE,\n> @@ -205,6 +207,17 @@ struct create_tag_options {\n> \t} cleanup_mode;\n> };\n> \n> +static const char message_advice_recursive_tag[] =\n> +\tN_(\"The object '%s' referred to by your new tag is already a tag.\\n\"\n> +\t \"\\n\"\n> +\t \"If you meant to create a tag of a tag, use:\\n\"\n> +\t \"\\n\"\n> +\t \"\\tgit tag --allow-recursive-tag %s\\n\"\n\nMy bad, left an extra space before the quote.\n\n> +\t \"\\n\"\n> +\t \"If you meant to tag the object that it points to, use:\\n\"\n> +\t \"\\n\"\n> +\t \"\\tgit tag %s^{}\");\n> +\n> static void create_tag(const struct object_id *object, const char *tag,\n> \t\t struct strbuf *buf, struct create_tag_options *opt,\n> \t\t struct object_id *prev, struct object_id *result)\n> @@ -215,7 +228,14 @@ static void create_tag(const struct object_id *object, const char *tag,\n> \n> \ttype = oid_object_info(the_repository, object, NULL);\n> \tif (type <= OBJ_NONE)\n> -\t die(_(\"bad object type.\"));\n> +\t\tdie(_(\"bad object type.\"));\n> +\n> +\tif (type == OBJ_TAG && !opt->allow_recursive_tag) {\n> +\t\terror(_(\"refusing to make a recursive tag\"));\n> +\t\tif (advice_recursive_tag)\n> +\t\t\tadvise(_(message_advice_recursive_tag), tag, tag, tag);\n> +\t\texit(1);\n> +\t}\n> \n> \tstrbuf_addf(&header,\n> \t\t \"object %s\\n\"\n> @@ -403,6 +423,8 @@ int cmd_tag(int argc, const char **argv, const char *prefix)\n> \t\t\t\t\tN_(\"use another key to sign the tag\")),\n> \t\tOPT__FORCE(&force, N_(\"replace the tag if exists\"), 0),\n> \t\tOPT_BOOL(0, \"create-reflog\", &create_reflog, N_(\"create a reflog\")),\n> +\t\tOPT_BOOL(0, \"allow-recursive-tag\", &opt.allow_recursive_tag,\n> +\t\t\t\t\tN_(\"allow recursive tags to be made\")),\n> \n> \t\tOPT_GROUP(N_(\"Tag listing options\")),\n> \t\tOPT_COLUMN(0, \"column\", &colopts, N_(\"show tag list in columns\")),\n> diff --git a/t/annotate-tests.sh b/t/annotate-tests.sh\n> index 6da48a2e0a..841f922e07 100644\n> --- a/t/annotate-tests.sh\n> +++ b/t/annotate-tests.sh\n> @@ -70,7 +70,7 @@ test_expect_success 'blame 1 author' '\n> \n> test_expect_success 'blame by tag objects' '\n> \tgit tag -m \"test tag\" testTag &&\n> -\tgit tag -m \"test tag #2\" testTag2 testTag &&\n> +\tgit tag -m \"test tag #2\" --allow-recursive-tag testTag2 testTag &&\n> \tcheck_count -h testTag A 2 &&\n> \tcheck_count -h testTag2 A 2\n> '\n> diff --git a/t/t0410-partial-clone.sh b/t/t0410-partial-clone.sh\n> index bce02788e6..5f06c2d76f 100755\n> --- a/t/t0410-partial-clone.sh\n> +++ b/t/t0410-partial-clone.sh\n> @@ -16,7 +16,7 @@ pack_as_from_promisor () {\n> \n> promise_and_delete () {\n> \tHASH=$(git -C repo rev-parse \"$1\") &&\n> -\tgit -C repo tag -a -m message my_annotated_tag \"$HASH\" &&\n> +\tgit -C repo tag -a -m message my_annotated_tag --allow-recursive-tag \"$HASH\" &&\n> \tgit -C repo rev-parse my_annotated_tag | pack_as_from_promisor &&\n> \t# tag -d prints a message to stdout, so redirect it\n> \tgit -C repo tag -d my_annotated_tag >/dev/null &&\n> diff --git a/t/t4205-log-pretty-formats.sh b/t/t4205-log-pretty-formats.sh\n> index f42a69faa2..018550f3b2 100755\n> --- a/t/t4205-log-pretty-formats.sh\n> +++ b/t/t4205-log-pretty-formats.sh\n> @@ -511,7 +511,7 @@ test_expect_success 'set up log decoration tests' '\n> \n> test_expect_success 'log decoration properly follows tag chain' '\n> \tgit tag -a tag1 -m tag1 &&\n> -\tgit tag -a tag2 -m tag2 tag1 &&\n> +\tgit tag -a tag2 -m tag2 --allow-recursive-tag tag1 &&\n> \tgit tag -d tag1 &&\n> \tgit commit --amend -m shorter &&\n> \tgit log --no-walk --tags --pretty=\"%H %d\" --decorate=full >actual &&\n> diff --git a/t/t5305-include-tag.sh b/t/t5305-include-tag.sh\n> index a5eca210b8..c99850c1c0 100755\n> --- a/t/t5305-include-tag.sh\n> +++ b/t/t5305-include-tag.sh\n> @@ -68,7 +68,7 @@ test_expect_success 'check unpacked result (have commit, have tag)' '\n> test_expect_success 'create hidden inner tag' '\n> \ttest_commit commit &&\n> \tgit tag -m inner inner HEAD &&\n> -\tgit tag -m outer outer inner &&\n> +\tgit tag -m outer --allow-recursive-tag outer inner &&\n> \tgit tag -d inner\n> '\n> \n> diff --git a/t/t5500-fetch-pack.sh b/t/t5500-fetch-pack.sh\n> index 49c540b1e1..c549b37aec 100755\n> --- a/t/t5500-fetch-pack.sh\n> +++ b/t/t5500-fetch-pack.sh\n> @@ -562,7 +562,7 @@ test_expect_success 'test --all wrt tag to non-commits' '\n> \t\thello tag\n> \tEOF\n> \t) &&\n> -\tgit tag -a -m \"tag -> tag\" tag-to-tag $tag &&\n> +\tgit tag -a -m \"tag -> tag\" --allow-recursive-tag tag-to-tag $tag &&\n> \n> \t# `fetch-pack --all` should succeed fetching all those objects.\n> \tmkdir fetchall &&\n> diff --git a/t/t6302-for-each-ref-filter.sh b/t/t6302-for-each-ref-filter.sh\n> index fc067ed672..f7b56ae195 100755\n> --- a/t/t6302-for-each-ref-filter.sh\n> +++ b/t/t6302-for-each-ref-filter.sh\n> @@ -12,7 +12,7 @@ test_expect_success 'setup some history and refs' '\n> \tgit checkout -b side &&\n> \ttest_commit four &&\n> \tgit tag -m \"An annotated tag\" annotated-tag &&\n> -\tgit tag -m \"Annonated doubly\" doubly-annotated-tag annotated-tag &&\n> +\tgit tag -m \"Annonated doubly\" --allow-recursive-tag doubly-annotated-tag annotated-tag &&\n> \n> \t# Note that these \"signed\" tags might not actually be signed.\n> \t# Tests which care about the distinction should be marked\n> @@ -24,7 +24,7 @@ test_expect_success 'setup some history and refs' '\n> \t\tsign=\n> \tfi &&\n> \tgit tag $sign -m \"A signed tag\" signed-tag &&\n> -\tgit tag $sign -m \"Signed doubly\" doubly-signed-tag signed-tag &&\n> +\tgit tag $sign -m \"Signed doubly\" --allow-recursive-tag doubly-signed-tag signed-tag &&\n> \n> \tgit checkout master &&\n> \tgit update-ref refs/odd/spot master\n> diff --git a/t/t7004-tag.sh b/t/t7004-tag.sh\n> index 0b01862c23..7a7c0ccee9 100755\n> --- a/t/t7004-tag.sh\n> +++ b/t/t7004-tag.sh\n> @@ -1265,7 +1265,7 @@ echo \"A message for another tag\" >>expect\n> echo '-----BEGIN PGP SIGNATURE-----' >>expect\n> test_expect_success GPG \\\n> \t'creating a signed tag pointing to another tag should succeed' '\n> -\tgit tag -s -m \"A message for another tag\" tag-signed-tag signed-tag &&\n> +\tgit tag -s -m \"A message for another tag\" --allow-recursive-tag tag-signed-tag signed-tag &&\n> \tget_tag_msg tag-signed-tag >actual &&\n> \ttest_cmp expect actual\n> '\n> @@ -1690,7 +1690,7 @@ test_expect_success '--points-at finds annotated tags of commits' '\n> '\n> \n> test_expect_success '--points-at finds annotated tags of tags' '\n> -\tgit tag -m \"describing the v4.0 tag object\" \\\n> +\tgit tag -m \"describing the v4.0 tag object\" --allow-recursive-tag \\\n> \t\tannotated-again-v4.0 annotated-v4.0 &&\n> \tcat >expect <<-\\EOF &&\n> \tannotated-again-v4.0\n> diff --git a/t/t9350-fast-export.sh b/t/t9350-fast-export.sh\n> index 5690fe2810..b5ed7e119a 100755\n> --- a/t/t9350-fast-export.sh\n> +++ b/t/t9350-fast-export.sh\n> @@ -441,8 +441,8 @@ test_expect_success 'set-up a few more tags for tag export tests' '\n> \tHEAD_TREE=$(git show -s --pretty=raw HEAD | grep tree | sed \"s/tree //\") &&\n> \tgit tag tree_tag -m \"tagging a tree\" $HEAD_TREE &&\n> \tgit tag -a tree_tag-obj -m \"tagging a tree\" $HEAD_TREE &&\n> -\tgit tag tag-obj_tag -m \"tagging a tag\" tree_tag-obj &&\n> -\tgit tag -a tag-obj_tag-obj -m \"tagging a tag\" tree_tag-obj\n> +\tgit tag tag-obj_tag -m \"tagging a tag\" --allow-recursive-tag tree_tag-obj &&\n> +\tgit tag -a tag-obj_tag-obj -m \"tagging a tag\" --allow-recursive-tag tree_tag-obj\n> '\n> \n> test_expect_success 'tree_tag' '\n> -- \n> 2.21.0.512.g57bf1b23e1\n> \n"},{"id":"372507","messageId":"87imw6aquw.fsf@evledraar.gmail.com","threadId":"50796","inReplyTo":"c371a653b4049256f3427e467b144385ee47ef43.1553586707.git.liu.denton@gmail.com","subject":"Re: [PATCH 1/3] tag: prevent recursive tags","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2019-03-26T10:10:15Z","receivedAt":"2019-03-26T10:10:21Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Tue, Mar 26 2019, Denton Liu wrote:\n\n> static const char * const git_tag_usage[] = {\n> -\tN_(\"git tag [-a | -s | -u ] [-f] [-m | -F ] []\"),\n> +\tN_(\"git tag [-a | -s | -u ] [-f] [-m | -F ] [--allow-recursive-tag]\\n\"\n> +\t\t\"\\t\\t []\"),\n> \tN_(\"git tag -d ...\"),\n> -\tN_(\"git tag -l [-n[]] [--contains ] [--no-contains ] [--points-at ]\"\n> -\t\t\"\\n\\t\\t[--format=] [--[no-]merged []] [...]\"),\n> +\tN_(\"git tag -l [-n[]] [--contains ] [--no-contains ] [--points-at ]\\n\"\n> +\t\t\"\\t\\t[--format=] [--[no-]merged []] [...]\"),\n> \tN_(\"git tag -v [--format=] ...\"),\n> \tNULL\n\nBetter to split the refactoring part of this into a leading\nchange. I.e. the latter 2 removed/2 added here, and the former could\nalso start with a wrap to 79 chars...\n\n> \ttype = oid_object_info(the_repository, object, NULL);\n> \tif (type <= OBJ_NONE)\n> -\t die(_(\"bad object type.\"));\n> +\t\tdie(_(\"bad object type.\"));\n\nDitto this 4 space -> tab change.\n\n> +\n> +\tif (type == OBJ_TAG && !opt->allow_recursive_tag) {\n> +\t\terror(_(\"refusing to make a recursive tag\"));\n> +\t\tif (advice_recursive_tag)\n> +\t\t\tadvise(_(message_advice_recursive_tag), tag, tag, tag);\n> +\t\texit(1);\n> +\t}\n\nThis patch of mine didn't end up making it in, but shows how instead of\n\"tag, tag, tag\" here you can just pass it once and use positional printf\narguments in the message. It makes things a lot more self-explanatory:\nhttps://public-inbox.org/git/20181026192734.9609-8-avarab@gmail.com/\n\n>\n> \tstrbuf_addf(&header,\n> \t\t \"object %s\\n\"\n> @@ -403,6 +423,8 @@ int cmd_tag(int argc, const char **argv, const char *prefix)\n> \t\t\t\t\tN_(\"use another key to sign the tag\")),\n> \t\tOPT__FORCE(&force, N_(\"replace the tag if exists\"), 0),\n> \t\tOPT_BOOL(0, \"create-reflog\", &create_reflog, N_(\"create a reflog\")),\n> +\t\tOPT_BOOL(0, \"allow-recursive-tag\", &opt.allow_recursive_tag,\n> +\t\t\t\t\tN_(\"allow recursive tags to be made\")),\n>\n> \t\tOPT_GROUP(N_(\"Tag listing options\")),\n> \t\tOPT_COLUMN(0, \"column\", &colopts, N_(\"show tag list in columns\")),\n> diff --git a/t/annotate-tests.sh b/t/annotate-tests.sh\n> index 6da48a2e0a..841f922e07 100644\n> --- a/t/annotate-tests.sh\n> +++ b/t/annotate-tests.sh\n> @@ -70,7 +70,7 @@ test_expect_success 'blame 1 author' '\n>\n> test_expect_success 'blame by tag objects' '\n> \tgit tag -m \"test tag\" testTag &&\n> -\tgit tag -m \"test tag #2\" testTag2 testTag &&\n> +\tgit tag -m \"test tag #2\" --allow-recursive-tag testTag2 testTag &&\n> \tcheck_count -h testTag A 2 &&\n> \tcheck_count -h testTag2 A 2\n> '\n> diff --git a/t/t0410-partial-clone.sh b/t/t0410-partial-clone.sh\n> index bce02788e6..5f06c2d76f 100755\n> --- a/t/t0410-partial-clone.sh\n> +++ b/t/t0410-partial-clone.sh\n> @@ -16,7 +16,7 @@ pack_as_from_promisor () {\n>\n> promise_and_delete () {\n> \tHASH=$(git -C repo rev-parse \"$1\") &&\n> -\tgit -C repo tag -a -m message my_annotated_tag \"$HASH\" &&\n> +\tgit -C repo tag -a -m message my_annotated_tag --allow-recursive-tag \"$HASH\" &&\n> \tgit -C repo rev-parse my_annotated_tag | pack_as_from_promisor &&\n> \t# tag -d prints a message to stdout, so redirect it\n> \tgit -C repo tag -d my_annotated_tag >/dev/null &&\n> diff --git a/t/t4205-log-pretty-formats.sh b/t/t4205-log-pretty-formats.sh\n> index f42a69faa2..018550f3b2 100755\n> --- a/t/t4205-log-pretty-formats.sh\n> +++ b/t/t4205-log-pretty-formats.sh\n> @@ -511,7 +511,7 @@ test_expect_success 'set up log decoration tests' '\n>\n> test_expect_success 'log decoration properly follows tag chain' '\n> \tgit tag -a tag1 -m tag1 &&\n> -\tgit tag -a tag2 -m tag2 tag1 &&\n> +\tgit tag -a tag2 -m tag2 --allow-recursive-tag tag1 &&\n> \tgit tag -d tag1 &&\n> \tgit commit --amend -m shorter &&\n> \tgit log --no-walk --tags --pretty=\"%H %d\" --decorate=full >actual &&\n> diff --git a/t/t5305-include-tag.sh b/t/t5305-include-tag.sh\n> index a5eca210b8..c99850c1c0 100755\n> --- a/t/t5305-include-tag.sh\n> +++ b/t/t5305-include-tag.sh\n> @@ -68,7 +68,7 @@ test_expect_success 'check unpacked result (have commit, have tag)' '\n> test_expect_success 'create hidden inner tag' '\n> \ttest_commit commit &&\n> \tgit tag -m inner inner HEAD &&\n> -\tgit tag -m outer outer inner &&\n> +\tgit tag -m outer --allow-recursive-tag outer inner &&\n> \tgit tag -d inner\n> '\n>\n> diff --git a/t/t5500-fetch-pack.sh b/t/t5500-fetch-pack.sh\n> index 49c540b1e1..c549b37aec 100755\n> --- a/t/t5500-fetch-pack.sh\n> +++ b/t/t5500-fetch-pack.sh\n> @@ -562,7 +562,7 @@ test_expect_success 'test --all wrt tag to non-commits' '\n> \t\thello tag\n> \tEOF\n> \t) &&\n> -\tgit tag -a -m \"tag -> tag\" tag-to-tag $tag &&\n> +\tgit tag -a -m \"tag -> tag\" --allow-recursive-tag tag-to-tag $tag &&\n>\n> \t# `fetch-pack --all` should succeed fetching all those objects.\n> \tmkdir fetchall &&\n> diff --git a/t/t6302-for-each-ref-filter.sh b/t/t6302-for-each-ref-filter.sh\n> index fc067ed672..f7b56ae195 100755\n> --- a/t/t6302-for-each-ref-filter.sh\n> +++ b/t/t6302-for-each-ref-filter.sh\n> @@ -12,7 +12,7 @@ test_expect_success 'setup some history and refs' '\n> \tgit checkout -b side &&\n> \ttest_commit four &&\n> \tgit tag -m \"An annotated tag\" annotated-tag &&\n> -\tgit tag -m \"Annonated doubly\" doubly-annotated-tag annotated-tag &&\n> +\tgit tag -m \"Annonated doubly\" --allow-recursive-tag doubly-annotated-tag annotated-tag &&\n>\n> \t# Note that these \"signed\" tags might not actually be signed.\n> \t# Tests which care about the distinction should be marked\n> @@ -24,7 +24,7 @@ test_expect_success 'setup some history and refs' '\n> \t\tsign=\n> \tfi &&\n> \tgit tag $sign -m \"A signed tag\" signed-tag &&\n> -\tgit tag $sign -m \"Signed doubly\" doubly-signed-tag signed-tag &&\n> +\tgit tag $sign -m \"Signed doubly\" --allow-recursive-tag doubly-signed-tag signed-tag &&\n>\n> \tgit checkout master &&\n> \tgit update-ref refs/odd/spot master\n> diff --git a/t/t7004-tag.sh b/t/t7004-tag.sh\n> index 0b01862c23..7a7c0ccee9 100755\n> --- a/t/t7004-tag.sh\n> +++ b/t/t7004-tag.sh\n> @@ -1265,7 +1265,7 @@ echo \"A message for another tag\" >>expect\n> echo '-----BEGIN PGP SIGNATURE-----' >>expect\n> test_expect_success GPG \\\n> \t'creating a signed tag pointing to another tag should succeed' '\n> -\tgit tag -s -m \"A message for another tag\" tag-signed-tag signed-tag &&\n> +\tgit tag -s -m \"A message for another tag\" --allow-recursive-tag tag-signed-tag signed-tag &&\n> \tget_tag_msg tag-signed-tag >actual &&\n> \ttest_cmp expect actual\n> '\n> @@ -1690,7 +1690,7 @@ test_expect_success '--points-at finds annotated tags of commits' '\n> '\n>\n> test_expect_success '--points-at finds annotated tags of tags' '\n> -\tgit tag -m \"describing the v4.0 tag object\" \\\n> +\tgit tag -m \"describing the v4.0 tag object\" --allow-recursive-tag \\\n> \t\tannotated-again-v4.0 annotated-v4.0 &&\n> \tcat >expect <<-\\EOF &&\n> \tannotated-again-v4.0\n> diff --git a/t/t9350-fast-export.sh b/t/t9350-fast-export.sh\n> index 5690fe2810..b5ed7e119a 100755\n> --- a/t/t9350-fast-export.sh\n> +++ b/t/t9350-fast-export.sh\n> @@ -441,8 +441,8 @@ test_expect_success 'set-up a few more tags for tag export tests' '\n> \tHEAD_TREE=$(git show -s --pretty=raw HEAD | grep tree | sed \"s/tree //\") &&\n> \tgit tag tree_tag -m \"tagging a tree\" $HEAD_TREE &&\n> \tgit tag -a tree_tag-obj -m \"tagging a tree\" $HEAD_TREE &&\n> -\tgit tag tag-obj_tag -m \"tagging a tag\" tree_tag-obj &&\n> -\tgit tag -a tag-obj_tag-obj -m \"tagging a tag\" tree_tag-obj\n> +\tgit tag tag-obj_tag -m \"tagging a tag\" --allow-recursive-tag tree_tag-obj &&\n> +\tgit tag -a tag-obj_tag-obj -m \"tagging a tag\" --allow-recursive-tag tree_tag-obj\n> '\n>\n> test_expect_success 'tree_tag' '\n\nAt least some of these tests should change the existing test line to a\n\"test_must_fail\", i.e. assert that without the new option these aren't\ncreated, maybe for good measure test --force too.\n"},{"id":"372508","messageId":"87h8bqaqt4.fsf@evledraar.gmail.com","threadId":"50796","inReplyTo":"d545bed9d3686b41ba67cfb25e00bb5a8b61c77b.1553586707.git.liu.denton@gmail.com","subject":"Re: [PATCH 2/3] t7004: ensure recursive tag behavior is working","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2019-03-26T10:11:19Z","receivedAt":"2019-03-26T10:11:24Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Tue, Mar 26 2019, Denton Liu wrote:\n\n> Add tests to ensure that recursive tags are disallowed unless the\n> \"--allow-recursive-tag\" option is provided.\n>\n> Signed-off-by: Denton Liu \n> ---\n> t/t7004-tag.sh | 8 ++++++++\n> 1 file changed, 8 insertions(+)\n>\n> diff --git a/t/t7004-tag.sh b/t/t7004-tag.sh\n> index 7a7c0ccee9..5297da952d 100755\n> --- a/t/t7004-tag.sh\n> +++ b/t/t7004-tag.sh\n> @@ -1700,6 +1700,14 @@ test_expect_success '--points-at finds annotated tags of tags' '\n> \ttest_cmp expect actual\n> '\n>\n> +test_expect_success 'recursive tagging should fail without --allow-recursive-tag' '\n> +\ttest_must_fail git tag -m recursive recursive annotated-v4.0\n> +'\n> +\n> +test_expect_success 'recursive tagging should pass with --allow-recursive-tag' '\n> +\tgit tag --allow-recursive-tag -m recursive recursive annotated-v4.0\n> +'\n> +\n> test_expect_success 'multiple --points-at are OR-ed together' '\n> \tcat >expect <<-\\EOF &&\n> \tv2.0\n\nMmm, I see part of my feedback on 1/3 is addressed here. IMO better just\nto squash this into that, or at least say in the commit message \"this\ncommit leaves us with a test blind spot for XYZ reason, but it will be\nadded back...\".\n"},{"id":"372510","messageId":"87ftraaqjr.fsf@evledraar.gmail.com","threadId":"50796","inReplyTo":"fe503ddc8f01f3c0c598b00da22c201a7f16ba7f.1553586707.git.liu.denton@gmail.com","subject":"Re: [PATCH 3/3] git-tag.txt: document --allow-recursive-tag option","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2019-03-26T10:16:56Z","receivedAt":"2019-03-26T10:17:01Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Tue, Mar 26 2019, Denton Liu wrote:\n\n> Signed-off-by: Denton Liu \n> ---\n> Documentation/git-tag.txt | 7 ++++++-\n> 1 file changed, 6 insertions(+), 1 deletion(-)\n>\n> diff --git a/Documentation/git-tag.txt b/Documentation/git-tag.txt\n> index a74e7b926d..7e7eb9a7e9 100644\n> --- a/Documentation/git-tag.txt\n> +++ b/Documentation/git-tag.txt\n> @@ -10,7 +10,7 @@ SYNOPSIS\n> --------\n> [verse]\n> 'git tag' [-a | -s | -u ] [-f] [-m | -F ] [-e]\n> -\t [ | ]\n> +\t[--allow-recursive-tag] [ | ]\n> 'git tag' -d ...\n> 'git tag' [-n[]] -l [--contains ] [--no-contains ]\n> \t[--points-at ] [--column[=] | --no-column]\n> @@ -193,6 +193,11 @@ This option is only applicable when listing tags without annotation lines.\n> \tthat of linkgit:git-for-each-ref[1]. When unspecified,\n> \tdefaults to `%(refname:strip=2)`.\n>\n> +--allow-recursive-tag::\n> +\tUsually recursively tagging a tag object is a mistake and the\n> +\tcommand prevents you from making such a tag. This option\n> +\tbypasses the safety and allows this to happen.\n> +\n> ::\n> \tThe name of the tag to create, delete, or describe.\n> \tThe new tag name must pass all checks defined by\n\nLike 1/3 I this should just be squashed into the one patch. No reason to\nleave the doc change in another change.\n\nI see the existing --allow-unrelated-histories uses a similar tone, but\nmaybe something closer to noting that there's nothing wrong with doing\nthis, and maybe even point out the signed \"chain of custody\" tagging as\none use-case, then finally noting that since people usually don't really\nwant this it's disabled by default.\n\nFinally, since \"git tag\" gets used by scripts a lot in practice, maybe\nnote that this behavior changed in \"Git version 2.22.0\".\n"},{"id":"372528","messageId":"20190326161818.GA29627@sigill.intra.peff.net","threadId":"50796","inReplyTo":"cover.1553586707.git.liu.denton@gmail.com","subject":"Re: [PATCH 0/3] tag: prevent recursive tags","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2019-03-26T16:18:18Z","receivedAt":"2019-03-26T16:18:27Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Mar 26, 2019 at 12:53:14AM -0700, Denton Liu wrote:\n\n> Peff said:\n> > Yeah, that's probably a good idea. Now we just need somebody to write\n> > the patch...\n> \n> Hey would you look at that, somebody wrote the patch!\n\nThe system works. :)\n\n> Earlier in the mailing list[1], Robert Dailey reported confusion over\n> some recursive tags.\n> \n> Peff noted that he hasn't seen a tag-to-a-tag in the wild so in most\n> cases, it'd probably be a mistake on the part of a user. He also\n> suggested we error out on a recursive tag unless \"--allow-recursive-tag\"\n> is provided.\n> \n> This patchset implements those suggestions.\n\nThanks. I agree with all of the comments Ævar left, but other than that\nthis looks pretty good to me.\n\nThe only hesitation I'd have is that turning this case into a hard error\n(rather than just an informative warning) may be too sudden for some\npeople's tastes. I'm on the fence myself; I'll be curious what Junio\nthinks when he gets back.\n\n-Peff\n"},{"id":"372556","messageId":"CABPp-BGKWxJVGfQC3imj8Z6RHxQ6zLOGRpkOjyn93a3RxmE0Lw@mail.gmail.com","threadId":"50796","inReplyTo":"c371a653b4049256f3427e467b144385ee47ef43.1553586707.git.liu.denton@gmail.com","subject":"Re: [PATCH 1/3] tag: prevent recursive tags","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2019-03-27T04:57:06Z","receivedAt":"2019-03-27T04:57:21Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"On Tue, Mar 26, 2019 at 12:56 AM Denton Liu wrote:\n>\n> Robert Dailey reported confusion on the mailing list about a recursive\n> tag which was most likely created by mistake. Jeff King noted that this\n> isn't a very common case so, most likely, creating a tag-to-a-tag is a\n> user-error.\n>\n> Prevent mistakes by erroring and providing advice on recursive tags,\n> unless \"--allow-recursive-tag\" is specified. Fix tests that fail as a\n> result of this change.\n\nAny chance we could use the term \"nested tag\" instead of \"recursive tag\"?\n"},{"id":"372567","messageId":"875zs4boib.fsf@evledraar.gmail.com","threadId":"50796","inReplyTo":"CABPp-BGKWxJVGfQC3imj8Z6RHxQ6zLOGRpkOjyn93a3RxmE0Lw@mail.gmail.com","subject":"Re: [PATCH 1/3] tag: prevent recursive tags","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2019-03-27T10:27:56Z","receivedAt":"2019-03-27T10:28:01Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Wed, Mar 27 2019, Elijah Newren wrote:\n\n> On Tue, Mar 26, 2019 at 12:56 AM Denton Liu wrote:\n>>\n>> Robert Dailey reported confusion on the mailing list about a recursive\n>> tag which was most likely created by mistake. Jeff King noted that this\n>> isn't a very common case so, most likely, creating a tag-to-a-tag is a\n>> user-error.\n>>\n>> Prevent mistakes by erroring and providing advice on recursive tags,\n>> unless \"--allow-recursive-tag\" is specified. Fix tests that fail as a\n>> result of this change.\n>\n> Any chance we could use the term \"nested tag\" instead of \"recursive tag\"?\n\n+1. \"Recursive\" sounded wrong to me, but I couldn't think of the\nnow-obvious alternative.\n\nSome grepping around shows we use \"nested submodules\" fairly\nconsistently, and in gitrevisions(7) we say the peel syntax will\nrecursively peel tags (but don't call them nested).\n\nSo makes sense to refer to the object type as nested, and when we're\nreferring to the operation that'll iterate over that nested structure\nsay it'll be done \"recursively\".\n"},{"id":"372643","messageId":"CAHd499AJzEL27fNUHa9WXquaOFzXYuPLCpDOx4xvPF_CfRQwLA@mail.gmail.com","threadId":"50796","inReplyTo":"875zs4boib.fsf@evledraar.gmail.com","subject":"Re: [PATCH 1/3] tag: prevent recursive tags","fromName":"Robert Dailey","fromEmail":"rcdailey.lists@gmail.com","sentAt":"2019-03-28T19:02:59Z","receivedAt":"2019-03-28T19:03:15Z","isPatch":true,"sender":{"key":"rcdailey.lists@gmail.com","avatar":null},"body":"On Wed, Mar 27, 2019 at 5:27 AM Ævar Arnfjörð Bjarmason\n wrote:\n>\n>\n> On Wed, Mar 27 2019, Elijah Newren wrote:\n>\n> > On Tue, Mar 26, 2019 at 12:56 AM Denton Liu wrote:\n> >>\n> >> Robert Dailey reported confusion on the mailing list about a recursive\n> >> tag which was most likely created by mistake. Jeff King noted that this\n> >> isn't a very common case so, most likely, creating a tag-to-a-tag is a\n> >> user-error.\n> >>\n> >> Prevent mistakes by erroring and providing advice on recursive tags,\n> >> unless \"--allow-recursive-tag\" is specified. Fix tests that fail as a\n> >> result of this change.\n> >\n> > Any chance we could use the term \"nested tag\" instead of \"recursive tag\"?\n>\n> +1. \"Recursive\" sounded wrong to me, but I couldn't think of the\n> now-obvious alternative.\n>\n> Some grepping around shows we use \"nested submodules\" fairly\n> consistently, and in gitrevisions(7) we say the peel syntax will\n> recursively peel tags (but don't call them nested).\n>\n> So makes sense to refer to the object type as nested, and when we're\n> referring to the operation that'll iterate over that nested structure\n> say it'll be done \"recursively\".\n\nWow thanks for fixing this, you guys are awesome. I wasn't expecting\nanyone to fix this since it seemed kinda niche. You're right that I\ncreated this nested tag unintentionally. And due to `git-lfs migrate`\nnot handling nested annotated tags, it took days of debugging to\neventually figure out that nothing was working because of 1 farkled\ntag.\n\nJust to make sure I understand the change, is a tag pointing to\nanother tag object now going to be forbidden by default? And to allow\nit, you must do `git tag --allow-nested-tag`?\n\nSo for example, going forward, if we have this:\n\n$ git tag -m 'Tag 1' 1.0\n$ git tag -m 'Tag 2' 2.0\n\nThis will fail:\n\n$ git tag -f 2.0 1.0\n\nUnless I do either:\n\n$ git tag --allow-nested-tag -f 2.0 1.0\n\nOr (this is the intended behavior in my case):\n\n$ git tag -f 2.0 1.0^{}\n"},{"id":"372966","messageId":"cover.1554183429.git.liu.denton@gmail.com","threadId":"50796","inReplyTo":"cover.1553586707.git.liu.denton@gmail.com","subject":"[PATCH v2 0/2] tag: prevent nested tags","fromName":"Denton Liu","fromEmail":"liu.denton@gmail.com","sentAt":"2019-04-02T05:38:37Z","receivedAt":"2019-04-02T05:38:43Z","isPatch":true,"sender":{"key":"liu.denton@gmail.com","avatar":"https://avatars.githubusercontent.com/u/9620836?v=4"},"body":"Earlier in the mailing list, Robert Dailey reported confusion over some\nnested tags. [1]\n\nPeff noted that he hasn't seen a tag-to-a-tag in the wild so in most cases,\nit'd probably be a mistake on the part of a user. He also suggested we error\nout on a nested tag unless \"--allow-nested-tag\" is provided. [2]\n\nThis patchset implements those suggestions.\n\nChanges since v1:\n\n* Squashed patches into two patches, one to clean up tag.c, one to do the rest\n* s/recursive/nested/g (so the new option is --allow-nested-tag)\n* Add more details to --allow-nested-tag documentation\n\n[1]: https://public-inbox.org/git/CAHd499BM91tf7f8=phR4Az8vMsHAHUGYsSb1x9as=WukUVZHJw@mail.gmail.com/\n[2]: https://public-inbox.org/git/20190325144930.GA19929@sigill.intra.peff.net/\n\n\nDenton Liu (2):\n tag: fix formatting\n tag: prevent nested tags\n\n Documentation/config/advice.txt | 2 ++\n Documentation/git-tag.txt | 16 +++++++++++++++-\n advice.c | 2 ++\n advice.h | 1 +\n builtin/tag.c | 30 ++++++++++++++++++++++++++----\n t/annotate-tests.sh | 2 +-\n t/t0410-partial-clone.sh | 2 +-\n t/t4205-log-pretty-formats.sh | 2 +-\n t/t5305-include-tag.sh | 2 +-\n t/t5500-fetch-pack.sh | 2 +-\n t/t6302-for-each-ref-filter.sh | 4 ++--\n t/t7004-tag.sh | 12 ++++++++++--\n t/t9350-fast-export.sh | 4 ++--\n 13 files changed, 65 insertions(+), 16 deletions(-)\n\n-- \n2.21.0.741.g2c528c8f87\n\n"},{"id":"372967","messageId":"c0d39a2829a82e5735b34535cc37ff824ba3a445.1554183429.git.liu.denton@gmail.com","threadId":"50796","inReplyTo":"cover.1554183429.git.liu.denton@gmail.com","subject":"[PATCH v2 1/2] tag: fix formatting","fromName":"Denton Liu","fromEmail":"liu.denton@gmail.com","sentAt":"2019-04-02T05:38:41Z","receivedAt":"2019-04-02T05:38:46Z","isPatch":true,"sender":{"key":"liu.denton@gmail.com","avatar":"https://avatars.githubusercontent.com/u/9620836?v=4"},"body":"Wrap usage line at ''. Also, wrap strings with '\\n' at the end\nof string fragments instead of at the beginning of the next string\nfragment.\n\nConvert a space-indent into a tab-indent for style.\n\nSigned-off-by: Denton Liu \n---\n builtin/tag.c | 9 +++++----\n 1 file changed, 5 insertions(+), 4 deletions(-)\n\ndiff --git a/builtin/tag.c b/builtin/tag.c\nindex 02f6bd1279..faae364e0f 100644\n--- a/builtin/tag.c\n+++ b/builtin/tag.c\n@@ -22,10 +22,11 @@\n #include \"ref-filter.h\"\n \n static const char * const git_tag_usage[] = {\n-\tN_(\"git tag [-a | -s | -u ] [-f] [-m | -F ] []\"),\n+\tN_(\"git tag [-a | -s | -u ] [-f] [-m | -F ]\\n\"\n+\t\t\"\\t\\t []\"),\n \tN_(\"git tag -d ...\"),\n-\tN_(\"git tag -l [-n[]] [--contains ] [--no-contains ] [--points-at ]\"\n-\t\t\"\\n\\t\\t[--format=] [--[no-]merged []] [...]\"),\n+\tN_(\"git tag -l [-n[]] [--contains ] [--no-contains ] [--points-at ]\\n\"\n+\t\t\"\\t\\t[--format=] [--[no-]merged []] [...]\"),\n \tN_(\"git tag -v [--format=] ...\"),\n \tNULL\n };\n@@ -215,7 +216,7 @@ static void create_tag(const struct object_id *object, const char *tag,\n \n \ttype = oid_object_info(the_repository, object, NULL);\n \tif (type <= OBJ_NONE)\n-\t die(_(\"bad object type.\"));\n+\t\tdie(_(\"bad object type.\"));\n \n \tstrbuf_addf(&header,\n \t\t \"object %s\\n\"\n-- \n2.21.0.741.g2c528c8f87\n\n"},{"id":"372968","messageId":"1bd9ee28bc8726490ec0a93286056beeb147fc49.1554183429.git.liu.denton@gmail.com","threadId":"50796","inReplyTo":"cover.1554183429.git.liu.denton@gmail.com","subject":"[PATCH v2 2/2] tag: prevent nested tags","fromName":"Denton Liu","fromEmail":"liu.denton@gmail.com","sentAt":"2019-04-02T05:38:43Z","receivedAt":"2019-04-02T05:38:49Z","isPatch":true,"sender":{"key":"liu.denton@gmail.com","avatar":"https://avatars.githubusercontent.com/u/9620836?v=4"},"body":"Robert Dailey reported confusion on the mailing list about a nested tag\nwhich was most likely created by mistake. Jeff King noted that this\nisn't a very common case so, most likely, creating a tag-to-a-tag is a\nuser-error.\n\nPrevent mistakes by erroring and providing advice on nested tags, unless\n\"--allow-nested-tag\" is specified. Fix tests that fail as a result of\nthis change.\n\nAdd tests to ensure that nested tags are disallowed unless the\n\"--allow-nested-tag\" option is provided.\n\nReported-by: Robert Dailey \nHelped-by: Jeff King \nHelped-by: Ævar Arnfjörð Bjarmason \nSigned-off-by: Denton Liu \n---\n Documentation/config/advice.txt | 2 ++\n Documentation/git-tag.txt | 16 +++++++++++++++-\n advice.c | 2 ++\n advice.h | 1 +\n builtin/tag.c | 23 ++++++++++++++++++++++-\n t/annotate-tests.sh | 2 +-\n t/t0410-partial-clone.sh | 2 +-\n t/t4205-log-pretty-formats.sh | 2 +-\n t/t5305-include-tag.sh | 2 +-\n t/t5500-fetch-pack.sh | 2 +-\n t/t6302-for-each-ref-filter.sh | 4 ++--\n t/t7004-tag.sh | 12 ++++++++++--\n t/t9350-fast-export.sh | 4 ++--\n 13 files changed, 61 insertions(+), 13 deletions(-)\n\ndiff --git a/Documentation/config/advice.txt b/Documentation/config/advice.txt\nindex 88620429ea..ec4f6ae658 100644\n--- a/Documentation/config/advice.txt\n+++ b/Documentation/config/advice.txt\n@@ -90,4 +90,6 @@ advice.*::\n \twaitingForEditor::\n \t\tPrint a message to the terminal whenever Git is waiting for\n \t\teditor input from the user.\n+\tnestedTag::\n+\t\tAdvice shown if a user attempts to recursively tag a tag object.\n --\ndiff --git a/Documentation/git-tag.txt b/Documentation/git-tag.txt\nindex a74e7b926d..e65548b1a0 100644\n--- a/Documentation/git-tag.txt\n+++ b/Documentation/git-tag.txt\n@@ -10,7 +10,7 @@ SYNOPSIS\n --------\n [verse]\n 'git tag' [-a | -s | -u ] [-f] [-m | -F ] [-e]\n-\t [ | ]\n+\t[--allow-nested-tag] [ | ]\n 'git tag' -d ...\n 'git tag' [-n[]] -l [--contains ] [--no-contains ]\n \t[--points-at ] [--column[=] | --no-column]\n@@ -193,6 +193,20 @@ This option is only applicable when listing tags without annotation lines.\n \tthat of linkgit:git-for-each-ref[1]. When unspecified,\n \tdefaults to `%(refname:strip=2)`.\n \n+--allow-nested-tag::\n+\tUsually nestedly tagging a tag object is a mistake and the\n+\tcommand prevents you from making such a tag. This option\n+\tbypasses the safety and allows this to happen.\n++\n+Note that there is nothing logically wrong with nesting tags and, in\n+fact, there may be some valid use-cases, such as showing a cryptographic\n+chain of custody by signing someone else's signed tag. However, in\n+practice, this is typically a mistake so we prevent it from happening by\n+default unless specifically requested.\n++\n+Automatically erroring on nested tags was introduced in Git version\n+2.22.0.\n+\n ::\n \tThe name of the tag to create, delete, or describe.\n \tThe new tag name must pass all checks defined by\ndiff --git a/advice.c b/advice.c\nindex 567209aa79..ce5f374ecd 100644\n--- a/advice.c\n+++ b/advice.c\n@@ -26,6 +26,7 @@ int advice_ignored_hook = 1;\n int advice_waiting_for_editor = 1;\n int advice_graft_file_deprecated = 1;\n int advice_checkout_ambiguous_remote_branch_name = 1;\n+int advice_nested_tag = 1;\n \n static int advice_use_color = -1;\n static char advice_colors[][COLOR_MAXLEN] = {\n@@ -81,6 +82,7 @@ static struct {\n \t{ \"waitingForEditor\", &advice_waiting_for_editor },\n \t{ \"graftFileDeprecated\", &advice_graft_file_deprecated },\n \t{ \"checkoutAmbiguousRemoteBranchName\", &advice_checkout_ambiguous_remote_branch_name },\n+\t{ \"nestedTag\", &advice_nested_tag },\n \n \t/* make this an alias for backward compatibility */\n \t{ \"pushNonFastForward\", &advice_push_update_rejected }\ndiff --git a/advice.h b/advice.h\nindex f875f8cd8d..cb5d361614 100644\n--- a/advice.h\n+++ b/advice.h\n@@ -26,6 +26,7 @@ extern int advice_ignored_hook;\n extern int advice_waiting_for_editor;\n extern int advice_graft_file_deprecated;\n extern int advice_checkout_ambiguous_remote_branch_name;\n+extern int advice_nested_tag;\n \n int git_default_advice_config(const char *var, const char *value);\n __attribute__((format (printf, 1, 2)))\ndiff --git a/builtin/tag.c b/builtin/tag.c\nindex faae364e0f..66da4775b1 100644\n--- a/builtin/tag.c\n+++ b/builtin/tag.c\n@@ -22,7 +22,7 @@\n #include \"ref-filter.h\"\n \n static const char * const git_tag_usage[] = {\n-\tN_(\"git tag [-a | -s | -u ] [-f] [-m | -F ]\\n\"\n+\tN_(\"git tag [-a | -s | -u ] [-f] [-m | -F ] [--allow-nested-tag]\\n\"\n \t\t\"\\t\\t []\"),\n \tN_(\"git tag -d ...\"),\n \tN_(\"git tag -l [-n[]] [--contains ] [--no-contains ] [--points-at ]\\n\"\n@@ -198,6 +198,7 @@ static int build_tag_object(struct strbuf *buf, int sign, struct object_id *resu\n struct create_tag_options {\n \tunsigned int message_given:1;\n \tunsigned int use_editor:1;\n+\tunsigned int allow_nested_tag;\n \tunsigned int sign;\n \tenum {\n \t\tCLEANUP_NONE,\n@@ -206,6 +207,17 @@ struct create_tag_options {\n \t} cleanup_mode;\n };\n \n+static const char message_advice_nested_tag[] =\n+\tN_(\"The object '%s' referred to by your new tag is already a tag.\\n\"\n+\t \"\\n\"\n+\t \"If you meant to create a tag of a tag, use:\\n\"\n+\t \"\\n\"\n+\t \"\\tgit tag --allow-nested-tag %s\\n\"\n+\t \"\\n\"\n+\t \"If you meant to tag the object that it points to, use:\\n\"\n+\t \"\\n\"\n+\t \"\\tgit tag %s^{}\");\n+\n static void create_tag(const struct object_id *object, const char *tag,\n \t\t struct strbuf *buf, struct create_tag_options *opt,\n \t\t struct object_id *prev, struct object_id *result)\n@@ -218,6 +230,13 @@ static void create_tag(const struct object_id *object, const char *tag,\n \tif (type <= OBJ_NONE)\n \t\tdie(_(\"bad object type.\"));\n \n+\tif (type == OBJ_TAG && !opt->allow_nested_tag) {\n+\t\terror(_(\"refusing to make a nested tag\"));\n+\t\tif (advice_nested_tag)\n+\t\t\tadvise(_(message_advice_nested_tag), tag, tag, tag);\n+\t\texit(1);\n+\t}\n+\n \tstrbuf_addf(&header,\n \t\t \"object %s\\n\"\n \t\t \"type %s\\n\"\n@@ -404,6 +423,8 @@ int cmd_tag(int argc, const char **argv, const char *prefix)\n \t\t\t\t\tN_(\"use another key to sign the tag\")),\n \t\tOPT__FORCE(&force, N_(\"replace the tag if exists\"), 0),\n \t\tOPT_BOOL(0, \"create-reflog\", &create_reflog, N_(\"create a reflog\")),\n+\t\tOPT_BOOL(0, \"allow-nested-tag\", &opt.allow_nested_tag,\n+\t\t\t\t\tN_(\"allow nested tags to be made\")),\n \n \t\tOPT_GROUP(N_(\"Tag listing options\")),\n \t\tOPT_COLUMN(0, \"column\", &colopts, N_(\"show tag list in columns\")),\ndiff --git a/t/annotate-tests.sh b/t/annotate-tests.sh\nindex 6da48a2e0a..9849ee30ea 100644\n--- a/t/annotate-tests.sh\n+++ b/t/annotate-tests.sh\n@@ -70,7 +70,7 @@ test_expect_success 'blame 1 author' '\n \n test_expect_success 'blame by tag objects' '\n \tgit tag -m \"test tag\" testTag &&\n-\tgit tag -m \"test tag #2\" testTag2 testTag &&\n+\tgit tag -m \"test tag #2\" --allow-nested-tag testTag2 testTag &&\n \tcheck_count -h testTag A 2 &&\n \tcheck_count -h testTag2 A 2\n '\ndiff --git a/t/t0410-partial-clone.sh b/t/t0410-partial-clone.sh\nindex bce02788e6..00922d4649 100755\n--- a/t/t0410-partial-clone.sh\n+++ b/t/t0410-partial-clone.sh\n@@ -16,7 +16,7 @@ pack_as_from_promisor () {\n \n promise_and_delete () {\n \tHASH=$(git -C repo rev-parse \"$1\") &&\n-\tgit -C repo tag -a -m message my_annotated_tag \"$HASH\" &&\n+\tgit -C repo tag -a -m message my_annotated_tag --allow-nested-tag \"$HASH\" &&\n \tgit -C repo rev-parse my_annotated_tag | pack_as_from_promisor &&\n \t# tag -d prints a message to stdout, so redirect it\n \tgit -C repo tag -d my_annotated_tag >/dev/null &&\ndiff --git a/t/t4205-log-pretty-formats.sh b/t/t4205-log-pretty-formats.sh\nindex f42a69faa2..039f652418 100755\n--- a/t/t4205-log-pretty-formats.sh\n+++ b/t/t4205-log-pretty-formats.sh\n@@ -511,7 +511,7 @@ test_expect_success 'set up log decoration tests' '\n \n test_expect_success 'log decoration properly follows tag chain' '\n \tgit tag -a tag1 -m tag1 &&\n-\tgit tag -a tag2 -m tag2 tag1 &&\n+\tgit tag -a tag2 -m tag2 --allow-nested-tag tag1 &&\n \tgit tag -d tag1 &&\n \tgit commit --amend -m shorter &&\n \tgit log --no-walk --tags --pretty=\"%H %d\" --decorate=full >actual &&\ndiff --git a/t/t5305-include-tag.sh b/t/t5305-include-tag.sh\nindex a5eca210b8..be17bfa9b4 100755\n--- a/t/t5305-include-tag.sh\n+++ b/t/t5305-include-tag.sh\n@@ -68,7 +68,7 @@ test_expect_success 'check unpacked result (have commit, have tag)' '\n test_expect_success 'create hidden inner tag' '\n \ttest_commit commit &&\n \tgit tag -m inner inner HEAD &&\n-\tgit tag -m outer outer inner &&\n+\tgit tag -m outer --allow-nested-tag outer inner &&\n \tgit tag -d inner\n '\n \ndiff --git a/t/t5500-fetch-pack.sh b/t/t5500-fetch-pack.sh\nindex 49c540b1e1..a71ac97a61 100755\n--- a/t/t5500-fetch-pack.sh\n+++ b/t/t5500-fetch-pack.sh\n@@ -562,7 +562,7 @@ test_expect_success 'test --all wrt tag to non-commits' '\n \t\thello tag\n \tEOF\n \t) &&\n-\tgit tag -a -m \"tag -> tag\" tag-to-tag $tag &&\n+\tgit tag -a -m \"tag -> tag\" --allow-nested-tag tag-to-tag $tag &&\n \n \t# `fetch-pack --all` should succeed fetching all those objects.\n \tmkdir fetchall &&\ndiff --git a/t/t6302-for-each-ref-filter.sh b/t/t6302-for-each-ref-filter.sh\nindex fc067ed672..5eed5da6d2 100755\n--- a/t/t6302-for-each-ref-filter.sh\n+++ b/t/t6302-for-each-ref-filter.sh\n@@ -12,7 +12,7 @@ test_expect_success 'setup some history and refs' '\n \tgit checkout -b side &&\n \ttest_commit four &&\n \tgit tag -m \"An annotated tag\" annotated-tag &&\n-\tgit tag -m \"Annonated doubly\" doubly-annotated-tag annotated-tag &&\n+\tgit tag -m \"Annonated doubly\" --allow-nested-tag doubly-annotated-tag annotated-tag &&\n \n \t# Note that these \"signed\" tags might not actually be signed.\n \t# Tests which care about the distinction should be marked\n@@ -24,7 +24,7 @@ test_expect_success 'setup some history and refs' '\n \t\tsign=\n \tfi &&\n \tgit tag $sign -m \"A signed tag\" signed-tag &&\n-\tgit tag $sign -m \"Signed doubly\" doubly-signed-tag signed-tag &&\n+\tgit tag $sign -m \"Signed doubly\" --allow-nested-tag doubly-signed-tag signed-tag &&\n \n \tgit checkout master &&\n \tgit update-ref refs/odd/spot master\ndiff --git a/t/t7004-tag.sh b/t/t7004-tag.sh\nindex 0b01862c23..d5e705fa1d 100755\n--- a/t/t7004-tag.sh\n+++ b/t/t7004-tag.sh\n@@ -1265,7 +1265,7 @@ echo \"A message for another tag\" >>expect\n echo '-----BEGIN PGP SIGNATURE-----' >>expect\n test_expect_success GPG \\\n \t'creating a signed tag pointing to another tag should succeed' '\n-\tgit tag -s -m \"A message for another tag\" tag-signed-tag signed-tag &&\n+\tgit tag -s -m \"A message for another tag\" --allow-nested-tag tag-signed-tag signed-tag &&\n \tget_tag_msg tag-signed-tag >actual &&\n \ttest_cmp expect actual\n '\n@@ -1690,7 +1690,7 @@ test_expect_success '--points-at finds annotated tags of commits' '\n '\n \n test_expect_success '--points-at finds annotated tags of tags' '\n-\tgit tag -m \"describing the v4.0 tag object\" \\\n+\tgit tag -m \"describing the v4.0 tag object\" --allow-nested-tag \\\n \t\tannotated-again-v4.0 annotated-v4.0 &&\n \tcat >expect <<-\\EOF &&\n \tannotated-again-v4.0\n@@ -1700,6 +1700,14 @@ test_expect_success '--points-at finds annotated tags of tags' '\n \ttest_cmp expect actual\n '\n \n+test_expect_success 'recursive tagging should fail without --allow-nested-tag' '\n+\ttest_must_fail git tag -m nested nested annotated-v4.0\n+'\n+\n+test_expect_success 'recursive tagging should pass with --allow-nested-tag' '\n+\tgit tag --allow-nested-tag -m nested nested annotated-v4.0\n+'\n+\n test_expect_success 'multiple --points-at are OR-ed together' '\n \tcat >expect <<-\\EOF &&\n \tv2.0\ndiff --git a/t/t9350-fast-export.sh b/t/t9350-fast-export.sh\nindex 5690fe2810..3f48d60d7f 100755\n--- a/t/t9350-fast-export.sh\n+++ b/t/t9350-fast-export.sh\n@@ -441,8 +441,8 @@ test_expect_success 'set-up a few more tags for tag export tests' '\n \tHEAD_TREE=$(git show -s --pretty=raw HEAD | grep tree | sed \"s/tree //\") &&\n \tgit tag tree_tag -m \"tagging a tree\" $HEAD_TREE &&\n \tgit tag -a tree_tag-obj -m \"tagging a tree\" $HEAD_TREE &&\n-\tgit tag tag-obj_tag -m \"tagging a tag\" tree_tag-obj &&\n-\tgit tag -a tag-obj_tag-obj -m \"tagging a tag\" tree_tag-obj\n+\tgit tag tag-obj_tag -m \"tagging a tag\" --allow-nested-tag tree_tag-obj &&\n+\tgit tag -a tag-obj_tag-obj -m \"tagging a tag\" --allow-nested-tag tree_tag-obj\n '\n \n test_expect_success 'tree_tag' '\n-- \n2.21.0.741.g2c528c8f87\n\n"},{"id":"372996","messageId":"20190402230345.GA5004@dev-l","threadId":"50796","inReplyTo":"1bd9ee28bc8726490ec0a93286056beeb147fc49.1554183429.git.liu.denton@gmail.com","subject":"[PATCH v2.5 2/2] tag: prevent nested tags","fromName":"Denton Liu","fromEmail":"liu.denton@gmail.com","sentAt":"2019-04-02T23:03:45Z","receivedAt":"2019-04-02T23:03:52Z","isPatch":true,"sender":{"key":"liu.denton@gmail.com","avatar":"https://avatars.githubusercontent.com/u/9620836?v=4"},"body":"Robert Dailey reported confusion on the mailing list about a nested tag\nwhich was most likely created by mistake. Jeff King noted that this\nisn't a very common case so, most likely, creating a tag-to-a-tag is a\nuser-error.\n\nPrevent mistakes by erroring and providing advice on nested tags, unless\n\"--allow-nested-tag\" is specified. Fix tests that fail as a result of\nthis change.\n\nAdd tests to ensure that nested tags are disallowed unless the\n\"--allow-nested-tag\" option is provided.\n\nThis is the first use of the '%n$' style of printf format in the\n*.[ch] files in our codebase, but it's supported by POSIX[1] and there\nare existing uses for it in po/*.po files, so hopefully it won't cause\nany trouble. It's more obvious for translators than having to repeat the\nargument three times because each use of '%s' could potentially have a\ndifferent string but this makes it obvious that they're the same.\n\n[1]: http://pubs.opengroup.org/onlinepubs/7908799/xsh/fprintf.html\n\nReported-by: Robert Dailey \nHelped-by: Jeff King \nHelped-by: Ævar Arnfjörð Bjarmason \nSigned-off-by: Denton Liu \n---\n\nHi all,\n\nI forgot to incorporate Ævar's recommendation of using the '%n$'\nstyle of printf format. This patch now includes that.\n\nThanks,\n\nDenton\n\n Documentation/config/advice.txt | 2 ++\n Documentation/git-tag.txt | 16 +++++++++++++++-\n advice.c | 2 ++\n advice.h | 1 +\n builtin/tag.c | 23 ++++++++++++++++++++++-\n t/annotate-tests.sh | 2 +-\n t/t0410-partial-clone.sh | 2 +-\n t/t4205-log-pretty-formats.sh | 2 +-\n t/t5305-include-tag.sh | 2 +-\n t/t5500-fetch-pack.sh | 2 +-\n t/t6302-for-each-ref-filter.sh | 4 ++--\n t/t7004-tag.sh | 12 ++++++++++--\n t/t9350-fast-export.sh | 4 ++--\n 13 files changed, 61 insertions(+), 13 deletions(-)\n\ndiff --git a/Documentation/config/advice.txt b/Documentation/config/advice.txt\nindex 88620429ea..ec4f6ae658 100644\n--- a/Documentation/config/advice.txt\n+++ b/Documentation/config/advice.txt\n@@ -90,4 +90,6 @@ advice.*::\n \twaitingForEditor::\n \t\tPrint a message to the terminal whenever Git is waiting for\n \t\teditor input from the user.\n+\tnestedTag::\n+\t\tAdvice shown if a user attempts to recursively tag a tag object.\n --\ndiff --git a/Documentation/git-tag.txt b/Documentation/git-tag.txt\nindex a74e7b926d..e65548b1a0 100644\n--- a/Documentation/git-tag.txt\n+++ b/Documentation/git-tag.txt\n@@ -10,7 +10,7 @@ SYNOPSIS\n --------\n [verse]\n 'git tag' [-a | -s | -u ] [-f] [-m | -F ] [-e]\n-\t [ | ]\n+\t[--allow-nested-tag] [ | ]\n 'git tag' -d ...\n 'git tag' [-n[]] -l [--contains ] [--no-contains ]\n \t[--points-at ] [--column[=] | --no-column]\n@@ -193,6 +193,20 @@ This option is only applicable when listing tags without annotation lines.\n \tthat of linkgit:git-for-each-ref[1]. When unspecified,\n \tdefaults to `%(refname:strip=2)`.\n \n+--allow-nested-tag::\n+\tUsually nestedly tagging a tag object is a mistake and the\n+\tcommand prevents you from making such a tag. This option\n+\tbypasses the safety and allows this to happen.\n++\n+Note that there is nothing logically wrong with nesting tags and, in\n+fact, there may be some valid use-cases, such as showing a cryptographic\n+chain of custody by signing someone else's signed tag. However, in\n+practice, this is typically a mistake so we prevent it from happening by\n+default unless specifically requested.\n++\n+Automatically erroring on nested tags was introduced in Git version\n+2.22.0.\n+\n ::\n \tThe name of the tag to create, delete, or describe.\n \tThe new tag name must pass all checks defined by\ndiff --git a/advice.c b/advice.c\nindex 567209aa79..ce5f374ecd 100644\n--- a/advice.c\n+++ b/advice.c\n@@ -26,6 +26,7 @@ int advice_ignored_hook = 1;\n int advice_waiting_for_editor = 1;\n int advice_graft_file_deprecated = 1;\n int advice_checkout_ambiguous_remote_branch_name = 1;\n+int advice_nested_tag = 1;\n \n static int advice_use_color = -1;\n static char advice_colors[][COLOR_MAXLEN] = {\n@@ -81,6 +82,7 @@ static struct {\n \t{ \"waitingForEditor\", &advice_waiting_for_editor },\n \t{ \"graftFileDeprecated\", &advice_graft_file_deprecated },\n \t{ \"checkoutAmbiguousRemoteBranchName\", &advice_checkout_ambiguous_remote_branch_name },\n+\t{ \"nestedTag\", &advice_nested_tag },\n \n \t/* make this an alias for backward compatibility */\n \t{ \"pushNonFastForward\", &advice_push_update_rejected }\ndiff --git a/advice.h b/advice.h\nindex f875f8cd8d..cb5d361614 100644\n--- a/advice.h\n+++ b/advice.h\n@@ -26,6 +26,7 @@ extern int advice_ignored_hook;\n extern int advice_waiting_for_editor;\n extern int advice_graft_file_deprecated;\n extern int advice_checkout_ambiguous_remote_branch_name;\n+extern int advice_nested_tag;\n \n int git_default_advice_config(const char *var, const char *value);\n __attribute__((format (printf, 1, 2)))\ndiff --git a/builtin/tag.c b/builtin/tag.c\nindex faae364e0f..8df31e13f0 100644\n--- a/builtin/tag.c\n+++ b/builtin/tag.c\n@@ -22,7 +22,7 @@\n #include \"ref-filter.h\"\n \n static const char * const git_tag_usage[] = {\n-\tN_(\"git tag [-a | -s | -u ] [-f] [-m | -F ]\\n\"\n+\tN_(\"git tag [-a | -s | -u ] [-f] [-m | -F ] [--allow-nested-tag]\\n\"\n \t\t\"\\t\\t []\"),\n \tN_(\"git tag -d ...\"),\n \tN_(\"git tag -l [-n[]] [--contains ] [--no-contains ] [--points-at ]\\n\"\n@@ -198,6 +198,7 @@ static int build_tag_object(struct strbuf *buf, int sign, struct object_id *resu\n struct create_tag_options {\n \tunsigned int message_given:1;\n \tunsigned int use_editor:1;\n+\tunsigned int allow_nested_tag;\n \tunsigned int sign;\n \tenum {\n \t\tCLEANUP_NONE,\n@@ -206,6 +207,17 @@ struct create_tag_options {\n \t} cleanup_mode;\n };\n \n+static const char message_advice_nested_tag[] =\n+\tN_(\"The object '%1$s' referred to by your new tag is already a tag.\\n\"\n+\t \"\\n\"\n+\t \"If you meant to create a tag of a tag, use:\\n\"\n+\t \"\\n\"\n+\t \"\\tgit tag --allow-nested-tag %1$s\\n\"\n+\t \"\\n\"\n+\t \"If you meant to tag the object that it points to, use:\\n\"\n+\t \"\\n\"\n+\t \"\\tgit tag %1$s^{}\");\n+\n static void create_tag(const struct object_id *object, const char *tag,\n \t\t struct strbuf *buf, struct create_tag_options *opt,\n \t\t struct object_id *prev, struct object_id *result)\n@@ -218,6 +230,13 @@ static void create_tag(const struct object_id *object, const char *tag,\n \tif (type <= OBJ_NONE)\n \t\tdie(_(\"bad object type.\"));\n \n+\tif (type == OBJ_TAG && !opt->allow_nested_tag) {\n+\t\terror(_(\"refusing to make a nested tag\"));\n+\t\tif (advice_nested_tag)\n+\t\t\tadvise(_(message_advice_nested_tag), tag);\n+\t\texit(1);\n+\t}\n+\n \tstrbuf_addf(&header,\n \t\t \"object %s\\n\"\n \t\t \"type %s\\n\"\n@@ -404,6 +423,8 @@ int cmd_tag(int argc, const char **argv, const char *prefix)\n \t\t\t\t\tN_(\"use another key to sign the tag\")),\n \t\tOPT__FORCE(&force, N_(\"replace the tag if exists\"), 0),\n \t\tOPT_BOOL(0, \"create-reflog\", &create_reflog, N_(\"create a reflog\")),\n+\t\tOPT_BOOL(0, \"allow-nested-tag\", &opt.allow_nested_tag,\n+\t\t\t\t\tN_(\"allow nested tags to be made\")),\n \n \t\tOPT_GROUP(N_(\"Tag listing options\")),\n \t\tOPT_COLUMN(0, \"column\", &colopts, N_(\"show tag list in columns\")),\ndiff --git a/t/annotate-tests.sh b/t/annotate-tests.sh\nindex 6da48a2e0a..9849ee30ea 100644\n--- a/t/annotate-tests.sh\n+++ b/t/annotate-tests.sh\n@@ -70,7 +70,7 @@ test_expect_success 'blame 1 author' '\n \n test_expect_success 'blame by tag objects' '\n \tgit tag -m \"test tag\" testTag &&\n-\tgit tag -m \"test tag #2\" testTag2 testTag &&\n+\tgit tag -m \"test tag #2\" --allow-nested-tag testTag2 testTag &&\n \tcheck_count -h testTag A 2 &&\n \tcheck_count -h testTag2 A 2\n '\ndiff --git a/t/t0410-partial-clone.sh b/t/t0410-partial-clone.sh\nindex bce02788e6..00922d4649 100755\n--- a/t/t0410-partial-clone.sh\n+++ b/t/t0410-partial-clone.sh\n@@ -16,7 +16,7 @@ pack_as_from_promisor () {\n \n promise_and_delete () {\n \tHASH=$(git -C repo rev-parse \"$1\") &&\n-\tgit -C repo tag -a -m message my_annotated_tag \"$HASH\" &&\n+\tgit -C repo tag -a -m message my_annotated_tag --allow-nested-tag \"$HASH\" &&\n \tgit -C repo rev-parse my_annotated_tag | pack_as_from_promisor &&\n \t# tag -d prints a message to stdout, so redirect it\n \tgit -C repo tag -d my_annotated_tag >/dev/null &&\ndiff --git a/t/t4205-log-pretty-formats.sh b/t/t4205-log-pretty-formats.sh\nindex f42a69faa2..039f652418 100755\n--- a/t/t4205-log-pretty-formats.sh\n+++ b/t/t4205-log-pretty-formats.sh\n@@ -511,7 +511,7 @@ test_expect_success 'set up log decoration tests' '\n \n test_expect_success 'log decoration properly follows tag chain' '\n \tgit tag -a tag1 -m tag1 &&\n-\tgit tag -a tag2 -m tag2 tag1 &&\n+\tgit tag -a tag2 -m tag2 --allow-nested-tag tag1 &&\n \tgit tag -d tag1 &&\n \tgit commit --amend -m shorter &&\n \tgit log --no-walk --tags --pretty=\"%H %d\" --decorate=full >actual &&\ndiff --git a/t/t5305-include-tag.sh b/t/t5305-include-tag.sh\nindex a5eca210b8..be17bfa9b4 100755\n--- a/t/t5305-include-tag.sh\n+++ b/t/t5305-include-tag.sh\n@@ -68,7 +68,7 @@ test_expect_success 'check unpacked result (have commit, have tag)' '\n test_expect_success 'create hidden inner tag' '\n \ttest_commit commit &&\n \tgit tag -m inner inner HEAD &&\n-\tgit tag -m outer outer inner &&\n+\tgit tag -m outer --allow-nested-tag outer inner &&\n \tgit tag -d inner\n '\n \ndiff --git a/t/t5500-fetch-pack.sh b/t/t5500-fetch-pack.sh\nindex 49c540b1e1..a71ac97a61 100755\n--- a/t/t5500-fetch-pack.sh\n+++ b/t/t5500-fetch-pack.sh\n@@ -562,7 +562,7 @@ test_expect_success 'test --all wrt tag to non-commits' '\n \t\thello tag\n \tEOF\n \t) &&\n-\tgit tag -a -m \"tag -> tag\" tag-to-tag $tag &&\n+\tgit tag -a -m \"tag -> tag\" --allow-nested-tag tag-to-tag $tag &&\n \n \t# `fetch-pack --all` should succeed fetching all those objects.\n \tmkdir fetchall &&\ndiff --git a/t/t6302-for-each-ref-filter.sh b/t/t6302-for-each-ref-filter.sh\nindex fc067ed672..5eed5da6d2 100755\n--- a/t/t6302-for-each-ref-filter.sh\n+++ b/t/t6302-for-each-ref-filter.sh\n@@ -12,7 +12,7 @@ test_expect_success 'setup some history and refs' '\n \tgit checkout -b side &&\n \ttest_commit four &&\n \tgit tag -m \"An annotated tag\" annotated-tag &&\n-\tgit tag -m \"Annonated doubly\" doubly-annotated-tag annotated-tag &&\n+\tgit tag -m \"Annonated doubly\" --allow-nested-tag doubly-annotated-tag annotated-tag &&\n \n \t# Note that these \"signed\" tags might not actually be signed.\n \t# Tests which care about the distinction should be marked\n@@ -24,7 +24,7 @@ test_expect_success 'setup some history and refs' '\n \t\tsign=\n \tfi &&\n \tgit tag $sign -m \"A signed tag\" signed-tag &&\n-\tgit tag $sign -m \"Signed doubly\" doubly-signed-tag signed-tag &&\n+\tgit tag $sign -m \"Signed doubly\" --allow-nested-tag doubly-signed-tag signed-tag &&\n \n \tgit checkout master &&\n \tgit update-ref refs/odd/spot master\ndiff --git a/t/t7004-tag.sh b/t/t7004-tag.sh\nindex 0b01862c23..d5e705fa1d 100755\n--- a/t/t7004-tag.sh\n+++ b/t/t7004-tag.sh\n@@ -1265,7 +1265,7 @@ echo \"A message for another tag\" >>expect\n echo '-----BEGIN PGP SIGNATURE-----' >>expect\n test_expect_success GPG \\\n \t'creating a signed tag pointing to another tag should succeed' '\n-\tgit tag -s -m \"A message for another tag\" tag-signed-tag signed-tag &&\n+\tgit tag -s -m \"A message for another tag\" --allow-nested-tag tag-signed-tag signed-tag &&\n \tget_tag_msg tag-signed-tag >actual &&\n \ttest_cmp expect actual\n '\n@@ -1690,7 +1690,7 @@ test_expect_success '--points-at finds annotated tags of commits' '\n '\n \n test_expect_success '--points-at finds annotated tags of tags' '\n-\tgit tag -m \"describing the v4.0 tag object\" \\\n+\tgit tag -m \"describing the v4.0 tag object\" --allow-nested-tag \\\n \t\tannotated-again-v4.0 annotated-v4.0 &&\n \tcat >expect <<-\\EOF &&\n \tannotated-again-v4.0\n@@ -1700,6 +1700,14 @@ test_expect_success '--points-at finds annotated tags of tags' '\n \ttest_cmp expect actual\n '\n \n+test_expect_success 'recursive tagging should fail without --allow-nested-tag' '\n+\ttest_must_fail git tag -m nested nested annotated-v4.0\n+'\n+\n+test_expect_success 'recursive tagging should pass with --allow-nested-tag' '\n+\tgit tag --allow-nested-tag -m nested nested annotated-v4.0\n+'\n+\n test_expect_success 'multiple --points-at are OR-ed together' '\n \tcat >expect <<-\\EOF &&\n \tv2.0\ndiff --git a/t/t9350-fast-export.sh b/t/t9350-fast-export.sh\nindex 5690fe2810..3f48d60d7f 100755\n--- a/t/t9350-fast-export.sh\n+++ b/t/t9350-fast-export.sh\n@@ -441,8 +441,8 @@ test_expect_success 'set-up a few more tags for tag export tests' '\n \tHEAD_TREE=$(git show -s --pretty=raw HEAD | grep tree | sed \"s/tree //\") &&\n \tgit tag tree_tag -m \"tagging a tree\" $HEAD_TREE &&\n \tgit tag -a tree_tag-obj -m \"tagging a tree\" $HEAD_TREE &&\n-\tgit tag tag-obj_tag -m \"tagging a tag\" tree_tag-obj &&\n-\tgit tag -a tag-obj_tag-obj -m \"tagging a tag\" tree_tag-obj\n+\tgit tag tag-obj_tag -m \"tagging a tag\" --allow-nested-tag tree_tag-obj &&\n+\tgit tag -a tag-obj_tag-obj -m \"tagging a tag\" --allow-nested-tag tree_tag-obj\n '\n \n test_expect_success 'tree_tag' '\n-- \n2.21.0.832.gd5ec0d3bee\n\n"},{"id":"373004","messageId":"xmqqzhp7sfw4.fsf@gitster-ct.c.googlers.com","threadId":"50796","inReplyTo":"20190402230345.GA5004@dev-l","subject":"Re: [PATCH v2.5 2/2] tag: prevent nested tags","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2019-04-03T07:32:27Z","receivedAt":"2019-04-03T07:32:34Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Denton Liu writes:\n\n> This is the first use of the '%n$' style of printf format in the\n> *.[ch] files in our codebase, but it's supported by POSIX[1] and there\n> are existing uses for it in po/*.po files, so hopefully it won't cause\n\nThe latter is a stronger indication that this should be OK than the\nformer ;-) Thanks for digging and noting.\n\n> diff --git a/Documentation/config/advice.txt b/Documentation/config/advice.txt\n> index 88620429ea..ec4f6ae658 100644\n> --- a/Documentation/config/advice.txt\n> +++ b/Documentation/config/advice.txt\n> @@ -90,4 +90,6 @@ advice.*::\n> \twaitingForEditor::\n> \t\tPrint a message to the terminal whenever Git is waiting for\n> \t\teditor input from the user.\n> +\tnestedTag::\n> +\t\tAdvice shown if a user attempts to recursively tag a tag object.\n> --\n\nIn addition to 'advice', we may have to add a configuration to help\nprojects that wants to tag tag objects regularly so that they do not\nhave to keep typing \"--allow-nested-tag\". But that can wait until a\nparticipant of such a project comes forward and makes a case for\ntheir workflow.\n\n> +chain of custody by signing someone else's signed tag. However, in\n> +practice, this is typically a mistake so we prevent it from happening by\n> +default unless specifically requested.\n\nI am not sure if this is so bad, actually. Why do we need to treat\nit as a mistake? When a command that wants a commit is fed a tag\n(either a tag that directly refers to a commit, or a tag that tags\nanother tag that refers to a commit), the command knows how to peel\nso it's not like the user is forced to say \"git log T^{commit}\".\n\nAnd if something that *MUST* take a commit refuses to (or more\nlikely, forges to) peel a tag down to a commit and yields an error,\nI think that is what needs fixing, not the command that creates a\ntag.\n\nSo, I am fairly negative on this change---unless it is made much\nmore clear in the doc and/or in the proposed log message what\npractical downside there are to the end users if we do not stop this\n\"mistake\", that is.\n\n> +Automatically erroring on nested tags was introduced in Git version\n> +2.22.0.\n\nAnd please do not write something like this. A feature gets in a\nrelease when it is ready, and we may not ship this in 2.22.\n\n\"git tag --help\" the user is running may or may not have the\nparagraph about \"nested tag\", depending on the existence of the\nfeature in the version of Git the user is running, so there is no\nneed to say something like that.\n\nAnd no, I do not buy arguments like \"random web servers serve\ndifferent versions of documentation without identifying which\nversion of Git they describe\".\n\n"},{"id":"373010","messageId":"xmqq7ecbscay.fsf@gitster-ct.c.googlers.com","threadId":"50796","inReplyTo":"xmqqzhp7sfw4.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH v2.5 2/2] tag: prevent nested tags","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2019-04-03T08:49:57Z","receivedAt":"2019-04-03T08:50:02Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano writes:\n\n> I am not sure if this is so bad, actually. Why do we need to treat\n> it as a mistake? When a command that wants a commit is fed a tag\n> (either a tag that directly refers to a commit, or a tag that tags\n> another tag that refers to a commit), the command knows how to peel\n> so it's not like the user is forced to say \"git log T^{commit}\".\n>\n> And if something that *MUST* take a commit refuses to (or more\n> likely, forges to) peel a tag down to a commit and yields an error,\n> I think that is what needs fixing, not the command that creates a\n> tag.\n>\n> So, I am fairly negative on this change---unless it is made much\n> more clear in the doc and/or in the proposed log message what\n> practical downside there are to the end users if we do not stop this\n> \"mistake\", that is.\n\nHaving said all that, I can sort-of see that it may make sense to\nforbid tagging anything but a commit-ish, either by default, or a\n\"git tag --forbid-no-committish\" that can be turned on with a\nconfiguration.\n"},{"id":"373063","messageId":"3a7a9348-188e-e2c1-58d9-52310b96081d@kdbg.org","threadId":"50796","inReplyTo":"xmqqzhp7sfw4.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH v2.5 2/2] tag: prevent nested tags","fromName":"Johannes Sixt","fromEmail":"j6t@kdbg.org","sentAt":"2019-04-03T18:16:09Z","receivedAt":"2019-04-03T18:16:14Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Am 03.04.19 um 09:32 schrieb Junio C Hamano:\n> Denton Liu writes:\n> \n>> This is the first use of the '%n$' style of printf format in the\n>> *.[ch] files in our codebase, but it's supported by POSIX[1] and there\n>> are existing uses for it in po/*.po files, so hopefully it won't cause\n> \n> The latter is a stronger indication that this should be OK than the\n> former ;-) Thanks for digging and noting.\n\nHowever, there is a difference in whether %n$ is in the code or just in\nthe translations: When it is not in the code, Git can be made to work on\na platform that does not support it by compiling with NO_GETTEXT. When\nit is in the code, that won't be possible anymore.\n\nI don't know whether there are any platforms that do not support %n$,\nthough.\n\n-- Hannes\n"},{"id":"373064","messageId":"CAHd499C9g3yPvs=wuaSuLrW=R09yjT5DcKHfpH9=PYYkAcpuhg@mail.gmail.com","threadId":"50796","inReplyTo":"xmqq7ecbscay.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH v2.5 2/2] tag: prevent nested tags","fromName":"Robert Dailey","fromEmail":"rcdailey.lists@gmail.com","sentAt":"2019-04-03T18:26:51Z","receivedAt":"2019-04-03T18:27:06Z","isPatch":true,"sender":{"key":"rcdailey.lists@gmail.com","avatar":null},"body":"On Wed, Apr 3, 2019 at 3:50 AM Junio C Hamano wrote:\n>\n> Junio C Hamano writes:\n>\n> > I am not sure if this is so bad, actually. Why do we need to treat\n> > it as a mistake? When a command that wants a commit is fed a tag\n> > (either a tag that directly refers to a commit, or a tag that tags\n> > another tag that refers to a commit), the command knows how to peel\n> > so it's not like the user is forced to say \"git log T^{commit}\".\n> >\n> > And if something that *MUST* take a commit refuses to (or more\n> > likely, forges to) peel a tag down to a commit and yields an error,\n> > I think that is what needs fixing, not the command that creates a\n> > tag.\n> >\n> > So, I am fairly negative on this change---unless it is made much\n> > more clear in the doc and/or in the proposed log message what\n> > practical downside there are to the end users if we do not stop this\n> > \"mistake\", that is.\n>\n> Having said all that, I can sort-of see that it may make sense to\n> forbid tagging anything but a commit-ish, either by default, or a\n> \"git tag --forbid-no-committish\" that can be turned on with a\n> configuration.\n\nI don't really understand what part of the change is a negative for\nyou. Are you saying that `git tag` should peel the tags for you? Or\nare you saying you don't like nested tagging to be opt-in and an error\notherwise?\n"},{"id":"373071","messageId":"20190403213318.GA14137@dev-l","threadId":"50796","inReplyTo":"xmqqzhp7sfw4.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH v2.5 2/2] tag: prevent nested tags","fromName":"Denton Liu","fromEmail":"liu.denton@gmail.com","sentAt":"2019-04-03T21:33:18Z","receivedAt":"2019-04-03T21:33:26Z","isPatch":true,"sender":{"key":"liu.denton@gmail.com","avatar":"https://avatars.githubusercontent.com/u/9620836?v=4"},"body":"On Wed, Apr 03, 2019 at 04:32:27PM +0900, Junio C Hamano wrote:\n> Denton Liu writes:\n> \n> > This is the first use of the '%n$' style of printf format in the\n> > *.[ch] files in our codebase, but it's supported by POSIX[1] and there\n> > are existing uses for it in po/*.po files, so hopefully it won't cause\n> \n> The latter is a stronger indication that this should be OK than the\n> former ;-) Thanks for digging and noting.\n\nThank Ævar, I shamelessly stole this message from one of his patches\nthat didn't get included in[1].\n\n> \n> > diff --git a/Documentation/config/advice.txt b/Documentation/config/advice.txt\n> > index 88620429ea..ec4f6ae658 100644\n> > --- a/Documentation/config/advice.txt\n> > +++ b/Documentation/config/advice.txt\n> > @@ -90,4 +90,6 @@ advice.*::\n> > \twaitingForEditor::\n> > \t\tPrint a message to the terminal whenever Git is waiting for\n> > \t\teditor input from the user.\n> > +\tnestedTag::\n> > +\t\tAdvice shown if a user attempts to recursively tag a tag object.\n> > --\n> \n> In addition to 'advice', we may have to add a configuration to help\n> projects that wants to tag tag objects regularly so that they do not\n> have to keep typing \"--allow-nested-tag\". But that can wait until a\n> participant of such a project comes forward and makes a case for\n> their workflow.\n> \n> > +chain of custody by signing someone else's signed tag. However, in\n> > +practice, this is typically a mistake so we prevent it from happening by\n> > +default unless specifically requested.\n> \n> I am not sure if this is so bad, actually. Why do we need to treat\n> it as a mistake? When a command that wants a commit is fed a tag\n> (either a tag that directly refers to a commit, or a tag that tags\n> another tag that refers to a commit), the command knows how to peel\n> so it's not like the user is forced to say \"git log T^{commit}\".\n\nThis patch came about because Robert Dailey expressed confusion after\naccidentally creating a tag-to-a-tag a while back by mistake when he\nactually meant to amend a tag.\n\nIn the discussion upthread, Peff noted that he has never seen a\ntag-to-a-tag in the wild before. I think the conclusion was that for\nthe majority of users, doing this is an error. That is what this patch\nis guarding against.\n\n> \n> And if something that *MUST* take a commit refuses to (or more\n> likely, forges to) peel a tag down to a commit and yields an error,\n> I think that is what needs fixing, not the command that creates a\n> tag.\n> \n> So, I am fairly negative on this change---unless it is made much\n> more clear in the doc and/or in the proposed log message what\n> practical downside there are to the end users if we do not stop this\n> \"mistake\", that is.\n\nI can update the log message to include more detail.\n\n> \n> > +Automatically erroring on nested tags was introduced in Git version\n> > +2.22.0.\n> \n> And please do not write something like this. A feature gets in a\n> release when it is ready, and we may not ship this in 2.22.\n\nÆvar suggested that we include this because git tag gets used by a lot\nof scripts so in case one ever starts failing, a maintainer can more\neasily track down the reason why.\n\n> \n> \"git tag --help\" the user is running may or may not have the\n> paragraph about \"nested tag\", depending on the existence of the\n> feature in the version of Git the user is running, so there is no\n> need to say something like that.\n> \n> And no, I do not buy arguments like \"random web servers serve\n> different versions of documentation without identifying which\n> version of Git they describe\".\n> \n\nThanks for the comments,\n\nDenton\n\n[1]: https://public-inbox.org/git/20181026192734.9609-8-avarab@gmail.com/\n"},{"id":"373088","messageId":"20190404020226.GG4409@sigill.intra.peff.net","threadId":"50796","inReplyTo":"20190403213318.GA14137@dev-l","subject":"Re: [PATCH v2.5 2/2] tag: prevent nested tags","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2019-04-04T02:02:26Z","receivedAt":"2019-04-04T02:02:29Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Apr 03, 2019 at 02:33:18PM -0700, Denton Liu wrote:\n\n> > I am not sure if this is so bad, actually. Why do we need to treat\n> > it as a mistake? When a command that wants a commit is fed a tag\n> > (either a tag that directly refers to a commit, or a tag that tags\n> > another tag that refers to a commit), the command knows how to peel\n> > so it's not like the user is forced to say \"git log T^{commit}\".\n> \n> This patch came about because Robert Dailey expressed confusion after\n> accidentally creating a tag-to-a-tag a while back by mistake when he\n> actually meant to amend a tag.\n> \n> In the discussion upthread, Peff noted that he has never seen a\n> tag-to-a-tag in the wild before. I think the conclusion was that for\n> the majority of users, doing this is an error. That is what this patch\n> is guarding against.\n\nI do still think it is likely to be a mistake. I think Junio's point,\nthough is: who cares if the mistake was made? For the most part you can\ncontinue to use the tag as if the mistake had never been made, because\nGit peels through multiple layers as necessary.\n\nThe only thing that caused the discussion in the first place is that\nwhen \"git show\" displays each layer, there was a little head-scratching.\n\nSo if we were starting from scratch and designing the behavior, I think\nputting a safety valve around this mistake would probably be a\nno-brainer.\n\nBut given that we're changing long-standing behavior (that somebody\n_could_ be using intentionally), is it worth it to fix what is a mostly\nharmless outcome?\n\nI'm on the fence personally. I think I do still lean slightly towards\n\"yes, detect this in the porcelain\", but I can see an argument both\nways.\n\n-Peff\n"},{"id":"373105","messageId":"xmqqftqyf76a.fsf@gitster-ct.c.googlers.com","threadId":"50796","inReplyTo":"20190404020226.GG4409@sigill.intra.peff.net","subject":"Re: [PATCH v2.5 2/2] tag: prevent nested tags","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2019-04-04T09:31:25Z","receivedAt":"2019-04-04T09:31:31Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> I do still think it is likely to be a mistake. I think Junio's point,\n> though is: who cares if the mistake was made? For the most part you can\n> continue to use the tag as if the mistake had never been made, because\n> Git peels through multiple layers as necessary.\n\nNicely said.\n\nIf we forget to peel, that is a bigger problem, but I do not think\nit makes any sense to single out tag-of-tag as \"curious\" and forbid\nit, when we silently allow tag-of-blob or tag-of-tree happily.\n\nAn opt-in (i.e. default to false) tag.allowTaggingOnlyCommits I do\nnot have any problem with, and I could be persuaded into taking an\nopt-out (i.e. default to true) tag.forbidTaggingAnythingButCommits\nconfiguration, perhaps, though.\n"},{"id":"373106","messageId":"xmqqbm1mf74c.fsf@gitster-ct.c.googlers.com","threadId":"50796","inReplyTo":"CAHd499C9g3yPvs=wuaSuLrW=R09yjT5DcKHfpH9=PYYkAcpuhg@mail.gmail.com","subject":"Re: [PATCH v2.5 2/2] tag: prevent nested tags","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2019-04-04T09:32:35Z","receivedAt":"2019-04-04T09:32:40Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Robert Dailey writes:\n\n>> > more clear in the doc and/or in the proposed log message what\n>> > practical downside there are to the end users if we do not stop this\n>> > \"mistake\", that is.\n>>\n>> Having said all that, I can sort-of see that it may make sense to\n>> forbid tagging anything but a commit-ish, either by default, or a\n>> \"git tag --forbid-no-committish\" that can be turned on with a\n>> configuration.\n>\n> I don't really understand what part of the change is a negative for\n> you. Are you saying that `git tag` should peel the tags for you? Or\n> are you saying you don't like nested tagging to be opt-in and an error\n> otherwise?\n\nNo, no and no. I am saying \"git tag -a -m 'message' tag anothertag\"\nthat creates a tag that points at another tag is perfectly fine.\n"},{"id":"373118","messageId":"20190404122722.GA23024@sigill.intra.peff.net","threadId":"50796","inReplyTo":"xmqqftqyf76a.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH v2.5 2/2] tag: prevent nested tags","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2019-04-04T12:27:22Z","receivedAt":"2019-04-04T12:27:25Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Apr 04, 2019 at 06:31:25PM +0900, Junio C Hamano wrote:\n\n> Jeff King writes:\n> \n> > I do still think it is likely to be a mistake. I think Junio's point,\n> > though is: who cares if the mistake was made? For the most part you can\n> > continue to use the tag as if the mistake had never been made, because\n> > Git peels through multiple layers as necessary.\n> \n> Nicely said.\n> \n> If we forget to peel, that is a bigger problem, but I do not think\n> it makes any sense to single out tag-of-tag as \"curious\" and forbid\n> it, when we silently allow tag-of-blob or tag-of-tree happily.\n\nIMHO the difference between those cases is that it is very easy to type\nsomething natural to get a tag of tag, like:\n\n git tag -m foo mytag\n # oops, try again\n git tag -m bar -f mytag mytag\n\nbut you have to take pretty specific steps to get a tag of a blob or\ntree, like:\n\n git tag mytag HEAD:path\n\nor\n\n git tag mytag HEAD^{tree}\n\nUnless you have a ref pointing to a blob or tree in the first place, of\ncourse. But then it is a chicken-and-egg question. Didn't you have to do\nsomething specific to get that tag in the first place?\n\n-Peff\n"},{"id":"373119","messageId":"CAHd499Dr5sjzFCFYvkwcS0WOo0W51_RyL7nLAg_MaGeFy5eQKQ@mail.gmail.com","threadId":"50796","inReplyTo":"xmqqbm1mf74c.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH v2.5 2/2] tag: prevent nested tags","fromName":"Robert Dailey","fromEmail":"rcdailey.lists@gmail.com","sentAt":"2019-04-04T13:47:43Z","receivedAt":"2019-04-04T13:47:59Z","isPatch":true,"sender":{"key":"rcdailey.lists@gmail.com","avatar":null},"body":"On Thu, Apr 4, 2019 at 4:32 AM Junio C Hamano wrote:\n>\n> Robert Dailey writes:\n>\n> >> > more clear in the doc and/or in the proposed log message what\n> >> > practical downside there are to the end users if we do not stop this\n> >> > \"mistake\", that is.\n> >>\n> >> Having said all that, I can sort-of see that it may make sense to\n> >> forbid tagging anything but a commit-ish, either by default, or a\n> >> \"git tag --forbid-no-committish\" that can be turned on with a\n> >> configuration.\n> >\n> > I don't really understand what part of the change is a negative for\n> > you. Are you saying that `git tag` should peel the tags for you? Or\n> > are you saying you don't like nested tagging to be opt-in and an error\n> > otherwise?\n>\n> No, no and no. I am saying \"git tag -a -m 'message' tag anothertag\"\n> that creates a tag that points at another tag is perfectly fine.\n\nIt might be fine within the realm of git itself, because git knows how\nto deal with them by peeling, as you say, but there are 3 reasons I\ndislike that this is allowed:\n\n1. The intent by the user was to create a tag pointing to the commit\nthat another tag points to. So the peeling was expected to occur when\nthe tag was *created*, not when it is used. Again, the use case is\nthat I create a new annotated tag, then realize later I pointed it to\nthe wrong commit. So sometimes I correct it by pointing it at another\ntag, but my intent was for both tags to point at the same commit, not\nfor one tag to point to a commit and the other to point to the tag.\n\n2. When users on my team do a `git show tag`, they see 2 descriptions\nand 2 tags. This creates a LOT of confusion. I wasted hours trying to\nfind out what this meant. It was very obscure and indirect. Most users\ndo not have your expert level of understanding of the internals of\nGit. So I think there's a disconnect between how you like how the\nmachinery works internally with tags, and what users expect from the\nporcelain interface. I think we should go with what's pragmatic (tags\nthat point to commits, not other tags) and design the interfaces for\nthe majority use case. Tags pointing to tags is a minority use case,\nor an edge case. Given that, I think it should be opt-in.\n\n3. Even if Git internally handles peeling tags, external 3rd party\ntooling may not. As I mentioned in another thread, `git lfs migrate`\nwas not programmed to peel tags. I reported the issue here:\n\nhttps://github.com/git-lfs/git-lfs/issues/3568\n\nThe author there admitted that migrating the repository *and* keeping\nthe tags pointing to tags would be difficult. So I think the solution\nthey're going to end up going with is that when you migrate a\nrepository for LFS support, they will permanently peel your annotated\ntags. Which I personally think is the correct solution.\n\nSo in summary, I think it's better for tags to be peeled when they're\ncreated, or at least make the user aware of the fact that they're\ncreating something that they might not be expecting. Just because Git\nhandles peeled tags transparently doesn't mean that's what the user\nwants, or what the user expects. I've been using git for years and I\nnever knew tags pointing to other tags could exist. It sounds like a\ntechnicality that most users shouldn't care about.\n\nThat's my feedback as a user, take it or leave it. I'm not really\nconcerned with what's right or wrong from a git-internals perspective,\nwhat I want is a tool that makes sense for my day to day job, and I\nthink this PR aligns with that.\n"},{"id":"373127","messageId":"20190404182512.GA29737@dev-l","threadId":"50796","inReplyTo":"cover.1554183429.git.liu.denton@gmail.com","subject":"[PATCH v3 0/2] tag: advise on recursive tagging","fromName":"Denton Liu","fromEmail":"liu.denton@gmail.com","sentAt":"2019-04-04T18:25:12Z","receivedAt":"2019-04-04T18:25:16Z","isPatch":true,"sender":{"key":"liu.denton@gmail.com","avatar":"https://avatars.githubusercontent.com/u/9620836?v=4"},"body":"[ Sorry for the spam, I typoed the mailing list address the first time ]\n\nHi all,\n\nI've been following the discussion and the tl;dr of it seems to be this:\nJunio wants to keep the long-standing behaviour of being able to git tag\nanything while Robert seems to believe that for most users, it is a\nmistake and even though Git is able unpeel tags, other tool authors may\nnot be aware that this is even possible and not handle the case.\n\nI've tried to compromise between both sides. Now, we keep the old\nbehaviour but instead of silently ignoring nested tags, we print out a\nhint that the user can act on. Hopefully, this should address everyone's\nconcerns.\n\nThe added benefit is that this won't break any existing scripts' unless\nthey are parsing stderr for whatever reason, but no one would do that,\nright? ;)\n\nDenton Liu (2):\n tag: fix formatting\n tag: advise on nested tags\n\n Documentation/config/advice.txt | 2 ++\n advice.c | 2 ++\n advice.h | 1 +\n builtin/tag.c | 23 +++++++++++++++++------\n t/t7004-tag.sh | 11 +++++++++++\n 5 files changed, 33 insertions(+), 6 deletions(-)\n\n-- \n2.21.0.843.gd0ae0373aa\n\n"},{"id":"373128","messageId":"20190404182513.GA29747@dev-l","threadId":"50796","inReplyTo":"cover.1554183429.git.liu.denton@gmail.com","subject":"[PATCH v3 1/2] tag: fix formatting","fromName":"Denton Liu","fromEmail":"liu.denton@gmail.com","sentAt":"2019-04-04T18:25:13Z","receivedAt":"2019-04-04T18:25:17Z","isPatch":true,"sender":{"key":"liu.denton@gmail.com","avatar":"https://avatars.githubusercontent.com/u/9620836?v=4"},"body":"Wrap usage line at ''. Also, wrap strings with '\\n' at the end\nof string fragments instead of at the beginning of the next string\nfragment.\n\nConvert a space-indent into a tab-indent for style.\n\nSigned-off-by: Denton Liu \n---\n builtin/tag.c | 9 +++++----\n 1 file changed, 5 insertions(+), 4 deletions(-)\n\ndiff --git a/builtin/tag.c b/builtin/tag.c\nindex 02f6bd1279..faae364e0f 100644\n--- a/builtin/tag.c\n+++ b/builtin/tag.c\n@@ -22,10 +22,11 @@\n #include \"ref-filter.h\"\n \n static const char * const git_tag_usage[] = {\n-\tN_(\"git tag [-a | -s | -u ] [-f] [-m | -F ] []\"),\n+\tN_(\"git tag [-a | -s | -u ] [-f] [-m | -F ]\\n\"\n+\t\t\"\\t\\t []\"),\n \tN_(\"git tag -d ...\"),\n-\tN_(\"git tag -l [-n[]] [--contains ] [--no-contains ] [--points-at ]\"\n-\t\t\"\\n\\t\\t[--format=] [--[no-]merged []] [...]\"),\n+\tN_(\"git tag -l [-n[]] [--contains ] [--no-contains ] [--points-at ]\\n\"\n+\t\t\"\\t\\t[--format=] [--[no-]merged []] [...]\"),\n \tN_(\"git tag -v [--format=] ...\"),\n \tNULL\n };\n@@ -215,7 +216,7 @@ static void create_tag(const struct object_id *object, const char *tag,\n \n \ttype = oid_object_info(the_repository, object, NULL);\n \tif (type <= OBJ_NONE)\n-\t die(_(\"bad object type.\"));\n+\t\tdie(_(\"bad object type.\"));\n \n \tstrbuf_addf(&header,\n \t\t \"object %s\\n\"\n-- \n2.21.0.843.gd0ae0373aa\n\n"},{"id":"373129","messageId":"20190404182515.GA29754@dev-l","threadId":"50796","inReplyTo":"cover.1554183429.git.liu.denton@gmail.com","subject":"[PATCH v3 2/2] tag: advise on nested tags","fromName":"Denton Liu","fromEmail":"liu.denton@gmail.com","sentAt":"2019-04-04T18:25:15Z","receivedAt":"2019-04-04T18:25:21Z","isPatch":true,"sender":{"key":"liu.denton@gmail.com","avatar":"https://avatars.githubusercontent.com/u/9620836?v=4"},"body":"Robert Dailey reported confusion on the mailing list about a nested tag\nwhich was most likely created by mistake. Jeff King noted that this\nisn't a very common case so, most likely, creating a tag-to-a-tag is a\nuser-error.\n\nPrevent mistakes by providing advice on nested tags. Add tests to ensure\nthat advice is given for nested tags.\n\nReported-by: Robert Dailey \nHelped-by: Jeff King \nHelped-by: Ævar Arnfjörð Bjarmason \nSigned-off-by: Denton Liu \n---\n\nThe advice message probably needs more work. Also, we currently only\nwarn on nested tags because I agree with what Peff said here[1], but I\ndon't really have any strong opinions on the matter so I'm open to\nchanging it.\n\n[1]: https://public-inbox.org/git/20190404122722.GA23024@sigill.intra.peff.net/\n\n Documentation/config/advice.txt | 2 ++\n advice.c | 2 ++\n advice.h | 1 +\n builtin/tag.c | 14 ++++++++++++--\n t/t7004-tag.sh | 11 +++++++++++\n 5 files changed, 28 insertions(+), 2 deletions(-)\n\ndiff --git a/Documentation/config/advice.txt b/Documentation/config/advice.txt\nindex 88620429ea..ec4f6ae658 100644\n--- a/Documentation/config/advice.txt\n+++ b/Documentation/config/advice.txt\n@@ -90,4 +90,6 @@ advice.*::\n \twaitingForEditor::\n \t\tPrint a message to the terminal whenever Git is waiting for\n \t\teditor input from the user.\n+\tnestedTag::\n+\t\tAdvice shown if a user attempts to recursively tag a tag object.\n --\ndiff --git a/advice.c b/advice.c\nindex 567209aa79..ce5f374ecd 100644\n--- a/advice.c\n+++ b/advice.c\n@@ -26,6 +26,7 @@ int advice_ignored_hook = 1;\n int advice_waiting_for_editor = 1;\n int advice_graft_file_deprecated = 1;\n int advice_checkout_ambiguous_remote_branch_name = 1;\n+int advice_nested_tag = 1;\n \n static int advice_use_color = -1;\n static char advice_colors[][COLOR_MAXLEN] = {\n@@ -81,6 +82,7 @@ static struct {\n \t{ \"waitingForEditor\", &advice_waiting_for_editor },\n \t{ \"graftFileDeprecated\", &advice_graft_file_deprecated },\n \t{ \"checkoutAmbiguousRemoteBranchName\", &advice_checkout_ambiguous_remote_branch_name },\n+\t{ \"nestedTag\", &advice_nested_tag },\n \n \t/* make this an alias for backward compatibility */\n \t{ \"pushNonFastForward\", &advice_push_update_rejected }\ndiff --git a/advice.h b/advice.h\nindex f875f8cd8d..cb5d361614 100644\n--- a/advice.h\n+++ b/advice.h\n@@ -26,6 +26,7 @@ extern int advice_ignored_hook;\n extern int advice_waiting_for_editor;\n extern int advice_graft_file_deprecated;\n extern int advice_checkout_ambiguous_remote_branch_name;\n+extern int advice_nested_tag;\n \n int git_default_advice_config(const char *var, const char *value);\n __attribute__((format (printf, 1, 2)))\ndiff --git a/builtin/tag.c b/builtin/tag.c\nindex faae364e0f..32948fade0 100644\n--- a/builtin/tag.c\n+++ b/builtin/tag.c\n@@ -206,7 +206,14 @@ struct create_tag_options {\n \t} cleanup_mode;\n };\n \n-static void create_tag(const struct object_id *object, const char *tag,\n+static const char message_advice_nested_tag[] =\n+\tN_(\"You have created a nested tag. The object referred to by your new is\\n\"\n+\t \"already a tag. If you meant to tag the object that it points to, use:\\n\"\n+\t \"\\n\"\n+\t \"\\tgit tag -f %s %s^{}\");\n+\n+static void create_tag(const struct object_id *object, const char *object_ref,\n+\t\t const char *tag,\n \t\t struct strbuf *buf, struct create_tag_options *opt,\n \t\t struct object_id *prev, struct object_id *result)\n {\n@@ -218,6 +225,9 @@ static void create_tag(const struct object_id *object, const char *tag,\n \tif (type <= OBJ_NONE)\n \t\tdie(_(\"bad object type.\"));\n \n+\tif (type == OBJ_TAG && advice_nested_tag)\n+\t\tadvise(_(message_advice_nested_tag), tag, object_ref);\n+\n \tstrbuf_addf(&header,\n \t\t \"object %s\\n\"\n \t\t \"type %s\\n\"\n@@ -551,7 +561,7 @@ int cmd_tag(int argc, const char **argv, const char *prefix)\n \tif (create_tag_object) {\n \t\tif (force_sign_annotate && !annotate)\n \t\t\topt.sign = 1;\n-\t\tcreate_tag(&object, tag, &buf, &opt, &prev, &object);\n+\t\tcreate_tag(&object, object_ref, tag, &buf, &opt, &prev, &object);\n \t}\n \n \ttransaction = ref_transaction_begin(&err);\ndiff --git a/t/t7004-tag.sh b/t/t7004-tag.sh\nindex 0b01862c23..34c130ba1c 100755\n--- a/t/t7004-tag.sh\n+++ b/t/t7004-tag.sh\n@@ -1700,6 +1700,17 @@ test_expect_success '--points-at finds annotated tags of tags' '\n \ttest_cmp expect actual\n '\n \n+test_expect_success 'recursive tagging should give advice' '\n+\tcat <<-EOF >expected &&\n+\thint: You have created a nested tag. The object referred to by your new is\n+\thint: already a tag. If you meant to tag the object that it points to, use:\n+\thint: \n+\thint: \tgit tag -f nested annotated-v4.0^{}\n+\tEOF\n+\tgit tag -m nested nested annotated-v4.0 2>actual &&\n+\ttest_i18ncmp expected actual\n+'\n+\n test_expect_success 'multiple --points-at are OR-ed together' '\n \tcat >expect <<-\\EOF &&\n \tv2.0\n-- \n2.21.0.843.gd0ae0373aa\n\n"},{"id":"373133","messageId":"xmqqimvte8yr.fsf@gitster-ct.c.googlers.com","threadId":"50796","inReplyTo":"CAHd499Dr5sjzFCFYvkwcS0WOo0W51_RyL7nLAg_MaGeFy5eQKQ@mail.gmail.com","subject":"Re: [PATCH v2.5 2/2] tag: prevent nested tags","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2019-04-04T21:50:20Z","receivedAt":"2019-04-04T21:50:26Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Robert Dailey writes:\n\n> It might be fine within the realm of git itself, because git knows how\n> to deal with them by peeling, as you say, but there are 3 reasons I\n> dislike that this is allowed:\n\nThat sounds as if you have tools that forgets to peel when it should\nin mind. Isn't that what you should be looking into fixing? After\nall, tools that aim to work with Git should strive to come close to\n\"within the realm of git itsef\" in the modern world.\n\n> 1. The intent by the user was to create a tag pointing to the commit\n> that another tag points to.\n\nYou make it sound as if you are convinced that is the truth. I am\nnot. If I want to tag the commit pointed at by tag v1.0, I'd say I\nwant to tag \"v1.0^0\", because otherwise there won't be a way to say\n\n $ git tag -s -m \"i am aware of this tag\" initial v1.0\n\ni.e. making a tag that points at a tag. So I take the lack of ^0\n(or ^{commit}) peeling an explicit sign that shows the intent by the\nuser (well, those who know the tool, anyway) is clearly to create a\ntag pointing to that tag. In other words, peeling at the tagging\ntime is wrong, and rejecting tag creation is also wrong.\n\n> 2. When users on my team do a `git show tag`, they see 2 descriptions\n> and 2 tags. This creates a LOT of confusion.\n\nSo what? Not everybody will forever stay to be a newbie ;-) \n\nAs I said, an opt-in tag.allowCommitOnly is fine. That would train\ntheir fingers to peel with ^{commit} when they want to tag a commit.\nAn opt-in tag.autoPeelTags might also be fine, even though that\nwould not help training their fingers (so they will have to be\nprepared for the same \"confusion\" on a fresh machine)\n\nWhen the command line clearly tells us that the user wants to tag a\ntag, we should not get overly \"smart\" and refuse to create a tag of\na tag, unless the user tells us otherwise with some means.\n\n> 3. Even if Git internally handles peeling tags, external 3rd party\n> tooling may not. As I mentioned in another thread, `git lfs migrate`\n> was not programmed to peel tags. I reported the issue here:\n\nThat is good, and it is the right way. After all, external 3rd\nparty tooling may tag a tag even after we castrate \"git tag\" to be\nincapable of doing so, so a bug like the one you are helping to fix\nin lfs needs to be fixed anyway. In other words, thats the only\nsensible way forward when you care about the entire Git ecosystem,\nnot just the main/reference implementation we work on here.\n\n"},{"id":"373134","messageId":"xmqqef6he8sh.fsf@gitster-ct.c.googlers.com","threadId":"50796","inReplyTo":"20190404122722.GA23024@sigill.intra.peff.net","subject":"Re: [PATCH v2.5 2/2] tag: prevent nested tags","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2019-04-04T21:54:06Z","receivedAt":"2019-04-04T21:54:12Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> but you have to take pretty specific steps to get a tag of a blob or\n> tree, like:\n>\n> git tag mytag HEAD:path\n>\n> or\n>\n> git tag mytag HEAD^{tree}\n\nAnd the way to ask for commit and tag are\n\n git tag mytag master\n git tag mytag v1.0.0\n\nI do not see why only the last one should either be forbidden or\nautomaticallly peel. Each of these four names an object of a\nspecific type, and singling out a tag as \"curious\" makes the\ninterface uneven.\n\n"},{"id":"373136","messageId":"20190404221208.GA30798@sigill.intra.peff.net","threadId":"50796","inReplyTo":"xmqqef6he8sh.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH v2.5 2/2] tag: prevent nested tags","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2019-04-04T22:12:08Z","receivedAt":"2019-04-04T22:12:13Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Apr 05, 2019 at 06:54:06AM +0900, Junio C Hamano wrote:\n\n> Jeff King writes:\n> \n> > but you have to take pretty specific steps to get a tag of a blob or\n> > tree, like:\n> >\n> > git tag mytag HEAD:path\n> >\n> > or\n> >\n> > git tag mytag HEAD^{tree}\n> \n> And the way to ask for commit and tag are\n> \n> git tag mytag master\n> git tag mytag v1.0.0\n> \n> I do not see why only the last one should either be forbidden or\n> automaticallly peel. Each of these four names an object of a\n> specific type, and singling out a tag as \"curious\" makes the\n> interface uneven.\n\nMy point is that it's easy to accidentally tag a tag. It's not easy to\naccidentally tag a tree. The counter example would be something that\nlooks as easy as your \"git tag mytag v1.0.0\" but actually tags a tree.\nI couldn't think of one.\n\nI do buy the argument that arguing for safety valves from a point of\n\"what's the easiest mistake to make in our current interface\" might not\nbe the best one. It yields a set of operations that aren't necessarily\nsensible or orthogonal with respect to the underlying reality of what's\npossible and what's reasonable. Which I think is the point you're\nmaking.\n\n-Peff\n"},{"id":"373155","messageId":"CABPp-BHBq64-4jO4=rxUsA2WV5keh86wrai87i=oLTzXdcTb=w@mail.gmail.com","threadId":"50796","inReplyTo":"xmqqftqyf76a.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH v2.5 2/2] tag: prevent nested tags","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2019-04-05T00:36:48Z","receivedAt":"2019-04-05T00:37:22Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"On Thu, Apr 4, 2019 at 2:31 AM Junio C Hamano wrote:\n>\n> Jeff King writes:\n>\n> > I do still think it is likely to be a mistake. I think Junio's point,\n> > though is: who cares if the mistake was made? For the most part you can\n> > continue to use the tag as if the mistake had never been made, because\n> > Git peels through multiple layers as necessary.\n>\n> Nicely said.\n>\n> If we forget to peel, that is a bigger problem, but I do not think\n> it makes any sense to single out tag-of-tag as \"curious\" and forbid\n> it, when we silently allow tag-of-blob or tag-of-tree happily.\n>\n> An opt-in (i.e. default to false) tag.allowTaggingOnlyCommits I do\n> not have any problem with, and I could be persuaded into taking an\n> opt-out (i.e. default to true) tag.forbidTaggingAnythingButCommits\n> configuration, perhaps, though.\n\nI'm slightly in favor of the tag.forbidTaggingAnythingButCommits\nroute. Two reasons:\n\n * Even core git commands can't handle these properly after more than\na decade, making me suspect that tools in the greater git ecosystem\nare going to fail to handle them too. In more detail... Some\nexamples: fast-export with --tag-of-filtered-object=rewrite fails on\ntags of tags and tags of blobs. Without that flag, I think\nfast-export munges tags of tags, but maybe that was only under some\nother special case; I don't remember right now. Also, filter-branch\nmunges tags of tags (though maybe that's documented; it may have\ndecided that tags of tags are an error in need of fixing with no flag\nfor users to opt out). Considering core tools that have been part of\ngit for over a decade mishandle tags of anything other than commits\n(or maybe even treat them as erroneous), I don't see why we'd expect\ntools outside of git to handle them correctly. Thus, I think it'd be\nnice if people had to specify some kind of way to state they are sure\nthey want to tag something other than a commit.\n\n * The only two repositories I know of and have access to which has\nsuch tags are linux.git (a tag of a tree) and git.git (a tag of a blob\nand four or so tags of tags). Further, these tags are all pretty old\ntoo. So, I think disallowing the creation of tags of non-commit\nobjects would be unlikely to negatively impact existing users.\n\n\nThough, on the flipside, given how rare these seem to be in practice,\nit might not be worth the effort. Certainly not at the top of my\npriority list.\n\nElijah\n"},{"id":"373171","messageId":"CAHd499Bp0QaZGcWhibWpJZJjMn-QqMBzW3vet1HnuMTTffQBRA@mail.gmail.com","threadId":"50796","inReplyTo":"xmqqimvte8yr.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH v2.5 2/2] tag: prevent nested tags","fromName":"Robert Dailey","fromEmail":"rcdailey.lists@gmail.com","sentAt":"2019-04-05T02:51:26Z","receivedAt":"2019-04-05T02:51:41Z","isPatch":true,"sender":{"key":"rcdailey.lists@gmail.com","avatar":null},"body":"On Thu, Apr 4, 2019 at 4:50 PM Junio C Hamano wrote:\n>\n> Robert Dailey writes:\n>\n> > It might be fine within the realm of git itself, because git knows how\n> > to deal with them by peeling, as you say, but there are 3 reasons I\n> > dislike that this is allowed:\n>\n> That sounds as if you have tools that forgets to peel when it should\n> in mind. Isn't that what you should be looking into fixing? After\n> all, tools that aim to work with Git should strive to come close to\n> \"within the realm of git itsef\" in the modern world.\n>\n> > 1. The intent by the user was to create a tag pointing to the commit\n> > that another tag points to.\n>\n> You make it sound as if you are convinced that is the truth. I am\n> not. If I want to tag the commit pointed at by tag v1.0, I'd say I\n> want to tag \"v1.0^0\", because otherwise there won't be a way to say\n>\n> $ git tag -s -m \"i am aware of this tag\" initial v1.0\n>\n> i.e. making a tag that points at a tag. So I take the lack of ^0\n> (or ^{commit}) peeling an explicit sign that shows the intent by the\n> user (well, those who know the tool, anyway) is clearly to create a\n> tag pointing to that tag. In other words, peeling at the tagging\n> time is wrong, and rejecting tag creation is also wrong.\n>\n> > 2. When users on my team do a `git show tag`, they see 2 descriptions\n> > and 2 tags. This creates a LOT of confusion.\n>\n> So what? Not everybody will forever stay to be a newbie ;-)\n>\n> As I said, an opt-in tag.allowCommitOnly is fine. That would train\n> their fingers to peel with ^{commit} when they want to tag a commit.\n> An opt-in tag.autoPeelTags might also be fine, even though that\n> would not help training their fingers (so they will have to be\n> prepared for the same \"confusion\" on a fresh machine)\n>\n> When the command line clearly tells us that the user wants to tag a\n> tag, we should not get overly \"smart\" and refuse to create a tag of\n> a tag, unless the user tells us otherwise with some means.\n>\n> > 3. Even if Git internally handles peeling tags, external 3rd party\n> > tooling may not. As I mentioned in another thread, `git lfs migrate`\n> > was not programmed to peel tags. I reported the issue here:\n>\n> That is good, and it is the right way. After all, external 3rd\n> party tooling may tag a tag even after we castrate \"git tag\" to be\n> incapable of doing so, so a bug like the one you are helping to fix\n> in lfs needs to be fixed anyway. In other words, thats the only\n> sensible way forward when you care about the entire Git ecosystem,\n> not just the main/reference implementation we work on here.\n>\n\nLook, I'm not saying you're wrong. You're probably absolutely right\nabout all the technical details. You know a lot more about this than I\ndo, that's for sure. But based on my observations and experience,\neverything you're saying isn't set in reality. I have guys on my team\nat work that haven't learned anything significant about Git in the\npast 3 years; they don't care to learn it. They use a GUI tool for\neverything and it's a means to an end for them. Most folks just want\nto check out branches, merge, and push commits. They don't understand\nwhat objects or blobs are, or what a tree vs tag vs commit is. Hell, I\nhave trouble explaining rebase to most people, and that's arguably\nmuch more straightforward. But all of this is OK. We're hired to be\nprogrammers, not experts at Git.\n\nI think what needs to be right is what most people expect, and from my\nexperience that's not in alignment with your opinion on how this\nshould work. And I want to apologize if it seems like I'm arguing with\nyou. My intent is just to clarify my point of thinking. I feel like\nyou're still very wrapped up in the bowels of Git and the deep\ntechnical details. I just want to make sure I'm being clear about my\nperspective, since I'm approaching this from a somewhat non-technical\nangle.\n\nAgain, regardless of what gets decided, I very much appreciate\neveryone looking into this. I feel like whatever you folks come up\nwith will ultimately be the best decision, even if it may not be 100%\nwhat I'm expecting.\n"},{"id":"373181","messageId":"xmqq36mxdnpz.fsf@gitster-ct.c.googlers.com","threadId":"50796","inReplyTo":"CABPp-BHBq64-4jO4=rxUsA2WV5keh86wrai87i=oLTzXdcTb=w@mail.gmail.com","subject":"Re: [PATCH v2.5 2/2] tag: prevent nested tags","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2019-04-05T05:29:12Z","receivedAt":"2019-04-05T05:29:17Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Elijah Newren writes:\n\n> I'm slightly in favor of the tag.forbidTaggingAnythingButCommits\n> route. Two reasons:\n>\n> * Even core git commands can't handle these properly after more than\n> a decade, making me suspect that tools in the greater git ecosystem\n> are going to fail to handle them too. In more detail... Some\n> examples: fast-export with --tag-of-filtered-object=rewrite fails on\n> tags of tags and tags of blobs. Without that flag, I think ...\n\nFair enough.\n\nPersonally, I've never considered import/export tools as part of the\ngit core proper, and it is time like this that I am reminded that I\nhave been right all along---they just do not get attention to the\nsame degree as the truly core tools.\n\nBut as we ship them together with the core part of the system, the\nusers may have been trained to think that tags to tags are not\nsomething they want due to the limitation of these fringe tools.\n"},{"id":"373676","messageId":"20190411184031.GA20265@esm","threadId":"50796","inReplyTo":"20190404122722.GA23024@sigill.intra.peff.net","subject":"Re: [PATCH v2.5 2/2] tag: prevent nested tags","fromName":"Eckhard Maaß","fromEmail":"eckhard.s.maass@googlemail.com","sentAt":"2019-04-11T18:40:31Z","receivedAt":"2019-04-11T18:40:37Z","isPatch":true,"sender":{"key":"eckhard.s.maass@googlemail.com","avatar":"https://avatars.githubusercontent.com/u/21984134?v=4"},"body":"On Thu, Apr 04, 2019 at 08:27:22AM -0400, Jeff King wrote:\n> IMHO the difference between those cases is that it is very easy to type\n> something natural to get a tag of tag, like:\n> \n> git tag -m foo mytag\n> # oops, try again\n> git tag -m bar -f mytag mytag\n\nCould it be that the semantics of `-f` are unintuitive and the real\nculprit? The documentation talks about replacing the tag, which at least\nnaively to me has the sense of \"well, the old tag object gets garbage\ncollected sometimes\". But if the tag is still referenced, the old tag\nhangs around. And this gets quite confusing. For example:\n\ngit tag -m 'Message' -a myTag\ngit tag -m 'Message' -a myOtherTag myTag\ngit tag -m 'Message' -f -a myTag otherBranch\n\nNow a `git show myOtherTag` gives me the old myTag - and labels it as\nmyTag. Copy and pasting this directs me to somewhere completely else,\nthough. Wouldn't it be the more sane approach here to fail, when the old\ntag is (or as in your example will) still be referenced? Or make some\neffort with git show that it is clear that the referenced tag can no\nlonger be referenced with its original name?\n\nGreetings,\nEckhard\n"},{"id":"373707","messageId":"xmqqa7gvzz5c.fsf@gitster-ct.c.googlers.com","threadId":"50796","inReplyTo":"20190411184031.GA20265@esm","subject":"Re: [PATCH v2.5 2/2] tag: prevent nested tags","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2019-04-12T03:21:51Z","receivedAt":"2019-04-12T03:21:55Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Eckhard Maaß\" writes:\n\n> ... Wouldn't it be the more sane approach here to fail, when the old\n> tag is (or as in your example will) still be referenced?\n\nThat is unworkable in the distributed world. There may be somebody\nelse who has a copy of the old tag and you may end up getting that\nlater.\n"}]}