* [PATCH] git-svn: clone: Fail on missing url argument
@ 2016-07-03 5:39 Christopher Layne
2016-07-03 6:15 ` Eric Wong
0 siblings, 1 reply; 4+ messages in thread
From: Christopher Layne @ 2016-07-03 5:39 UTC (permalink / raw)
To: git; +Cc: Eric Wong
* cmd_clone should detect a missing $url arg before using it otherwise
an uninitialized value error is emitted in even the simplest case of
'git svn clone' without arguments.
Signed-off-by: Christopher Layne <clayne@anodized.com>
---
git-svn.perl | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/git-svn.perl b/git-svn.perl
index 05eced0..f609e54 100755
--- a/git-svn.perl
+++ b/git-svn.perl
@@ -507,7 +507,10 @@ sub init_subdir {
sub cmd_clone {
my ($url, $path) = @_;
- if (!defined $path &&
+ if (!$url) {
+ die "SVN repository location required ",
+ "as a command-line argument\n";
+ } elsif (!defined $path &&
(defined $_trunk || @_branches || @_tags ||
defined $_stdlayout) &&
$url !~ m#^[a-z\+]+://#) {
--
2.7.3
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] git-svn: clone: Fail on missing url argument
2016-07-03 5:39 [PATCH] git-svn: clone: Fail on missing url argument Christopher Layne
@ 2016-07-03 6:15 ` Eric Wong
2016-07-03 6:49 ` Christopher Layne
2016-07-06 18:15 ` Junio C Hamano
0 siblings, 2 replies; 4+ messages in thread
From: Eric Wong @ 2016-07-03 6:15 UTC (permalink / raw)
To: Christopher Layne; +Cc: git
Christopher Layne <clayne@anodized.com> wrote:
> * cmd_clone should detect a missing $url arg before using it otherwise
> an uninitialized value error is emitted in even the simplest case of
> 'git svn clone' without arguments.
Thanks, this patch looks obviously correct.
I've eliminated the '* ' and space prefix from the version I've
applied since it's not the convention around here.
> Signed-off-by: Christopher Layne <clayne@anodized.com>
Signed-off-by: Eric Wong <e@80x24.org>
And pushed to "master" of git://bogomips.org/git-svn
(I'll request for Junio to pull within a few days while
other changes pile up).
> sub cmd_clone {
> my ($url, $path) = @_;
> - if (!defined $path &&
> + if (!$url) {
> + die "SVN repository location required ",
> + "as a command-line argument\n";
"as a command-line argument" seems like an unnecessary phrase,
but I see we use it elsewhere; so it's fine here.
I might be tempted to queue up a separate patch
to eliminate this extra statement from the rest of git-svn,
though. Not sure if others feel the same way.
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] git-svn: clone: Fail on missing url argument
2016-07-03 6:15 ` Eric Wong
@ 2016-07-03 6:49 ` Christopher Layne
2016-07-06 18:15 ` Junio C Hamano
1 sibling, 0 replies; 4+ messages in thread
From: Christopher Layne @ 2016-07-03 6:49 UTC (permalink / raw)
To: Eric Wong; +Cc: git
> On Jul 2, 2016, at 2315 PT, Eric Wong <e@80x24.org> wrote:
>> sub cmd_clone {
>> my ($url, $path) = @_;
>> - if (!defined $path &&
>> + if (!$url) {
>> + die "SVN repository location required ",
>> + "as a command-line argument\n";
>
> "as a command-line argument" seems like an unnecessary phrase,
> but I see we use it elsewhere; so it's fine here.
>
> I might be tempted to queue up a separate patch
> to eliminate this extra statement from the rest of git-svn,
> though. Not sure if others feel the same way.
I basically went with the same logic/error message that cmd_init()
was using a couple of lines down in an attempt to stay consistent:
527 sub cmd_init {
528 if (defined $_stdlayout) {
529 $_trunk = 'trunk' if (!defined $_trunk);
530 @_tags = 'tags' if (! @_tags);
531 @_branches = 'branches' if (! @_branches);
532 }
533 if (defined $_trunk || @_branches || @_tags) {
534 return cmd_multi_init(@_);
535 }
536 my $url = shift or die "SVN repository location required ",
537 "as a command-line argument\n";
538 $url = canonicalize_url($url);
539 init_subdir(@_);
540 do_git_init_db();
541
542 if ($Git::SVN::_minimize_url eq 'unset') {
543 $Git::SVN::_minimize_url = 0;
544 }
545
546 Git::SVN->init($url);
547 }
-cl
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] git-svn: clone: Fail on missing url argument
2016-07-03 6:15 ` Eric Wong
2016-07-03 6:49 ` Christopher Layne
@ 2016-07-06 18:15 ` Junio C Hamano
1 sibling, 0 replies; 4+ messages in thread
From: Junio C Hamano @ 2016-07-06 18:15 UTC (permalink / raw)
To: Eric Wong; +Cc: Christopher Layne, git
Eric Wong <e@80x24.org> writes:
> Christopher Layne <clayne@anodized.com> wrote:
>> * cmd_clone should detect a missing $url arg before using it otherwise
>> an uninitialized value error is emitted in even the simplest case of
>> 'git svn clone' without arguments.
>
> Thanks, this patch looks obviously correct.
>
> I've eliminated the '* ' and space prefix from the version I've
> applied since it's not the convention around here.
>
>> Signed-off-by: Christopher Layne <clayne@anodized.com>
>
> Signed-off-by: Eric Wong <e@80x24.org>
>
> And pushed to "master" of git://bogomips.org/git-svn
> (I'll request for Junio to pull within a few days while
> other changes pile up).
Thanks.
>> sub cmd_clone {
>> my ($url, $path) = @_;
>> - if (!defined $path &&
>> + if (!$url) {
>> + die "SVN repository location required ",
>> + "as a command-line argument\n";
>
> "as a command-line argument" seems like an unnecessary phrase,
> but I see we use it elsewhere; so it's fine here.
>
> I might be tempted to queue up a separate patch
> to eliminate this extra statement from the rest of git-svn,
> though. Not sure if others feel the same way.
If it _can_ come from somewhere else (perhaps a future enhancement
may allow you to configure where to clone from? Not likely for
cmd_clone but other places in git-svn may be talking about something
that could be configured in the future), then "as a command-line
argument" is not just unnecessary but actively waiting to harm the
users.
But otherwise I do not think anybody cares either way.
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2016-07-06 18:15 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-07-03 5:39 [PATCH] git-svn: clone: Fail on missing url argument Christopher Layne
2016-07-03 6:15 ` Eric Wong
2016-07-03 6:49 ` Christopher Layne
2016-07-06 18:15 ` Junio C Hamano
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).