{"thread":{"id":"42767","subject":"[PATCH] git-svn: clone: Fail on missing url argument","startedAt":"2016-07-03T05:46:44Z","lastAt":"2016-07-06T18:15:37Z","messageCount":4,"participants":["Christopher Layne","Eric Wong","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"290753","messageId":"20160703053923.GA12956@ns1.signalpunk.com","threadId":"42767","inReplyTo":null,"subject":"[PATCH] git-svn: clone: Fail on missing url argument","fromName":"Christopher Layne","fromEmail":"clayne@anodized.com","sentAt":"2016-07-03T05:39:23Z","receivedAt":"2016-07-03T05:46:44Z","isPatch":true,"sender":{"key":"clayne@anodized.com","avatar":null},"body":"* cmd_clone should detect a missing $url arg before using it otherwise\n  an uninitialized value error is emitted in even the simplest case of\n  'git svn clone' without arguments.\n\nSigned-off-by: Christopher Layne <clayne@anodized.com>\n---\n git-svn.perl | 5 ++++-\n 1 file changed, 4 insertions(+), 1 deletion(-)\n\ndiff --git a/git-svn.perl b/git-svn.perl\nindex 05eced0..f609e54 100755\n--- a/git-svn.perl\n+++ b/git-svn.perl\n@@ -507,7 +507,10 @@ sub init_subdir {\n \n sub cmd_clone {\n \tmy ($url, $path) = @_;\n-\tif (!defined $path &&\n+\tif (!$url) {\n+\t\tdie \"SVN repository location required \",\n+\t\t    \"as a command-line argument\\n\";\n+\t} elsif (!defined $path &&\n \t    (defined $_trunk || @_branches || @_tags ||\n \t     defined $_stdlayout) &&\n \t    $url !~ m#^[a-z\\+]+://#) {\n-- \n2.7.3\n\n"},{"id":"290754","messageId":"20160703061553.GA20458@dcvr.yhbt.net","threadId":"42767","inReplyTo":"20160703053923.GA12956@ns1.signalpunk.com","subject":"Re: [PATCH] git-svn: clone: Fail on missing url argument","fromName":"Eric Wong","fromEmail":"e@80x24.org","sentAt":"2016-07-03T06:15:53Z","receivedAt":"2016-07-03T06:17:36Z","isPatch":true,"sender":{"key":"e@80x24.org","avatar":null},"body":"Christopher Layne <clayne@anodized.com> wrote:\n> * cmd_clone should detect a missing $url arg before using it otherwise\n>   an uninitialized value error is emitted in even the simplest case of\n>   'git svn clone' without arguments.\n\nThanks, this patch looks obviously correct.\n\nI've eliminated the '* ' and space prefix from the version I've\napplied since it's not the convention around here.\n\n> Signed-off-by: Christopher Layne <clayne@anodized.com>\n\nSigned-off-by: Eric Wong <e@80x24.org>\n\nAnd pushed to \"master\" of git://bogomips.org/git-svn\n(I'll request for Junio to pull within a few days while\n other changes pile up).\n\n>  sub cmd_clone {\n>  \tmy ($url, $path) = @_;\n> -\tif (!defined $path &&\n> +\tif (!$url) {\n> +\t\tdie \"SVN repository location required \",\n> +\t\t    \"as a command-line argument\\n\";\n\n\"as a command-line argument\" seems like an unnecessary phrase,\nbut I see we use it elsewhere; so it's fine here.\n\nI might be tempted to queue up a separate patch\nto eliminate this extra statement from the rest of git-svn,\nthough.  Not sure if others feel the same way.\n"},{"id":"290755","messageId":"6A207591-312F-4C62-8C69-2742E44146F4@anodized.com","threadId":"42767","inReplyTo":"20160703061553.GA20458@dcvr.yhbt.net","subject":"Re: [PATCH] git-svn: clone: Fail on missing url argument","fromName":"Christopher Layne","fromEmail":"clayne@anodized.com","sentAt":"2016-07-03T06:49:38Z","receivedAt":"2016-07-03T06:56:42Z","isPatch":true,"sender":{"key":"clayne@anodized.com","avatar":null},"body":"\n> On Jul 2, 2016, at 2315 PT, Eric Wong <e@80x24.org> wrote:\n>> sub cmd_clone {\n>> \tmy ($url, $path) = @_;\n>> -\tif (!defined $path &&\n>> +\tif (!$url) {\n>> +\t\tdie \"SVN repository location required \",\n>> +\t\t    \"as a command-line argument\\n\";\n> \n> \"as a command-line argument\" seems like an unnecessary phrase,\n> but I see we use it elsewhere; so it's fine here.\n> \n> I might be tempted to queue up a separate patch\n> to eliminate this extra statement from the rest of git-svn,\n> though.  Not sure if others feel the same way.\n\nI basically went with the same logic/error message that cmd_init()\nwas using a couple of lines down in an attempt to stay consistent:\n\n 527 sub cmd_init {\n 528         if (defined $_stdlayout) {\n 529                 $_trunk = 'trunk' if (!defined $_trunk);\n 530                 @_tags = 'tags' if (! @_tags);\n 531                 @_branches = 'branches' if (! @_branches);\n 532         }\n 533         if (defined $_trunk || @_branches || @_tags) {\n 534                 return cmd_multi_init(@_);\n 535         }\n 536         my $url = shift or die \"SVN repository location required \",\n 537                                \"as a command-line argument\\n\";\n 538         $url = canonicalize_url($url);\n 539         init_subdir(@_);\n 540         do_git_init_db();\n 541 \n 542         if ($Git::SVN::_minimize_url eq 'unset') {\n 543                 $Git::SVN::_minimize_url = 0;\n 544         }\n 545 \n 546         Git::SVN->init($url);\n 547 }\n\n-cl\n"},{"id":"290897","messageId":"xmqqvb0iske6.fsf@gitster.mtv.corp.google.com","threadId":"42767","inReplyTo":"20160703061553.GA20458@dcvr.yhbt.net","subject":"Re: [PATCH] git-svn: clone: Fail on missing url argument","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-07-06T18:15:29Z","receivedAt":"2016-07-06T18:15:37Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Eric Wong <e@80x24.org> writes:\n\n> Christopher Layne <clayne@anodized.com> wrote:\n>> * cmd_clone should detect a missing $url arg before using it otherwise\n>>   an uninitialized value error is emitted in even the simplest case of\n>>   'git svn clone' without arguments.\n>\n> Thanks, this patch looks obviously correct.\n>\n> I've eliminated the '* ' and space prefix from the version I've\n> applied since it's not the convention around here.\n>\n>> Signed-off-by: Christopher Layne <clayne@anodized.com>\n>\n> Signed-off-by: Eric Wong <e@80x24.org>\n>\n> And pushed to \"master\" of git://bogomips.org/git-svn\n> (I'll request for Junio to pull within a few days while\n>  other changes pile up).\n\nThanks.\n\n>>  sub cmd_clone {\n>>  \tmy ($url, $path) = @_;\n>> -\tif (!defined $path &&\n>> +\tif (!$url) {\n>> +\t\tdie \"SVN repository location required \",\n>> +\t\t    \"as a command-line argument\\n\";\n>\n> \"as a command-line argument\" seems like an unnecessary phrase,\n> but I see we use it elsewhere; so it's fine here.\n>\n> I might be tempted to queue up a separate patch\n> to eliminate this extra statement from the rest of git-svn,\n> though.  Not sure if others feel the same way.\n\nIf it _can_ come from somewhere else (perhaps a future enhancement\nmay allow you to configure where to clone from?  Not likely for\ncmd_clone but other places in git-svn may be talking about something\nthat could be configured in the future), then \"as a command-line\nargument\" is not just unnecessary but actively waiting to harm the\nusers.\n\nBut otherwise I do not think anybody cares either way.\n"}]}