{"thread":{"id":"23457","subject":"[PATCH] git-cvsimport: strip question-mark characters in tags","startedAt":"2010-04-14T13:38:31Z","lastAt":"2010-04-15T01:39:45Z","messageCount":7,"participants":["Ed Santiago","Junio C Hamano","Andreas Schwab","Johan Herland"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"139475","messageId":"20100414133831.GA12135@f.santiago.vpn.redhat.com","threadId":"23457","inReplyTo":null,"subject":"[PATCH] git-cvsimport: strip question-mark characters in tags","fromName":"Ed Santiago","fromEmail":"santiago@redhat.com","sentAt":"2010-04-14T13:38:31Z","receivedAt":"2010-04-14T13:38:31Z","isPatch":true,"sender":{"key":"santiago@redhat.com","avatar":"https://gravatar.com/avatar/ea76e06d92bea441997ae5b01983859fcee775de243c13c2efb1731b6f444328?d=mp&s=160"},"body":"Question mark character appears to be valid in a CVS tag,\nbut not a git one. Remove it. Leave open the possibility that there may\nbe more such characters; and comment (FIXME) that we may want to replace\nthose instead of removing them.\n\nAlso: if git tag command fails, do not include $! in our\nerror message: it is not useful after system(), and will\nonly serve as a red herring.\n---\n git-cvsimport.perl | 6 ++++--\n 1 files changed, 4 insertions(+), 2 deletions(-)\n\ndiff --git a/git-cvsimport.perl b/git-cvsimport.perl\nindex 9e03eee..48de2b4 100755\n--- a/git-cvsimport.perl\n+++ b/git-cvsimport.perl\n@@ -840,10 +840,12 @@ 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$xtag =~ s/\\[//g;\n+\t\t# The following characters are valid in CVS tags but not git.\n+\t\t# Remove them. (FIXME: optionally replace?)\n+\t\t$xtag =~ tr/\\[\\?//d;\n \n \t\tsystem('git' , 'tag', '-f', $xtag, $cid) == 0\n-\t\t\tor die \"Cannot create tag $xtag: $!\\n\";\n+\t\t\tor die \"Cannot create tag $xtag\\n\";\n \n \t\tprint \"Created tag '$xtag' on '$branch'\\n\" if $opt_v;\n \t}\n-- \n1.6.6.1\n\n\n-- \nEd Santiago Software Engineer santiago@redhat.com\n"},{"id":"139479","messageId":"7vfx2yqf9s.fsf@alter.siamese.dyndns.org","threadId":"23457","inReplyTo":"20100414133831.GA12135@f.santiago.vpn.redhat.com","subject":"Re: [PATCH] git-cvsimport: strip question-mark characters in tags","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2010-04-14T14:29:03Z","receivedAt":"2010-04-14T14:29:03Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ed Santiago writes:\n\n> Question mark character appears to be valid in a CVS tag,\n> but not a git one. Remove it. Leave open the possibility that there may\n> be more such characters; and comment (FIXME) that we may want to replace\n> those instead of removing them.\n\nI agree that people may want to optionally replace them to avoid mapping\ntwo originally different tags into the same one.\n\nDoes your regexp chain check other invalid refname sequences, such as \".name\",\n\"na..me\", etc.?\n\n> Also: if git tag command fails, do not include $! in our\n> error message: it is not useful after system(), and will\n> only serve as a red herring.\n> ---\n\nSign-off?\n"},{"id":"139487","messageId":"20100414154409.GA28268@f.santiago.vpn.redhat.com","threadId":"23457","inReplyTo":"7vfx2yqf9s.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] git-cvsimport: strip question-mark characters in tags","fromName":"Ed Santiago","fromEmail":"santiago@redhat.com","sentAt":"2010-04-14T15:44:10Z","receivedAt":"2010-04-14T15:44:10Z","isPatch":true,"sender":{"key":"santiago@redhat.com","avatar":"https://gravatar.com/avatar/ea76e06d92bea441997ae5b01983859fcee775de243c13c2efb1731b6f444328?d=mp&s=160"},"body":"On Wed, Apr 14, 2010 at 07:29:03AM -0700, Junio C Hamano wrote:\n> \n> I agree that people may want to optionally replace them to avoid mapping\n> two originally different tags into the same one.\n\nI considered that but decided that it was beyond the scope of\nwhat I wanted to tackle. My intent was to suggest an incremental\nfix, not a revolutionary one. I also wished to remove a misleading\ndiagnostic[1] and provide a way for others to do the same as needed.\n\n [1] The error I saw was \"Cannot create tag mumblefoo{?dist}: Illegal seek\",\n where \"Illegal seek\" is due to an uninitialized/invalid $!\n\nA proper solution would, I believe, be uncomfortably complex.\nSome of the approaches I considered are:\n\n * command-line option '--tag-replace =', eg '?=-'\n * replacing a (hardcoded) list of chars with %xx hex codes\n * option a la -A, with a file listing =\n * like above, but the file lists perl expressions such as s/\\?/./g\n\nI used git-cvsimport because I've used tailor[2] before and\ndreaded the thought of running it again. I love how easy\ngit-cvsimport was to use! I see it as a great tool for\nquick jobs, for proof-of-concept situations: it has a low\ncost of entry, works well with a remote CVS repo, and was\neasy to debug. But if it were to become as configurable -- and\nas complex -- as tailor, I think it might lose much of its value.\n\n [2] http://wiki.darcs.net/RelatedSoftware/Tailor\n\n> Does your regexp chain check other invalid refname sequences, such as \".name\",\n> \"na..me\", etc.?\n\nEek, no. Again, this was a purely incremental fix, an addition\nto an existing (simple) check. Duplicating the functionality\nof git-check-ref-format would be a terrible idea. But if there's\nalready a perlified library for this, or a portable hook into\ngit itself (I haven't looked), it might be wise to have\ngit-cvsimport use that to check/convert tags. At the very\nleast it would give a much better error to the end user.\n\n> Sign-off?\n\nMy apologies. I'm new to this.\n\nHow would you like me to proceed? I'm reluctant to code a\nfull general solution to the tag-validity problem, but am\nhappy to rework my changes into something better.\n\nThanks for your feedback,\nEd\n-- \nEd Santiago Software Engineer santiago@redhat.com\n"},{"id":"139498","messageId":"7v1vehrg0g.fsf@alter.siamese.dyndns.org","threadId":"23457","inReplyTo":"20100414154409.GA28268@f.santiago.vpn.redhat.com","subject":"Re: [PATCH] git-cvsimport: strip question-mark characters in tags","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2010-04-14T19:27:43Z","receivedAt":"2010-04-14T19:27:43Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ed Santiago writes:\n\n> On Wed, Apr 14, 2010 at 07:29:03AM -0700, Junio C Hamano wrote:\n>> \n>> I agree that people may want to optionally replace them to avoid mapping\n>> two originally different tags into the same one.\n>\n> I considered that but decided that it was beyond the scope of\n> what I wanted to tackle.\n\nOh, one step at a time is perfectly fine.\n"},{"id":"139514","messageId":"m27ho9sr0i.fsf@igel.home","threadId":"23457","inReplyTo":"20100414133831.GA12135@f.santiago.vpn.redhat.com","subject":"Re: [PATCH] git-cvsimport: strip question-mark characters in tags","fromName":"Andreas Schwab","fromEmail":"schwab@linux-m68k.org","sentAt":"2010-04-14T20:44:45Z","receivedAt":"2010-04-14T20:44:45Z","isPatch":true,"sender":{"key":"schwab@linux-m68k.org","avatar":"https://avatars.githubusercontent.com/u/2175493?v=4"},"body":"Ed Santiago writes:\n\n> Question mark character appears to be valid in a CVS tag,\n\nAccording to the CVS docs only letters, digits, '-' and '_' are valid\nfor tag names.\n\nAndreas.\n\n-- \nAndreas Schwab, schwab@linux-m68k.org\nGPG Key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5\n\"And now for something completely different.\"\n"},{"id":"139526","messageId":"20100414214229.GA31757@f.santiago.vpn.redhat.com","threadId":"23457","inReplyTo":"m27ho9sr0i.fsf@igel.home","subject":"Re: [PATCH] git-cvsimport: strip question-mark characters in tags","fromName":"Ed Santiago","fromEmail":"santiago@redhat.com","sentAt":"2010-04-14T21:42:29Z","receivedAt":"2010-04-14T21:42:29Z","isPatch":true,"sender":{"key":"santiago@redhat.com","avatar":"https://gravatar.com/avatar/ea76e06d92bea441997ae5b01983859fcee775de243c13c2efb1731b6f444328?d=mp&s=160"},"body":"On Wed, Apr 14, 2010 at 10:44:45PM +0200, Andreas Schwab wrote:\n> Ed Santiago writes:\n> \n> > Question mark character appears to be valid in a CVS tag,\n> \n> According to the CVS docs only letters, digits, '-' and '_' are valid\n> for tag names.\n\nPoor choice of words on my part. What I *should* have said is\nsomething like:\n\n Although question marks and curly braces are not among the\n set of characters which CVS considers to be valid for a tag,\n real-world situations have been encontered in which a CVS\n comma-v file has a tag including all those characters. This\n patch makes git-cvsimport accept and forgive that reality.\n\nHow that tag got created, I really don't know. I can imagine\nthree ways it could've happened (rcs commands; broken/old version\nof CVS; custom tool for mucking with comma-v files). My goal\nwas to recognize that this sort of thing happens, and to make\nit easier for the next person to find & fix this in the script.\nWith that goal in mind, removing $! and adding the comment is\nthe only important part of my patch. The question mark itself\nis not likely to be useful except in very rare and weird cases.\n\nEd\n-- \nEd Santiago Software Engineer santiago@redhat.com\n"},{"id":"139543","messageId":"201004150339.45879.johan@herland.net","threadId":"23457","inReplyTo":"20100414214229.GA31757@f.santiago.vpn.redhat.com","subject":"Re: [PATCH] git-cvsimport: strip question-mark characters in tags","fromName":"Johan Herland","fromEmail":"johan@herland.net","sentAt":"2010-04-15T01:39:45Z","receivedAt":"2010-04-15T01:39:45Z","isPatch":true,"sender":{"key":"johan@herland.net","avatar":"https://avatars.githubusercontent.com/u/547031?v=4"},"body":"On Wednesday 14 April 2010, Ed Santiago wrote:\n> On Wed, Apr 14, 2010 at 10:44:45PM +0200, Andreas Schwab wrote:\n> > Ed Santiago writes:\n> > > Question mark character appears to be valid in a CVS tag,\n> > \n> > According to the CVS docs only letters, digits, '-' and '_' are valid\n> > for tag names.\n> \n> Poor choice of words on my part. What I *should* have said is\n> something like:\n> \n> Although question marks and curly braces are not among the\n> set of characters which CVS considers to be valid for a tag,\n> real-world situations have been encontered in which a CVS\n> comma-v file has a tag including all those characters. This\n> patch makes git-cvsimport accept and forgive that reality.\n\nIndeed. I have even seen CVS tag names containing carriage returns (aka. CR, \n\\r) in the wild...\n\n\n...Johan\n\n\n> How that tag got created, I really don't know. I can imagine\n> three ways it could've happened (rcs commands; broken/old version\n> of CVS; custom tool for mucking with comma-v files). My goal\n> was to recognize that this sort of thing happens, and to make\n> it easier for the next person to find & fix this in the script.\n> With that goal in mind, removing $! and adding the comment is\n> the only important part of my patch. The question mark itself\n> is not likely to be useful except in very rare and weird cases.\n> \n> Ed\n\n\n-- \nJohan Herland, \nwww.herland.net\n"}]}