{"thread":{"id":"8408","subject":"[PATCH] Use git-tag in git-cvsimport","startedAt":"2007-06-03T06:56:36Z","lastAt":"2007-06-06T20:41:21Z","messageCount":7,"participants":["Elvis Pranskevichus","Junio C Hamano","Martin Waitz"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"43852","messageId":"11808537962798-git-send-email-el@prans.net","threadId":"8408","inReplyTo":null,"subject":"[PATCH] Use git-tag in git-cvsimport","fromName":"Elvis Pranskevichus","fromEmail":"el@prans.net","sentAt":"2007-06-03T06:56:36Z","receivedAt":"2007-06-03T06:56:36Z","isPatch":true,"sender":{"key":"el@prans.net","avatar":null},"body":"Currently git-cvsimport tries to create tag objects directly via git-mktag\nin a very broken way, e.g the stuff it writes into the tagger field of\nthe tag object doesn't really resemble the GIT_COMMITTER_IDENT. This makes\ngitweb and possibly other tools that try to interpret tag objects to be\nconfused about tag date and authorship.\n\nFix this by calling git-tag instead. This also has a nice side effect of\nnot creating the tag object but only the lightweight tag as that's the only\nthing CVS has anyways.\n\nSigned-off-by: Elvis Pranskevichus <el@prans.net>\n---\n git-cvsimport.perl |   26 ++------------------------\n 1 files changed, 2 insertions(+), 24 deletions(-)\n\ndiff --git a/git-cvsimport.perl b/git-cvsimport.perl\nindex f68afe7..d5ca66b 100755\n--- a/git-cvsimport.perl\n+++ b/git-cvsimport.perl\n@@ -771,31 +771,9 @@ sub commit {\n \t\t$xtag =~ s/\\s+\\*\\*.*$//; # Remove stuff like ** INVALID ** and ** FUNKY **\n \t\t$xtag =~ tr/_/\\./ if ( $opt_u );\n \t\t$xtag =~ s/[\\/]/$opt_s/g;\n-\t\t\n-\t\tmy $pid = open2($in, $out, 'git-mktag');\n-\t\tprint $out \"object $cid\\n\".\n-\t\t    \"type commit\\n\".\n-\t\t    \"tag $xtag\\n\".\n-\t\t    \"tagger $author_name <$author_email>\\n\"\n-\t\t    or die \"Cannot create tag object $xtag: $!\\n\";\n-\t\tclose($out)\n-\t\t    or die \"Cannot create tag object $xtag: $!\\n\";\n-\n-\t\tmy $tagobj = <$in>;\n-\t\tchomp $tagobj;\n-\n-\t\tif ( !close($in) or waitpid($pid, 0) != $pid or\n-\t\t     $? != 0 or $tagobj !~ /^[0123456789abcdef]{40}$/ ) {\n-\t\t    die \"Cannot create tag object $xtag: $!\\n\";\n-\t        }\n-\t\t\n-\n-\t\topen(C,\">$git_dir/refs/tags/$xtag\")\n+\n+\t\tsystem(\"git-tag $xtag $cid\") == 0\n \t\t\tor die \"Cannot create tag $xtag: $!\\n\";\n-\t\tprint C \"$tagobj\\n\"\n-\t\t\tor die \"Cannot write tag $xtag: $!\\n\";\n-\t\tclose(C)\n-\t\t\tor die \"Cannot write tag $xtag: $!\\n\";\n \n \t\tprint \"Created tag '$xtag' on '$branch'\\n\" if $opt_v;\n \t}\n-- \n1.5.2\n"},{"id":"43853","messageId":"7v1wgto2mh.fsf@assigned-by-dhcp.cox.net","threadId":"8408","inReplyTo":"11808537962798-git-send-email-el@prans.net","subject":"Re: [PATCH] Use git-tag in git-cvsimport","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2007-06-03T08:37:10Z","receivedAt":"2007-06-03T08:37:10Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Elvis Pranskevichus <el@prans.net> writes:\n\n> Currently git-cvsimport tries to create tag objects directly via git-mktag\n> in a very broken way, e.g the stuff it writes into the tagger field of\n> the tag object doesn't really resemble the GIT_COMMITTER_IDENT. This makes\n> gitweb and possibly other tools that try to interpret tag objects to be\n> confused about tag date and authorship.\n>\n> Fix this by calling git-tag instead. This also has a nice side effect of\n> not creating the tag object but only the lightweight tag as that's the only\n> thing CVS has anyways.\n>\n> Signed-off-by: Elvis Pranskevichus <el@prans.net>\n\nThis sounds very sane, although I have not thought through the\npossible ramifications.\n\n>  git-cvsimport.perl |   26 ++------------------------\n>  1 files changed, 2 insertions(+), 24 deletions(-)\n>\n> diff --git a/git-cvsimport.perl b/git-cvsimport.perl\n> index f68afe7..d5ca66b 100755\n> --- a/git-cvsimport.perl\n> +++ b/git-cvsimport.perl\n> @@ -771,31 +771,9 @@ sub commit {\n>  \t\t$xtag =~ s/\\s+\\*\\*.*$//; # Remove stuff like ** INVALID ** and ** FUNKY **\n>  \t\t$xtag =~ tr/_/\\./ if ( $opt_u );\n>  \t\t$xtag =~ s/[\\/]/$opt_s/g;\n> - ...\n> +\n> +\t\tsystem(\"git-tag $xtag $cid\") == 0\n>  \t\t\tor die \"Cannot create tag $xtag: $!\\n\";\n> - ...\n>  \n>  \t\tprint \"Created tag '$xtag' on '$branch'\\n\" if $opt_v;\n>  \t}\n> -- \n> 1.5.2\n\nOther than that I would write the \"system\" in a slightly newer\nstyle, i.e.\n\n\tsystem('git-tag', $xtag, $cid)\n\nI do not think of any obvious downside, either in the code nor\nthe change to use unannotated tag.\n\nAnybody on the list see downsides with this?\n"},{"id":"43922","messageId":"20070603225354.GB16637@admingilde.org","threadId":"8408","inReplyTo":"11808537962798-git-send-email-el@prans.net","subject":"Re: [PATCH] Use git-tag in git-cvsimport","fromName":"Martin Waitz","fromEmail":"tali@admingilde.org","sentAt":"2007-06-03T22:53:55Z","receivedAt":"2007-06-03T22:53:55Z","isPatch":true,"sender":{"key":"tali@admingilde.org","avatar":"https://gravatar.com/avatar/3f89b03eee362187effabe257898735b475673a12265c398ea9161259ae91553?d=mp&s=160"},"body":"hoi :)\n\nOn Sun, Jun 03, 2007 at 02:56:36AM -0400, Elvis Pranskevichus wrote:\n> Fix this by calling git-tag instead. This also has a nice side effect of\n> not creating the tag object but only the lightweight tag as that's the only\n> thing CVS has anyways.\n\nbut lightweight tags are not fetched by default.\nAnd only leaving them in the repository that actually did the cvsimport\nmakes them much less valuable.\n\n> +\n> +\t\tsystem(\"git-tag $xtag $cid\") == 0\n\nI suggest adding something like -m \"import label from CVS\".\n\n-- \nMartin Waitz\n"},{"id":"43926","messageId":"7vabvgmvuo.fsf@assigned-by-dhcp.cox.net","threadId":"8408","inReplyTo":"20070603225354.GB16637@admingilde.org","subject":"Re: [PATCH] Use git-tag in git-cvsimport","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2007-06-04T00:01:03Z","receivedAt":"2007-06-04T00:01:03Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Martin Waitz <tali@admingilde.org> writes:\n\n> hoi :)\n>\n> On Sun, Jun 03, 2007 at 02:56:36AM -0400, Elvis Pranskevichus wrote:\n>> Fix this by calling git-tag instead. This also has a nice side effect of\n>> not creating the tag object but only the lightweight tag as that's the only\n>> thing CVS has anyways.\n>\n> but lightweight tags are not fetched by default.\n\nAre you sure about that?\n"},{"id":"43947","messageId":"20070604071810.GD16637@admingilde.org","threadId":"8408","inReplyTo":"7vabvgmvuo.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH] Use git-tag in git-cvsimport","fromName":"Martin Waitz","fromEmail":"tali@admingilde.org","sentAt":"2007-06-04T07:18:11Z","receivedAt":"2007-06-04T07:18:11Z","isPatch":true,"sender":{"key":"tali@admingilde.org","avatar":"https://gravatar.com/avatar/3f89b03eee362187effabe257898735b475673a12265c398ea9161259ae91553?d=mp&s=160"},"body":"hoi :)\n\nOn Sun, Jun 03, 2007 at 05:01:03PM -0700, Junio C Hamano wrote:\n> Martin Waitz <tali@admingilde.org> writes:\n> > but lightweight tags are not fetched by default.\n> \n> Are you sure about that?\n\nnot any more now that you questioned it ;-)\n\nBut at least there is a hook script which refuses to receive\nun-annotated tags and I always considered those tags to be temporary\ntags in the local repository.\n\n-- \nMartin Waitz\n"},{"id":"44017","messageId":"200706041955.52779.elprans@gmail.com","threadId":"8408","inReplyTo":"7v1wgto2mh.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH] Use git-tag in git-cvsimport","fromName":"Elvis Pranskevichus","fromEmail":"elprans@gmail.com","sentAt":"2007-06-04T23:55:52Z","receivedAt":"2007-06-04T23:55:52Z","isPatch":true,"sender":{"key":"elprans@gmail.com","avatar":null},"body":"> hoi :)\n\nHi =)\n\n> On Sun, Jun 03, 2007 at 05:01:03PM -0700, Junio C Hamano wrote:\n> > Martin Waitz <tali@admingilde.org> writes:\n> > > but lightweight tags are not fetched by default.\n> > \n> > Are you sure about that?\n\n> not any more now that you questioned it ;-)\n\nLast time I checked, unannotated tags are git-cloned and git-fetched just \nfine. I'm not sure about the exact definition and behaviour of lightweight \ntags, though. \n\nCVS just doesn't attach any valuable info to the tags, it's just a point in \ntime.\n\n> But at least there is a hook script which refuses to receive\n> un-annotated tags and I always considered those tags to be temporary\n> tags in the local repository.\n\nWell, there's no mention about that in the docs. And I don't think that the \nnotion of git losing valid objects along the way is the good one =)\n\nAnyways, the patch wasn't about the tag type change. As I mentioned it's just \na side effect. The main point is to fix the cvsimport tag breakage. I've \ntested that change on a few big (and messy) CVS repos, and it works just \nfine.\n\nCheers,\n-- \n\n                 Elvis\n"},{"id":"44200","messageId":"7vhcpkzuha.fsf@assigned-by-dhcp.cox.net","threadId":"8408","inReplyTo":"20070604071810.GD16637@admingilde.org","subject":"Re: [PATCH] Use git-tag in git-cvsimport","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2007-06-06T20:41:21Z","receivedAt":"2007-06-06T20:41:21Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Martin Waitz <tali@admingilde.org> writes:\n\n> On Sun, Jun 03, 2007 at 05:01:03PM -0700, Junio C Hamano wrote:\n>> Martin Waitz <tali@admingilde.org> writes:\n>> > but lightweight tags are not fetched by default.\n>> \n>> Are you sure about that?\n>\n> not any more now that you questioned it ;-)\n>\n> But at least there is a hook script which refuses to receive\n> un-annotated tags and I always considered those tags to be temporary\n> tags in the local repository.\n\nI'll take the patch to 'next' and see if anybody screams.  We\ncan add the '-m \"Label from CVS\"' bit easily if annotated tags\nturns out to be easier for people before it graduates to\n'master', although I suspect we probably do not have to.\n"}]}