{"thread":{"id":"19188","subject":"[PATCH] Fix sloppy Getopt::Long.","startedAt":"2009-05-05T18:16:14Z","lastAt":"2009-05-08T18:28:14Z","messageCount":6,"participants":["Robin H. Johnson","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"113048","messageId":"1241547374-6737-1-git-send-email-robbat2@gentoo.org","threadId":"19188","inReplyTo":null,"subject":"[PATCH] Fix sloppy Getopt::Long.","fromName":"Robin H. Johnson","fromEmail":"robbat2@gentoo.org","sentAt":"2009-05-05T18:16:14Z","receivedAt":"2009-05-05T18:16:14Z","isPatch":true,"sender":{"key":"robbat2@gentoo.org","avatar":"https://avatars.githubusercontent.com/u/373898?v=4"},"body":"Getopt-Long v2.38 is much stricter about sloppy getopt usage. The\ntrailing pipe causes git-svn testcases to fail for all of the --stdin\nargument calls.\n\nSigned-off-by: Robin H. Johnson \n\n===\nShould be applied to both the stable 1.6.2.x tree and the new 1.6.3\ntree.\n---\n git-svn.perl | 2 +-\n 1 files changed, 1 insertions(+), 1 deletions(-)\n\ndiff --git a/git-svn.perl b/git-svn.perl\nindex c5965c9..ef1d30d 100755\n--- a/git-svn.perl\n+++ b/git-svn.perl\n@@ -147,7 +147,7 @@ my %cmd = (\n \t 'dry-run|n' => \\$_dry_run } ],\n \t'set-tree' => [ \\&cmd_set_tree,\n \t \"Set an SVN repository to a git tree-ish\",\n-\t\t\t{ 'stdin|' => \\$_stdin, %cmt_opts, %fc_opts, } ],\n+\t\t\t{ 'stdin' => \\$_stdin, %cmt_opts, %fc_opts, } ],\n \t'create-ignore' => [ \\&cmd_create_ignore,\n \t\t\t 'Create a .gitignore per svn:ignore',\n \t\t\t { 'revision|r=i' => \\$_revision\n-- \n1.6.2.3\n"},{"id":"113051","messageId":"7vfxfj1gu9.fsf@alter.siamese.dyndns.org","threadId":"19188","inReplyTo":"1241547374-6737-1-git-send-email-robbat2@gentoo.org","subject":"Re: [PATCH] Fix sloppy Getopt::Long.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-05-05T19:37:34Z","receivedAt":"2009-05-05T19:37:34Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Robin H. Johnson\" writes:\n\n> Getopt-Long v2.38 is much stricter about sloppy getopt usage. The\n> trailing pipe causes git-svn testcases to fail for all of the --stdin\n> argument calls.\n\nI am not objecting at all; just asking for clarification.\n\n> -\t\t\t{ 'stdin|' => \\$_stdin, %cmt_opts, %fc_opts, } ],\n> +\t\t\t{ 'stdin' => \\$_stdin, %cmt_opts, %fc_opts, } ],\n\nIs this \"pipe\" supposed to be followed by an alternative spelling of the\noption, as in\n\n\t'stdin|standard-input' => \\$_stdin, ...\n\nand is the sloppyness that it would be crazy to accept either --stdin or\njust -- (without actual option name) for this option?\n\nCould an older version of Getopt::Long() have accepted\n\n\t$ command --foo --bar - other args\n\nto set $_stdin to true with that \"sloppy\" syntax? If so people could have\nrelied on such a behaviour, which is a bit worrying.\n"},{"id":"113054","messageId":"robbat2.20090505T200652.976942941Z@orbis-terrarum.net","threadId":"19188","inReplyTo":"7vfxfj1gu9.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] Fix sloppy Getopt::Long.","fromName":"Robin H. Johnson","fromEmail":"robbat2@gentoo.org","sentAt":"2009-05-05T20:21:16Z","receivedAt":"2009-05-05T20:21:16Z","isPatch":true,"sender":{"key":"robbat2@gentoo.org","avatar":"https://avatars.githubusercontent.com/u/373898?v=4"},"body":"On Tue, May 05, 2009 at 12:37:34PM -0700, Junio C Hamano wrote:\n> \"Robin H. Johnson\" writes:\n> \n> > Getopt-Long v2.38 is much stricter about sloppy getopt usage. The\n> > trailing pipe causes git-svn testcases to fail for all of the --stdin\n> > argument calls.\n> \n> I am not objecting at all; just asking for clarification.\n> \n> > -\t\t\t{ 'stdin|' => \\$_stdin, %cmt_opts, %fc_opts, } ],\n> > +\t\t\t{ 'stdin' => \\$_stdin, %cmt_opts, %fc_opts, } ],\n> \n> Is this \"pipe\" supposed to be followed by an alternative spelling of the\n> option, as in\n> \t'stdin|standard-input' => \\$_stdin, ...\nYes. Short form or alternative long form.\n\n> and is the sloppyness that it would be crazy to accept either --stdin or\n> just -- (without actual option name) for this option?\nWithin the main loop, both '--' and '-' are treated as special cases\nearlier on before the matching of options is done. '--' is the explicit\nseparator, while '-' is an argument (or a value to an option), not an\noption in itself.\n\n> Could an older version of Getopt::Long() have accepted\n> \n> \t$ command --foo --bar - other args\n> \n> to set $_stdin to true with that \"sloppy\" syntax? If so people could have\n> relied on such a behaviour, which is a bit worrying.\nAs far as I can follow in the Getopt::Long code, with the old case of\n'stdin|', the empty string case would never have matched anyway.\n\nThe v2.38 change in respect to this is not described in the upstream\nCHANGES, but boils down to this single modification:\n\nGetopt-Long-2.38/lib/Getopt/Long.pm:\n@@ -777,7 +776,7 @@\n # Option name\n (?: \\w+[-\\w]* )\n # Alias names, or \"?\"\n- (?: \\| (?: \\? | \\w[-\\w]* )? )*\n+ (?: \\| (?: \\? | \\w[-\\w]* ) )*\n )?\n\n-- \nRobin Hugh Johnson\nGentoo Linux Developer & Infra Guy\nE-Mail : robbat2@gentoo.org\nGnuPG FP : 11AC BA4F 4778 E3F6 E4ED F38E B27B 944E 3488 4E85\n"},{"id":"113093","messageId":"20090506161309.GC10702@curie-int","threadId":"19188","inReplyTo":"20090506064949.GB29479@dcvr.yhbt.net","subject":"Re: [PATCH] Fix sloppy Getopt::Long.","fromName":"Robin H. Johnson","fromEmail":"robbat2@gentoo.org","sentAt":"2009-05-06T16:13:09Z","receivedAt":"2009-05-06T16:13:09Z","isPatch":true,"sender":{"key":"robbat2@gentoo.org","avatar":"https://avatars.githubusercontent.com/u/373898?v=4"},"body":"On Tue, May 05, 2009 at 11:49:49PM -0700, Eric Wong wrote:\n> Junio C Hamano wrote:\n> > \"Robin H. Johnson\" writes:\n> > \n> > > Getopt-Long v2.38 is much stricter about sloppy getopt usage. The\n> > > trailing pipe causes git-svn testcases to fail for all of the --stdin\n> > > argument calls.\n> > >\n> > > Signed-off-by: Robin H. Johnson \n> > \n> > Eric, I'll take this directly to my tree. Ok?\n> \n> The empty \"\" after the \"|\" was intended for the set-tree command to take\n> a lone \"-\" as a parameter to also mean \"--stdin\".\n> \n> The following should work, too, but I don't have time to test right now:\n> \n> +\t\t\t{ '' => \\$_stdin, 'stdin' => \\$_stdin,\n> +\t\t\t %cmt_opts, %fc_opts, } ],\nI confirm that it does correctly set the $_stdin variable (tested\nbriefly).\n\nTestcase:\n=====\nuse Getopt::Long qw/:config gnu_getopt no_ignore_case auto_abbrev/;\nmy $_stdin;\nmy $rc = GetOptions( 'stdin' => \\$_stdin, '' => \\$_stdin);\nprintf \"rc:%s s:%s\\n\",$rc,$_stdin;\n=====\n\n-- \nRobin Hugh Johnson\nGentoo Linux Developer & Infra Guy\nE-Mail : robbat2@gentoo.org\nGnuPG FP : 11AC BA4F 4778 E3F6 E4ED F38E B27B 944E 3488 4E85\n"},{"id":"113099","messageId":"7v63gew3dp.fsf@alter.siamese.dyndns.org","threadId":"19188","inReplyTo":"20090506161309.GC10702@curie-int","subject":"Re: [PATCH] Fix sloppy Getopt::Long.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-05-06T17:24:50Z","receivedAt":"2009-05-06T17:24:50Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Robin H. Johnson\" writes:\n\n> On Tue, May 05, 2009 at 11:49:49PM -0700, Eric Wong wrote:\n>> Junio C Hamano wrote:\n>> > \"Robin H. Johnson\" writes:\n>> > \n>> > > Getopt-Long v2.38 is much stricter about sloppy getopt usage. The\n>> > > trailing pipe causes git-svn testcases to fail for all of the --stdin\n>> > > argument calls.\n>> > >\n>> > > Signed-off-by: Robin H. Johnson \n>> > \n>> > Eric, I'll take this directly to my tree. Ok?\n>> \n>> The empty \"\" after the \"|\" was intended for the set-tree command to take\n>> a lone \"-\" as a parameter to also mean \"--stdin\".\n>> \n>> The following should work, too, but I don't have time to test right now:\n>> \n>> +\t\t\t{ '' => \\$_stdin, 'stdin' => \\$_stdin,\n>> +\t\t\t %cmt_opts, %fc_opts, } ],\n> I confirm that it does correctly set the $_stdin variable (tested\n> briefly).\n\nWait a minute. Do you mean we would also need the above \"explicit empty\nargument sets $_stdin\"? Wasn't it your earlier analysis/claim that the\ncaller already takes care of a lone \"-\"?\n\nOr do you mean \"yes it would also work but it is not necessary\"?\n"},{"id":"113365","messageId":"robbat2.20090508T182516.673117391Z@orbis-terrarum.net","threadId":"19188","inReplyTo":"7v63gew3dp.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] Fix sloppy Getopt::Long.","fromName":"Robin H. Johnson","fromEmail":"robbat2@gentoo.org","sentAt":"2009-05-08T18:28:14Z","receivedAt":"2009-05-08T18:28:14Z","isPatch":true,"sender":{"key":"robbat2@gentoo.org","avatar":"https://avatars.githubusercontent.com/u/373898?v=4"},"body":"On Wed, May 06, 2009 at 10:24:50AM -0700, Junio C Hamano wrote:\n> >> +\t\t\t{ '' => \\$_stdin, 'stdin' => \\$_stdin,\n> >> +\t\t\t %cmt_opts, %fc_opts, } ],\n> > I confirm that it does correctly set the $_stdin variable (tested\n> > briefly).\n> Wait a minute. Do you mean we would also need the above \"explicit empty\n> argument sets $_stdin\"? Wasn't it your earlier analysis/claim that the\n> caller already takes care of a lone \"-\"?\nI'd originally considered a single '-' as an argument not an option,\nmeaning stdin, not being processed within the getopt framework, but\ninstead being handled later.\n\n> Or do you mean \"yes it would also work but it is not necessary\"?\nUsing Eric's change makes it explicit what is expected: Passing\n'--stdin' is the same as passing '-'.\n\n-- \nRobin Hugh Johnson\nGentoo Linux Developer & Infra Guy\nE-Mail : robbat2@gentoo.org\nGnuPG FP : 11AC BA4F 4778 E3F6 E4ED F38E B27B 944E 3488 4E85\n"}]}