Re: [PATCH] git-svn: Remove unnecessary Git::SVN::Util package

[Eric Wong <normalperson@yhbt.net>]

"David D. Kilzer" <ddkilzer@kilzer.net> wrote:
> Digest::MD5 is loaded regardless of the package in which it's
> declared, so move its 'use' statement and the md5sum() function
> into the main package.
> 
> Signed-off-by: David D. Kilzer <ddkilzer@kilzer.net>

Acked-by: Eric Wong <normalperson@yhbt.net>

> ---
> 
> Ask Bjørn Hansen <ask@develooper.com> wrote:
> > On Nov 21, 2007, at 11:57, David D. Kilzer wrote:
> > > Created new Git::SVN::Util package with an md5sum() function.  A
> > > new package was created so that Digest::MD5 did not have to be
> > > loaded in the main package.
> > Huh?  It's all in the same file anyway, so what difference does it
> > make?
> 
> None!

I have floated the idea of splitting git-svn into several files to make
it easier to maintain and navigate.  However, I'm not looking foward to
doing a install system for that...

Maybe it'll just cat all the files together during build and
still install as one file... *shrug*

>  git-svn.perl |   23 +++++++++--------------
>  1 files changed, 9 insertions(+), 14 deletions(-)
> 
> diff --git a/git-svn.perl b/git-svn.perl
> index 62801c8..17d3020 100755
> --- a/git-svn.perl
> +++ b/git-svn.perl
> @@ -35,6 +35,7 @@ push @Git::SVN::Ra::ISA, 'SVN::Ra';
>  push @SVN::Git::Editor::ISA, 'SVN::Delta::Editor';
>  push @SVN::Git::Fetcher::ISA, 'SVN::Delta::Editor';
>  use Carp qw/croak/;
> +use Digest::MD5;
>  use IO::File qw//;
>  use File::Basename qw/dirname basename/;
>  use File::Path qw/mkpath/;
> @@ -48,8 +49,7 @@ BEGIN {
>  	foreach (qw/command command_oneline command_noisy command_output_pipe
>  	            command_input_pipe command_close_pipe/) {
>  		for my $package ( qw(SVN::Git::Editor SVN::Git::Fetcher
> -			Git::SVN::Migration Git::SVN::Log Git::SVN
> -			Git::SVN::Util),
> +			Git::SVN::Migration Git::SVN::Log Git::SVN),
>  			__PACKAGE__) {
>  			*{"${package}::$_"} = \&{"Git::$_"};
>  		}
> @@ -835,19 +835,19 @@ sub cmd_info {
>  			    command_output_pipe(qw(cat-file blob), "HEAD:$path");
>  			if ($file_type eq "link") {
>  				my $file_name = <$fh>;
> -				$checksum = Git::SVN::Util::md5sum("link $file_name");
> +				$checksum = md5sum("link $file_name");
>  			} else {
> -				$checksum = Git::SVN::Util::md5sum($fh);
> +				$checksum = md5sum($fh);
>  			}
>  			command_close_pipe($fh, $ctx);
>  		} elsif ($file_type eq "link") {
>  			my $file_name =
>  			    command(qw(cat-file blob), "HEAD:$path");
>  			$checksum =
> -			    Git::SVN::Util::md5sum("link " . $file_name);
> +			    md5sum("link " . $file_name);
>  		} else {
>  			open FILE, "<", $path or die $!;
> -			$checksum = Git::SVN::Util::md5sum(\*FILE);
> +			$checksum = md5sum(\*FILE);
>  			close FILE or die $!;
>  		}
>  		$result .= "Checksum: " . $checksum . "\n";
> @@ -1187,11 +1187,6 @@ sub find_file_type_and_diff_status {
>  	return ("file", $diff_status);
>  }
>  
> -package Git::SVN::Util;
> -use strict;
> -use warnings;
> -use Digest::MD5;
> -
>  sub md5sum {
>  	my $arg = shift;
>  	my $ref = ref $arg;
> @@ -2926,7 +2921,7 @@ sub apply_textdelta {
>  
>  		if (defined $exp) {
>  			seek $base, 0, 0 or croak $!;
> -			my $got = Git::SVN::Util::md5sum($base);
> +			my $got = ::md5sum($base);
>  			die "Checksum mismatch: $fb->{path} $fb->{blob}\n",
>  			    "expected: $exp\n",
>  			    "     got: $got\n" if ($got ne $exp);
> @@ -2945,7 +2940,7 @@ sub close_file {
>  	if (my $fh = $fb->{fh}) {
>  		if (defined $exp) {
>  			seek($fh, 0, 0) or croak $!;
> -			my $got = Git::SVN::Util::md5sum($fh);
> +			my $got = ::md5sum($fh);
>  			if ($got ne $exp) {
>  				die "Checksum mismatch: $path\n",
>  				    "expected: $exp\n    got: $got\n";
> @@ -3300,7 +3295,7 @@ sub chg_file {
>  	$fh->flush == 0 or croak $!;
>  	seek $fh, 0, 0 or croak $!;
>  
> -	my $exp = Git::SVN::Util::md5sum($fh);
> +	my $exp = ::md5sum($fh);
>  	seek $fh, 0, 0 or croak $!;
>  
>  	my $pool = SVN::Pool->new;
> -- 
> 1.5.3.4
> 
> -
> To unsubscribe from this list: send the line "unsubscribe git" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

-- 
Eric Wong