Re: [PATCH 1/2] Add password parameter to git svn commands and use it when provided instead of defaulting to end-user prompt

From: Jonathan Nieder <jrnieder@gmail.com>
Date: Oct 14, 2013, 14:11 UTC
Message-ID: <20131014141127.GA21200@google.com>
In-Reply-To: <1381569810-2167-1-git-send-email-arnaud.brejeon@gmail.com>

Hi,

arnaud.brejeon@gmail.com wrote:

> Signed-off-by: Arnaud Brejeon <arnaud.brejeon <at> gmail.com>

Thanks.

Can you say a little more about the context? Do you run a script that wants to pass a password to 'git svn', do you type it each time on the command line, or something else? Is it ok that the password would show up in "ps" output? Would the platform's keyring or netrc be usable here, or is there something in the context that avoids that?

> ---
>  git-svn.perl           |    3 +++
>  perl/Git/SVN/Prompt.pm |   12 +++++++++---
>  2 files changed, 12 insertions(+), 3 deletions(-)

If we're going to do this, it would also need to be documented in Documentation/git-svn.txt, and ideally tested in one of the t/t91*.sh test scripts.

The implementation looks reasonable from a quick glance. Cc-ing Eric for other comments.

Hope that helps, Jonathan

(patch left unsnipped for reference)

Show 60 quoted lines

> diff --git a/git-svn.perl b/git-svn.perl
> index ff1ce3d..07f0a6c 100755
> --- a/git-svn.perl
> +++ b/git-svn.perl
> @@ -123,6 +123,7 @@ sub opt_prefix { return $_prefix || '' }
>  $Git::SVN::Fetcher::_placeholder_filename = ".gitignore";
>  $_q ||= 0;
>  my %remote_opts = ( 'username=s' => \$Git::SVN::Prompt::_username,
> +                    'password=s' => \$Git::SVN::Prompt::_password,
>                      'config-dir=s' => \$Git::SVN::Ra::config_dir,
>                      'no-auth-cache' => \$Git::SVN::Prompt::_no_auth_cache,
>                      'ignore-paths=s' => \$Git::SVN::Fetcher::_ignore_regex,
> @@ -206,6 +207,7 @@ my %cmd = (
>  	              'parents' => \$_parents,
>  	              'tag|t' => \$_tag,
>  	              'username=s' => \$Git::SVN::Prompt::_username,
> +	              'password=s' => \$Git::SVN::Prompt::_password,
>  	              'commit-url=s' => \$_commit_url } ],
>  	tag => [ sub { $_tag = 1; cmd_branch(@_) },
>  	         'Create a tag in the SVN repository',
> @@ -214,6 +216,7 @@ my %cmd = (
>  	           'dry-run|n' => \$_dry_run,
>  	           'parents' => \$_parents,
>  	           'username=s' => \$Git::SVN::Prompt::_username,
> +	           'password=s' => \$Git::SVN::Prompt::_password,
>  	           'commit-url=s' => \$_commit_url } ],
>  	'set-tree' => [ \&cmd_set_tree,
>  	                "Set an SVN repository to a git tree-ish",
> diff --git a/perl/Git/SVN/Prompt.pm b/perl/Git/SVN/Prompt.pm
> index e940b08..a94a847 100644
> --- a/perl/Git/SVN/Prompt.pm
> +++ b/perl/Git/SVN/Prompt.pm
> @@ -2,7 +2,7 @@ package Git::SVN::Prompt;
>  use strict;
>  use warnings;
>  require SVN::Core;
> -use vars qw/$_no_auth_cache $_username/;
> +use vars qw/$_no_auth_cache $_username $_password/;
>  
>  sub simple {
>  	my ($cred, $realm, $default_username, $may_save, $pool) = @_;
> @@ -17,8 +17,14 @@ sub simple {
>  	} else {
>  		username($cred, $realm, $may_save, $pool);
>  	}
> -	$cred->password(_read_password("Password for '" .
> -	                               $cred->username . "': ", $realm));
> +
> +	if (defined $_password && length $_password) {
> +		$cred->password($_password);
> +	} else {
> +		$cred->password(_read_password("Password for '" .
> +		                               $cred->username . "': ", $realm));
> +	}
> +
>  	$cred->may_save($may_save);
>  	$SVN::_Core::SVN_NO_ERROR;
>  }
> -- 
> 1.7.10.2 (Apple Git-33)