git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [RFC/PATCH] git-svn: use YAML format for mergeinfo cache when possible

From
Jonathan Nieder <jrnieder@gmail.com>
Date
May 28, 2012, 00:39 UTC
Message-ID
<20120528003901.GA11103@burratino>
In-Reply-To
<20120527201450.GA3630@dcvr.yhbt.net>
Eric Wong wrote:
> Jonathan Nieder <jrnieder@gmail.com> wrote:
>> +package Git::SVN::Memoize::YAML;
>
> Can we use this as an opportunity to start splitting git-svn.perl into
> multiple .pm files?

Not a bad idea. I've included an example patch to sanity-check the approach below.

Show 5 quoted lines
>> +	my $truehash = (-e $filename) ? YAML::Any::LoadFile($filename) : {};
>
>> +	YAML::Any::DumpFile($self->{FILENAME}, $self->{H});
>
> These should die on errors, right?
At least in YAML::Old, they use Carp::croak.  Maybe something like
	local @CARP_NOT = qw(YAML::Any);
to blame the caller for the error would bring sanity.
Show 5 quoted lines
>> +=head1 BUGS
>
>> +Error handling is awkward.
>
> How so?

I mostly meant that it's not obvious what the state of %hash is at the point marked with (*) below:

	if (not eval {
		tie my %hash => 'Foo::Bar', @params;
		1;
	}) {
		my $err = $@ ||
			# a destructor might have clobbered $@
			"Zombie error";
		die $err if worth_dying($err);
		(*) ... try to recover ...
	}

That's not specific to Memoize::YAML, though. It probably is not awkward for wizards who know the details. :)

-- >8 --
Subject: git-svn: move Git::SVN::Prompt into its own file

git-svn.perl is very long (around 6500 lines) and although it is nicely split into modules, some new readers do not even notice --- it is too distracting to see all this functionality collected in a single file.

Splitting it into multiple files would make it easier for people to read individual modules straight through and to experiment with components separately.

Let's start with Git::SVN::Prompt. For simplicity, we install this as a module in the standard search path, just like the existing Git and Git::I18N modules. In the process, add a manpage explaining its interface and that it is not likely to be useful for other projects to avoid confusion.

Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
Thanks for a thoughtful review.
Jonathan
 git-svn.perl           |  145 +---------------------------------
 perl/Git/SVN/Prompt.pm |  202 ++++++++++++++++++++++++++++++++++++++++++++++++
 perl/Makefile.PL       |    1 +
 3 files changed, 204 insertions(+), 144 deletions(-)
 create mode 100644 perl/Git/SVN/Prompt.pm
diff --git a/git-svn.perl b/git-svn.perl
index 1b4ef68f..ef60b874 100755
--- a/git-svn.perl
+++ b/git-svn.perl
@@ -80,6 +80,7 @@ use File::Find;
 use Getopt::Long qw/:config gnu_getopt no_ignore_case auto_abbrev/;
 use IPC::Open3;
 use Git;
+use Git::SVN::Prompt qw//;
 use Memoize;  # core since 5.8.0, Jul 2002
 
 BEGIN {
@@ -4441,150 +4442,6 @@ sub remove_username {
 	$_[0] =~ s{^([^:]*://)[^@]+@}{$1};
 }
 
-package Git::SVN::Prompt;
-use strict;
-use warnings;
-require SVN::Core;
-use vars qw/$_no_auth_cache $_username/;
-
-sub simple {
-	my ($cred, $realm, $default_username, $may_save, $pool) = @_;
-	$may_save = undef if $_no_auth_cache;
-	$default_username = $_username if defined $_username;
-	if (defined $default_username && length $default_username) {
-		if (defined $realm && length $realm) {
-			print STDERR "Authentication realm: $realm\n";
-			STDERR->flush;
-		}
-		$cred->username($default_username);
-	} else {
-		username($cred, $realm, $may_save, $pool);
-	}
-	$cred->password(_read_password("Password for '" .
-	                               $cred->username . "': ", $realm));
-	$cred->may_save($may_save);
-	$SVN::_Core::SVN_NO_ERROR;
-}
-
-sub ssl_server_trust {
-	my ($cred, $realm, $failures, $cert_info, $may_save, $pool) = @_;
-	$may_save = undef if $_no_auth_cache;
-	print STDERR "Error validating server certificate for '$realm':\n";
-	{
-		no warnings 'once';
-		# All variables SVN::Auth::SSL::* are used only once,
-		# so we're shutting up Perl warnings about this.
-		if ($failures & $SVN::Auth::SSL::UNKNOWNCA) {
-			print STDERR " - The certificate is not issued ",
-			    "by a trusted authority. Use the\n",
-			    "   fingerprint to validate ",
-			    "the certificate manually!\n";
-		}
-		if ($failures & $SVN::Auth::SSL::CNMISMATCH) {
-			print STDERR " - The certificate hostname ",
-			    "does not match.\n";
-		}
-		if ($failures & $SVN::Auth::SSL::NOTYETVALID) {
-			print STDERR " - The certificate is not yet valid.\n";
-		}
-		if ($failures & $SVN::Auth::SSL::EXPIRED) {
-			print STDERR " - The certificate has expired.\n";
-		}
-		if ($failures & $SVN::Auth::SSL::OTHER) {
-			print STDERR " - The certificate has ",
-			    "an unknown error.\n";
-		}
-	} # no warnings 'once'
-	printf STDERR
-	        "Certificate information:\n".
-	        " - Hostname: %s\n".
-	        " - Valid: from %s until %s\n".
-	        " - Issuer: %s\n".
-	        " - Fingerprint: %s\n",
-	        map $cert_info->$_, qw(hostname valid_from valid_until
-	                               issuer_dname fingerprint);
-	my $choice;
-prompt:
-	print STDERR $may_save ?
-	      "(R)eject, accept (t)emporarily or accept (p)ermanently? " :
-	      "(R)eject or accept (t)emporarily? ";
-	STDERR->flush;
-	$choice = lc(substr(<STDIN> || 'R', 0, 1));
-	if ($choice =~ /^t$/i) {
-		$cred->may_save(undef);
-	} elsif ($choice =~ /^r$/i) {
-		return -1;
-	} elsif ($may_save && $choice =~ /^p$/i) {
-		$cred->may_save($may_save);
-	} else {
-		goto prompt;
-	}
-	$cred->accepted_failures($failures);
-	$SVN::_Core::SVN_NO_ERROR;
-}
-
-sub ssl_client_cert {
-	my ($cred, $realm, $may_save, $pool) = @_;
-	$may_save = undef if $_no_auth_cache;
-	print STDERR "Client certificate filename: ";
-	STDERR->flush;
-	chomp(my $filename = <STDIN>);
-	$cred->cert_file($filename);
-	$cred->may_save($may_save);
-	$SVN::_Core::SVN_NO_ERROR;
-}
-
-sub ssl_client_cert_pw {
-	my ($cred, $realm, $may_save, $pool) = @_;
-	$may_save = undef if $_no_auth_cache;
-	$cred->password(_read_password("Password: ", $realm));
-	$cred->may_save($may_save);
-	$SVN::_Core::SVN_NO_ERROR;
-}
-
-sub username {
-	my ($cred, $realm, $may_save, $pool) = @_;
-	$may_save = undef if $_no_auth_cache;
-	if (defined $realm && length $realm) {
-		print STDERR "Authentication realm: $realm\n";
-	}
-	my $username;
-	if (defined $_username) {
-		$username = $_username;
-	} else {
-		print STDERR "Username: ";
-		STDERR->flush;
-		chomp($username = <STDIN>);
-	}
-	$cred->username($username);
-	$cred->may_save($may_save);
-	$SVN::_Core::SVN_NO_ERROR;
-}
-
-sub _read_password {
-	my ($prompt, $realm) = @_;
-	my $password = '';
-	if (exists $ENV{GIT_ASKPASS}) {
-		open(PH, "-|", $ENV{GIT_ASKPASS}, $prompt);
-		$password = <PH>;
-		$password =~ s/[\012\015]//; # \n\r
-		close(PH);
-	} else {
-		print STDERR $prompt;
-		STDERR->flush;
-		require Term::ReadKey;
-		Term::ReadKey::ReadMode('noecho');
-		while (defined(my $key = Term::ReadKey::ReadKey(0))) {
-			last if $key =~ /[\012\015]/; # \n\r
-			$password .= $key;
-		}
-		Term::ReadKey::ReadMode('restore');
-		print STDERR "\n";
-		STDERR->flush;
-	}
-	$password;
-}
-
 package SVN::Git::Fetcher;
 use vars qw/@ISA $_ignore_regex $_preserve_empty_dirs $_placeholder_filename
             @deleted_gpath %added_placeholder $repo_id/;
diff --git a/perl/Git/SVN/Prompt.pm b/perl/Git/SVN/Prompt.pm
new file mode 100644
index 00000000..3a6f8af0
--- /dev/null
+++ b/perl/Git/SVN/Prompt.pm
@@ -0,0 +1,202 @@
+package Git::SVN::Prompt;
+use strict;
+use warnings;
+require SVN::Core;
+use vars qw/$_no_auth_cache $_username/;
+
+sub simple {
+	my ($cred, $realm, $default_username, $may_save, $pool) = @_;
+	$may_save = undef if $_no_auth_cache;
+	$default_username = $_username if defined $_username;
+	if (defined $default_username && length $default_username) {
+		if (defined $realm && length $realm) {
+			print STDERR "Authentication realm: $realm\n";
+			STDERR->flush;
+		}
+		$cred->username($default_username);
+	} else {
+		username($cred, $realm, $may_save, $pool);
+	}
+	$cred->password(_read_password("Password for '" .
+	                               $cred->username . "': ", $realm));
+	$cred->may_save($may_save);
+	$SVN::_Core::SVN_NO_ERROR;
+}
+
+sub ssl_server_trust {
+	my ($cred, $realm, $failures, $cert_info, $may_save, $pool) = @_;
+	$may_save = undef if $_no_auth_cache;
+	print STDERR "Error validating server certificate for '$realm':\n";
+	{
+		no warnings 'once';
+		# All variables SVN::Auth::SSL::* are used only once,
+		# so we're shutting up Perl warnings about this.
+		if ($failures & $SVN::Auth::SSL::UNKNOWNCA) {
+			print STDERR " - The certificate is not issued ",
+			    "by a trusted authority. Use the\n",
+			    "   fingerprint to validate ",
+			    "the certificate manually!\n";
+		}
+		if ($failures & $SVN::Auth::SSL::CNMISMATCH) {
+			print STDERR " - The certificate hostname ",
+			    "does not match.\n";
+		}
+		if ($failures & $SVN::Auth::SSL::NOTYETVALID) {
+			print STDERR " - The certificate is not yet valid.\n";
+		}
+		if ($failures & $SVN::Auth::SSL::EXPIRED) {
+			print STDERR " - The certificate has expired.\n";
+		}
+		if ($failures & $SVN::Auth::SSL::OTHER) {
+			print STDERR " - The certificate has ",
+			    "an unknown error.\n";
+		}
+	} # no warnings 'once'
+	printf STDERR
+	        "Certificate information:\n".
+	        " - Hostname: %s\n".
+	        " - Valid: from %s until %s\n".
+	        " - Issuer: %s\n".
+	        " - Fingerprint: %s\n",
+	        map $cert_info->$_, qw(hostname valid_from valid_until
+	                               issuer_dname fingerprint);
+	my $choice;
+prompt:
+	print STDERR $may_save ?
+	      "(R)eject, accept (t)emporarily or accept (p)ermanently? " :
+	      "(R)eject or accept (t)emporarily? ";
+	STDERR->flush;
+	$choice = lc(substr(<STDIN> || 'R', 0, 1));
+	if ($choice =~ /^t$/i) {
+		$cred->may_save(undef);
+	} elsif ($choice =~ /^r$/i) {
+		return -1;
+	} elsif ($may_save && $choice =~ /^p$/i) {
+		$cred->may_save($may_save);
+	} else {
+		goto prompt;
+	}
+	$cred->accepted_failures($failures);
+	$SVN::_Core::SVN_NO_ERROR;
+}
+
+sub ssl_client_cert {
+	my ($cred, $realm, $may_save, $pool) = @_;
+	$may_save = undef if $_no_auth_cache;
+	print STDERR "Client certificate filename: ";
+	STDERR->flush;
+	chomp(my $filename = <STDIN>);
+	$cred->cert_file($filename);
+	$cred->may_save($may_save);
+	$SVN::_Core::SVN_NO_ERROR;
+}
+
+sub ssl_client_cert_pw {
+	my ($cred, $realm, $may_save, $pool) = @_;
+	$may_save = undef if $_no_auth_cache;
+	$cred->password(_read_password("Password: ", $realm));
+	$cred->may_save($may_save);
+	$SVN::_Core::SVN_NO_ERROR;
+}
+
+sub username {
+	my ($cred, $realm, $may_save, $pool) = @_;
+	$may_save = undef if $_no_auth_cache;
+	if (defined $realm && length $realm) {
+		print STDERR "Authentication realm: $realm\n";
+	}
+	my $username;
+	if (defined $_username) {
+		$username = $_username;
+	} else {
+		print STDERR "Username: ";
+		STDERR->flush;
+		chomp($username = <STDIN>);
+	}
+	$cred->username($username);
+	$cred->may_save($may_save);
+	$SVN::_Core::SVN_NO_ERROR;
+}
+
+sub _read_password {
+	my ($prompt, $realm) = @_;
+	my $password = '';
+	if (exists $ENV{GIT_ASKPASS}) {
+		open(PH, "-|", $ENV{GIT_ASKPASS}, $prompt);
+		$password = <PH>;
+		$password =~ s/[\012\015]//; # \n\r
+		close(PH);
+	} else {
+		print STDERR $prompt;
+		STDERR->flush;
+		require Term::ReadKey;
+		Term::ReadKey::ReadMode('noecho');
+		while (defined(my $key = Term::ReadKey::ReadKey(0))) {
+			last if $key =~ /[\012\015]/; # \n\r
+			$password .= $key;
+		}
+		Term::ReadKey::ReadMode('restore');
+		print STDERR "\n";
+		STDERR->flush;
+	}
+	$password;
+}
+
+1;
+__END__
+
+Git::SVN::Prompt - authentication callbacks for git-svn
+
+=head1 SYNOPSIS
+
+    use Git::SVN::Prompt qw(simple ssl_client_cert ssl_client_cert_pw
+                            ssl_server_trust username);
+    use SVN::Client ();
+
+    my $cached_simple = SVN::Client::get_simple_provider();
+    my $git_simple = SVN::Client::get_simple_prompt_provider(\&simple, 2);
+    my $cached_ssl = SVN::Client::get_ssl_server_trust_file_provider();
+    my $git_ssl = SVN::Client::get_ssl_server_trust_prompt_provider(
+        \&ssl_server_trust);
+    my $cached_cert = SVN::Client::get_ssl_client_cert_file_provider();
+    my $git_cert = SVN::Client::get_ssl_client_cert_prompt_provider(
+        \&ssl_client_cert, 2);
+    my $cached_cert_pw = SVN::Client::get_ssl_client_cert_pw_file_provider();
+    my $git_cert_pw = SVN::Client::get_ssl_client_cert_pw_prompt_provider(
+        \&ssl_client_cert_pw, 2);
+    my $cached_username = SVN::Client::get_username_provider();
+    my $git_username = SVN::Client::get_username_prompt_provider(
+        \&username, 2);
+
+    my $ctx = new SVN::Client(
+        auth => [
+            $cached_simple, $git_simple,
+            $cached_ssl, $git_ssl,
+            $cached_cert, $git_cert,
+            $cached_cert_pw, $git_cert_pw,
+            $cached_username, $git_username
+        ]);
+
+=head1 DESCRIPTION
+
+This module is an implementation detail of the "git svn" command.
+It implements git-svn's authentication policy.  Do not use it unless
+you are developing git-svn.
+
+The interface will change as git-svn evolves.
+
+=head1 DEPENDENCIES
+
+L<SVN::Core>.
+
+=head1 SEE ALSO
+
+L<SVN::Client>.
+
+=head1 INCOMPATIBILITIES
+
+None reported.
+
+=head1 BUGS
+
+None.
diff --git a/perl/Makefile.PL b/perl/Makefile.PL
index 456d45bf..4d8e31d2 100644
--- a/perl/Makefile.PL
+++ b/perl/Makefile.PL
@@ -27,6 +27,7 @@ MAKE_FRAG
 my %pm = (
 	'Git.pm' => '$(INST_LIBDIR)/Git.pm',
 	'Git/I18N.pm' => '$(INST_LIBDIR)/Git/I18N.pm',
+	'Git/SVN/Prompt.pm' => '$(INST_LIBDIR)/Git/SVN/Prompt.pm',
 );
 
 # We come with our own bundled Error.pm. It's not in the set of default

-- 
1.7.10
Previous: Eric WongNext: Jonathan Nieder
Message 13 of 26 in “git-svn: Destroy the cache when we fail to read it”
  1. git-svn: Destroy the cache when we fail to read itJason Gross, Aug 22, 2011
  2. Jason GrossAug 22, 2011
  3. Add tests for handling corrupted cachesJason Gross, Aug 22, 2011
  4. Jonathan NiederAug 23, 2011
  5. Jonathan NiederAug 23, 2011
  6. Jason GrossAug 23, 2011
  7. Eric WongAug 23, 2011
  8. Junio C HamanoAug 23, 2011
  9. Eric WongAug 23, 2011
  10. git-svn: use YAML format for mergeinfo cache when possibleJonathan Nieder, May 27, 2012
  11. 2/1 fixup! git-svn: use YAML format for mergeinfo cache when possibleJonathan Nieder, May 27, 2012
  12. Eric WongMay 27, 2012
  13. Jonathan NiederMay 28, 2012
  14. 0/2 git-svn: give SVN::Git::Fetcher its own fileJonathan Nieder, May 28, 2012
  15. 1/2 git-svn: rename SVN::Git::* packages to Git::SVN::*Jonathan Nieder, May 28, 2012
  16. 2/2 git-svn: make Git::SVN::Fetcher a separate fileJonathan Nieder, May 28, 2012
  17. Eric WongMay 29, 2012
  18. Junio C HamanoMay 29, 2012
  19. 0/3 git-svn: use YAML format for mergeinfo cache when possibleJonathan Nieder, Jun 9, 2012
  20. 1/3 git-svn: make Git::SVN::Editor a separate fileJonathan Nieder, Jun 9, 2012
  21. 2/3 git-svn: make Git::SVN::RA a separate fileJonathan Nieder, Jun 9, 2012
  22. 3/3 git-svn: use YAML format for mergeinfo cache when possibleJonathan Nieder, Jun 9, 2012
  23. Jonathan NiederJun 13, 2012
  24. Eric WongJun 10, 2012
  25. Jonathan NiederJun 10, 2012
  26. Junio C HamanoJun 11, 2012

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.