All of lore.kernel.org
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: "Kyle J. McKay" <mackyle@gmail.com>
Cc: Johan Herland <johan@herland.net>, git@vger.kernel.org
Subject: Re: [ANNOUNCE] Git v2.0.0-rc0
Date: Tue, 22 Apr 2014 12:56:51 -0700	[thread overview]
Message-ID: <xmqqbnvt3zp8.fsf@gitster.dls.corp.google.com> (raw)
In-Reply-To: 475e137b5095e45c92a87a9969f58f0@74d39fa044aa309eaea14b9f57fe79c

"Kyle J. McKay" <mackyle@gmail.com> writes:

> Alternatively this change can be made in git-svn.perl:
> |diff --git a/git-svn.perl b/git-svn.perl
> |index 7349ffea..284f458a 100755
> |--- a/git-svn.perl
> |+++ b/git-svn.perl
> |@@ -149,7 +149,7 @@ my ($_trunk, @_tags, @_branches, $_stdlayout);
>   my %icv;
>   my %init_opts = ( 'template=s' => \$_template, 'shared:s' => \$_shared,
>                     'trunk|T=s' => \$_trunk, 'tags|t=s@' => \@_tags,
> -                  'branches|b=s@' => \@_branches, 'prefix=s' => \$_prefix,
> +                  'branches|b=s@' => \@_branches, 'prefix:s' => \$_prefix,
>                     'stdlayout|s' => \$_stdlayout,
>                     'minimize-url|m!' => \$Git::SVN::_minimize_url,
>                    'no-metadata' => sub { $icv{noMetadata} = 1 },
>
> Which makes the argument to --prefix optional (in which case it is set  
> to "").

We do want to learn what prefix the user wants to use when they
start their command line with "git svn --prefix".  Stopping the
command line right there and expecting it to default to whatever
built-in prefix is simply strange; the user could have omitted that
"--prefix" that lacks the argument.  If the user did want us to use
the default by passing an empty string as its argument, both forms,
i.e. "--prefix ''" and "--prefix=''", ought to be usable, but if
older Getopt::Long() does not allow the latter form, at least the
former is the right way for the user to tell us that.

So I think your fix (workaround for a bug in older Getopt::Long) in
the patch is the right one.  Johan may want to help me by pointing
out if I am missing something.

Thanks.


> I'm not really sure what the best thing to do here is.  I thought the  
> documentation talked somewhere about using --prefix="" (or just --prefix=)
> to get the old behavior, but now I can't find that.  In that  
> case I think probably just changing the tests to use --prefix ""  
> instead of --prefix="" should take care of it especially since the log  
> message for fe191fca (which actually changes the default) talks about  
> using --prefix "" and not --prefix="".  I have attached a patch below  
> (against master) to make that change to the two affected subtests of t9117.
>
> --Kyle
>
> ---- >8 ----
> Subject: [PATCH] t9117: use --prefix "" instead of --prefix=""
>
> Versions of Perl's Getopt::Long module before 2.37 do not contain
> this fix that first appeared in Getopt::Long version 2.37:
>
> * Bugfix: With gnu_compat, --foo= will no longer trigger "Option
>   requires an argument" but return the empty string.
>
> Instead of using --prefix="" use --prefix "" when testing an
> explictly empty prefix string in order to work with older versions
> of Perl's Getopt::Long module.
>
> Signed-off-by: Kyle J. McKay <mackyle@gmail.com>
> ---
>  t/t9117-git-svn-init-clone.sh | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/t/t9117-git-svn-init-clone.sh b/t/t9117-git-svn-init-clone.sh
> index dfed76fe..a66f43c6 100755
> --- a/t/t9117-git-svn-init-clone.sh
> +++ b/t/t9117-git-svn-init-clone.sh
> @@ -101,18 +101,18 @@ test_expect_success 'clone with -s/-T/-b/-t assumes --prefix=origin/' '
>  	rm -f warning
>  	'
>  
> -test_expect_success 'init with -s/-T/-b/-t and --prefix="" still works' '
> +test_expect_success 'init with -s/-T/-b/-t and --prefix "" still works' '
>  	test ! -d project &&
> -	git svn init -s "$svnrepo"/project project --prefix="" 2>warning &&
> +	git svn init -s "$svnrepo"/project project --prefix "" 2>warning &&
>  	test_must_fail grep -q prefix warning &&
>  	test_svn_configured_prefix "" &&
>  	rm -rf project &&
>  	rm -f warning
>  	'
>  
> -test_expect_success 'clone with -s/-T/-b/-t and --prefix="" still works' '
> +test_expect_success 'clone with -s/-T/-b/-t and --prefix "" still works' '
>  	test ! -d project &&
> -	git svn clone -s "$svnrepo"/project --prefix="" 2>warning &&
> +	git svn clone -s "$svnrepo"/project --prefix "" 2>warning &&
>  	test_must_fail grep -q prefix warning &&
>  	test_svn_configured_prefix "" &&
>  	rm -rf project &&

      parent reply	other threads:[~2014-04-22 19:57 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-04-18 19:37 [ANNOUNCE] Git v2.0.0-rc0 Junio C Hamano
2014-04-19  1:13 ` Johan Herland
2014-04-19  7:20   ` Junio C Hamano
2014-04-20 19:08 ` Felipe Contreras
2014-04-20 21:14   ` Junio C Hamano
2014-04-22 11:16 ` Kyle J. McKay
2014-04-22 18:58   ` Jonathan Nieder
2014-04-22 20:12     ` Junio C Hamano
2014-04-22 21:00       ` Jonathan Nieder
2014-04-22 21:26         ` Junio C Hamano
2014-04-22 22:11           ` Jonathan Nieder
2014-04-22 22:25             ` brian m. carlson
2014-04-22 22:47               ` Junio C Hamano
2014-04-23  7:33                 ` Johan Herland
2014-04-23 16:40                   ` Junio C Hamano
2014-04-22 19:56   ` Junio C Hamano [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=xmqqbnvt3zp8.fsf@gitster.dls.corp.google.com \
    --to=gitster@pobox.com \
    --cc=git@vger.kernel.org \
    --cc=johan@herland.net \
    --cc=mackyle@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.