Re: [PATCH] Add git svn gc command

[Eric Wong <normalperson@yhbt.net>]

Robert Zeh <robert.a.zeh@gmail.com> wrote:
> Add a git svn gc command that gzips all unhandled.log files, and removes 
> all index files under .git/svn.
>
> Signed-off-by: Robert Allan Zeh <robert.a.zeh@gmail.com>
> ---
>  Documentation/git-svn.txt |    4 ++++
>  git-svn.perl              |   37 +++++++++++++++++++++++++++++++++++++
>  t/t9140-git-svn-gc.sh     |   45 ++++++++++++++++++++++++++++++++++++ 
> +++++++++
>  3 files changed, 86 insertions(+), 0 deletions(-)
>  create mode 100755 t/t9140-git-svn-gc.sh

Hi Robert,

Your mailer is mangling whitespace badly so the patch isn't applying
at all.  Make sure indents are real tabs like the rest of the code.

Some more comments below, but I think this will be a good addition
to git svn.

> --- a/git-svn.perl
> +++ b/git-svn.perl
> @@ -35,11 +35,14 @@ 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 Compress::Zlib;

I'd "require" Compress::Zlib lazilly so it's not loaded at startup.
It's not a stock component of Perl and not needed for the majority of
commands.

> +sub cmd_gc
> +{
> +    use Cwd;
> +       my $git_dir = $ENV{GIT_DIR};
> +    my $svn_dir = "$git_dir/svn";
> +    print getcwd;
> +    print "\n";

I'm not sure why you need to print getcwd there, leftover code from
testing/debugging?

> +sub gc_directory {
> +    if (-f $_ && basename($_) eq "unhandled.log") {
> +        my $out_filename = $_ . ".gz";
> +        print $out_filename, "\n";
> +        open my $in_fh, "<", $_ or die "Unable to open $_: $!\n";
> +        binmode $in_fh;
> +        my $gz = gzopen($out_filename, "wb") or die "Unable to open  
> $out_filename: $!\n";

gzopen with "ab" might be a better idea than "wb".  I'm not sure if
clobbering the existing log every time we run this command is a good
idea.

-- 
Eric Wong