{"thread":{"id":"31110","subject":"Extract Git::SVN from git-svn, take 2.","startedAt":"2012-07-26T23:22:21Z","lastAt":"2012-07-27T23:01:18Z","messageCount":30,"participants":["Michael G. Schwern","Junio C Hamano","Jonathan Nieder","Michael G Schwern","Eric Wong"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"195905","messageId":"1343344945-3717-1-git-send-email-schwern@pobox.com","threadId":"31110","inReplyTo":null,"subject":"Extract Git::SVN from git-svn, take 2.","fromName":"Michael G. Schwern","fromEmail":"schwern@pobox.com","sentAt":"2012-07-26T23:22:21Z","receivedAt":"2012-07-26T23:22:21Z","isPatch":false,"sender":{"key":"schwern@pobox.com","avatar":"https://avatars.githubusercontent.com/u/25888?v=4"},"body":"Same as before, now with tab indentation in the new Perl tests.\n\nAs before, patch #3 is 132k and will be rejected by some of the lists.\n"},{"id":"195906","messageId":"1343344945-3717-2-git-send-email-schwern@pobox.com","threadId":"31110","inReplyTo":"1343344945-3717-1-git-send-email-schwern@pobox.com","subject":"[PATCH 1/4] Extract some utilities from git-svn to allow extracting Git::SVN.","fromName":"Michael G. Schwern","fromEmail":"schwern@pobox.com","sentAt":"2012-07-26T23:22:22Z","receivedAt":"2012-07-26T23:22:22Z","isPatch":true,"sender":{"key":"schwern@pobox.com","avatar":"https://avatars.githubusercontent.com/u/25888?v=4"},"body":"From: \"Michael G. Schwern\" <schwern@pobox.com>\n\nPut them in a new module called Git::SVN::Utils.  Yeah, not terribly\noriginal and it will be a dumping ground.  But its better than having\nthem in the main git-svn program.  At least they can be documented\nand tested.\n\n* fatal() is used by many classes.\n* Change the $can_compress lexical into a function.\n\nThis should be enough to extract Git::SVN.\n\nSigned-off-by: Michael G. Schwern <schwern@pobox.com>\n---\n git-svn.perl                   | 34 +++++++++++++-----------\n perl/Git/SVN/Utils.pm          | 59 ++++++++++++++++++++++++++++++++++++++++++\n perl/Makefile                  |  1 +\n t/Git-SVN/00compile.t          |  8 ++++++\n t/Git-SVN/Utils/can_compress.t | 11 ++++++++\n t/Git-SVN/Utils/fatal.t        | 34 ++++++++++++++++++++++++\n 6 files changed, 132 insertions(+), 15 deletions(-)\n create mode 100644 perl/Git/SVN/Utils.pm\n create mode 100644 t/Git-SVN/00compile.t\n create mode 100644 t/Git-SVN/Utils/can_compress.t\n create mode 100644 t/Git-SVN/Utils/fatal.t\n\ndiff --git a/git-svn.perl b/git-svn.perl\nindex 0b074c4..79fe4a4 100755\n--- a/git-svn.perl\n+++ b/git-svn.perl\n@@ -10,6 +10,8 @@ use vars qw/\t$AUTHOR $VERSION\n $AUTHOR = 'Eric Wong <normalperson@yhbt.net>';\n $VERSION = '@@GIT_VERSION@@';\n \n+use Git::SVN::Utils qw(fatal can_compress);\n+\n # From which subdir have we been invoked?\n my $cmd_dir_prefix = eval {\n \tcommand_oneline([qw/rev-parse --show-prefix/], STDERR => 0)\n@@ -35,8 +37,6 @@ $Git::SVN::Log::TZ = $ENV{TZ};\n $ENV{TZ} = 'UTC';\n $| = 1; # unbuffer STDOUT\n \n-sub fatal (@) { print STDERR \"@_\\n\"; exit 1 }\n-\n # All SVN commands do it.  Otherwise we may die on SIGPIPE when the remote\n # repository decides to close the connection which we expect to be kept alive.\n $SIG{PIPE} = 'IGNORE';\n@@ -66,7 +66,7 @@ sub _req_svn {\n \t\tfatal \"Need SVN::Core 1.1.0 or better (got $SVN::Core::VERSION)\";\n \t}\n }\n-my $can_compress = eval { require Compress::Zlib; 1};\n+\n use Carp qw/croak/;\n use Digest::MD5;\n use IO::File qw//;\n@@ -1578,7 +1578,7 @@ sub cmd_reset {\n }\n \n sub cmd_gc {\n-\tif (!$can_compress) {\n+\tif (!can_compress()) {\n \t\twarn \"Compress::Zlib could not be found; unhandled.log \" .\n \t\t     \"files will not be compressed.\\n\";\n \t}\n@@ -2014,13 +2014,13 @@ sub md5sum {\n \t} elsif (!$ref) {\n \t\t$md5->add($arg) or croak $!;\n \t} else {\n-\t\t::fatal \"Can't provide MD5 hash for unknown ref type: '\", $ref, \"'\";\n+\t\tfatal \"Can't provide MD5 hash for unknown ref type: '\", $ref, \"'\";\n \t}\n \treturn $md5->hexdigest();\n }\n \n sub gc_directory {\n-\tif ($can_compress && -f $_ && basename($_) eq \"unhandled.log\") {\n+\tif (can_compress() && -f $_ && basename($_) eq \"unhandled.log\") {\n \t\tmy $out_filename = $_ . \".gz\";\n \t\topen my $in_fh, \"<\", $_ or die \"Unable to open $_: $!\\n\";\n \t\tbinmode $in_fh;\n@@ -2055,6 +2055,9 @@ use Time::Local;\n use Memoize;  # core since 5.8.0, Jul 2002\n use Memoize::Storable;\n use POSIX qw(:signal_h);\n+\n+use Git::SVN::Utils qw(fatal can_compress);\n+\n my $can_use_yaml;\n BEGIN {\n \t$can_use_yaml = eval { require Git::SVN::Memoize::YAML; 1};\n@@ -2880,8 +2883,8 @@ sub assert_index_clean {\n \t\tcommand_noisy('read-tree', $treeish);\n \t\t$x = command_oneline('write-tree');\n \t\tif ($y ne $x) {\n-\t\t\t::fatal \"trees ($treeish) $y != $x\\n\",\n-\t\t\t        \"Something is seriously wrong...\";\n+\t\t\tfatal \"trees ($treeish) $y != $x\\n\",\n+\t\t\t      \"Something is seriously wrong...\";\n \t\t}\n \t});\n }\n@@ -3236,7 +3239,7 @@ sub mkemptydirs {\n \tmy %empty_dirs = ();\n \tmy $gz_file = \"$self->{dir}/unhandled.log.gz\";\n \tif (-f $gz_file) {\n-\t\tif (!$can_compress) {\n+\t\tif (!can_compress()) {\n \t\t\twarn \"Compress::Zlib could not be found; \",\n \t\t\t     \"empty directories in $gz_file will not be read\\n\";\n \t\t} else {\n@@ -3919,7 +3922,7 @@ sub set_tree {\n \tmy ($self, $tree) = (shift, shift);\n \tmy $log_entry = ::get_commit_entry($tree);\n \tunless ($self->{last_rev}) {\n-\t\t::fatal(\"Must have an existing revision to commit\");\n+\t\tfatal(\"Must have an existing revision to commit\");\n \t}\n \tmy %ed_opts = ( r => $self->{last_rev},\n \t                log => $log_entry->{log},\n@@ -4348,6 +4351,7 @@ sub remove_username {\n package Git::SVN::Log;\n use strict;\n use warnings;\n+use Git::SVN::Utils qw(fatal);\n use POSIX qw/strftime/;\n use constant commit_log_separator => ('-' x 72) . \"\\n\";\n use vars qw/$TZ $limit $color $pager $non_recursive $verbose $oneline\n@@ -4446,15 +4450,15 @@ sub config_pager {\n sub run_pager {\n \treturn unless defined $pager;\n \tpipe my ($rfd, $wfd) or return;\n-\tdefined(my $pid = fork) or ::fatal \"Can't fork: $!\";\n+\tdefined(my $pid = fork) or fatal \"Can't fork: $!\";\n \tif (!$pid) {\n \t\topen STDOUT, '>&', $wfd or\n-\t\t                     ::fatal \"Can't redirect to stdout: $!\";\n+\t\t                     fatal \"Can't redirect to stdout: $!\";\n \t\treturn;\n \t}\n-\topen STDIN, '<&', $rfd or ::fatal \"Can't redirect stdin: $!\";\n+\topen STDIN, '<&', $rfd or fatal \"Can't redirect stdin: $!\";\n \t$ENV{LESS} ||= 'FRSX';\n-\texec $pager or ::fatal \"Can't run pager: $! ($pager)\";\n+\texec $pager or fatal \"Can't run pager: $! ($pager)\";\n }\n \n sub format_svn_date {\n@@ -4603,7 +4607,7 @@ sub cmd_show_log {\n \t\t} elsif ($::_revision =~ /^\\d+$/) {\n \t\t\t$r_min = $r_max = $::_revision;\n \t\t} else {\n-\t\t\t::fatal \"-r$::_revision is not supported, use \",\n+\t\t\tfatal \"-r$::_revision is not supported, use \",\n \t\t\t\t\"standard 'git log' arguments instead\";\n \t\t}\n \t}\ndiff --git a/perl/Git/SVN/Utils.pm b/perl/Git/SVN/Utils.pm\nnew file mode 100644\nindex 0000000..3d0bfa4\n--- /dev/null\n+++ b/perl/Git/SVN/Utils.pm\n@@ -0,0 +1,59 @@\n+package Git::SVN::Utils;\n+\n+use strict;\n+use warnings;\n+\n+use base qw(Exporter);\n+\n+our @EXPORT_OK = qw(fatal can_compress);\n+\n+\n+=head1 NAME\n+\n+Git::SVN::Utils - utility functions used across Git::SVN\n+\n+=head1 SYNOPSIS\n+\n+    use Git::SVN::Utils qw(functions to import);\n+\n+=head1 DESCRIPTION\n+\n+This module contains functions which are useful across many different\n+parts of Git::SVN.  Mostly it's a place to put utility functions\n+rather than duplicate the code or have classes grabbing at other\n+classes.\n+\n+=head1 FUNCTIONS\n+\n+All functions can be imported only on request.\n+\n+=head3 fatal\n+\n+    fatal(@message);\n+\n+Display a message and exit with a fatal error code.\n+\n+=cut\n+\n+# Note: not certain why this is in use instead of die.  Probably because\n+# the exit code of die is 255?  Doesn't appear to be used consistently.\n+sub fatal (@) { print STDERR \"@_\\n\"; exit 1 }\n+\n+\n+=head3 can_compress\n+\n+    my $can_compress = can_compress;\n+\n+Returns true if Compress::Zlib is available, false otherwise.\n+\n+=cut\n+\n+my $can_compress;\n+sub can_compress {\n+    return $can_compress if defined $can_compress;\n+\n+    return $can_compress = eval { require Compress::Zlib; } ? 1 : 0;\n+}\n+\n+\n+1;\ndiff --git a/perl/Makefile b/perl/Makefile\nindex 6ca7d47..24a9f5a 100644\n--- a/perl/Makefile\n+++ b/perl/Makefile\n@@ -31,6 +31,7 @@ modules += Git/SVN/Fetcher\n modules += Git/SVN/Editor\n modules += Git/SVN/Prompt\n modules += Git/SVN/Ra\n+modules += Git/SVN/Utils\n \n $(makfile): ../GIT-CFLAGS Makefile\n \techo all: private-Error.pm Git.pm Git/I18N.pm > $@\ndiff --git a/t/Git-SVN/00compile.t b/t/Git-SVN/00compile.t\nnew file mode 100644\nindex 0000000..a7aa85a\n--- /dev/null\n+++ b/t/Git-SVN/00compile.t\n@@ -0,0 +1,8 @@\n+#!/usr/bin/env perl\n+\n+use strict;\n+use warnings;\n+\n+use Test::More tests => 1;\n+\n+require_ok 'Git::SVN::Utils';\ndiff --git a/t/Git-SVN/Utils/can_compress.t b/t/Git-SVN/Utils/can_compress.t\nnew file mode 100644\nindex 0000000..d7b49b8\n--- /dev/null\n+++ b/t/Git-SVN/Utils/can_compress.t\n@@ -0,0 +1,11 @@\n+#!/usr/bin/perl\n+\n+use strict;\n+use warnings;\n+\n+use Test::More 'no_plan';\n+\n+use Git::SVN::Utils qw(can_compress);\n+\n+# !! is the \"convert this to boolean\" operator.\n+is !!can_compress(), !!eval { require Compress::Zlib };\ndiff --git a/t/Git-SVN/Utils/fatal.t b/t/Git-SVN/Utils/fatal.t\nnew file mode 100644\nindex 0000000..49e1438\n--- /dev/null\n+++ b/t/Git-SVN/Utils/fatal.t\n@@ -0,0 +1,34 @@\n+#!/usr/bin/perl\n+\n+use strict;\n+use warnings;\n+\n+use Test::More 'no_plan';\n+\n+BEGIN {\n+\t# Override exit at BEGIN time before Git::SVN::Utils is loaded\n+\t# so it will see our local exit later.\n+\t*CORE::GLOBAL::exit = sub(;$) {\n+\treturn @_ ? CORE::exit($_[0]) : CORE::exit();\n+\t};\n+}\n+\n+use Git::SVN::Utils qw(fatal);\n+\n+# fatal()\n+{\n+\t# Capture the exit code and prevent exit.\n+\tmy $exit_status;\n+\tno warnings 'redefine';\n+\tlocal *CORE::GLOBAL::exit = sub { $exit_status = $_[0] || 0 };\n+\n+\t# Trap fatal's message to STDERR\n+\tmy $stderr;\n+\tclose STDERR;\n+\tok open STDERR, \">\", \\$stderr;\n+\n+\tfatal \"Some\", \"Stuff\", \"Happened\";\n+\n+\tis $stderr, \"Some Stuff Happened\\n\";\n+\tis $exit_status, 1;\n+}\n-- \n1.7.11.1\n"},{"id":"195907","messageId":"1343344945-3717-3-git-send-email-schwern@pobox.com","threadId":"31110","inReplyTo":"1343344945-3717-1-git-send-email-schwern@pobox.com","subject":"[PATCH 2/4] Prepare Git::SVN for extraction into its own file.","fromName":"Michael G. Schwern","fromEmail":"schwern@pobox.com","sentAt":"2012-07-26T23:22:23Z","receivedAt":"2012-07-26T23:22:23Z","isPatch":true,"sender":{"key":"schwern@pobox.com","avatar":"https://avatars.githubusercontent.com/u/25888?v=4"},"body":"From: \"Michael G. Schwern\" <schwern@pobox.com>\n\nThis means it should be able to load without git-svn being loaded.\n\n* Load Git.pm on its own and all the needed command functions.\n\n* It needs to grab at a git-svn lexical $_prefix representing the --prefix\n  option.  Provide opt_prefix() for that.  This is a refactoring artifact.\n  The prefix should really be passed into Git::SVN->new.\n\n* Unqualify unnecessarily fully qualified globals like\n  $Git::SVN::default_repo_id.\n\n* Lexically isolate the class just to make sure nothing is leaking out.\n---\n git-svn.perl | 22 ++++++++++++++++++----\n 1 file changed, 18 insertions(+), 4 deletions(-)\n\ndiff --git a/git-svn.perl b/git-svn.perl\nindex 79fe4a4..9cdf6fc 100755\n--- a/git-svn.perl\n+++ b/git-svn.perl\n@@ -89,7 +89,7 @@ BEGIN {\n \tforeach (qw/command command_oneline command_noisy command_output_pipe\n \t            command_input_pipe command_close_pipe\n \t            command_bidi_pipe command_close_bidi_pipe/) {\n-\t\tfor my $package ( qw(Git::SVN::Migration Git::SVN::Log Git::SVN),\n+\t\tfor my $package ( qw(Git::SVN::Migration Git::SVN::Log),\n \t\t\t__PACKAGE__) {\n \t\t\t*{\"${package}::$_\"} = \\&{\"Git::$_\"};\n \t\t}\n@@ -109,6 +109,10 @@ my ($_stdin, $_help, $_edit,\n \t$_merge, $_strategy, $_preserve_merges, $_dry_run, $_local,\n \t$_prefix, $_no_checkout, $_url, $_verbose,\n \t$_git_format, $_commit_url, $_tag, $_merge_info, $_interactive);\n+\n+# This is a refactoring artifact so Git::SVN can get at this git-svn switch.\n+sub opt_prefix { return $_prefix || '' }\n+\n $Git::SVN::_follow_parent = 1;\n $Git::SVN::Fetcher::_placeholder_filename = \".gitignore\";\n $_q ||= 0;\n@@ -2038,6 +2042,7 @@ sub gc_directory {\n \t}\n }\n \n+{\n package Git::SVN;\n use strict;\n use warnings;\n@@ -2056,6 +2061,13 @@ use Memoize;  # core since 5.8.0, Jul 2002\n use Memoize::Storable;\n use POSIX qw(:signal_h);\n \n+use Git qw(\n+    command\n+    command_oneline\n+    command_noisy\n+    command_output_pipe\n+    command_close_pipe\n+);\n use Git::SVN::Utils qw(fatal can_compress);\n \n my $can_use_yaml;\n@@ -4280,12 +4292,13 @@ sub find_rev_after {\n sub _new {\n \tmy ($class, $repo_id, $ref_id, $path) = @_;\n \tunless (defined $repo_id && length $repo_id) {\n-\t\t$repo_id = $Git::SVN::default_repo_id;\n+\t\t$repo_id = $default_repo_id;\n \t}\n \tunless (defined $ref_id && length $ref_id) {\n-\t\t$_prefix = '' unless defined($_prefix);\n+\t\t# Access the prefix option from the git-svn main program if it's loaded.\n+\t\tmy $prefix = defined &::opt_prefix ? ::opt_prefix() : \"\";\n \t\t$_[2] = $ref_id =\n-\t\t             \"refs/remotes/$_prefix$Git::SVN::default_ref_id\";\n+\t\t             \"refs/remotes/$prefix$default_ref_id\";\n \t}\n \t$_[1] = $repo_id;\n \tmy $dir = \"$ENV{GIT_DIR}/svn/$ref_id\";\n@@ -4347,6 +4360,7 @@ sub uri_decode {\n sub remove_username {\n \t$_[0] =~ s{^([^:]*://)[^@]+@}{$1};\n }\n+}\n \n package Git::SVN::Log;\n use strict;\n-- \n1.7.11.1\n"},{"id":"195908","messageId":"1343344945-3717-5-git-send-email-schwern@pobox.com","threadId":"31110","inReplyTo":"1343344945-3717-1-git-send-email-schwern@pobox.com","subject":"[PATCH 4/4] Move initialization of Git::SVN variables into Git::SVN.","fromName":"Michael G. Schwern","fromEmail":"schwern@pobox.com","sentAt":"2012-07-26T23:22:25Z","receivedAt":"2012-07-26T23:22:25Z","isPatch":true,"sender":{"key":"schwern@pobox.com","avatar":"https://avatars.githubusercontent.com/u/25888?v=4"},"body":"From: \"Michael G. Schwern\" <schwern@pobox.com>\n\nAlso it can compile on its own now, yay!\n---\n git-svn.perl          | 4 ----\n perl/Git/SVN.pm       | 9 +++++++--\n t/Git-SVN/00compile.t | 3 ++-\n 3 files changed, 9 insertions(+), 7 deletions(-)\n\ndiff --git a/git-svn.perl b/git-svn.perl\nindex 4c77f69..ef10f6f 100755\n--- a/git-svn.perl\n+++ b/git-svn.perl\n@@ -20,10 +20,7 @@ my $cmd_dir_prefix = eval {\n \n my $git_dir_user_set = 1 if defined $ENV{GIT_DIR};\n $ENV{GIT_DIR} ||= '.git';\n-$Git::SVN::default_repo_id = 'svn';\n-$Git::SVN::default_ref_id = $ENV{GIT_SVN_ID} || 'git-svn';\n $Git::SVN::Ra::_log_window_size = 100;\n-$Git::SVN::_minimize_url = 'unset';\n \n if (! exists $ENV{SVN_SSH} && exists $ENV{GIT_SSH}) {\n \t$ENV{SVN_SSH} = $ENV{GIT_SSH};\n@@ -114,7 +111,6 @@ my ($_stdin, $_help, $_edit,\n # This is a refactoring artifact so Git::SVN can get at this git-svn switch.\n sub opt_prefix { return $_prefix || '' }\n \n-$Git::SVN::_follow_parent = 1;\n $Git::SVN::Fetcher::_placeholder_filename = \".gitignore\";\n $_q ||= 0;\n my %remote_opts = ( 'username=s' => \\$Git::SVN::Prompt::_username,\ndiff --git a/perl/Git/SVN.pm b/perl/Git/SVN.pm\nindex c71c041..2e0d7f0 100644\n--- a/perl/Git/SVN.pm\n+++ b/perl/Git/SVN.pm\n@@ -3,9 +3,9 @@ use strict;\n use warnings;\n use Fcntl qw/:DEFAULT :seek/;\n use constant rev_map_fmt => 'NH40';\n-use vars qw/$default_repo_id $default_ref_id $_no_metadata $_follow_parent\n+use vars qw/$_no_metadata\n             $_repack $_repack_flags $_use_svm_props $_head\n-            $_use_svnsync_props $no_reuse_existing $_minimize_url\n+            $_use_svnsync_props $no_reuse_existing\n \t    $_use_log_author $_add_author_from $_localtime/;\n use Carp qw/croak/;\n use File::Path qw/mkpath/;\n@@ -30,6 +30,11 @@ BEGIN {\n \t$can_use_yaml = eval { require Git::SVN::Memoize::YAML; 1};\n }\n \n+our $_follow_parent  = 1;\n+our $_minimize_url   = 'unset';\n+our $default_repo_id = 'svn';\n+our $default_ref_id  = $ENV{GIT_SVN_ID} || 'git-svn';\n+\n my ($_gc_nr, $_gc_period);\n \n # properties that we do not log:\ndiff --git a/t/Git-SVN/00compile.t b/t/Git-SVN/00compile.t\nindex a7aa85a..97475d9 100644\n--- a/t/Git-SVN/00compile.t\n+++ b/t/Git-SVN/00compile.t\n@@ -3,6 +3,7 @@\n use strict;\n use warnings;\n \n-use Test::More tests => 1;\n+use Test::More tests => 2;\n \n require_ok 'Git::SVN::Utils';\n+require_ok 'Git::SVN';\n-- \n1.7.11.1\n"},{"id":"195922","messageId":"7vvch93hpy.fsf@alter.siamese.dyndns.org","threadId":"31110","inReplyTo":"1343344945-3717-3-git-send-email-schwern@pobox.com","subject":"Re: [PATCH 2/4] Prepare Git::SVN for extraction into its own file.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-07-27T05:18:01Z","receivedAt":"2012-07-27T05:18:01Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Michael G. Schwern\" <schwern@pobox.com> writes:\n\n> From: \"Michael G. Schwern\" <schwern@pobox.com>\n>\n> This means it should be able to load without git-svn being loaded.\n>\n> * Load Git.pm on its own and all the needed command functions.\n>\n> * It needs to grab at a git-svn lexical $_prefix representing the --prefix\n>   option.  Provide opt_prefix() for that.  This is a refactoring artifact.\n>   The prefix should really be passed into Git::SVN->new.\n\nI agree that the prefix is part of SVN->new arguments in the final\nstate after applying the whole series (not just these four but also\nwith the follow-up patches).\n\n> * Unqualify unnecessarily fully qualified globals like\n>   $Git::SVN::default_repo_id.\n>\n> * Lexically isolate the class just to make sure nothing is leaking out.\n> ---\n\nForgot to sign-off, or are you still unsure about this step?\n\n> diff --git a/git-svn.perl b/git-svn.perl\n> index 79fe4a4..9cdf6fc 100755\n> --- a/git-svn.perl\n> +++ b/git-svn.perl\n> @@ -109,6 +109,10 @@ my ($_stdin, $_help, $_edit,\n>  \t$_merge, $_strategy, $_preserve_merges, $_dry_run, $_local,\n>  \t$_prefix, $_no_checkout, $_url, $_verbose,\n>  \t$_git_format, $_commit_url, $_tag, $_merge_info, $_interactive);\n> +\n> +# This is a refactoring artifact so Git::SVN can get at this git-svn switch.\n> +sub opt_prefix { return $_prefix || '' }\n> +\n>  $Git::SVN::_follow_parent = 1;\n>  $Git::SVN::Fetcher::_placeholder_filename = \".gitignore\";\n>  $_q ||= 0;\n> @@ -4280,12 +4292,13 @@ sub find_rev_after {\n>  sub _new {\n>  \tmy ($class, $repo_id, $ref_id, $path) = @_;\n>  \tunless (defined $repo_id && length $repo_id) {\n> -\t\t$repo_id = $Git::SVN::default_repo_id;\n> +\t\t$repo_id = $default_repo_id;\n>  \t}\n>  \tunless (defined $ref_id && length $ref_id) {\n> -\t\t$_prefix = '' unless defined($_prefix);\n> +\t\t# Access the prefix option from the git-svn main program if it's loaded.\n> +\t\tmy $prefix = defined &::opt_prefix ? ::opt_prefix() : \"\";\n\nAgain, I agree with you that passing $prefix as one of the arguments\nto ->new is the right thing to do in the final state after applying\nthe whole series.  I don't know if later steps in your patch series\nwill do so, but it _might_ make more sense to update ->new and its\ncallers to do so without doing anything else first, so that you do\nnot have to call out to the ::opt_prefix() when you split things\nout.\n"},{"id":"195923","messageId":"7vobn13hps.fsf@alter.siamese.dyndns.org","threadId":"31110","inReplyTo":"1343344945-3717-2-git-send-email-schwern@pobox.com","subject":"Re: [PATCH 1/4] Extract some utilities from git-svn to allow extracting Git::SVN.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-07-27T05:18:07Z","receivedAt":"2012-07-27T05:18:07Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Michael G. Schwern\" <schwern@pobox.com> writes:\n\n> From: \"Michael G. Schwern\" <schwern@pobox.com>\n>\n> Put them in a new module called Git::SVN::Utils.  Yeah, not terribly\n> original and it will be a dumping ground.  But its better than having\n> them in the main git-svn program.  At least they can be documented\n> and tested.\n>\n> * fatal() is used by many classes.\n> * Change the $can_compress lexical into a function.\n>\n> This should be enough to extract Git::SVN.\n>\n> Signed-off-by: Michael G. Schwern <schwern@pobox.com>\n> ---\n\nLooks good.\n\n> diff --git a/perl/Git/SVN/Utils.pm b/perl/Git/SVN/Utils.pm\n> new file mode 100644\n> index 0000000..3d0bfa4\n> --- /dev/null\n> +++ b/perl/Git/SVN/Utils.pm\n> @@ -0,0 +1,59 @@\n> ...\n> +=head1 FUNCTIONS\n> +\n> +All functions can be imported only on request.\n> +\n> +=head3 fatal\n> +\n> +    fatal(@message);\n> +\n> +Display a message and exit with a fatal error code.\n> +\n> +=cut\n> +\n> +# Note: not certain why this is in use instead of die.  Probably because\n> +# the exit code of die is 255?  Doesn't appear to be used consistently.\n> +sub fatal (@) { print STDERR \"@_\\n\"; exit 1 }\n\nVery true.  Also I do not think the line-noise prototype buys us\nanything (other than making the code look mysterious to non Perl\nprogrammers); we are not emulating any Perl's builtin with this\nfunction, and I do not see a reason why we want to force list\ncontext to its arguments, either.  But removal of it is not part of\nthis step anyway, so I wouldn't complain.\n\n> +=head3 can_compress\n> +\n> +    my $can_compress = can_compress;\n> +\n> +Returns true if Compress::Zlib is available, false otherwise.\n> +\n> +=cut\n> +\n> +my $can_compress;\n> +sub can_compress {\n> +    return $can_compress if defined $can_compress;\n> +\n> +    return $can_compress = eval { require Compress::Zlib; } ? 1 : 0;\n> +}\n\nThe original said \"eval { require Compress::Zlib; 1; }\"; presumably,\nwhen require does succeed, the value inside is the \"1;\" that has to\nbe at the end of Compress::Zlib, so the difference should not matter.\n"},{"id":"195924","messageId":"7vhast3hpb.fsf@alter.siamese.dyndns.org","threadId":"31110","inReplyTo":"1343344945-3717-5-git-send-email-schwern@pobox.com","subject":"Re: [PATCH 4/4] Move initialization of Git::SVN variables into Git::SVN.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-07-27T05:18:24Z","receivedAt":"2012-07-27T05:18:24Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Michael G. Schwern\" <schwern@pobox.com> writes:\n\n> From: \"Michael G. Schwern\" <schwern@pobox.com>\n>\n> Also it can compile on its own now, yay!\n\nHmmm.\n\nIf you swap the order of steps 3/4 and 4/4 by creating Git/SVN.pm\nthat only has these variable definitions (i.e. \"our $X\" and \"use\nvars $X\") and make git-svn.perl use them from Git::SVN in the first\nstep, and then do the bulk-moving (equivalent of your 3/4) in the\nsecond step, would it free you from having to say \"it's doubtful it\nwill compile by itself\"?\n\nIn short:\n\n - I didn't see anything questionable in 1/4;\n\n - Calling up ::opt_prefix() from module in 2/4 looked ugly to me\n   but I suspect it should be easy to fix;\n\n - 3/4 was a straight move and I didn't see anything questionable in\n   it, but I think it would be nicer if intermediate steps can be\n   made to still work by making 4/4 come first or something\n   similarly simple.\n\nIf the issues in 2/4 and 3/4 are easily fixable by going the route I\nhandwaved above, the result of doing so based on this round is ready\nto be applied, I think.\n\nEric, Jonathan, what do you think?\n\n> ---\n>  git-svn.perl          | 4 ----\n>  perl/Git/SVN.pm       | 9 +++++++--\n>  t/Git-SVN/00compile.t | 3 ++-\n>  3 files changed, 9 insertions(+), 7 deletions(-)\n>\n> diff --git a/git-svn.perl b/git-svn.perl\n> index 4c77f69..ef10f6f 100755\n> --- a/git-svn.perl\n> +++ b/git-svn.perl\n> @@ -20,10 +20,7 @@ my $cmd_dir_prefix = eval {\n>  \n>  my $git_dir_user_set = 1 if defined $ENV{GIT_DIR};\n>  $ENV{GIT_DIR} ||= '.git';\n> -$Git::SVN::default_repo_id = 'svn';\n> -$Git::SVN::default_ref_id = $ENV{GIT_SVN_ID} || 'git-svn';\n>  $Git::SVN::Ra::_log_window_size = 100;\n> -$Git::SVN::_minimize_url = 'unset';\n>  \n>  if (! exists $ENV{SVN_SSH} && exists $ENV{GIT_SSH}) {\n>  \t$ENV{SVN_SSH} = $ENV{GIT_SSH};\n> @@ -114,7 +111,6 @@ my ($_stdin, $_help, $_edit,\n>  # This is a refactoring artifact so Git::SVN can get at this git-svn switch.\n>  sub opt_prefix { return $_prefix || '' }\n>  \n> -$Git::SVN::_follow_parent = 1;\n>  $Git::SVN::Fetcher::_placeholder_filename = \".gitignore\";\n>  $_q ||= 0;\n>  my %remote_opts = ( 'username=s' => \\$Git::SVN::Prompt::_username,\n> diff --git a/perl/Git/SVN.pm b/perl/Git/SVN.pm\n> index c71c041..2e0d7f0 100644\n> --- a/perl/Git/SVN.pm\n> +++ b/perl/Git/SVN.pm\n> @@ -3,9 +3,9 @@ use strict;\n>  use warnings;\n>  use Fcntl qw/:DEFAULT :seek/;\n>  use constant rev_map_fmt => 'NH40';\n> -use vars qw/$default_repo_id $default_ref_id $_no_metadata $_follow_parent\n> +use vars qw/$_no_metadata\n>              $_repack $_repack_flags $_use_svm_props $_head\n> -            $_use_svnsync_props $no_reuse_existing $_minimize_url\n> +            $_use_svnsync_props $no_reuse_existing\n>  \t    $_use_log_author $_add_author_from $_localtime/;\n>  use Carp qw/croak/;\n>  use File::Path qw/mkpath/;\n> @@ -30,6 +30,11 @@ BEGIN {\n>  \t$can_use_yaml = eval { require Git::SVN::Memoize::YAML; 1};\n>  }\n>  \n> +our $_follow_parent  = 1;\n> +our $_minimize_url   = 'unset';\n> +our $default_repo_id = 'svn';\n> +our $default_ref_id  = $ENV{GIT_SVN_ID} || 'git-svn';\n> +\n>  my ($_gc_nr, $_gc_period);\n>  \n>  # properties that we do not log:\n> diff --git a/t/Git-SVN/00compile.t b/t/Git-SVN/00compile.t\n> index a7aa85a..97475d9 100644\n> --- a/t/Git-SVN/00compile.t\n> +++ b/t/Git-SVN/00compile.t\n> @@ -3,6 +3,7 @@\n>  use strict;\n>  use warnings;\n>  \n> -use Test::More tests => 1;\n> +use Test::More tests => 2;\n>  \n>  require_ok 'Git::SVN::Utils';\n> +require_ok 'Git::SVN';\n"},{"id":"195925","messageId":"7va9yl3hgf.fsf@alter.siamese.dyndns.org","threadId":"31110","inReplyTo":"7vvch93hpy.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 2/4] Prepare Git::SVN for extraction into its own file.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-07-27T05:23:44Z","receivedAt":"2012-07-27T05:23:44Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano <gitster@pobox.com> writes:\n\n> \"Michael G. Schwern\" <schwern@pobox.com> writes:\n>\n>> From: \"Michael G. Schwern\" <schwern@pobox.com>\n>>\n>> This means it should be able to load without git-svn being loaded.\n>>\n>> * Load Git.pm on its own and all the needed command functions.\n>>\n>> * It needs to grab at a git-svn lexical $_prefix representing the --prefix\n>>   option.  Provide opt_prefix() for that.  This is a refactoring artifact.\n>>   The prefix should really be passed into Git::SVN->new.\n>\n> I agree that the prefix is part of SVN->new arguments in the final\n\ns/is/should be/; sorry for the noise.\n\n> state after applying the whole series (not just these four but also\n> with the follow-up patches).\n> ...\n> Again, I agree with you that passing $prefix as one of the arguments\n> to ->new is the right thing to do in the final state after applying\n> the whole series.  I don't know if later steps in your patch series\n> will do so, but it _might_ make more sense to update ->new and its\n> callers to do so without doing anything else first, so that you do\n> not have to call out to the ::opt_prefix() when you split things\n> out.\n"},{"id":"195926","messageId":"20120727053800.GC4685@burratino","threadId":"31110","inReplyTo":"7vhast3hpb.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 4/4] Move initialization of Git::SVN variables into Git::SVN.","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2012-07-27T05:38:00Z","receivedAt":"2012-07-27T05:38:00Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Junio C Hamano wrote:\n> \"Michael G. Schwern\" <schwern@pobox.com> writes:\n\n>> Also it can compile on its own now, yay!\n>\n> Hmmm.\n\nI agree with Michael's \"yay\" and also think it's fine that after\npatch 3 it isn't there yet.\n\nThat's because git-svn.perl doesn't use Git::SVN on its own but helps\nit out a little.  So even if we only applied patches 1-3, git-svn\nwould still work (maybe it's worth testing \"perl -MGit::SVN\" by hand\nto avoid the \"it's doubtful\" about whether Git::SVN is self-contained\nand replace it with a more certain statement?), and patch 4 just makes\nit even better.\n\n[...]\n> In short:\n>\n>  - I didn't see anything questionable in 1/4;\n>\n>  - Calling up ::opt_prefix() from module in 2/4 looked ugly to me\n>    but I suspect it should be easy to fix;\n>\n>  - 3/4 was a straight move and I didn't see anything questionable in\n>    it, but I think it would be nicer if intermediate steps can be\n>    made to still work by making 4/4 come first or something\n>    similarly simple.\n>\n> If the issues in 2/4 and 3/4 are easily fixable by going the route I\n> handwaved above, the result of doing so based on this round is ready\n> to be applied, I think.\n>\n> Eric, Jonathan, what do you think?\n\nI think this is pretty good already, though I also like your\nsuggestion re 2/4.\n\nI haven't reviewed the tests these introduce and assume Eric has that\ncovered.\n\nThanks,\nJonathan\n"},{"id":"195927","messageId":"7v394d3ffc.fsf@alter.siamese.dyndns.org","threadId":"31110","inReplyTo":"20120727053800.GC4685@burratino","subject":"Re: [PATCH 4/4] Move initialization of Git::SVN variables into Git::SVN.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-07-27T06:07:35Z","receivedAt":"2012-07-27T06:07:35Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jonathan Nieder <jrnieder@gmail.com> writes:\n\n>> In short:\n>>\n>>  - I didn't see anything questionable in 1/4;\n>>\n>>  - Calling up ::opt_prefix() from module in 2/4 looked ugly to me\n>>    but I suspect it should be easy to fix;\n>>\n>>  - 3/4 was a straight move and I didn't see anything questionable in\n>>    it, but I think it would be nicer if intermediate steps can be\n>>    made to still work by making 4/4 come first or something\n>>    similarly simple.\n>>\n>> If the issues in 2/4 and 3/4 are easily fixable by going the route I\n>> handwaved above, the result of doing so based on this round is ready\n>> to be applied, I think.\n>>\n>> Eric, Jonathan, what do you think?\n>\n> I think this is pretty good already, though I also like your\n> suggestion re 2/4.\n>\n> I haven't reviewed the tests these introduce and assume Eric has that\n> covered.\n\nI didn't mean to say \"Unless you prove that the two suggestions are\nnot easy to implement, I will veto the series until they are fixed.\"\nEspecially, I consider that the ordering between 3 and 4 falls into\nthe \"it would be nicer if this wart weren't there\" category.\n\nThe result will be queued tentatively near the tip of 'pu', but as\nthis is primarily about git-svn, I would prefer a copy that is\nvetted by Eric to be fed from him.\n\nThanks.\n\nP.S.\n\nt91XX series seem to fail in 'pu' with \"Can't locate Git/SVN.pm in\n@INC\" for me.  I see perl/blib/lib/Git/SVN/ directory and files\nunder it, but there is no perl/blib/lib/Git/SVN.pm installed.  I see\nGit/I18N.pm and Git/SVN/Ra.pm (and friends) mentioned in\nperl/perl.mak generated by MakeMaker, but Git/SVN.pm does not appear\nanywhere.\n\nI think it is some interaction with other topics, as the tip of\nms/git-svn-pm topic that parks this series does not exhibit the\nsymptom, but it is getting late for me already, so I won't dig into\nthis further.\n"},{"id":"195930","messageId":"7vpq7h1z1q.fsf@alter.siamese.dyndns.org","threadId":"31110","inReplyTo":"7v394d3ffc.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 4/4] Move initialization of Git::SVN variables into Git::SVN.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-07-27T06:46:41Z","receivedAt":"2012-07-27T06:46:41Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano <gitster@pobox.com> writes:\n\n> t91XX series seem to fail in 'pu' with \"Can't locate Git/SVN.pm in\n> @INC\" for me.  I see perl/blib/lib/Git/SVN/ directory and files\n> under it, but there is no perl/blib/lib/Git/SVN.pm installed.  I see\n> Git/I18N.pm and Git/SVN/Ra.pm (and friends) mentioned in\n> perl/perl.mak generated by MakeMaker, but Git/SVN.pm does not appear\n> anywhere.\n>\n> I think it is some interaction with other topics, as the tip of\n> ms/git-svn-pm topic that parks this series does not exhibit the\n> symptom, but it is getting late for me already, so I won't dig into\n> this further.\n\nActually there is no difference between ms/git-svn-pm and pu in perl/\ndirectory. I _think_ there is some dependency missing that makes\nthis sequence break:\n\n\t(in one repository)\n\tgit checkout pu ;# older pu without ms/git-svn-pm\n        make ; make test\n\n\t(in another repository that shares the refs)\n\tgit checkout pu\n        git merge ms/git-svn-pm\n\n\t(in the first repository)\n        git reset --hard ;# update the working tree\n        make ; make test\n"},{"id":"195931","messageId":"7vlii51xz4.fsf@alter.siamese.dyndns.org","threadId":"31110","inReplyTo":"7vpq7h1z1q.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 4/4] Move initialization of Git::SVN variables into Git::SVN.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-07-27T07:09:51Z","receivedAt":"2012-07-27T07:09:51Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano <gitster@pobox.com> writes:\n\n> Junio C Hamano <gitster@pobox.com> writes:\n>\n>> t91XX series seem to fail in 'pu' with \"Can't locate Git/SVN.pm in\n>> @INC\" for me.  I see perl/blib/lib/Git/SVN/ directory and files\n>> under it, but there is no perl/blib/lib/Git/SVN.pm installed.  I see\n>> Git/I18N.pm and Git/SVN/Ra.pm (and friends) mentioned in\n>> perl/perl.mak generated by MakeMaker, but Git/SVN.pm does not appear\n>> anywhere.\n>> \n>> I think it is some interaction with other topics, as the tip of\n>> ms/git-svn-pm topic that parks this series does not exhibit the\n>> symptom, but it is getting late for me already, so I won't dig into\n>> this further.\n>\n> Actually there is no difference between ms/git-svn-pm and pu in perl/\n> directory. I _think_ there is some dependency missing that makes\n> this sequence break:\n>\n> \t(in one repository)\n> \tgit checkout pu ;# older pu without ms/git-svn-pm\n>         make ; make test\n>\n> \t(in another repository that shares the refs)\n> \tgit checkout pu\n>         git merge ms/git-svn-pm\n>\n> \t(in the first repository)\n>         git reset --hard ;# update the working tree\n>         make ; make test\n\nWhat was happening was that originally, pu had ms/makefile-pl but\nnot ms/git-svn-pm.  Hence, perl/Git/SVN.pm did not exist.  I ran\n\"make\" and it created perl/perl.mak that does not know about\nGit/SVN.pm;\n\nThen ms/git-svn-pm is merged to pu and now we have perl/Git/SVN.pm.\nBut there is nothing in ms/makefile-pl that says on what files\nperl.mak depends on.\n\nI think there needs to be a dependency in to recreate perl/perl.mak\nwhen any of the *.pm files are changed, perhaps like this.\n\nI am not sure why perl/perl.mak is built by the top-level Makefile,\ninstead of just using \"$(MAKE) -C perl/\", though...\n\n Makefile      | 7 +++++++\n perl/Makefile | 1 +\n 2 files changed, 8 insertions(+)\n\ndiff --git a/Makefile b/Makefile\nindex b0b3493..e2a4ac7 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -2090,6 +2090,13 @@ $(SCRIPT_LIB) : % : %.sh GIT-SCRIPT-DEFINES\n ifndef NO_PERL\n $(patsubst %.perl,%,$(SCRIPT_PERL)): perl/perl.mak\n \n+perl/perl.mak: perl/PM.stamp\n+\n+perl/PM.stamp: FORCE\n+\t$(QUIET_GEN)find perl -type f -name '*.pm' | sort >$@+ && \\\n+\t{ cmp $@+ $@ >/dev/null 2>/dev/null || mv $@+ $@; } && \\\n+\t$(RM) $@+\n+\n perl/perl.mak: GIT-CFLAGS GIT-PREFIX perl/Makefile perl/Makefile.PL\n \t$(QUIET_SUBDIR0)perl $(QUIET_SUBDIR1) PERL_PATH='$(PERL_PATH_SQ)' prefix='$(prefix_SQ)' $(@F)\n \ndiff --git a/perl/Makefile b/perl/Makefile\nindex 6ca7d47..d6f8478 100644\n--- a/perl/Makefile\n+++ b/perl/Makefile\n@@ -20,6 +20,7 @@ clean:\n \t$(RM) ppport.h\n \t$(RM) $(makfile)\n \t$(RM) $(makfile).old\n+\t$(RM) PM.stamp\n \n ifdef NO_PERL_MAKEMAKER\n instdir_SQ = $(subst ','\\'',$(prefix)/lib)\n"},{"id":"195933","messageId":"50124E71.2010302@pobox.com","threadId":"31110","inReplyTo":"7vvch93hpy.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 2/4] Prepare Git::SVN for extraction into its own file.","fromName":"Michael G Schwern","fromEmail":"schwern@pobox.com","sentAt":"2012-07-27T08:16:49Z","receivedAt":"2012-07-27T08:16:49Z","isPatch":true,"sender":{"key":"schwern@pobox.com","avatar":"https://avatars.githubusercontent.com/u/25888?v=4"},"body":"On 2012.7.26 10:18 PM, Junio C Hamano wrote:\n> Forgot to sign-off, or are you still unsure about this step?\n\nI just never think to do it.  It's just a line in the commit message, right?\nThere's no crypto involved like tag -s.  Is it a blocker?  I guess I can write\na msg-filter if it's important.\n\n\n> Again, I agree with you that passing $prefix as one of the arguments\n> to ->new is the right thing to do in the final state after applying\n> the whole series.  I don't know if later steps in your patch series\n> will do so, but it _might_ make more sense to update ->new and its\n> callers to do so without doing anything else first, so that you do\n> not have to call out to the ::opt_prefix() when you split things\n> out.\n\nI don't personally plan on doing any more about it, no.  It isn't needed for\nSVN 1.7, there's very little real code change (which you could see by looking\nat my remote instead of waiting to be fed patches...) and its a very, very\nminor problem in the grand scheme.\n\nHow git-svn structures its switches needs a ton of work, and there are far\ndeeper problems with Git::SVN.  For one, it's completely undocumented.  For\nanother, Git::SVN can't instantiate an object without git-svn being loaded and\nso is very difficult to unit test.  I wouldn't want to change the constructor\ninterface until I could construct an object.\n\nThe first step toward that would be to change git-svn so it can be loaded as a\nlibrary using the standard \"main() unless caller\" trick.  Then Git::SVN unit\ntests can require git-svn as a library without executing it and get some tests\nwritten with a minimum of Git::SVN code change.\n\nStep zero would be to allow Perl unit tests to either use or emulate the work\ndone in lib-git-svn.sh.  The major problem being how to communicate the\nlocation of the trash directory, currently done by environment variables.  A\nsimple trick would be for the Perl tests to execute a shell wrapper that\noutputs the relevant information.\n\nNone of which I plan to get into just now.\n\n\n-- \nemacs -- THAT'S NO EDITOR... IT'S AN OPERATING SYSTEM!\n"},{"id":"195934","messageId":"50124F19.1050502@pobox.com","threadId":"31110","inReplyTo":"7vobn13hps.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 1/4] Extract some utilities from git-svn to allow extracting Git::SVN.","fromName":"Michael G Schwern","fromEmail":"schwern@pobox.com","sentAt":"2012-07-27T08:19:37Z","receivedAt":"2012-07-27T08:19:37Z","isPatch":true,"sender":{"key":"schwern@pobox.com","avatar":"https://avatars.githubusercontent.com/u/25888?v=4"},"body":"On 2012.7.26 10:18 PM, Junio C Hamano wrote:\n>> +# Note: not certain why this is in use instead of die.  Probably because\n>> +# the exit code of die is 255?  Doesn't appear to be used consistently.\n>> +sub fatal (@) { print STDERR \"@_\\n\"; exit 1 }\n> \n> Very true.  Also I do not think the line-noise prototype buys us\n> anything (other than making the code look mysterious to non Perl\n> programmers); we are not emulating any Perl's builtin with this\n> function, and I do not see a reason why we want to force list\n> context to its arguments, either.  But removal of it is not part of\n> this step anyway, so I wouldn't complain.\n\nThe prototype does absolutely nothing since @ is the default prototype.  But\nyes, I'm doing a very rote refactoring here.\n\n\n>> +sub can_compress {\n>> +    return $can_compress if defined $can_compress;\n>> +\n>> +    return $can_compress = eval { require Compress::Zlib; } ? 1 : 0;\n>> +}\n> \n> The original said \"eval { require Compress::Zlib; 1; }\"; presumably,\n> when require does succeed, the value inside is the \"1;\" that has to\n> be at the end of Compress::Zlib, so the difference should not matter.\n\nYes.  In other situations where you cannot guarantee that the statement in the\neval will return true it makes sense, but here it's redundant.\n\n\n-- \nBeing faith-based doesn't trump reality.\n\t-- Bruce Sterling\n"},{"id":"195935","messageId":"50125433.1000702@pobox.com","threadId":"31110","inReplyTo":"7vhast3hpb.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 4/4] Move initialization of Git::SVN variables into Git::SVN.","fromName":"Michael G Schwern","fromEmail":"schwern@pobox.com","sentAt":"2012-07-27T08:41:23Z","receivedAt":"2012-07-27T08:41:23Z","isPatch":true,"sender":{"key":"schwern@pobox.com","avatar":"https://avatars.githubusercontent.com/u/25888?v=4"},"body":"On 2012.7.26 10:18 PM, Junio C Hamano wrote:\n> If you swap the order of steps 3/4 and 4/4 by creating Git/SVN.pm\n> that only has these variable definitions (i.e. \"our $X\" and \"use\n> vars $X\") and make git-svn.perl use them from Git::SVN in the first\n> step, and then do the bulk-moving (equivalent of your 3/4) in the\n> second step, would it free you from having to say \"it's doubtful it\n> will compile by itself\"?\n\nIf it wasn't clear, all tests pass with every patch using SVN 1.6.\n\n\"Compile on its own\" wasn't entirely clear.  I meant that Git::SVN doesn't\ndepend on git-svn to set its defaults.  Git::SVN still depends on it for A LOT\nof other things, and will likely remain that way for a long time, so it's\nkinda splitting hairs to worry about it.\n\n4/4 was done last to ensure the phase of git-svn when the Git::SVN globals are\ninitialized remains basically the same.  If they were moved into Git::SVN\nbefore it was split out they'd be getting initialized *after* the git-svn\ncommand has been executed.  I didn't want to expend the energy or risk the\nbugs to get around that.\n\n\n> In short:\n> \n>  - I didn't see anything questionable in 1/4;\n> \n>  - Calling up ::opt_prefix() from module in 2/4 looked ugly to me\n>    but I suspect it should be easy to fix;\n\nOriginally I tried to refactor new().  It rapidly turned into a lot of work on\nundocumented code with no unit tests for no use to the SVN 1.7 issue for one\nvariable.  This is a very cheap way to let far more important work move\nforward and it has a very narrow effect.  It could be made a Git::SVN global\nthat git-svn grabs at, but that's not really any better.  I'd rather leave it be.\n\n\n-- \n91. I am not authorized to initiate Jihad.\n    -- The 213 Things Skippy Is No Longer Allowed To Do In The U.S. Army\n           http://skippyslist.com/list/\n"},{"id":"195938","messageId":"20120727113433.GA20883@dcvr.yhbt.net","threadId":"31110","inReplyTo":"7vobn13hps.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 1/4] Extract some utilities from git-svn to allow extracting Git::SVN.","fromName":"Eric Wong","fromEmail":"normalperson@yhbt.net","sentAt":"2012-07-27T11:34:33Z","receivedAt":"2012-07-27T11:34:33Z","isPatch":true,"sender":{"key":"e@80x24.org","avatar":null},"body":"Junio C Hamano <gitster@pobox.com> wrote:\n> \"Michael G. Schwern\" <schwern@pobox.com> writes:\n> > +# Note: not certain why this is in use instead of die.  Probably because\n> > +# the exit code of die is 255?  Doesn't appear to be used consistently.\n\nYes, 255 caused problems for the test suite:\n\ncommit d25c26e771fdf771f264dc85be348719886d354f\nAuthor: Eric Wong <normalperson@yhbt.net>\nDate:   Fri Nov 24 22:38:18 2006 -0800\n\n    git-svn: exit with status 1 for test failures\n    \n    Some versions of the SVN libraries cause die() to exit with 255,\n    and 40cf043389ef4cdf3e56e7c4268d6f302e387fa0 tightened up\n    test_expect_failure to reject return values >128.\n\n> > +sub fatal (@) { print STDERR \"@_\\n\"; exit 1 }\n> \n> Very true.  Also I do not think the line-noise prototype buys us\n> anything (other than making the code look mysterious to non Perl\n> programmers); we are not emulating any Perl's builtin with this\n> function, and I do not see a reason why we want to force list\n> context to its arguments, either.  But removal of it is not part of\n> this step anyway, so I wouldn't complain.\n\nI think I just learned Perl prototypes around that time :x\nWe can certainly remove it later.\n\n> > +my $can_compress;\n> > +sub can_compress {\n> > +    return $can_compress if defined $can_compress;\n> > +\n> > +    return $can_compress = eval { require Compress::Zlib; } ? 1 : 0;\n> > +}\n> \n> The original said \"eval { require Compress::Zlib; 1; }\"; presumably,\n> when require does succeed, the value inside is the \"1;\" that has to\n> be at the end of Compress::Zlib, so the difference should not matter.\n\nI just squashed in the simplification and made the indentation\nconsistent with other .pm files:\n\n\treturn $can_compress = eval { require Compress::Zlib; };\n"},{"id":"195940","messageId":"20120727115326.GA8890@dcvr.yhbt.net","threadId":"31110","inReplyTo":"50124E71.2010302@pobox.com","subject":"Re: [PATCH 2/4] Prepare Git::SVN for extraction into its own file.","fromName":"Eric Wong","fromEmail":"normalperson@yhbt.net","sentAt":"2012-07-27T11:53:26Z","receivedAt":"2012-07-27T11:53:26Z","isPatch":true,"sender":{"key":"e@80x24.org","avatar":null},"body":"Michael G Schwern <schwern@pobox.com> wrote:\n> On 2012.7.26 10:18 PM, Junio C Hamano wrote:\n> > Again, I agree with you that passing $prefix as one of the arguments\n> > to ->new is the right thing to do in the final state after applying\n> > the whole series.  I don't know if later steps in your patch series\n> > will do so, but it _might_ make more sense to update ->new and its\n> > callers to do so without doing anything else first, so that you do\n> > not have to call out to the ::opt_prefix() when you split things\n> > out.\n> \n> I don't personally plan on doing any more about it, no.  It isn't needed for\n> SVN 1.7, there's very little real code change (which you could see by looking\n> at my remote instead of waiting to be fed patches...) and its a very, very\n> minor problem in the grand scheme.\n\nI agree, its not worth it right now.\n\n> The first step toward that would be to change git-svn so it can be loaded as a\n> library using the standard \"main() unless caller\" trick.  Then Git::SVN unit\n> tests can require git-svn as a library without executing it and get some tests\n> written with a minimum of Git::SVN code change.\n\n> None of which I plan to get into just now.\n\nThat's fine.  The modules were an afterthought and not intended at the\ntime for standalone use, so it'd take a bit of work.  I doubt the\nmodules will be useful elsewhere, but will make code easier to\nmaintain in the future.\n\nI also value functional/integration tests far more than unit tests.\n"},{"id":"195941","messageId":"20120727115959.GA31784@dcvr.yhbt.net","threadId":"31110","inReplyTo":"7v394d3ffc.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 4/4] Move initialization of Git::SVN variables into Git::SVN.","fromName":"Eric Wong","fromEmail":"normalperson@yhbt.net","sentAt":"2012-07-27T11:59:59Z","receivedAt":"2012-07-27T11:59:59Z","isPatch":true,"sender":{"key":"e@80x24.org","avatar":null},"body":"Junio C Hamano <gitster@pobox.com> wrote:\n> The result will be queued tentatively near the tip of 'pu', but as\n> this is primarily about git-svn, I would prefer a copy that is\n> vetted by Eric to be fed from him.\n\nOK.  I've signed-off on the 7 patches you have in pu.  Will look at the\nother series in a few hours.\n\nThe following changes since commit cdd159b2f56c9e69e37bbb8f5af301abd93e5407:\n\n  Merge branch 'jc/test-lib-source-build-options-early' (2012-07-25 15:47:08 -0700)\n\nare available in the git repository at:\n\n  git://bogomips.org/git-svn master\n\nfor you to fetch changes up to 8d1ddbdc877cf1f430ea8e79bf800ce806875565:\n\n  Move initialization of Git::SVN variables into Git::SVN. (2012-07-27 11:29:21 +0000)\n\n----------------------------------------------------------------\nMichael G. Schwern (7):\n      Quiet warning if Makefile.PL is run with -w and no --localedir\n      Don't lose Error.pm if $@ gets clobbered.\n      The Makefile.PL will now find .pm files itself.\n      Extract some utilities from git-svn to allow extracting Git::SVN.\n      Prepare Git::SVN for extraction into its own file.\n      Extract Git::SVN from git-svn into its own .pm file.\n      Move initialization of Git::SVN variables into Git::SVN.\n\n git-svn.perl                   | 2340 +---------------------------------------\n perl/Git/SVN.pm                | 2324 +++++++++++++++++++++++++++++++++++++++\n perl/Git/SVN/Utils.pm          |   59 +\n perl/Makefile                  |    2 +\n perl/Makefile.PL               |   35 +-\n t/Git-SVN/00compile.t          |    9 +\n t/Git-SVN/Utils/can_compress.t |   11 +\n t/Git-SVN/Utils/fatal.t        |   34 +\n 8 files changed, 2476 insertions(+), 2338 deletions(-)\n create mode 100644 perl/Git/SVN.pm\n create mode 100644 perl/Git/SVN/Utils.pm\n create mode 100644 t/Git-SVN/00compile.t\n create mode 100644 t/Git-SVN/Utils/can_compress.t\n create mode 100644 t/Git-SVN/Utils/fatal.t\n\n> Thanks.\n> \n> P.S.\n> \n> t91XX series seem to fail in 'pu' with \"Can't locate Git/SVN.pm in\n> @INC\" for me.  I see perl/blib/lib/Git/SVN/ directory and files\n> under it, but there is no perl/blib/lib/Git/SVN.pm installed.  I see\n> Git/I18N.pm and Git/SVN/Ra.pm (and friends) mentioned in\n> perl/perl.mak generated by MakeMaker, but Git/SVN.pm does not appear\n> anywhere.\n> \n> I think it is some interaction with other topics, as the tip of\n> ms/git-svn-pm topic that parks this series does not exhibit the\n> symptom, but it is getting late for me already, so I won't dig into\n> this further.\n\nI think your proposed patch in the followup should work.  We should\nprobably squash that into this series avoid breaking bisect in the\nfuture.\n"},{"id":"195963","messageId":"20120727200703.GA2034@dcvr.yhbt.net","threadId":"31110","inReplyTo":"7vlii51xz4.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 4/4] Move initialization of Git::SVN variables into Git::SVN.","fromName":"Eric Wong","fromEmail":"normalperson@yhbt.net","sentAt":"2012-07-27T20:07:03Z","receivedAt":"2012-07-27T20:07:03Z","isPatch":true,"sender":{"key":"e@80x24.org","avatar":null},"body":"Junio C Hamano <gitster@pobox.com> wrote:\n> What was happening was that originally, pu had ms/makefile-pl but\n> not ms/git-svn-pm.  Hence, perl/Git/SVN.pm did not exist.  I ran\n> \"make\" and it created perl/perl.mak that does not know about\n> Git/SVN.pm;\n> \n> Then ms/git-svn-pm is merged to pu and now we have perl/Git/SVN.pm.\n> But there is nothing in ms/makefile-pl that says on what files\n> perl.mak depends on.\n> \n> I think there needs to be a dependency in to recreate perl/perl.mak\n> when any of the *.pm files are changed, perhaps like this.\n> \n> I am not sure why perl/perl.mak is built by the top-level Makefile,\n> instead of just using \"$(MAKE) -C perl/\", though...\n\nI'm not sure why perl/perl.mak is built by the top-level Makefile,\neither.\n\nI'll put the following after ms/makefile-pl but before ms/git-svn-pm:\n\n>From a6ea2301d1bb6fd7c7415fed3aa7673542a563bd Mon Sep 17 00:00:00 2001\nFrom: Junio C Hamano <gitster@pobox.com>\nDate: Fri, 27 Jul 2012 20:04:20 +0000\nSubject: [PATCH] perl: detect new files in MakeMaker builds\n\nWhile Makefile.PL now finds .pm files on its own, it does not\ndetect new files after it generates perl/perl.mak.\n\n[ew: commit message]\n\nref: http://mid.gmane.org/7vlii51xz4.fsf@alter.siamese.dyndns.org\n\nSigned-off-by: Eric Wong <normalperson@yhbt.net>\n---\n Makefile      | 7 +++++++\n perl/Makefile | 1 +\n 2 files changed, 8 insertions(+)\n\ndiff --git a/Makefile b/Makefile\nindex b0b3493..e2a4ac7 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -2090,6 +2090,13 @@ $(SCRIPT_LIB) : % : %.sh GIT-SCRIPT-DEFINES\n ifndef NO_PERL\n $(patsubst %.perl,%,$(SCRIPT_PERL)): perl/perl.mak\n \n+perl/perl.mak: perl/PM.stamp\n+\n+perl/PM.stamp: FORCE\n+\t$(QUIET_GEN)find perl -type f -name '*.pm' | sort >$@+ && \\\n+\t{ cmp $@+ $@ >/dev/null 2>/dev/null || mv $@+ $@; } && \\\n+\t$(RM) $@+\n+\n perl/perl.mak: GIT-CFLAGS GIT-PREFIX perl/Makefile perl/Makefile.PL\n \t$(QUIET_SUBDIR0)perl $(QUIET_SUBDIR1) PERL_PATH='$(PERL_PATH_SQ)' prefix='$(prefix_SQ)' $(@F)\n \ndiff --git a/perl/Makefile b/perl/Makefile\nindex 8493d76..4969ef8 100644\n--- a/perl/Makefile\n+++ b/perl/Makefile\n@@ -20,6 +20,7 @@ clean:\n \t$(RM) ppport.h\n \t$(RM) $(makfile)\n \t$(RM) $(makfile).old\n+\t$(RM) PM.stamp\n \n ifdef NO_PERL_MAKEMAKER\n instdir_SQ = $(subst ','\\'',$(prefix)/lib)\n-- \nEric Wong\n"},{"id":"195964","messageId":"50130062.7090901@pobox.com","threadId":"31110","inReplyTo":"20120727200703.GA2034@dcvr.yhbt.net","subject":"Re: [PATCH 4/4] Move initialization of Git::SVN variables into Git::SVN.","fromName":"Michael G Schwern","fromEmail":"schwern@pobox.com","sentAt":"2012-07-27T20:56:02Z","receivedAt":"2012-07-27T20:56:02Z","isPatch":true,"sender":{"key":"schwern@pobox.com","avatar":"https://avatars.githubusercontent.com/u/25888?v=4"},"body":"On 2012.7.27 1:07 PM, Eric Wong wrote:\n> While Makefile.PL now finds .pm files on its own, it does not\n> detect new files after it generates perl/perl.mak.\n\nAre you saying this doesn't work?\n\nperl Makefile.PL\nmake -f perl.mak\ntouch Git/Foo.pm\nperl Makefile.PL\nmake -f perl.mak\n\nor this?\n\nperl Makefile.PL\nmake -f perl.mak\ntouch Git/Foo.pm\nmake -f perl.mak\n\nThe former should work.  The latter is a MakeMaker limitation.  Makefile.PL\nhard codes the list of .pm files into the Makefile.\n\n\n-- \nWho invented the eponym?\n"},{"id":"195965","messageId":"20120727205925.GA14046@dcvr.yhbt.net","threadId":"31110","inReplyTo":"50130062.7090901@pobox.com","subject":"Re: [PATCH 4/4] Move initialization of Git::SVN variables into Git::SVN.","fromName":"Eric Wong","fromEmail":"normalperson@yhbt.net","sentAt":"2012-07-27T20:59:25Z","receivedAt":"2012-07-27T20:59:25Z","isPatch":true,"sender":{"key":"e@80x24.org","avatar":null},"body":"Michael G Schwern <schwern@pobox.com> wrote:\n> On 2012.7.27 1:07 PM, Eric Wong wrote:\n> > While Makefile.PL now finds .pm files on its own, it does not\n> > detect new files after it generates perl/perl.mak.\n> \n> Are you saying this doesn't work?\n> \n> perl Makefile.PL\n> make -f perl.mak\n> touch Git/Foo.pm\n> perl Makefile.PL\n> make -f perl.mak\n\nThis works.\n\n> or this?\n> \n> perl Makefile.PL\n> make -f perl.mak\n> touch Git/Foo.pm\n> make -f perl.mak\n> \n> The former should work.  The latter is a MakeMaker limitation.  Makefile.PL\n> hard codes the list of .pm files into the Makefile.\n\nYup, Junio's patch works around the MM limitation so the latter works.\n"},{"id":"195966","messageId":"7v1ujw28n4.fsf@alter.siamese.dyndns.org","threadId":"31110","inReplyTo":"50130062.7090901@pobox.com","subject":"Re: [PATCH 4/4] Move initialization of Git::SVN variables into Git::SVN.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-07-27T21:31:43Z","receivedAt":"2012-07-27T21:31:43Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Michael G Schwern <schwern@pobox.com> writes:\n\n> On 2012.7.27 1:07 PM, Eric Wong wrote:\n>> While Makefile.PL now finds .pm files on its own, it does not\n>> detect new files after it generates perl/perl.mak.\n>\n> Are you saying this doesn't work?\n>\n> perl Makefile.PL\n> make -f perl.mak\n> touch Git/Foo.pm\n> perl Makefile.PL\n> make -f perl.mak\n>\n> or this?\n>\n> perl Makefile.PL\n> make -f perl.mak\n> touch Git/Foo.pm\n> make -f perl.mak\n\nNeither of the above.  Nobody should be typing \"perl Makefile.PL\"\ninside our source tree unless he is trying to debug our Makefiles\nanyway.\n\nWhat does not work is this sequence:\n\n\tmake\n        >perl/Git/Foo.pm\n        make\n\nMakefile at the top-level, which builds perl/perl.mak by running\n\"perl Makefile.PL\" in perl/ subdirectory, doesn't have dependencies\n[*1*], so in the above sequence, the second invocation of \"make\"\nfails to rebuild perl/perl.mak, which causes Git/Foo.pm forgotten\nfrom the build/installation step.\n\nAnd that is what happened to Git/SVN.pm.\n\n\n[Footnote]\n\n*1* I also suspect perl/Makefile lacks this dependency even though\nit has its own rule to build perl/perl.mak---don't they need to be\ncleaned-up and merged???\n\n\t\n"},{"id":"195969","messageId":"7vsjcczxgu.fsf@alter.siamese.dyndns.org","threadId":"31110","inReplyTo":"20120727200703.GA2034@dcvr.yhbt.net","subject":"Re: [PATCH 4/4] Move initialization of Git::SVN variables into Git::SVN.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-07-27T21:49:05Z","receivedAt":"2012-07-27T21:49:05Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Eric Wong <normalperson@yhbt.net> writes:\n\n> I'll put the following after ms/makefile-pl but before ms/git-svn-pm:\n\nOK, it seems that you haven't pushed out the result yet (which is\nfine); how do we want to proceed?\n\nI generally prefer pulling from maintainer trees directly to my\n'master' without staging them in 'next', so when the above is done\nand you feel the tip of your tree is good for 1.7.12-rc1 without\nregression, please throw me a \"pull, now!\".\n\nThanks.\n"},{"id":"195971","messageId":"20120727220753.GA7378@dcvr.yhbt.net","threadId":"31110","inReplyTo":"7vsjcczxgu.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 4/4] Move initialization of Git::SVN variables into Git::SVN.","fromName":"Eric Wong","fromEmail":"normalperson@yhbt.net","sentAt":"2012-07-27T22:07:53Z","receivedAt":"2012-07-27T22:07:53Z","isPatch":true,"sender":{"key":"e@80x24.org","avatar":null},"body":"Junio C Hamano <gitster@pobox.com> wrote:\n> Eric Wong <normalperson@yhbt.net> writes:\n> \n> > I'll put the following after ms/makefile-pl but before ms/git-svn-pm:\n> \n> OK, it seems that you haven't pushed out the result yet (which is\n> fine); how do we want to proceed?\n\nOops, got sidetracked into something else.  Before I got sidetracked,\nmy application of another patch in Michael's 3rd series failed\n(even with your updated Makefile patch):\n\n  [PATCH 7/8] Extract Git::SVN::GlobSpec from git-svn\n\nGlobSpec.pm did not get picked up and placed into blib/ and some tests\n(t9118-git-svn-funky-branch-names.sh) failed as a result\n\nSo I'll hold off until we can fix the build regressions (working on it\nnow)\n"},{"id":"195972","messageId":"20120727221924.GA8700@dcvr.yhbt.net","threadId":"31110","inReplyTo":"20120727220753.GA7378@dcvr.yhbt.net","subject":"Re: [PATCH 4/4] Move initialization of Git::SVN variables into Git::SVN.","fromName":"Eric Wong","fromEmail":"normalperson@yhbt.net","sentAt":"2012-07-27T22:19:24Z","receivedAt":"2012-07-27T22:19:24Z","isPatch":true,"sender":{"key":"e@80x24.org","avatar":null},"body":"Eric Wong <normalperson@yhbt.net> wrote:\n> Junio C Hamano <gitster@pobox.com> wrote:\n> > Eric Wong <normalperson@yhbt.net> writes:\n> > \n> > > I'll put the following after ms/makefile-pl but before ms/git-svn-pm:\n> > \n> > OK, it seems that you haven't pushed out the result yet (which is\n> > fine); how do we want to proceed?\n\n> So I'll hold off until we can fix the build regressions (working on it\n> now)\n\nOK, all fixed, all I needed was this (squashed in):\n\n--- a/perl/Makefile\n+++ b/perl/Makefile\n@@ -22,6 +22,8 @@ clean:\n \t$(RM) $(makfile).old\n \t$(RM) PM.stamp\n \n+$(makfile): PM.stamp\n+\n ifdef NO_PERL_MAKEMAKER\n instdir_SQ = $(subst ','\\'',$(prefix)/lib)\n\n-------------\nThe redundant dependencies are biting us :<  I agree there presence in\nthe top-level Makefile needs to be reviewed.\n\nAnyways, you can pull this now from my master:\n\nThe following changes since commit cdd159b2f56c9e69e37bbb8f5af301abd93e5407:\n\n  Merge branch 'jc/test-lib-source-build-options-early' (2012-07-25 15:47:08 -0700)\n\nare available in the git repository at:\n\n  git://bogomips.org/git-svn master\n\nfor you to fetch changes up to 5c71028fced46d03bf81b8625680d9ac87c8f4f0:\n\n  Move initialization of Git::SVN variables into Git::SVN. (2012-07-27 22:14:54 +0000)\n\n----------------------------------------------------------------\nJunio C Hamano (1):\n      perl: detect new files in MakeMaker builds\n\nMichael G. Schwern (7):\n      Quiet warning if Makefile.PL is run with -w and no --localedir\n      Don't lose Error.pm if $@ gets clobbered.\n      The Makefile.PL will now find .pm files itself.\n      Extract some utilities from git-svn to allow extracting Git::SVN.\n      Prepare Git::SVN for extraction into its own file.\n      Extract Git::SVN from git-svn into its own .pm file.\n      Move initialization of Git::SVN variables into Git::SVN.\n\n Makefile                       |    7 +\n git-svn.perl                   | 2340 +---------------------------------------\n perl/.gitignore                |    1 +\n perl/Git/SVN.pm                | 2324 +++++++++++++++++++++++++++++++++++++++\n perl/Git/SVN/Utils.pm          |   59 +\n perl/Makefile                  |    5 +\n perl/Makefile.PL               |   35 +-\n t/Git-SVN/00compile.t          |    9 +\n t/Git-SVN/Utils/can_compress.t |   11 +\n t/Git-SVN/Utils/fatal.t        |   34 +\n 10 files changed, 2487 insertions(+), 2338 deletions(-)\n create mode 100644 perl/Git/SVN.pm\n create mode 100644 perl/Git/SVN/Utils.pm\n create mode 100644 t/Git-SVN/00compile.t\n create mode 100644 t/Git-SVN/Utils/can_compress.t\n create mode 100644 t/Git-SVN/Utils/fatal.t\n"},{"id":"195974","messageId":"7vboj0zv7t.fsf@alter.siamese.dyndns.org","threadId":"31110","inReplyTo":"20120727221924.GA8700@dcvr.yhbt.net","subject":"Re: [PATCH 4/4] Move initialization of Git::SVN variables into Git::SVN.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-07-27T22:37:42Z","receivedAt":"2012-07-27T22:37:42Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Eric Wong <normalperson@yhbt.net> writes:\n\n> OK, all fixed, all I needed was this (squashed in):\n>\n> --- a/perl/Makefile\n> +++ b/perl/Makefile\n> @@ -22,6 +22,8 @@ clean:\n>  \t$(RM) $(makfile).old\n>  \t$(RM) PM.stamp\n>  \n> +$(makfile): PM.stamp\n> +\n>  ifdef NO_PERL_MAKEMAKER\n>  instdir_SQ = $(subst ','\\'',$(prefix)/lib)\n>\n> -------------\n> The redundant dependencies are biting us :<  I agree there presence in\n> the top-level Makefile needs to be reviewed.\n\nDo you feel confident enough that we can leave that question hanging\naround and still merge this before 1.7.12 safely?\n\nI do not think it is a regression at the Makefile level per-se---we\ndidn't have right dependencies to keep perl.mak up to date, which\nwas the root cause of what we observed.\n\nBut the lack of dependencies did not matter before this series\nbecause the list of *.pm files never changed, so in that sense the\nseries is what introduced the build regression, and I do not have a\nsolid feeling that we squashed it.\n\n> Anyways, you can pull this now from my master:\n>\n> The following changes since commit cdd159b2f56c9e69e37bbb8f5af301abd93e5407:\n>\n>   Merge branch 'jc/test-lib-source-build-options-early' (2012-07-25 15:47:08 -0700)\n>\n> are available in the git repository at:\n>\n>   git://bogomips.org/git-svn master\n>\n> for you to fetch changes up to 5c71028fced46d03bf81b8625680d9ac87c8f4f0:\n>\n>   Move initialization of Git::SVN variables into Git::SVN. (2012-07-27 22:14:54 +0000)\n>\n> ----------------------------------------------------------------\n> Junio C Hamano (1):\n>       perl: detect new files in MakeMaker builds\n>\n> Michael G. Schwern (7):\n>       Quiet warning if Makefile.PL is run with -w and no --localedir\n>       Don't lose Error.pm if $@ gets clobbered.\n>       The Makefile.PL will now find .pm files itself.\n>       Extract some utilities from git-svn to allow extracting Git::SVN.\n>       Prepare Git::SVN for extraction into its own file.\n>       Extract Git::SVN from git-svn into its own .pm file.\n>       Move initialization of Git::SVN variables into Git::SVN.\n>\n>  Makefile                       |    7 +\n>  git-svn.perl                   | 2340 +---------------------------------------\n>  perl/.gitignore                |    1 +\n>  perl/Git/SVN.pm                | 2324 +++++++++++++++++++++++++++++++++++++++\n>  perl/Git/SVN/Utils.pm          |   59 +\n>  perl/Makefile                  |    5 +\n>  perl/Makefile.PL               |   35 +-\n>  t/Git-SVN/00compile.t          |    9 +\n>  t/Git-SVN/Utils/can_compress.t |   11 +\n>  t/Git-SVN/Utils/fatal.t        |   34 +\n>  10 files changed, 2487 insertions(+), 2338 deletions(-)\n>  create mode 100644 perl/Git/SVN.pm\n>  create mode 100644 perl/Git/SVN/Utils.pm\n>  create mode 100644 t/Git-SVN/00compile.t\n>  create mode 100644 t/Git-SVN/Utils/can_compress.t\n>  create mode 100644 t/Git-SVN/Utils/fatal.t\n"},{"id":"195976","messageId":"20120727224554.GA30385@dcvr.yhbt.net","threadId":"31110","inReplyTo":"7vboj0zv7t.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 4/4] Move initialization of Git::SVN variables into Git::SVN.","fromName":"Eric Wong","fromEmail":"normalperson@yhbt.net","sentAt":"2012-07-27T22:45:54Z","receivedAt":"2012-07-27T22:45:54Z","isPatch":true,"sender":{"key":"e@80x24.org","avatar":null},"body":"Junio C Hamano <gitster@pobox.com> wrote:\n> Eric Wong <normalperson@yhbt.net> writes:\n> > The redundant dependencies are biting us :<  I agree there presence in\n> > the top-level Makefile needs to be reviewed.\n> \n> Do you feel confident enough that we can leave that question hanging\n> around and still merge this before 1.7.12 safely?\n\nYes.\n\n> I do not think it is a regression at the Makefile level per-se---we\n> didn't have right dependencies to keep perl.mak up to date, which\n> was the root cause of what we observed.\n> \n> But the lack of dependencies did not matter before this series\n> because the list of *.pm files never changed, so in that sense the\n> series is what introduced the build regression, and I do not have a\n> solid feeling that we squashed it.\n\nRight, I agree the original dependencies are not good and it's not\na recent regression in the Makefile level.\n\nI do feel our patch deals with the problem for now.  I've been going\nbetween commits in Michael's 3rd series and haven't noticed new issues\nwhen running the tests.\n"},{"id":"195978","messageId":"7vwr1oyfyc.fsf@alter.siamese.dyndns.org","threadId":"31110","inReplyTo":"20120727221924.GA8700@dcvr.yhbt.net","subject":"Re: [PATCH 4/4] Move initialization of Git::SVN variables into Git::SVN.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-07-27T22:52:43Z","receivedAt":"2012-07-27T22:52:43Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Eric Wong <normalperson@yhbt.net> writes:\n\n> Eric Wong <normalperson@yhbt.net> wrote:\n>> So I'll hold off until we can fix the build regressions (working on it\n>> now)\n>\n> OK, all fixed, all I needed was this (squashed in):\n>\n> --- a/perl/Makefile\n> +++ b/perl/Makefile\n> @@ -22,6 +22,8 @@ clean:\n>  \t$(RM) $(makfile).old\n>  \t$(RM) PM.stamp\n>  \n> +$(makfile): PM.stamp\n> +\n>  ifdef NO_PERL_MAKEMAKER\n>  instdir_SQ = $(subst ','\\'',$(prefix)/lib)\n\nAnother thing I noticed but didn't say was that the top-level\nMakefile seems to think without NO_PERL the way to regenerate\nperl/perl.mak is to run perl/Makefile.PL, which is not true if the\nbuild is done with NO_PERL_MAKEMAKER.\n\nI do not offhand know why we even need to have dependency on\nperl/perl.mak in the toplevel Makefile (other than \"otherwise nobody\ndescends into perl/ and run make in it\", which is a bogus\nreason---there should be a rule to run \"$(MAKE) -C perl/ $@\" when\ndoing \"make all\" at the top-level if that is the case), but I think\nat least the duplicated rule in the toplevel Makefile should read\nsomething like:\n\n\tperl/perl.mak: ... (the dependencies) ...\n\t\t$(QUIET_SUBDIR0)perl ... (make variables) ... perl.mak\n\nso that the real knowledge of how to rebuild it (with or without\nNO_PERL_MAKEMAKER) should be in perl/Makefile.\n"},{"id":"195980","messageId":"7vobn0yfm8.fsf@alter.siamese.dyndns.org","threadId":"31110","inReplyTo":"20120727224554.GA30385@dcvr.yhbt.net","subject":"Re: [PATCH 4/4] Move initialization of Git::SVN variables into Git::SVN.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-07-27T22:59:59Z","receivedAt":"2012-07-27T22:59:59Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Eric Wong <normalperson@yhbt.net> writes:\n\n> Junio C Hamano <gitster@pobox.com> wrote:\n>> Eric Wong <normalperson@yhbt.net> writes:\n>> > The redundant dependencies are biting us :<  I agree there presence in\n>> > the top-level Makefile needs to be reviewed.\n>> \n>> Do you feel confident enough that we can leave that question hanging\n>> around and still merge this before 1.7.12 safely?\n>\n> Yes.\n>\n>> I do not think it is a regression at the Makefile level per-se---we\n>> didn't have right dependencies to keep perl.mak up to date, which\n>> was the root cause of what we observed.\n>> \n>> But the lack of dependencies did not matter before this series\n>> because the list of *.pm files never changed, so in that sense the\n>> series is what introduced the build regression, and I do not have a\n>> solid feeling that we squashed it.\n>\n> Right, I agree the original dependencies are not good and it's not\n> a recent regression in the Makefile level.\n>\n> I do feel our patch deals with the problem for now.  I've been going\n> between commits in Michael's 3rd series and haven't noticed new issues\n> when running the tests.\n\nOk, please don't forget to add necessary .gitignore rule for the new\nstamp file.\n"},{"id":"195981","messageId":"20120727230118.GA29949@dcvr.yhbt.net","threadId":"31110","inReplyTo":"7vobn0yfm8.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 4/4] Move initialization of Git::SVN variables into Git::SVN.","fromName":"Eric Wong","fromEmail":"normalperson@yhbt.net","sentAt":"2012-07-27T23:01:18Z","receivedAt":"2012-07-27T23:01:18Z","isPatch":true,"sender":{"key":"e@80x24.org","avatar":null},"body":"Junio C Hamano <gitster@pobox.com> wrote:\n> Ok, please don't forget to add necessary .gitignore rule for the new\n> stamp file.\n\nI noticed/remembered that, but I forgot to mention I squashed that in,\ntoo :)\n"}]}