* Re: Fix git-svn tests for SVN 1.7.5.
2012-07-17 17:44 ` Jonathan Nieder
@ 2012-07-17 18:58 ` Michael G Schwern
2012-07-17 23:13 ` Extract Git classes from git-svn (4/10) (was Re: Fix git-svn tests for SVN 1.7.5.) Michael G Schwern
2012-07-17 23:14 ` Extract Git classes from git-svn (5/10) " Michael G Schwern
2012-07-17 23:05 ` Find .pm files automatically " Michael G Schwern
` (9 subsequent siblings)
10 siblings, 2 replies; 34+ messages in thread
From: Michael G Schwern @ 2012-07-17 18:58 UTC (permalink / raw)
To: Jonathan Nieder; +Cc: git, gitster, robbat2, Eric Wong, Ben Walton
On 2012.7.17 10:44 AM, Jonathan Nieder wrote:
> Michael G Schwern wrote:
>> I've fixed the git-svn tests for SVN 1.7 and tested with SVN 1.7.5.
>
> Thanks. git-svn is not maintained by Junio but by Eric and others on
> the list. I'm cc-ing Eric and Ben Walton so they can benefit from
> your work.
Thanks.
>> There's a lot of work in this change, so I felt it better to submit the
>> patches as a link to a git repository rather than attach a pile of patches.
>> Here is my repository, the work is in the fix-canonical branch.
>> https://github.com/schwern/git
>
> It is indeed quite the intimidating pile of patches, so I do not think
> we will be able to apply it all in one chunk as-is. :(
>
> My advice would be to send five or so of the patches that you would
> like to be reviewed first, inline, one per message, in reply to this
> message so we can start to work on that. Presumably the patches do
> not regress git-svn's behavior but only make it saner, so even if this
> is not a complete fix it should allow us to get started. See
> Documentation/SubmittingPatches for more hints.
Yes, the refactorings are all as rote as I could make them and only lightly
touch the code enough to make the canonicalization possible... with a bit more
work than was strictly necessary around the Perl build system.
Let me do a bit of rebase work to make things work better as a series of
submissions and I'll get back to you.
I'm new here, and I'll play nice, but let me go on record to state that Git
asking for individual emails with inline patches feels like Sendmail Corp
asking to be faxed an email thread. I was kinda hoping SubmittingPatches
wasn't serious about that and it was some sort of policy artifact that was
never updated. :-/
--
151. The proper way to report to my Commander is "Specialist Schwarz,
reporting as ordered, Sir" not "You can't prove a thing!"
-- The 213 Things Skippy Is No Longer Allowed To Do In The U.S. Army
http://skippyslist.com/list/
^ permalink raw reply [flat|nested] 34+ messages in thread* Extract Git classes from git-svn (4/10) (was Re: Fix git-svn tests for SVN 1.7.5.)
2012-07-17 18:58 ` Michael G Schwern
@ 2012-07-17 23:13 ` Michael G Schwern
2012-07-17 23:14 ` Extract Git classes from git-svn (5/10) " Michael G Schwern
1 sibling, 0 replies; 34+ messages in thread
From: Michael G Schwern @ 2012-07-17 23:13 UTC (permalink / raw)
To: Michael G Schwern
Cc: Jonathan Nieder, git, gitster, robbat2, Eric Wong, Ben Walton
>From 8f70be0424a770c299b6a0c5bf99e4030e5e4d92 Mon Sep 17 00:00:00 2001
From: "Michael G. Schwern" <schwern@pobox.com>
Date: Thu, 12 Jul 2012 16:58:53 -0700
Subject: [PATCH 05/11] Make Git::SVN::Log work.
Changes to Git::SVN::Log to make it compile....
* Change the $_git_format lexical only used by Git::SVN::Log into a
Git::SVN::Log global
* Have it load the Git command functions itself
---
git-svn.perl | 8 +++++---
perl/Git/SVN/Log.pm | 10 +++++++++-
perl/Makefile | 1 +
t/Git-SVN/00compile.t | 4 +++-
4 files changed, 18 insertions(+), 5 deletions(-)
diff --git a/git-svn.perl b/git-svn.perl
index 5e6e3b5..7c8ca49 100755
--- a/git-svn.perl
+++ b/git-svn.perl
@@ -11,6 +11,8 @@ $AUTHOR = 'Eric Wong <normalperson@yhbt.net>';
$VERSION = '@@GIT_VERSION@@';
use Git::SVN;
+use Git::SVN::Log;
+
use Git::SVN::Utils qw(fatal can_compress);
# From which subdir have we been invoked?
@@ -88,7 +90,7 @@ BEGIN {
foreach (qw/command command_oneline command_noisy command_output_pipe
command_input_pipe command_close_pipe
command_bidi_pipe command_close_bidi_pipe/) {
- for my $package ( qw(Git::SVN::Migration Git::SVN::Log),
+ for my $package ( qw(Git::SVN::Migration),
__PACKAGE__) {
*{"${package}::$_"} = \&{"Git::$_"};
}
@@ -107,7 +109,7 @@ my ($_stdin, $_help, $_edit,
$_version, $_fetch_all, $_no_rebase, $_fetch_parent,
$_merge, $_strategy, $_preserve_merges, $_dry_run, $_local,
$_prefix, $_no_checkout, $_url, $_verbose,
- $_git_format, $_commit_url, $_tag, $_merge_info, $_interactive);
+ $_commit_url, $_tag, $_merge_info, $_interactive);
# This is a refactoring artifact so Git::SVN can get at this variable.
sub opt_prefix { return $_prefix || '' }
@@ -271,7 +273,7 @@ my %cmd = (
{ 'url' => \$_url, } ],
'blame' => [ \&Git::SVN::Log::cmd_blame,
"Show what revision and author last modified each line of a file",
- { 'git-format' => \$_git_format } ],
+ { 'git-format' => \$Git::SVN::Log::_git_format } ],
'reset' => [ \&cmd_reset,
"Undo fetches back to the specified SVN revision",
{ 'revision|r=s' => \$_revision,
diff --git a/perl/Git/SVN/Log.pm b/perl/Git/SVN/Log.pm
index bbec3b0..7f3cb87 100644
--- a/perl/Git/SVN/Log.pm
+++ b/perl/Git/SVN/Log.pm
@@ -1,12 +1,17 @@
package Git::SVN::Log;
+
use strict;
use warnings;
+
+use Git qw(command command_oneline command_output_pipe command_close_pipe);
use Git::SVN::Utils qw(fatal);
use POSIX qw/strftime/;
use constant commit_log_separator => ('-' x 72) . "\n";
use vars qw/$TZ $limit $color $pager $non_recursive $verbose $oneline
%rusers $show_commit $incremental/;
-my $l_fmt;
+
+# Options set in git-svn
+our $_git_format;
sub cmt_showable {
my ($c) = @_;
@@ -52,6 +57,7 @@ sub git_svn_log_cmd {
}
my ($url, $rev, $uuid, $gs) = ::working_head_info($head);
+ require Git::SVN;
$gs ||= Git::SVN->_new;
my @cmd = (qw/log --abbrev-commit --pretty=raw --default/,
$gs->refname);
@@ -113,6 +119,7 @@ sub run_pager {
sub format_svn_date {
my $t = shift || time;
+ require Git::SVN;
my $gmoff = Git::SVN::get_tz($t);
return strftime("%Y-%m-%d %H:%M:%S $gmoff (%a, %d %b %Y)", localtime($t));
}
@@ -183,6 +190,7 @@ sub process_commit {
return 1;
}
+my $l_fmt;
sub show_commit {
my $c = shift;
if ($oneline) {
diff --git a/perl/Makefile b/perl/Makefile
index d0a0c5c..2a4ca57 100644
--- a/perl/Makefile
+++ b/perl/Makefile
@@ -30,6 +30,7 @@ modules += Git/SVN
modules += Git/SVN/Memoize/YAML
modules += Git/SVN/Fetcher
modules += Git/SVN/Editor
+modules += Git/SVN/Log
modules += Git/SVN/Prompt
modules += Git/SVN/Ra
modules += Git/SVN/Utils
diff --git a/t/Git-SVN/00compile.t b/t/Git-SVN/00compile.t
index c32ee4b..37626f4 100644
--- a/t/Git-SVN/00compile.t
+++ b/t/Git-SVN/00compile.t
@@ -3,7 +3,9 @@
use strict;
use warnings;
-use Test::More tests => 2;
+use Test::More tests => 4;
require_ok 'Git::SVN';
require_ok 'Git::SVN::Utils';
+require_ok 'Git::SVN::Ra';
+require_ok 'Git::SVN::Log';
--
1.7.11.1
^ permalink raw reply related [flat|nested] 34+ messages in thread* Extract Git classes from git-svn (5/10) (was Re: Fix git-svn tests for SVN 1.7.5.)
2012-07-17 18:58 ` Michael G Schwern
2012-07-17 23:13 ` Extract Git classes from git-svn (4/10) (was Re: Fix git-svn tests for SVN 1.7.5.) Michael G Schwern
@ 2012-07-17 23:14 ` Michael G Schwern
1 sibling, 0 replies; 34+ messages in thread
From: Michael G Schwern @ 2012-07-17 23:14 UTC (permalink / raw)
To: Michael G Schwern
Cc: Jonathan Nieder, git, gitster, robbat2, Eric Wong, Ben Walton
>From ab67ab421dbfd248b9a198b8cc1cd9944ba6178d Mon Sep 17 00:00:00 2001
From: "Michael G. Schwern" <schwern@pobox.com>
Date: Tue, 17 Jul 2012 15:46:44 -0700
Subject: [PATCH 06/11] Move Git::SVN::Migration into its own file.
Just a straight cut & paste, the fixes come next commit.
---
git-svn.perl | 246 ---------------------------------------------
perl/Git/SVN/Migration.pm | 247 ++++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 247 insertions(+), 246 deletions(-)
create mode 100644 perl/Git/SVN/Migration.pm
diff --git a/git-svn.perl b/git-svn.perl
index 7c8ca49..f2bf759 100755
--- a/git-svn.perl
+++ b/git-svn.perl
@@ -2043,252 +2043,6 @@ sub gc_directory {
}
-package Git::SVN::Migration;
-# these version numbers do NOT correspond to actual version numbers
-# of git nor git-svn. They are just relative.
-#
-# v0 layout: .git/$id/info/url, refs/heads/$id-HEAD
-#
-# v1 layout: .git/$id/info/url, refs/remotes/$id
-#
-# v2 layout: .git/svn/$id/info/url, refs/remotes/$id
-#
-# v3 layout: .git/svn/$id, refs/remotes/$id
-# - info/url may remain for backwards compatibility
-# - this is what we migrate up to this layout automatically,
-# - this will be used by git svn init on single branches
-# v3.1 layout (auto migrated):
-# - .rev_db => .rev_db.$UUID, .rev_db will remain as a symlink
-# for backwards compatibility
-#
-# v4 layout: .git/svn/$repo_id/$id, refs/remotes/$repo_id/$id
-# - this is only created for newly multi-init-ed
-# repositories. Similar in spirit to the
-# --use-separate-remotes option in git-clone (now default)
-# - we do not automatically migrate to this (following
-# the example set by core git)
-#
-# v5 layout: .rev_db.$UUID => .rev_map.$UUID
-# - newer, more-efficient format that uses 24-bytes per record
-# with no filler space.
-# - use xxd -c24 < .rev_map.$UUID to view and debug
-# - This is a one-way migration, repositories updated to the
-# new format will not be able to use old git-svn without
-# rebuilding the .rev_db. Rebuilding the rev_db is not
-# possible if noMetadata or useSvmProps are set; but should
-# be no problem for users that use the (sensible) defaults.
-use strict;
-use warnings;
-use Carp qw/croak/;
-use File::Path qw/mkpath/;
-use File::Basename qw/dirname basename/;
-use vars qw/$_minimize/;
-
-sub migrate_from_v0 {
- my $git_dir = $ENV{GIT_DIR};
- return undef unless -d $git_dir;
- my ($fh, $ctx) = command_output_pipe(qw/rev-parse --symbolic --all/);
- my $migrated = 0;
- while (<$fh>) {
- chomp;
- my ($id, $orig_ref) = ($_, $_);
- next unless $id =~ s#^refs/heads/(.+)-HEAD$#$1#;
- next unless -f "$git_dir/$id/info/url";
- my $new_ref = "refs/remotes/$id";
- if (::verify_ref("$new_ref^0")) {
- print STDERR "W: $orig_ref is probably an old ",
- "branch used by an ancient version of ",
- "git-svn.\n",
- "However, $new_ref also exists.\n",
- "We will not be able ",
- "to use this branch until this ",
- "ambiguity is resolved.\n";
- next;
- }
- print STDERR "Migrating from v0 layout...\n" if !$migrated;
- print STDERR "Renaming ref: $orig_ref => $new_ref\n";
- command_noisy('update-ref', $new_ref, $orig_ref);
- command_noisy('update-ref', '-d', $orig_ref, $orig_ref);
- $migrated++;
- }
- command_close_pipe($fh, $ctx);
- print STDERR "Done migrating from v0 layout...\n" if $migrated;
- $migrated;
-}
-
-sub migrate_from_v1 {
- my $git_dir = $ENV{GIT_DIR};
- my $migrated = 0;
- return $migrated unless -d $git_dir;
- my $svn_dir = "$git_dir/svn";
-
- # just in case somebody used 'svn' as their $id at some point...
- return $migrated if -d $svn_dir && ! -f "$svn_dir/info/url";
-
- print STDERR "Migrating from a git-svn v1 layout...\n";
- mkpath([$svn_dir]);
- print STDERR "Data from a previous version of git-svn exists, but\n\t",
- "$svn_dir\n\t(required for this version ",
- "($::VERSION) of git-svn) does not exist.\n";
- my ($fh, $ctx) = command_output_pipe(qw/rev-parse --symbolic --all/);
- while (<$fh>) {
- my $x = $_;
- next unless $x =~ s#^refs/remotes/##;
- chomp $x;
- next unless -f "$git_dir/$x/info/url";
- my $u = eval { ::file_to_s("$git_dir/$x/info/url") };
- next unless $u;
- my $dn = dirname("$git_dir/svn/$x");
- mkpath([$dn]) unless -d $dn;
- if ($x eq 'svn') { # they used 'svn' as GIT_SVN_ID:
- mkpath(["$git_dir/svn/svn"]);
- print STDERR " - $git_dir/$x/info => ",
- "$git_dir/svn/$x/info\n";
- rename "$git_dir/$x/info", "$git_dir/svn/$x/info" or
- croak "$!: $x";
- # don't worry too much about these, they probably
- # don't exist with repos this old (save for index,
- # and we can easily regenerate that)
- foreach my $f (qw/unhandled.log index .rev_db/) {
- rename "$git_dir/$x/$f", "$git_dir/svn/$x/$f";
- }
- } else {
- print STDERR " - $git_dir/$x => $git_dir/svn/$x\n";
- rename "$git_dir/$x", "$git_dir/svn/$x" or
- croak "$!: $x";
- }
- $migrated++;
- }
- command_close_pipe($fh, $ctx);
- print STDERR "Done migrating from a git-svn v1 layout\n";
- $migrated;
-}
-
-sub read_old_urls {
- my ($l_map, $pfx, $path) = @_;
- my @dir;
- foreach (<$path/*>) {
- if (-r "$_/info/url") {
- $pfx .= '/' if $pfx && $pfx !~ m!/$!;
- my $ref_id = $pfx . basename $_;
- my $url = ::file_to_s("$_/info/url");
- $l_map->{$ref_id} = $url;
- } elsif (-d $_) {
- push @dir, $_;
- }
- }
- foreach (@dir) {
- my $x = $_;
- $x =~ s!^\Q$ENV{GIT_DIR}\E/svn/!!o;
- read_old_urls($l_map, $x, $_);
- }
-}
-
-sub migrate_from_v2 {
- my @cfg = command(qw/config -l/);
- return if grep /^svn-remote\..+\.url=/, @cfg;
- my %l_map;
- read_old_urls(\%l_map, '', "$ENV{GIT_DIR}/svn");
- my $migrated = 0;
-
- foreach my $ref_id (sort keys %l_map) {
- eval { Git::SVN->init($l_map{$ref_id}, '', undef, $ref_id) };
- if ($@) {
- Git::SVN->init($l_map{$ref_id}, '', $ref_id, $ref_id);
- }
- $migrated++;
- }
- $migrated;
-}
-
-sub minimize_connections {
- my $r = Git::SVN::read_all_remotes();
- my $new_urls = {};
- my $root_repos = {};
- foreach my $repo_id (keys %$r) {
- my $url = $r->{$repo_id}->{url} or next;
- my $fetch = $r->{$repo_id}->{fetch} or next;
- my $ra = Git::SVN::Ra->new($url);
-
- # skip existing cases where we already connect to the root
- if (($ra->{url} eq $ra->{repos_root}) ||
- ($ra->{repos_root} eq $repo_id)) {
- $root_repos->{$ra->{url}} = $repo_id;
- next;
- }
-
- my $root_ra = Git::SVN::Ra->new($ra->{repos_root});
- my $root_path = $ra->{url};
- $root_path =~ s#^\Q$ra->{repos_root}\E(/|$)##;
- foreach my $path (keys %$fetch) {
- my $ref_id = $fetch->{$path};
- my $gs = Git::SVN->new($ref_id, $repo_id, $path);
-
- # make sure we can read when connecting to
- # a higher level of a repository
- my ($last_rev, undef) = $gs->last_rev_commit;
- if (!defined $last_rev) {
- $last_rev = eval {
- $root_ra->get_latest_revnum;
- };
- next if $@;
- }
- my $new = $root_path;
- $new .= length $path ? "/$path" : '';
- eval {
- $root_ra->get_log([$new], $last_rev, $last_rev,
- 0, 0, 1, sub { });
- };
- next if $@;
- $new_urls->{$ra->{repos_root}}->{$new} =
- { ref_id => $ref_id,
- old_repo_id => $repo_id,
- old_path => $path };
- }
- }
-
- my @emptied;
- foreach my $url (keys %$new_urls) {
- # see if we can re-use an existing [svn-remote "repo_id"]
- # instead of creating a(n ugly) new section:
- my $repo_id = $root_repos->{$url} || $url;
-
- my $fetch = $new_urls->{$url};
- foreach my $path (keys %$fetch) {
- my $x = $fetch->{$path};
- Git::SVN->init($url, $path, $repo_id, $x->{ref_id});
- my $pfx = "svn-remote.$x->{old_repo_id}";
-
- my $old_fetch = quotemeta("$x->{old_path}:".
- "$x->{ref_id}");
- command_noisy(qw/config --unset/,
- "$pfx.fetch", '^'. $old_fetch . '$');
- delete $r->{$x->{old_repo_id}}->
- {fetch}->{$x->{old_path}};
- if (!keys %{$r->{$x->{old_repo_id}}->{fetch}}) {
- command_noisy(qw/config --unset/,
- "$pfx.url");
- push @emptied, $x->{old_repo_id}
- }
- }
- }
- if (@emptied) {
- my $file = $ENV{GIT_CONFIG} || "$ENV{GIT_DIR}/config";
- print STDERR <<EOF;
-The following [svn-remote] sections in your config file ($file) are empty
-and can be safely removed:
-EOF
- print STDERR "[svn-remote \"$_\"]\n" foreach @emptied;
- }
-}
-
-sub migration_check {
- migrate_from_v0();
- migrate_from_v1();
- migrate_from_v2();
- minimize_connections() if $_minimize;
-}
-
package Git::IndexInfo;
use strict;
use warnings;
diff --git a/perl/Git/SVN/Migration.pm b/perl/Git/SVN/Migration.pm
new file mode 100644
index 0000000..082a788
--- /dev/null
+++ b/perl/Git/SVN/Migration.pm
@@ -0,0 +1,247 @@
+package Git::SVN::Migration;
+# these version numbers do NOT correspond to actual version numbers
+# of git nor git-svn. They are just relative.
+#
+# v0 layout: .git/$id/info/url, refs/heads/$id-HEAD
+#
+# v1 layout: .git/$id/info/url, refs/remotes/$id
+#
+# v2 layout: .git/svn/$id/info/url, refs/remotes/$id
+#
+# v3 layout: .git/svn/$id, refs/remotes/$id
+# - info/url may remain for backwards compatibility
+# - this is what we migrate up to this layout automatically,
+# - this will be used by git svn init on single branches
+# v3.1 layout (auto migrated):
+# - .rev_db => .rev_db.$UUID, .rev_db will remain as a symlink
+# for backwards compatibility
+#
+# v4 layout: .git/svn/$repo_id/$id, refs/remotes/$repo_id/$id
+# - this is only created for newly multi-init-ed
+# repositories. Similar in spirit to the
+# --use-separate-remotes option in git-clone (now default)
+# - we do not automatically migrate to this (following
+# the example set by core git)
+#
+# v5 layout: .rev_db.$UUID => .rev_map.$UUID
+# - newer, more-efficient format that uses 24-bytes per record
+# with no filler space.
+# - use xxd -c24 < .rev_map.$UUID to view and debug
+# - This is a one-way migration, repositories updated to the
+# new format will not be able to use old git-svn without
+# rebuilding the .rev_db. Rebuilding the rev_db is not
+# possible if noMetadata or useSvmProps are set; but should
+# be no problem for users that use the (sensible) defaults.
+use strict;
+use warnings;
+use Carp qw/croak/;
+use File::Path qw/mkpath/;
+use File::Basename qw/dirname basename/;
+use vars qw/$_minimize/;
+
+sub migrate_from_v0 {
+ my $git_dir = $ENV{GIT_DIR};
+ return undef unless -d $git_dir;
+ my ($fh, $ctx) = command_output_pipe(qw/rev-parse --symbolic --all/);
+ my $migrated = 0;
+ while (<$fh>) {
+ chomp;
+ my ($id, $orig_ref) = ($_, $_);
+ next unless $id =~ s#^refs/heads/(.+)-HEAD$#$1#;
+ next unless -f "$git_dir/$id/info/url";
+ my $new_ref = "refs/remotes/$id";
+ if (::verify_ref("$new_ref^0")) {
+ print STDERR "W: $orig_ref is probably an old ",
+ "branch used by an ancient version of ",
+ "git-svn.\n",
+ "However, $new_ref also exists.\n",
+ "We will not be able ",
+ "to use this branch until this ",
+ "ambiguity is resolved.\n";
+ next;
+ }
+ print STDERR "Migrating from v0 layout...\n" if !$migrated;
+ print STDERR "Renaming ref: $orig_ref => $new_ref\n";
+ command_noisy('update-ref', $new_ref, $orig_ref);
+ command_noisy('update-ref', '-d', $orig_ref, $orig_ref);
+ $migrated++;
+ }
+ command_close_pipe($fh, $ctx);
+ print STDERR "Done migrating from v0 layout...\n" if $migrated;
+ $migrated;
+}
+
+sub migrate_from_v1 {
+ my $git_dir = $ENV{GIT_DIR};
+ my $migrated = 0;
+ return $migrated unless -d $git_dir;
+ my $svn_dir = "$git_dir/svn";
+
+ # just in case somebody used 'svn' as their $id at some point...
+ return $migrated if -d $svn_dir && ! -f "$svn_dir/info/url";
+
+ print STDERR "Migrating from a git-svn v1 layout...\n";
+ mkpath([$svn_dir]);
+ print STDERR "Data from a previous version of git-svn exists, but\n\t",
+ "$svn_dir\n\t(required for this version ",
+ "($::VERSION) of git-svn) does not exist.\n";
+ my ($fh, $ctx) = command_output_pipe(qw/rev-parse --symbolic --all/);
+ while (<$fh>) {
+ my $x = $_;
+ next unless $x =~ s#^refs/remotes/##;
+ chomp $x;
+ next unless -f "$git_dir/$x/info/url";
+ my $u = eval { ::file_to_s("$git_dir/$x/info/url") };
+ next unless $u;
+ my $dn = dirname("$git_dir/svn/$x");
+ mkpath([$dn]) unless -d $dn;
+ if ($x eq 'svn') { # they used 'svn' as GIT_SVN_ID:
+ mkpath(["$git_dir/svn/svn"]);
+ print STDERR " - $git_dir/$x/info => ",
+ "$git_dir/svn/$x/info\n";
+ rename "$git_dir/$x/info", "$git_dir/svn/$x/info" or
+ croak "$!: $x";
+ # don't worry too much about these, they probably
+ # don't exist with repos this old (save for index,
+ # and we can easily regenerate that)
+ foreach my $f (qw/unhandled.log index .rev_db/) {
+ rename "$git_dir/$x/$f", "$git_dir/svn/$x/$f";
+ }
+ } else {
+ print STDERR " - $git_dir/$x => $git_dir/svn/$x\n";
+ rename "$git_dir/$x", "$git_dir/svn/$x" or
+ croak "$!: $x";
+ }
+ $migrated++;
+ }
+ command_close_pipe($fh, $ctx);
+ print STDERR "Done migrating from a git-svn v1 layout\n";
+ $migrated;
+}
+
+sub read_old_urls {
+ my ($l_map, $pfx, $path) = @_;
+ my @dir;
+ foreach (<$path/*>) {
+ if (-r "$_/info/url") {
+ $pfx .= '/' if $pfx && $pfx !~ m!/$!;
+ my $ref_id = $pfx . basename $_;
+ my $url = ::file_to_s("$_/info/url");
+ $l_map->{$ref_id} = $url;
+ } elsif (-d $_) {
+ push @dir, $_;
+ }
+ }
+ foreach (@dir) {
+ my $x = $_;
+ $x =~ s!^\Q$ENV{GIT_DIR}\E/svn/!!o;
+ read_old_urls($l_map, $x, $_);
+ }
+}
+
+sub migrate_from_v2 {
+ my @cfg = command(qw/config -l/);
+ return if grep /^svn-remote\..+\.url=/, @cfg;
+ my %l_map;
+ read_old_urls(\%l_map, '', "$ENV{GIT_DIR}/svn");
+ my $migrated = 0;
+
+ foreach my $ref_id (sort keys %l_map) {
+ eval { Git::SVN->init($l_map{$ref_id}, '', undef, $ref_id) };
+ if ($@) {
+ Git::SVN->init($l_map{$ref_id}, '', $ref_id, $ref_id);
+ }
+ $migrated++;
+ }
+ $migrated;
+}
+
+sub minimize_connections {
+ my $r = Git::SVN::read_all_remotes();
+ my $new_urls = {};
+ my $root_repos = {};
+ foreach my $repo_id (keys %$r) {
+ my $url = $r->{$repo_id}->{url} or next;
+ my $fetch = $r->{$repo_id}->{fetch} or next;
+ my $ra = Git::SVN::Ra->new($url);
+
+ # skip existing cases where we already connect to the root
+ if (($ra->{url} eq $ra->{repos_root}) ||
+ ($ra->{repos_root} eq $repo_id)) {
+ $root_repos->{$ra->{url}} = $repo_id;
+ next;
+ }
+
+ my $root_ra = Git::SVN::Ra->new($ra->{repos_root});
+ my $root_path = $ra->{url};
+ $root_path =~ s#^\Q$ra->{repos_root}\E(/|$)##;
+ foreach my $path (keys %$fetch) {
+ my $ref_id = $fetch->{$path};
+ my $gs = Git::SVN->new($ref_id, $repo_id, $path);
+
+ # make sure we can read when connecting to
+ # a higher level of a repository
+ my ($last_rev, undef) = $gs->last_rev_commit;
+ if (!defined $last_rev) {
+ $last_rev = eval {
+ $root_ra->get_latest_revnum;
+ };
+ next if $@;
+ }
+ my $new = $root_path;
+ $new .= length $path ? "/$path" : '';
+ eval {
+ $root_ra->get_log([$new], $last_rev, $last_rev,
+ 0, 0, 1, sub { });
+ };
+ next if $@;
+ $new_urls->{$ra->{repos_root}}->{$new} =
+ { ref_id => $ref_id,
+ old_repo_id => $repo_id,
+ old_path => $path };
+ }
+ }
+
+ my @emptied;
+ foreach my $url (keys %$new_urls) {
+ # see if we can re-use an existing [svn-remote "repo_id"]
+ # instead of creating a(n ugly) new section:
+ my $repo_id = $root_repos->{$url} || $url;
+
+ my $fetch = $new_urls->{$url};
+ foreach my $path (keys %$fetch) {
+ my $x = $fetch->{$path};
+ Git::SVN->init($url, $path, $repo_id, $x->{ref_id});
+ my $pfx = "svn-remote.$x->{old_repo_id}";
+
+ my $old_fetch = quotemeta("$x->{old_path}:".
+ "$x->{ref_id}");
+ command_noisy(qw/config --unset/,
+ "$pfx.fetch", '^'. $old_fetch . '$');
+ delete $r->{$x->{old_repo_id}}->
+ {fetch}->{$x->{old_path}};
+ if (!keys %{$r->{$x->{old_repo_id}}->{fetch}}) {
+ command_noisy(qw/config --unset/,
+ "$pfx.url");
+ push @emptied, $x->{old_repo_id}
+ }
+ }
+ }
+ if (@emptied) {
+ my $file = $ENV{GIT_CONFIG} || "$ENV{GIT_DIR}/config";
+ print STDERR <<EOF;
+The following [svn-remote] sections in your config file ($file) are empty
+and can be safely removed:
+EOF
+ print STDERR "[svn-remote \"$_\"]\n" foreach @emptied;
+ }
+}
+
+sub migration_check {
+ migrate_from_v0();
+ migrate_from_v1();
+ migrate_from_v2();
+ minimize_connections() if $_minimize;
+}
+
+1;
--
1.7.11.1
^ permalink raw reply related [flat|nested] 34+ messages in thread
* Find .pm files automatically (was Re: Fix git-svn tests for SVN 1.7.5.)
2012-07-17 17:44 ` Jonathan Nieder
2012-07-17 18:58 ` Michael G Schwern
@ 2012-07-17 23:05 ` Michael G Schwern
2012-07-18 0:01 ` Jonathan Nieder
2012-07-17 23:12 ` Extract Git classes from git-svn (2/10) " Michael G Schwern
` (8 subsequent siblings)
10 siblings, 1 reply; 34+ messages in thread
From: Michael G Schwern @ 2012-07-17 23:05 UTC (permalink / raw)
To: Jonathan Nieder; +Cc: git, gitster, robbat2, Eric Wong, Ben Walton
On 2012.7.17 10:44 AM, Jonathan Nieder wrote:
> My advice would be to send five or so of the patches that you would
> like to be reviewed first, inline, one per message, in reply to this
> message so we can start to work on that. Presumably the patches do
> not regress git-svn's behavior but only make it saner, so even if this
> is not a complete fix it should allow us to get started. See
> Documentation/SubmittingPatches for more hints.
Ok, here goes.
First patch overhauls perl/Makefile.PL to make it easier to add .pm files,
which I'm going to be doing a lot of. Instead of having to manually add to
the %pm hash, it scans for .pm files.
It also moves Error.pm into a bundle directory. This both makes it just
another directory to scan (or not scan), but it also makes it possible to
bundle additional modules in the future. ExtUtils::MakeMaker uses this
technique itself.
You still have to remember to add them to the other Makefile.
This is available as a branch.
https://github.com/schwern/git/tree/git-svn/easier_modules
>From 47a723a860cded6b16a716ea74c5bc029ee5b0ac Mon Sep 17 00:00:00 2001
From: "Michael G. Schwern" <schwern@pobox.com>
Date: Thu, 12 Jul 2012 00:05:38 -0700
Subject: [PATCH 01/11] Make the process of adding a module less blecherous.
* Scan for .pm files and build %pms rather than having to do it by hand.
* Move the bundled Error into its own directory so we can bundle other modules.
In addition...
* Add all the .pm files to the all dependency in the alternative Makefile
---
perl/Makefile | 6 ++--
perl/Makefile.PL | 42 +++++++++++++----------
perl/{private-Error.pm => bundles/Error/Error.pm} | 0
perl/bundles/README | 10 ++++++
4 files changed, 36 insertions(+), 22 deletions(-)
rename perl/{private-Error.pm => bundles/Error/Error.pm} (100%)
create mode 100644 perl/bundles/README
diff --git a/perl/Makefile b/perl/Makefile
index 6ca7d47..4f25930 100644
--- a/perl/Makefile
+++ b/perl/Makefile
@@ -33,7 +33,7 @@ modules += Git/SVN/Prompt
modules += Git/SVN/Ra
$(makfile): ../GIT-CFLAGS Makefile
- echo all: private-Error.pm Git.pm Git/I18N.pm > $@
+ echo all: bundles/Error/Error.pm $(modules) > $@
set -e; \
for i in $(modules); \
do \
@@ -49,7 +49,7 @@ $(makfile): ../GIT-CFLAGS Makefile
done
echo ' $(RM) blib/lib/Error.pm' >> $@
'$(PERL_PATH_SQ)' -MError -e 'exit($$Error::VERSION < 0.15009)' || \
- echo ' cp private-Error.pm blib/lib/Error.pm' >> $@
+ echo ' cp bundles/Error/Error.pm blib/lib/Error.pm' >> $@
echo install: >> $@
set -e; \
for i in $(modules); \
@@ -66,7 +66,7 @@ $(makfile): ../GIT-CFLAGS Makefile
done
echo ' $(RM) "$$(DESTDIR)$(instdir_SQ)/Error.pm"' >> $@
'$(PERL_PATH_SQ)' -MError -e 'exit($$Error::VERSION < 0.15009)' || \
- echo ' cp private-Error.pm "$$(DESTDIR)$(instdir_SQ)/Error.pm"' >> $@
+ echo ' cp bundles/Error/Error.pm "$$(DESTDIR)$(instdir_SQ)/Error.pm"' >> $@
echo instlibdir: >> $@
echo ' echo $(instdir_SQ)' >> $@
else
diff --git a/perl/Makefile.PL b/perl/Makefile.PL
index b54b04a..000d370 100644
--- a/perl/Makefile.PL
+++ b/perl/Makefile.PL
@@ -2,11 +2,16 @@ use strict;
use warnings;
use ExtUtils::MakeMaker;
use Getopt::Long;
+use File::Find;
+
+# Don't forget to update the perl/Makefile, too.
+# Don't forget to test with NO_PERL_MAKEMAKER=YesPlease
# Sanity: die at first unknown option
Getopt::Long::Configure qw/ pass_through /;
-GetOptions("localedir=s" => \my $localedir);
+my $localedir = '';
+GetOptions("localedir=s" => \$localedir);
sub MY::postamble {
return <<'MAKE_FRAG';
@@ -24,27 +29,25 @@ endif
MAKE_FRAG
}
-# XXX. When editing this list:
-#
-# * Please update perl/Makefile, too.
-# * Don't forget to test with NO_PERL_MAKEMAKER=YesPlease
-my %pm = (
- 'Git.pm' => '$(INST_LIBDIR)/Git.pm',
- 'Git/I18N.pm' => '$(INST_LIBDIR)/Git/I18N.pm',
- 'Git/SVN/Memoize/YAML.pm' => '$(INST_LIBDIR)/Git/SVN/Memoize/YAML.pm',
- 'Git/SVN/Fetcher.pm' => '$(INST_LIBDIR)/Git/SVN/Fetcher.pm',
- 'Git/SVN/Editor.pm' => '$(INST_LIBDIR)/Git/SVN/Editor.pm',
- 'Git/SVN/Prompt.pm' => '$(INST_LIBDIR)/Git/SVN/Prompt.pm',
- 'Git/SVN/Ra.pm' => '$(INST_LIBDIR)/Git/SVN/Ra.pm',
-);
-
+my @pmlibdirs = ("Git");
# We come with our own bundled Error.pm. It's not in the set of default
# Perl modules so install it if it's not available on the system yet.
-eval { require Error };
-if ($@ || $Error::VERSION < 0.15009) {
- $pm{'private-Error.pm'} = '$(INST_LIBDIR)/Error.pm';
+if ( !eval { require Error } || $Error::VERSION < 0.15009) {
+ push @pmlibdirs, "bundles/Error";
}
+
+# Find all the .pm files in @pmlibdirs which includes our bundled modules.
+my %pms;
+find sub {
+ return unless /\.pm$/;
+
+ my $inst = $File::Find::name;
+ $inst =~ s{bundles/[^/]+/?}{};
+ $pms{$File::Find::name} = '$(INST_LIBDIR)/'.$inst;
+}, @pmlibdirs;
+
+
# redirect stdout, otherwise the message "Writing perl.mak for Git"
# disrupts the output for the target 'instlibdir'
open STDOUT, ">&STDERR";
@@ -52,8 +55,9 @@ open STDOUT, ">&STDERR";
WriteMakefile(
NAME => 'Git',
VERSION_FROM => 'Git.pm',
- PM => \%pm,
+ PM => \%pms,
PM_FILTER => qq[\$(PERL) -pe "s<\\Q++LOCALEDIR++\\E><$localedir>"],
MAKEFILE => 'perl.mak',
INSTALLSITEMAN3DIR => '$(SITEPREFIX)/share/man/man3'
);
+
diff --git a/perl/private-Error.pm b/perl/bundles/Error/Error.pm
similarity index 100%
rename from perl/private-Error.pm
rename to perl/bundles/Error/Error.pm
diff --git a/perl/bundles/README b/perl/bundles/README
new file mode 100644
index 0000000..8a9ce39
--- /dev/null
+++ b/perl/bundles/README
@@ -0,0 +1,10 @@
+This is any Perl modules we might want to bundle because Perl doesn't (or didn't)
+ship with them. Each directory is a distribution containing all the PM files.
+
+For example, if you wanted to bundle URI...
+1) mkdir bundles/URI/
+
+2) build URI & cp -r blib/lib/* into bundles/URI
+
+3) add bundles/URI to @pmlibdirs in the Makefile.PL with the
+ appropriate check for existance and high enough version
--
1.7.11.1
--
Alligator sandwich, and make it snappy!
^ permalink raw reply related [flat|nested] 34+ messages in thread* Re: Find .pm files automatically (was Re: Fix git-svn tests for SVN 1.7.5.)
2012-07-17 23:05 ` Find .pm files automatically " Michael G Schwern
@ 2012-07-18 0:01 ` Jonathan Nieder
2012-07-18 1:41 ` Michael G Schwern
0 siblings, 1 reply; 34+ messages in thread
From: Jonathan Nieder @ 2012-07-18 0:01 UTC (permalink / raw)
To: Michael G Schwern; +Cc: git, gitster, robbat2, Eric Wong, Ben Walton
Hi,
Michael G Schwern wrote:
> Ok, here goes.
>
> First patch overhauls perl/Makefile.PL to make it easier to add .pm files,
> which I'm going to be doing a lot of. Instead of having to manually add to
> the %pm hash, it scans for .pm files.
An excellent goal.
> It also moves Error.pm into a bundle directory. This both makes it just
> another directory to scan (or not scan), but it also makes it possible to
> bundle additional modules in the future. ExtUtils::MakeMaker uses this
> technique itself.
This is not so much "also" as "as an example to demonstrate the
technique", no? I guess I'd prefer it to be in a separate patch, but
this way's fine, too.
[...]
> From 47a723a860cded6b16a716ea74c5bc029ee5b0ac Mon Sep 17 00:00:00 2001
> From: "Michael G. Schwern" <schwern@pobox.com>
> Date: Thu, 12 Jul 2012 00:05:38 -0700
> Subject: [PATCH 01/11] Make the process of adding a module less blecherous.
>
> * Scan for .pm files and build %pms rather than having to do it by hand.
> * Move the bundled Error into its own directory so we can bundle other modules.
>
> In addition...
> * Add all the .pm files to the all dependency in the alternative Makefile
> ---
You'll probably hate this. Because we have a bunch of patches to
incorporate, I think it's worth spending the time to make that go as
smoothly as possible for later patches.
- the "From 47a723..." line is for your mailer. Please do not
include it in the body of your message.
- likewise for the From: and Date: lines which are redundant next to
the corresponding fields in the mail header
- comments that are useful for posterity, like
This patch overhauls perl/Makefile.PL to make it easier to add .pm files,
which I'm going to be doing a lot of. Instead of having to manually add to
the %pm hash, it scans for .pm files.
should go above the triple-dash marker, while comments that are
less useful, like "Hi", go after the triple-dash.
- using a different subject line for each patch makes the reader's
life much easier, so please do use the subject lines from your
commits in the mail header.
The git-format-patch(1) manpage has some instructions for using
Thunderbird to send patches, which should take care of all this
automatically.
If everything is right, then maintainers will love you because they
can save a bunch of your patches that look ready into a single mbox
and apply them all at once with "git am".
As Documentation/SubmittingPatches explains under "MUA specific
hints", you can test that a patch is being sent correctly by emailing
it to yourself, saving as an mbox, and trying to apply it with "git am
<path to mbox file>". If the resulting commit is as expected, then
you've succeeded.
> perl/Makefile | 6 ++--
> perl/Makefile.PL | 42 +++++++++++++----------
> perl/{private-Error.pm => bundles/Error/Error.pm} | 0
> perl/bundles/README | 10 ++++++
> 4 files changed, 36 insertions(+), 22 deletions(-)
> rename perl/{private-Error.pm => bundles/Error/Error.pm} (100%)
> create mode 100644 perl/bundles/README
Ok, on to the patch proper.
> diff --git a/perl/Makefile b/perl/Makefile
> index 6ca7d47..4f25930 100644
> --- a/perl/Makefile
> +++ b/perl/Makefile
> @@ -33,7 +33,7 @@ modules += Git/SVN/Prompt
> modules += Git/SVN/Ra
>
> $(makfile): ../GIT-CFLAGS Makefile
> - echo all: private-Error.pm Git.pm Git/I18N.pm > $@
> + echo all: bundles/Error/Error.pm $(modules) > $@
The word "bundles/" left me a little nervous, because I (ignorantly)
imagined that this might be some specialized facility like Python eggs
or Ruby gems. Is the intent that this directory contains CPAN modules
we want to be able to depend on? Is there really any intention of
having more of them than Error.pm?
Before this patch, in the default case (with MakeMaker), "make
install" wrote a manpage in <mandir>/man3/private-Error.3pm. Does it
still do so after the patch? Will people who have installation
scripts that expected that manpage have to change them, and if so, is
the new behavior better to make up for that effort?
[...]
> --- a/perl/Makefile.PL
> +++ b/perl/Makefile.PL
> @@ -2,11 +2,16 @@ use strict;
> use warnings;
> use ExtUtils::MakeMaker;
> use Getopt::Long;
> +use File::Find;
> +
> +# Don't forget to update the perl/Makefile, too.
> +# Don't forget to test with NO_PERL_MAKEMAKER=YesPlease
Now the reader will have no reason to be looking at this file, so
these comments are pretty much useless. In an ideal world, "make
test" in the MakeMaker build would automatically "grep perl/Makefile"
to catch modules that are not listed there, but that can wait, I
imagine.
Alternatively, maybe there could be a perl/modules.list that both
makefiles read? That way, if I drop in an unrelated .pm file for
reference while coding the build system would not be confused by
it, and since both build systems would use the same module list
there would be no risk of it falling out of date.
Thanks for some food for thought, and hope that helps,
Jonathan
^ permalink raw reply [flat|nested] 34+ messages in thread* Re: Find .pm files automatically (was Re: Fix git-svn tests for SVN 1.7.5.)
2012-07-18 0:01 ` Jonathan Nieder
@ 2012-07-18 1:41 ` Michael G Schwern
2012-07-18 2:14 ` Jonathan Nieder
0 siblings, 1 reply; 34+ messages in thread
From: Michael G Schwern @ 2012-07-18 1:41 UTC (permalink / raw)
To: Jonathan Nieder; +Cc: git, gitster, robbat2, Eric Wong, Ben Walton
On 2012.7.17 5:01 PM, Jonathan Nieder wrote:
>> It also moves Error.pm into a bundle directory. This both makes it just
>> another directory to scan (or not scan), but it also makes it possible to
>> bundle additional modules in the future. ExtUtils::MakeMaker uses this
>> technique itself.
>
> This is not so much "also" as "as an example to demonstrate the
> technique", no? I guess I'd prefer it to be in a separate patch, but
> this way's fine, too.
I wrote the MakeMaker system so I was just cribbing off that. It made more
sense to build a list of directories to scan and then scan them than to add
individual file exceptions later. I could put it in a separate patch, but it
would require some bending.
> You'll probably hate this. Because we have a bunch of patches to
> incorporate, I think it's worth spending the time to make that go as
> smoothly as possible for later patches.
Sorry. I have lots of experience with git but very little with the email
submission tools. I've always either just done everything via repositories or
used Github.
It sounds like I should figure out the git-send-email tool and do this very
slowly.
> The word "bundles/" left me a little nervous, because I (ignorantly)
> imagined that this might be some specialized facility like Python eggs
> or Ruby gems.
Nope, just copy .pm files in.
> Is the intent that this directory contains CPAN modules
> we want to be able to depend on?
Yes.
> Is there really any intention of having more of them than Error.pm?
No idea, this is my first look at the code, but now it's possible. In my
experience, if there's a barrier to using CPAN modules then people won't use
them. They'll rewrite the functionality poorly.
> Before this patch, in the default case (with MakeMaker), "make
> install" wrote a manpage in <mandir>/man3/private-Error.3pm. Does it
> still do so after the patch? Will people who have installation
> scripts that expected that manpage have to change them, and if so, is
> the new behavior better to make up for that effort?
The man page is now man3/bundles::Error::Error.3 which is equally as incorrect
as man3/private-Error.3. It is possible to correct that so it's man3/Error.3,
but that's going to require some effort. Basically its in the same boat as
PM. Once you have to change one you have to change them all.
Why do install scripts have specific code to look for that man page?
If it's going to be trouble I can put Error.pm back. It's just something I
did in passing.
>> +# Don't forget to update the perl/Makefile, too.
>> +# Don't forget to test with NO_PERL_MAKEMAKER=YesPlease
>
> Now the reader will have no reason to be looking at this file, so
> these comments are pretty much useless. In an ideal world, "make
> test" in the MakeMaker build would automatically "grep perl/Makefile"
> to catch modules that are not listed there, but that can wait, I
> imagine.
>
> Alternatively, maybe there could be a perl/modules.list that both
> makefiles read? That way, if I drop in an unrelated .pm file for
> reference while coding the build system would not be confused by
> it, and since both build systems would use the same module list
> there would be no risk of it falling out of date.
Ideally, that second Makefile would go away. Parallel build systems are extra
work and generate bugs.
The log suggests it might have something to do with people wanting to build
with an ActiveState Perl on Cygwin or something? MakeMaker builds different
Makefiles depending on the OS, so it may be as simple as telling Makefile.PL
what flavor of make you're using.
--
emacs -- THAT'S NO EDITOR... IT'S AN OPERATING SYSTEM!
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: Find .pm files automatically (was Re: Fix git-svn tests for SVN 1.7.5.)
2012-07-18 1:41 ` Michael G Schwern
@ 2012-07-18 2:14 ` Jonathan Nieder
0 siblings, 0 replies; 34+ messages in thread
From: Jonathan Nieder @ 2012-07-18 2:14 UTC (permalink / raw)
To: Michael G Schwern; +Cc: git, gitster, robbat2, Eric Wong, Ben Walton
Thanks for the reply. Quick clarifications:
Michael G Schwern wrote:
> The man page is now man3/bundles::Error::Error.3 which is equally as incorrect
> as man3/private-Error.3. It is possible to correct that so it's man3/Error.3,
> but that's going to require some effort. Basically its in the same boat as
> PM. Once you have to change one you have to change them all.
>
> Why do install scripts have specific code to look for that man page?
To delete it. :)
[...]
> Ideally, that second Makefile would go away. Parallel build systems are extra
> work and generate bugs.
Agreed --- I'd love to see the NO_PERL_MAKEMAKER option go away.
> The log suggests it might have something to do with people wanting to build
> with an ActiveState Perl on Cygwin or something? MakeMaker builds different
> Makefiles depending on the OS, so it may be as simple as telling Makefile.PL
> what flavor of make you're using.
I think the main user is the ordinary Git for Windows build (which
uses perl 5.8.8, from mingw or msys I imagine). If you have the
Windows expertise to help them or know someone who could, I'm sure
they'd be happy to switch their git build to use MakeMaker.
Website: http://msysgit.github.com/
Thanks,
Jonathan
^ permalink raw reply [flat|nested] 34+ messages in thread
* Extract Git classes from git-svn (2/10) (was Re: Fix git-svn tests for SVN 1.7.5.)
2012-07-17 17:44 ` Jonathan Nieder
2012-07-17 18:58 ` Michael G Schwern
2012-07-17 23:05 ` Find .pm files automatically " Michael G Schwern
@ 2012-07-17 23:12 ` Michael G Schwern
2012-07-18 0:08 ` Jonathan Nieder
2012-07-18 10:58 ` Eric Wong
2012-07-17 23:13 ` Extract Git classes from git-svn (3/10) " Michael G Schwern
` (7 subsequent siblings)
10 siblings, 2 replies; 34+ messages in thread
From: Michael G Schwern @ 2012-07-17 23:12 UTC (permalink / raw)
To: Jonathan Nieder; +Cc: git, gitster, robbat2, Eric Wong, Ben Walton
>From 683a230e439f1d5ac2727ce4c2a74e93804fc72b Mon Sep 17 00:00:00 2001
From: "Michael G. Schwern" <schwern@pobox.com>
Date: Wed, 11 Jul 2012 22:16:01 -0700
Subject: [PATCH 03/11] Fix Git::SVN so it can at least compile alone.
It's still very intertwined with git-svn, but that's a lot of work. This
gets things working and tests passing again (as well as they were).
This required some parallel refactorings...
* fatal() moved out of git-svn into a new Git::SVN::Utils
* The $can_compress lexical moved into Git::SVN::Utils::can_compress()
* The $_prefix variable which stores the --prefix option is wrapped
in a function (rather than made global) so access to it can be
controlled. Git::SVN does not rely on this function being
available so it can work without git-svn loaded. In general,
the options should be put back together into a hash and accessed
via an options() function.
* A new tree of unit tests for the Git::SVN modules has been created.
It doesn't work with the existing Makefile, that can be worried
about later.
* Move initialization of Git::SVN globals into Git::SVN
* Have Git::SVN load the Git command* functions on its own
---
git-svn.perl | 33 ++++++++++++++++++---------------
perl/Git/SVN.pm | 29 ++++++++++++++++++++---------
perl/Git/SVN/Utils.pm | 19 +++++++++++++++++++
perl/Makefile | 2 ++
t/Git-SVN/00compile.t | 9 +++++++++
t/Git-SVN/Utils/can_compress.t | 11 +++++++++++
t/Git-SVN/Utils/fatal.t | 34 ++++++++++++++++++++++++++++++++++
7 files changed, 113 insertions(+), 24 deletions(-)
create mode 100644 perl/Git/SVN/Utils.pm
create mode 100644 t/Git-SVN/00compile.t
create mode 100644 t/Git-SVN/Utils/can_compress.t
create mode 100644 t/Git-SVN/Utils/fatal.t
diff --git a/git-svn.perl b/git-svn.perl
index 59db0a4..8a02d1c 100755
--- a/git-svn.perl
+++ b/git-svn.perl
@@ -10,6 +10,9 @@ use vars qw/ $AUTHOR $VERSION
$AUTHOR = 'Eric Wong <normalperson@yhbt.net>';
$VERSION = '@@GIT_VERSION@@';
+use Git::SVN;
+use Git::SVN::Utils qw(fatal can_compress);
+
# From which subdir have we been invoked?
my $cmd_dir_prefix = eval {
command_oneline([qw/rev-parse --show-prefix/], STDERR => 0)
@@ -17,10 +20,8 @@ my $cmd_dir_prefix = eval {
my $git_dir_user_set = 1 if defined $ENV{GIT_DIR};
$ENV{GIT_DIR} ||= '.git';
-$Git::SVN::default_repo_id = 'svn';
-$Git::SVN::default_ref_id = $ENV{GIT_SVN_ID} || 'git-svn';
+
$Git::SVN::Ra::_log_window_size = 100;
-$Git::SVN::_minimize_url = 'unset';
if (! exists $ENV{SVN_SSH} && exists $ENV{GIT_SSH}) {
$ENV{SVN_SSH} = $ENV{GIT_SSH};
@@ -35,8 +36,6 @@ $Git::SVN::Log::TZ = $ENV{TZ};
$ENV{TZ} = 'UTC';
$| = 1; # unbuffer STDOUT
-sub fatal (@) { print STDERR "@_\n"; exit 1 }
-
# All SVN commands do it. Otherwise we may die on SIGPIPE when the remote
# repository decides to close the connection which we expect to be kept alive.
$SIG{PIPE} = 'IGNORE';
@@ -66,7 +65,7 @@ sub _req_svn {
fatal "Need SVN::Core 1.1.0 or better (got $SVN::Core::VERSION)";
}
}
-my $can_compress = eval { require Compress::Zlib; 1};
+
use Carp qw/croak/;
use Digest::MD5;
use IO::File qw//;
@@ -89,7 +88,7 @@ BEGIN {
foreach (qw/command command_oneline command_noisy command_output_pipe
command_input_pipe command_close_pipe
command_bidi_pipe command_close_bidi_pipe/) {
- for my $package ( qw(Git::SVN::Migration Git::SVN::Log Git::SVN),
+ for my $package ( qw(Git::SVN::Migration Git::SVN::Log),
__PACKAGE__) {
*{"${package}::$_"} = \&{"Git::$_"};
}
@@ -109,7 +108,10 @@ my ($_stdin, $_help, $_edit,
$_merge, $_strategy, $_preserve_merges, $_dry_run, $_local,
$_prefix, $_no_checkout, $_url, $_verbose,
$_git_format, $_commit_url, $_tag, $_merge_info, $_interactive);
-$Git::SVN::_follow_parent = 1;
+
+# This is a refactoring artifact so Git::SVN can get at this variable.
+sub opt_prefix { return $_prefix || '' }
+
$Git::SVN::Fetcher::_placeholder_filename = ".gitignore";
$_q ||= 0;
my %remote_opts = ( 'username=s' => \$Git::SVN::Prompt::_username,
@@ -1578,7 +1580,7 @@ sub cmd_reset {
}
sub cmd_gc {
- if (!$can_compress) {
+ if (!can_compress()) {
warn "Compress::Zlib could not be found; unhandled.log " .
"files will not be compressed.\n";
}
@@ -2020,7 +2022,7 @@ sub md5sum {
}
sub gc_directory {
- if ($can_compress && -f $_ && basename($_) eq "unhandled.log") {
+ if (can_compress() && -f $_ && basename($_) eq "unhandled.log") {
my $out_filename = $_ . ".gz";
open my $in_fh, "<", $_ or die "Unable to open $_: $!\n";
binmode $in_fh;
@@ -2042,6 +2044,7 @@ sub gc_directory {
package Git::SVN::Log;
use strict;
use warnings;
+use Git::SVN::Utils qw(fatal);
use POSIX qw/strftime/;
use constant commit_log_separator => ('-' x 72) . "\n";
use vars qw/$TZ $limit $color $pager $non_recursive $verbose $oneline
@@ -2140,15 +2143,15 @@ sub config_pager {
sub run_pager {
return unless defined $pager;
pipe my ($rfd, $wfd) or return;
- defined(my $pid = fork) or ::fatal "Can't fork: $!";
+ defined(my $pid = fork) or fatal "Can't fork: $!";
if (!$pid) {
open STDOUT, '>&', $wfd or
- ::fatal "Can't redirect to stdout: $!";
+ fatal "Can't redirect to stdout: $!";
return;
}
- open STDIN, '<&', $rfd or ::fatal "Can't redirect stdin: $!";
+ open STDIN, '<&', $rfd or fatal "Can't redirect stdin: $!";
$ENV{LESS} ||= 'FRSX';
- exec $pager or ::fatal "Can't run pager: $! ($pager)";
+ exec $pager or fatal "Can't run pager: $! ($pager)";
}
sub format_svn_date {
@@ -2297,7 +2300,7 @@ sub cmd_show_log {
} elsif ($::_revision =~ /^\d+$/) {
$r_min = $r_max = $::_revision;
} else {
- ::fatal "-r$::_revision is not supported, use ",
+ fatal "-r$::_revision is not supported, use ",
"standard 'git log' arguments instead";
}
}
diff --git a/perl/Git/SVN.pm b/perl/Git/SVN.pm
index 533f1da..02983d6 100644
--- a/perl/Git/SVN.pm
+++ b/perl/Git/SVN.pm
@@ -1,11 +1,13 @@
package Git::SVN;
+
use strict;
use warnings;
+
use Fcntl qw/:DEFAULT :seek/;
use constant rev_map_fmt => 'NH40';
-use vars qw/$default_repo_id $default_ref_id $_no_metadata $_follow_parent
+use vars qw/$_no_metadata
$_repack $_repack_flags $_use_svm_props $_head
- $_use_svnsync_props $no_reuse_existing $_minimize_url
+ $_use_svnsync_props $no_reuse_existing
$_use_log_author $_add_author_from $_localtime/;
use Carp qw/croak/;
use File::Path qw/mkpath/;
@@ -20,6 +22,14 @@ BEGIN {
$can_use_yaml = eval { require Git::SVN::Memoize::YAML; 1};
}
+use Git qw(command command_oneline command_noisy command_output_pipe
command_close_pipe);
+use Git::SVN::Utils qw(fatal can_compress);
+
+our $_follow_parent = 1;
+our $_minimize_url = 'unset';
+our $default_repo_id = 'svn';
+our $default_ref_id = $ENV{GIT_SVN_ID} || 'git-svn';
+
my ($_gc_nr, $_gc_period);
# properties that we do not log:
@@ -840,8 +850,8 @@ sub assert_index_clean {
command_noisy('read-tree', $treeish);
$x = command_oneline('write-tree');
if ($y ne $x) {
- ::fatal "trees ($treeish) $y != $x\n",
- "Something is seriously wrong...";
+ fatal "trees ($treeish) $y != $x\n",
+ "Something is seriously wrong...";
}
});
}
@@ -1196,7 +1206,7 @@ sub mkemptydirs {
my %empty_dirs = ();
my $gz_file = "$self->{dir}/unhandled.log.gz";
if (-f $gz_file) {
- if (!$can_compress) {
+ if (!can_compress()) {
warn "Compress::Zlib could not be found; ",
"empty directories in $gz_file will not be read\n";
} else {
@@ -1879,7 +1889,7 @@ sub set_tree {
my ($self, $tree) = (shift, shift);
my $log_entry = ::get_commit_entry($tree);
unless ($self->{last_rev}) {
- ::fatal("Must have an existing revision to commit");
+ fatal("Must have an existing revision to commit");
}
my %ed_opts = ( r => $self->{last_rev},
log => $log_entry->{log},
@@ -2237,12 +2247,13 @@ sub find_rev_after {
sub _new {
my ($class, $repo_id, $ref_id, $path) = @_;
unless (defined $repo_id && length $repo_id) {
- $repo_id = $Git::SVN::default_repo_id;
+ $repo_id = $default_repo_id;
}
unless (defined $ref_id && length $ref_id) {
- $_prefix = '' unless defined($_prefix);
+ # Access the prefix option from the git-svn main program if it's loaded.
+ my $prefix = defined &::opt_prefix ? ::opt_prefix() : "";
$_[2] = $ref_id =
- "refs/remotes/$_prefix$Git::SVN::default_ref_id";
+ "refs/remotes/$prefix$default_ref_id";
}
$_[1] = $repo_id;
my $dir = "$ENV{GIT_DIR}/svn/$ref_id";
diff --git a/perl/Git/SVN/Utils.pm b/perl/Git/SVN/Utils.pm
new file mode 100644
index 0000000..d8f63e6
--- /dev/null
+++ b/perl/Git/SVN/Utils.pm
@@ -0,0 +1,19 @@
+package Git::SVN::Utils;
+
+use strict;
+use warnings;
+
+use base qw(Exporter);
+
+our @EXPORT_OK = qw(fatal can_compress);
+
+sub fatal (@) { print STDERR "@_\n"; exit 1 }
+
+my $can_compress;
+sub can_compress {
+ return $can_compress if defined $can_compress;
+
+ return $can_compress = eval { require Compress::Zlib; } ? 1 : 0;
+}
+
+1;
diff --git a/perl/Makefile b/perl/Makefile
index 4f25930..d0a0c5c 100644
--- a/perl/Makefile
+++ b/perl/Makefile
@@ -26,11 +26,13 @@ instdir_SQ = $(subst ','\'',$(prefix)/lib)
modules += Git
modules += Git/I18N
+modules += Git/SVN
modules += Git/SVN/Memoize/YAML
modules += Git/SVN/Fetcher
modules += Git/SVN/Editor
modules += Git/SVN/Prompt
modules += Git/SVN/Ra
+modules += Git/SVN/Utils
$(makfile): ../GIT-CFLAGS Makefile
echo all: bundles/Error/Error.pm $(modules) > $@
diff --git a/t/Git-SVN/00compile.t b/t/Git-SVN/00compile.t
new file mode 100644
index 0000000..c32ee4b
--- /dev/null
+++ b/t/Git-SVN/00compile.t
@@ -0,0 +1,9 @@
+#!/usr/bin/env perl
+
+use strict;
+use warnings;
+
+use Test::More tests => 2;
+
+require_ok 'Git::SVN';
+require_ok 'Git::SVN::Utils';
diff --git a/t/Git-SVN/Utils/can_compress.t b/t/Git-SVN/Utils/can_compress.t
new file mode 100644
index 0000000..d7b49b8
--- /dev/null
+++ b/t/Git-SVN/Utils/can_compress.t
@@ -0,0 +1,11 @@
+#!/usr/bin/perl
+
+use strict;
+use warnings;
+
+use Test::More 'no_plan';
+
+use Git::SVN::Utils qw(can_compress);
+
+# !! is the "convert this to boolean" operator.
+is !!can_compress(), !!eval { require Compress::Zlib };
diff --git a/t/Git-SVN/Utils/fatal.t b/t/Git-SVN/Utils/fatal.t
new file mode 100644
index 0000000..b90746c
--- /dev/null
+++ b/t/Git-SVN/Utils/fatal.t
@@ -0,0 +1,34 @@
+#!/usr/bin/perl
+
+use strict;
+use warnings;
+
+use Test::More 'no_plan';
+
+BEGIN {
+ # Override exit at BEGIN time before Git::SVN::Utils is loaded
+ # so it will see our local exit later.
+ *CORE::GLOBAL::exit = sub(;$) {
+ return @_ ? CORE::exit($_[0]) : CORE::exit();
+ };
+}
+
+use Git::SVN::Utils qw(fatal);
+
+# fatal()
+{
+ # Capture the exit code and prevent exit.
+ my $exit_status;
+ no warnings 'redefine';
+ local *CORE::GLOBAL::exit = sub { $exit_status = $_[0] || 0 };
+
+ # Trap fatal's message to STDERR
+ my $stderr;
+ close STDERR;
+ ok open STDERR, ">", \$stderr;
+
+ fatal "Some", "Stuff", "Happened";
+
+ is $stderr, "Some Stuff Happened\n";
+ is $exit_status, 1;
+}
--
1.7.11.1
^ permalink raw reply related [flat|nested] 34+ messages in thread* Re: Extract Git classes from git-svn (2/10) (was Re: Fix git-svn tests for SVN 1.7.5.)
2012-07-17 23:12 ` Extract Git classes from git-svn (2/10) " Michael G Schwern
@ 2012-07-18 0:08 ` Jonathan Nieder
2012-07-18 10:58 ` Eric Wong
1 sibling, 0 replies; 34+ messages in thread
From: Jonathan Nieder @ 2012-07-18 0:08 UTC (permalink / raw)
To: Michael G Schwern; +Cc: git, gitster, robbat2, Eric Wong, Ben Walton
Hi,
Michael G Schwern wrote:
> From 683a230e439f1d5ac2727ce4c2a74e93804fc72b Mon Sep 17 00:00:00 2001
> From: "Michael G. Schwern" <schwern@pobox.com>
> Date: Wed, 11 Jul 2012 22:16:01 -0700
Just like with patch 1, the mail body should lose the above.
> Subject: [PATCH 03/11] Fix Git::SVN so it can at least compile alone.
Did I miss patch 2?
> It's still very intertwined with git-svn, but that's a lot of work. This
> gets things working and tests passing again (as well as they were).
>
> This required some parallel refactorings...
>
> * fatal() moved out of git-svn into a new Git::SVN::Utils
[...]
> git-svn.perl | 33 ++++++++++++++++++---------------
> perl/Git/SVN.pm | 29 ++++++++++++++++++++---------
> perl/Git/SVN/Utils.pm | 19 +++++++++++++++++++
> perl/Makefile | 2 ++
> t/Git-SVN/00compile.t | 9 +++++++++
> t/Git-SVN/Utils/can_compress.t | 11 +++++++++++
> t/Git-SVN/Utils/fatal.t | 34 ++++++++++++++++++++++++++++++++++
> 7 files changed, 113 insertions(+), 24 deletions(-)
> create mode 100644 perl/Git/SVN/Utils.pm
> create mode 100644 t/Git-SVN/00compile.t
> create mode 100644 t/Git-SVN/Utils/can_compress.t
> create mode 100644 t/Git-SVN/Utils/fatal.t
It seems like a lot is going on in the one patch. Probably most of
the changes are good, but if this causes a regression we would have no
choice but to revert the whole thing, which would be unfeasible
because of later patches building on it.
So in other words, a patch like this that makes a lot of changes at
once would make life very hard for the maintainer, I imagine.
What is the motivation behind these changes? Can they be untangled
from each other and applied one at a time, in such a way that each
incremental change looks obviously correct?
Since I'm missing the patch that created Git/SVN.pm in the first
place, I can't tell --- did that patch break the build and this one
fixes it? In that case, the order of the two patches should be
swapped to ensure "git bisect" is still usable.
Sorry, I wish I had better news to mix in with all this. I am
thrilled to see this is making the internal APIs saner and adding
tests so I hope we can get it in in a way that makes regressions
unlikely.
Thanks,
Jonathan
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: Extract Git classes from git-svn (2/10) (was Re: Fix git-svn tests for SVN 1.7.5.)
2012-07-17 23:12 ` Extract Git classes from git-svn (2/10) " Michael G Schwern
2012-07-18 0:08 ` Jonathan Nieder
@ 2012-07-18 10:58 ` Eric Wong
2012-07-19 0:11 ` Michael G Schwern
1 sibling, 1 reply; 34+ messages in thread
From: Eric Wong @ 2012-07-18 10:58 UTC (permalink / raw)
To: Michael G Schwern; +Cc: Jonathan Nieder, git, gitster, robbat2, Ben Walton
Michael G Schwern <schwern@pobox.com> wrote:
Hi Michael, thanks for taking your time to help with this.
I agree with everything Jonathan said (and thank him for taking
the time to point you in the right direction).
I too (very strongly) prefer email for code review. I doubt I would've
ever gotten involved if git were run differently. I'm actually
disappointed the mailing list culture that built git hasn't rubbed off
to other projects that adopt git.
> +++ b/t/Git-SVN/00compile.t
> +use Test::More tests => 2;
I prefer not declaring test counts and using done_testing() instead.
done_testing() is favorable to me in at least 2 ways:
* done_testing() closely matches the behavior of the existing
sh-based test suite in git (which calls test_done)
* maintaining test counts leads to unnecessary merge conflicts
Skipping the tests on old versions of Test::More (< 0.88) is acceptable
to me (especially since integration tests provide the real coverage
already).
> +++ b/t/Git-SVN/Utils/can_compress.t
> +use Test::More 'no_plan';
no_plan is the worst way to use Test::More, I think.
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: Extract Git classes from git-svn (2/10) (was Re: Fix git-svn tests for SVN 1.7.5.)
2012-07-18 10:58 ` Eric Wong
@ 2012-07-19 0:11 ` Michael G Schwern
0 siblings, 0 replies; 34+ messages in thread
From: Michael G Schwern @ 2012-07-19 0:11 UTC (permalink / raw)
To: Eric Wong; +Cc: Jonathan Nieder, git, gitster, robbat2, Ben Walton
On 2012.7.18 3:58 AM, Eric Wong wrote:
> I agree with everything Jonathan said (and thank him for taking
> the time to point you in the right direction).
Thanks, you guys have been very nice to my flailing and failing. I'm going to
back off and send out a sort of overview email so we can figure out how best
to chunk this up.
>> +++ b/t/Git-SVN/00compile.t
>
>> +use Test::More tests => 2;
>
> I prefer not declaring test counts and using done_testing() instead.
> done_testing() is favorable to me in at least 2 ways:
>
> * done_testing() closely matches the behavior of the existing
> sh-based test suite in git (which calls test_done)
>
> * maintaining test counts leads to unnecessary merge conflicts
Yes, I concur 100%. So much that I went back in my time machine and added
done_testing() to Test::More! Also I killed Hitler, so now WWII ends in 1945.
Things seem to have turned out for the better.
I love it when people advocate my features back to me. :) I didn't use
done_testing because I didn't know your stance on using non-5.8 core versions
of modules.
> Skipping the tests on old versions of Test::More (< 0.88) is acceptable
> to me (especially since integration tests provide the real coverage
> already).
It is very easy to bundle an uninstalled copy of Test::More, probably easier
than putting in the code necessary to check for it and skip it. A lot of Perl
modules do it. The usual thing is to put it into t/lib/ and add "use lib
't/lib'" to the tests. I don't see any reason why that basic technique
wouldn't work here, with some minor changes to match the Git test suite.
I can help you with that, but I'd like to get through this SVN 1.7 fix first.
--
Being faith-based doesn't trump reality.
-- Bruce Sterling
^ permalink raw reply [flat|nested] 34+ messages in thread
* Extract Git classes from git-svn (3/10) (was Re: Fix git-svn tests for SVN 1.7.5.)
2012-07-17 17:44 ` Jonathan Nieder
` (2 preceding siblings ...)
2012-07-17 23:12 ` Extract Git classes from git-svn (2/10) " Michael G Schwern
@ 2012-07-17 23:13 ` Michael G Schwern
2012-07-18 0:12 ` Jonathan Nieder
2012-07-17 23:16 ` Extract Git classes from git-svn (6/10) " Michael G Schwern
` (6 subsequent siblings)
10 siblings, 1 reply; 34+ messages in thread
From: Michael G Schwern @ 2012-07-17 23:13 UTC (permalink / raw)
To: Jonathan Nieder; +Cc: git, gitster, robbat2, Eric Wong, Ben Walton
>From 5f0b609e9b0a70c86c46b48f0b180c96c3355a14 Mon Sep 17 00:00:00 2001
From: "Michael G. Schwern" <schwern@pobox.com>
Date: Tue, 17 Jul 2012 15:40:03 -0700
Subject: [PATCH 04/11] Extract Git::SVN::Log from git-svn.
This is a straight cut & paste. Next commit will make it work. This will
make it easier to see the differences in Git::SVN::Log.
---
git-svn.perl | 387 ---------------------------------------------------
perl/Git/SVN/Log.pm | 388 ++++++++++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 388 insertions(+), 387 deletions(-)
create mode 100644 perl/Git/SVN/Log.pm
diff --git a/git-svn.perl b/git-svn.perl
index 8a02d1c..5e6e3b5 100755
--- a/git-svn.perl
+++ b/git-svn.perl
@@ -2041,393 +2041,6 @@ sub gc_directory {
}
-package Git::SVN::Log;
-use strict;
-use warnings;
-use Git::SVN::Utils qw(fatal);
-use POSIX qw/strftime/;
-use constant commit_log_separator => ('-' x 72) . "\n";
-use vars qw/$TZ $limit $color $pager $non_recursive $verbose $oneline
- %rusers $show_commit $incremental/;
-my $l_fmt;
-
-sub cmt_showable {
- my ($c) = @_;
- return 1 if defined $c->{r};
-
- # big commit message got truncated by the 16k pretty buffer in rev-list
- if ($c->{l} && $c->{l}->[-1] eq "...\n" &&
- $c->{a_raw} =~ /\@([a-f\d\-]+)>$/) {
- @{$c->{l}} = ();
- my @log = command(qw/cat-file commit/, $c->{c});
-
- # shift off the headers
- shift @log while ($log[0] ne '');
- shift @log;
-
- # TODO: make $c->{l} not have a trailing newline in the future
- @{$c->{l}} = map { "$_\n" } grep !/^git-svn-id: /, @log;
-
- (undef, $c->{r}, undef) = ::extract_metadata(
- (grep(/^git-svn-id: /, @log))[-1]);
- }
- return defined $c->{r};
-}
-
-sub log_use_color {
- return $color || Git->repository->get_colorbool('color.diff');
-}
-
-sub git_svn_log_cmd {
- my ($r_min, $r_max, @args) = @_;
- my $head = 'HEAD';
- my (@files, @log_opts);
- foreach my $x (@args) {
- if ($x eq '--' || @files) {
- push @files, $x;
- } else {
- if (::verify_ref("$x^0")) {
- $head = $x;
- } else {
- push @log_opts, $x;
- }
- }
- }
-
- my ($url, $rev, $uuid, $gs) = ::working_head_info($head);
- $gs ||= Git::SVN->_new;
- my @cmd = (qw/log --abbrev-commit --pretty=raw --default/,
- $gs->refname);
- push @cmd, '-r' unless $non_recursive;
- push @cmd, qw/--raw --name-status/ if $verbose;
- push @cmd, '--color' if log_use_color();
- push @cmd, @log_opts;
- if (defined $r_max && $r_max == $r_min) {
- push @cmd, '--max-count=1';
- if (my $c = $gs->rev_map_get($r_max)) {
- push @cmd, $c;
- }
- } elsif (defined $r_max) {
- if ($r_max < $r_min) {
- ($r_min, $r_max) = ($r_max, $r_min);
- }
- my (undef, $c_max) = $gs->find_rev_before($r_max, 1, $r_min);
- my (undef, $c_min) = $gs->find_rev_after($r_min, 1, $r_max);
- # If there are no commits in the range, both $c_max and $c_min
- # will be undefined. If there is at least 1 commit in the
- # range, both will be defined.
- return () if !defined $c_min || !defined $c_max;
- if ($c_min eq $c_max) {
- push @cmd, '--max-count=1', $c_min;
- } else {
- push @cmd, '--boundary', "$c_min..$c_max";
- }
- }
- return (@cmd, @files);
-}
-
-# adapted from pager.c
-sub config_pager {
- if (! -t *STDOUT) {
- $ENV{GIT_PAGER_IN_USE} = 'false';
- $pager = undef;
- return;
- }
- chomp($pager = command_oneline(qw(var GIT_PAGER)));
- if ($pager eq 'cat') {
- $pager = undef;
- }
- $ENV{GIT_PAGER_IN_USE} = defined($pager);
-}
-
-sub run_pager {
- return unless defined $pager;
- pipe my ($rfd, $wfd) or return;
- defined(my $pid = fork) or fatal "Can't fork: $!";
- if (!$pid) {
- open STDOUT, '>&', $wfd or
- fatal "Can't redirect to stdout: $!";
- return;
- }
- open STDIN, '<&', $rfd or fatal "Can't redirect stdin: $!";
- $ENV{LESS} ||= 'FRSX';
- exec $pager or fatal "Can't run pager: $! ($pager)";
-}
-
-sub format_svn_date {
- my $t = shift || time;
- my $gmoff = Git::SVN::get_tz($t);
- return strftime("%Y-%m-%d %H:%M:%S $gmoff (%a, %d %b %Y)", localtime($t));
-}
-
-sub parse_git_date {
- my ($t, $tz) = @_;
- # Date::Parse isn't in the standard Perl distro :(
- if ($tz =~ s/^\+//) {
- $t += tz_to_s_offset($tz);
- } elsif ($tz =~ s/^\-//) {
- $t -= tz_to_s_offset($tz);
- }
- return $t;
-}
-
-sub set_local_timezone {
- if (defined $TZ) {
- $ENV{TZ} = $TZ;
- } else {
- delete $ENV{TZ};
- }
-}
-
-sub tz_to_s_offset {
- my ($tz) = @_;
- $tz =~ s/(\d\d)$//;
- return ($1 * 60) + ($tz * 3600);
-}
-
-sub get_author_info {
- my ($dest, $author, $t, $tz) = @_;
- $author =~ s/(?:^\s*|\s*$)//g;
- $dest->{a_raw} = $author;
- my $au;
- if ($::_authors) {
- $au = $rusers{$author} || undef;
- }
- if (!$au) {
- ($au) = ($author =~ /<([^>]+)\@[^>]+>$/);
- }
- $dest->{t} = $t;
- $dest->{tz} = $tz;
- $dest->{a} = $au;
- $dest->{t_utc} = parse_git_date($t, $tz);
-}
-
-sub process_commit {
- my ($c, $r_min, $r_max, $defer) = @_;
- if (defined $r_min && defined $r_max) {
- if ($r_min == $c->{r} && $r_min == $r_max) {
- show_commit($c);
- return 0;
- }
- return 1 if $r_min == $r_max;
- if ($r_min < $r_max) {
- # we need to reverse the print order
- return 0 if (defined $limit && --$limit < 0);
- push @$defer, $c;
- return 1;
- }
- if ($r_min != $r_max) {
- return 1 if ($r_min < $c->{r});
- return 1 if ($r_max > $c->{r});
- }
- }
- return 0 if (defined $limit && --$limit < 0);
- show_commit($c);
- return 1;
-}
-
-sub show_commit {
- my $c = shift;
- if ($oneline) {
- my $x = "\n";
- if (my $l = $c->{l}) {
- while ($l->[0] =~ /^\s*$/) { shift @$l }
- $x = $l->[0];
- }
- $l_fmt ||= 'A' . length($c->{r});
- print 'r',pack($l_fmt, $c->{r}),' | ';
- print "$c->{c} | " if $show_commit;
- print $x;
- } else {
- show_commit_normal($c);
- }
-}
-
-sub show_commit_changed_paths {
- my ($c) = @_;
- return unless $c->{changed};
- print "Changed paths:\n", @{$c->{changed}};
-}
-
-sub show_commit_normal {
- my ($c) = @_;
- print commit_log_separator, "r$c->{r} | ";
- print "$c->{c} | " if $show_commit;
- print "$c->{a} | ", format_svn_date($c->{t_utc}), ' | ';
- my $nr_line = 0;
-
- if (my $l = $c->{l}) {
- while ($l->[$#$l] eq "\n" && $#$l > 0
- && $l->[($#$l - 1)] eq "\n") {
- pop @$l;
- }
- $nr_line = scalar @$l;
- if (!$nr_line) {
- print "1 line\n\n\n";
- } else {
- if ($nr_line == 1) {
- $nr_line = '1 line';
- } else {
- $nr_line .= ' lines';
- }
- print $nr_line, "\n";
- show_commit_changed_paths($c);
- print "\n";
- print $_ foreach @$l;
- }
- } else {
- print "1 line\n";
- show_commit_changed_paths($c);
- print "\n";
-
- }
- foreach my $x (qw/raw stat diff/) {
- if ($c->{$x}) {
- print "\n";
- print $_ foreach @{$c->{$x}}
- }
- }
-}
-
-sub cmd_show_log {
- my (@args) = @_;
- my ($r_min, $r_max);
- my $r_last = -1; # prevent dupes
- set_local_timezone();
- if (defined $::_revision) {
- if ($::_revision =~ /^(\d+):(\d+)$/) {
- ($r_min, $r_max) = ($1, $2);
- } elsif ($::_revision =~ /^\d+$/) {
- $r_min = $r_max = $::_revision;
- } else {
- fatal "-r$::_revision is not supported, use ",
- "standard 'git log' arguments instead";
- }
- }
-
- config_pager();
- @args = git_svn_log_cmd($r_min, $r_max, @args);
- if (!@args) {
- print commit_log_separator unless $incremental || $oneline;
- return;
- }
- my $log = command_output_pipe(@args);
- run_pager();
- my (@k, $c, $d, $stat);
- my $esc_color = qr/(?:\033\[(?:(?:\d+;)*\d*)?m)*/;
- while (<$log>) {
- if (/^${esc_color}commit (?:- )?($::sha1_short)/o) {
- my $cmt = $1;
- if ($c && cmt_showable($c) && $c->{r} != $r_last) {
- $r_last = $c->{r};
- process_commit($c, $r_min, $r_max, \@k) or
- goto out;
- }
- $d = undef;
- $c = { c => $cmt };
- } elsif (/^${esc_color}author (.+) (\d+) ([\-\+]?\d+)$/o) {
- get_author_info($c, $1, $2, $3);
- } elsif (/^${esc_color}(?:tree|parent|committer) /o) {
- # ignore
- } elsif (/^${esc_color}:\d{6} \d{6} $::sha1_short/o) {
- push @{$c->{raw}}, $_;
- } elsif (/^${esc_color}[ACRMDT]\t/) {
- # we could add $SVN->{svn_path} here, but that requires
- # remote access at the moment (repo_path_split)...
- s#^(${esc_color})([ACRMDT])\t#$1 $2 #o;
- push @{$c->{changed}}, $_;
- } elsif (/^${esc_color}diff /o) {
- $d = 1;
- push @{$c->{diff}}, $_;
- } elsif ($d) {
- push @{$c->{diff}}, $_;
- } elsif (/^\ .+\ \|\s*\d+\ $esc_color[\+\-]*
- $esc_color*[\+\-]*$esc_color$/x) {
- $stat = 1;
- push @{$c->{stat}}, $_;
- } elsif ($stat && /^ \d+ files changed, \d+ insertions/) {
- push @{$c->{stat}}, $_;
- $stat = undef;
- } elsif (/^${esc_color} (git-svn-id:.+)$/o) {
- ($c->{url}, $c->{r}, undef) = ::extract_metadata($1);
- } elsif (s/^${esc_color} //o) {
- push @{$c->{l}}, $_;
- }
- }
- if ($c && defined $c->{r} && $c->{r} != $r_last) {
- $r_last = $c->{r};
- process_commit($c, $r_min, $r_max, \@k);
- }
- if (@k) {
- ($r_min, $r_max) = ($r_max, $r_min);
- process_commit($_, $r_min, $r_max) foreach reverse @k;
- }
-out:
- close $log;
- print commit_log_separator unless $incremental || $oneline;
-}
-
-sub cmd_blame {
- my $path = pop;
-
- config_pager();
- run_pager();
-
- my ($fh, $ctx, $rev);
-
- if ($_git_format) {
- ($fh, $ctx) = command_output_pipe('blame', @_, $path);
- while (my $line = <$fh>) {
- if ($line =~ /^\^?([[:xdigit:]]+)\s/) {
- # Uncommitted edits show up as a rev ID of
- # all zeros, which we can't look up with
- # cmt_metadata
- if ($1 !~ /^0+$/) {
- (undef, $rev, undef) =
- ::cmt_metadata($1);
- $rev = '0' if (!$rev);
- } else {
- $rev = '0';
- }
- $rev = sprintf('%-10s', $rev);
- $line =~ s/^\^?[[:xdigit:]]+(\s)/$rev$1/;
- }
- print $line;
- }
- } else {
- ($fh, $ctx) = command_output_pipe('blame', '-p', @_, 'HEAD',
- '--', $path);
- my ($sha1);
- my %authors;
- my @buffer;
- my %dsha; #distinct sha keys
-
- while (my $line = <$fh>) {
- push @buffer, $line;
- if ($line =~ /^([[:xdigit:]]{40})\s\d+\s\d+/) {
- $dsha{$1} = 1;
- }
- }
-
- my $s2r = ::cmt_sha2rev_batch([keys %dsha]);
-
- foreach my $line (@buffer) {
- if ($line =~ /^([[:xdigit:]]{40})\s\d+\s\d+/) {
- $rev = $s2r->{$1};
- $rev = '0' if (!$rev)
- }
- elsif ($line =~ /^author (.*)/) {
- $authors{$rev} = $1;
- $authors{$rev} =~ s/\s/_/g;
- }
- elsif ($line =~ /^\t(.*)$/) {
- printf("%6s %10s %s\n", $rev, $authors{$rev}, $1);
- }
- }
- }
- command_close_pipe($fh, $ctx);
-}
-
package Git::SVN::Migration;
# these version numbers do NOT correspond to actual version numbers
# of git nor git-svn. They are just relative.
diff --git a/perl/Git/SVN/Log.pm b/perl/Git/SVN/Log.pm
new file mode 100644
index 0000000..bbec3b0
--- /dev/null
+++ b/perl/Git/SVN/Log.pm
@@ -0,0 +1,388 @@
+package Git::SVN::Log;
+use strict;
+use warnings;
+use Git::SVN::Utils qw(fatal);
+use POSIX qw/strftime/;
+use constant commit_log_separator => ('-' x 72) . "\n";
+use vars qw/$TZ $limit $color $pager $non_recursive $verbose $oneline
+ %rusers $show_commit $incremental/;
+my $l_fmt;
+
+sub cmt_showable {
+ my ($c) = @_;
+ return 1 if defined $c->{r};
+
+ # big commit message got truncated by the 16k pretty buffer in rev-list
+ if ($c->{l} && $c->{l}->[-1] eq "...\n" &&
+ $c->{a_raw} =~ /\@([a-f\d\-]+)>$/) {
+ @{$c->{l}} = ();
+ my @log = command(qw/cat-file commit/, $c->{c});
+
+ # shift off the headers
+ shift @log while ($log[0] ne '');
+ shift @log;
+
+ # TODO: make $c->{l} not have a trailing newline in the future
+ @{$c->{l}} = map { "$_\n" } grep !/^git-svn-id: /, @log;
+
+ (undef, $c->{r}, undef) = ::extract_metadata(
+ (grep(/^git-svn-id: /, @log))[-1]);
+ }
+ return defined $c->{r};
+}
+
+sub log_use_color {
+ return $color || Git->repository->get_colorbool('color.diff');
+}
+
+sub git_svn_log_cmd {
+ my ($r_min, $r_max, @args) = @_;
+ my $head = 'HEAD';
+ my (@files, @log_opts);
+ foreach my $x (@args) {
+ if ($x eq '--' || @files) {
+ push @files, $x;
+ } else {
+ if (::verify_ref("$x^0")) {
+ $head = $x;
+ } else {
+ push @log_opts, $x;
+ }
+ }
+ }
+
+ my ($url, $rev, $uuid, $gs) = ::working_head_info($head);
+ $gs ||= Git::SVN->_new;
+ my @cmd = (qw/log --abbrev-commit --pretty=raw --default/,
+ $gs->refname);
+ push @cmd, '-r' unless $non_recursive;
+ push @cmd, qw/--raw --name-status/ if $verbose;
+ push @cmd, '--color' if log_use_color();
+ push @cmd, @log_opts;
+ if (defined $r_max && $r_max == $r_min) {
+ push @cmd, '--max-count=1';
+ if (my $c = $gs->rev_map_get($r_max)) {
+ push @cmd, $c;
+ }
+ } elsif (defined $r_max) {
+ if ($r_max < $r_min) {
+ ($r_min, $r_max) = ($r_max, $r_min);
+ }
+ my (undef, $c_max) = $gs->find_rev_before($r_max, 1, $r_min);
+ my (undef, $c_min) = $gs->find_rev_after($r_min, 1, $r_max);
+ # If there are no commits in the range, both $c_max and $c_min
+ # will be undefined. If there is at least 1 commit in the
+ # range, both will be defined.
+ return () if !defined $c_min || !defined $c_max;
+ if ($c_min eq $c_max) {
+ push @cmd, '--max-count=1', $c_min;
+ } else {
+ push @cmd, '--boundary', "$c_min..$c_max";
+ }
+ }
+ return (@cmd, @files);
+}
+
+# adapted from pager.c
+sub config_pager {
+ if (! -t *STDOUT) {
+ $ENV{GIT_PAGER_IN_USE} = 'false';
+ $pager = undef;
+ return;
+ }
+ chomp($pager = command_oneline(qw(var GIT_PAGER)));
+ if ($pager eq 'cat') {
+ $pager = undef;
+ }
+ $ENV{GIT_PAGER_IN_USE} = defined($pager);
+}
+
+sub run_pager {
+ return unless defined $pager;
+ pipe my ($rfd, $wfd) or return;
+ defined(my $pid = fork) or fatal "Can't fork: $!";
+ if (!$pid) {
+ open STDOUT, '>&', $wfd or
+ fatal "Can't redirect to stdout: $!";
+ return;
+ }
+ open STDIN, '<&', $rfd or fatal "Can't redirect stdin: $!";
+ $ENV{LESS} ||= 'FRSX';
+ exec $pager or fatal "Can't run pager: $! ($pager)";
+}
+
+sub format_svn_date {
+ my $t = shift || time;
+ my $gmoff = Git::SVN::get_tz($t);
+ return strftime("%Y-%m-%d %H:%M:%S $gmoff (%a, %d %b %Y)", localtime($t));
+}
+
+sub parse_git_date {
+ my ($t, $tz) = @_;
+ # Date::Parse isn't in the standard Perl distro :(
+ if ($tz =~ s/^\+//) {
+ $t += tz_to_s_offset($tz);
+ } elsif ($tz =~ s/^\-//) {
+ $t -= tz_to_s_offset($tz);
+ }
+ return $t;
+}
+
+sub set_local_timezone {
+ if (defined $TZ) {
+ $ENV{TZ} = $TZ;
+ } else {
+ delete $ENV{TZ};
+ }
+}
+
+sub tz_to_s_offset {
+ my ($tz) = @_;
+ $tz =~ s/(\d\d)$//;
+ return ($1 * 60) + ($tz * 3600);
+}
+
+sub get_author_info {
+ my ($dest, $author, $t, $tz) = @_;
+ $author =~ s/(?:^\s*|\s*$)//g;
+ $dest->{a_raw} = $author;
+ my $au;
+ if ($::_authors) {
+ $au = $rusers{$author} || undef;
+ }
+ if (!$au) {
+ ($au) = ($author =~ /<([^>]+)\@[^>]+>$/);
+ }
+ $dest->{t} = $t;
+ $dest->{tz} = $tz;
+ $dest->{a} = $au;
+ $dest->{t_utc} = parse_git_date($t, $tz);
+}
+
+sub process_commit {
+ my ($c, $r_min, $r_max, $defer) = @_;
+ if (defined $r_min && defined $r_max) {
+ if ($r_min == $c->{r} && $r_min == $r_max) {
+ show_commit($c);
+ return 0;
+ }
+ return 1 if $r_min == $r_max;
+ if ($r_min < $r_max) {
+ # we need to reverse the print order
+ return 0 if (defined $limit && --$limit < 0);
+ push @$defer, $c;
+ return 1;
+ }
+ if ($r_min != $r_max) {
+ return 1 if ($r_min < $c->{r});
+ return 1 if ($r_max > $c->{r});
+ }
+ }
+ return 0 if (defined $limit && --$limit < 0);
+ show_commit($c);
+ return 1;
+}
+
+sub show_commit {
+ my $c = shift;
+ if ($oneline) {
+ my $x = "\n";
+ if (my $l = $c->{l}) {
+ while ($l->[0] =~ /^\s*$/) { shift @$l }
+ $x = $l->[0];
+ }
+ $l_fmt ||= 'A' . length($c->{r});
+ print 'r',pack($l_fmt, $c->{r}),' | ';
+ print "$c->{c} | " if $show_commit;
+ print $x;
+ } else {
+ show_commit_normal($c);
+ }
+}
+
+sub show_commit_changed_paths {
+ my ($c) = @_;
+ return unless $c->{changed};
+ print "Changed paths:\n", @{$c->{changed}};
+}
+
+sub show_commit_normal {
+ my ($c) = @_;
+ print commit_log_separator, "r$c->{r} | ";
+ print "$c->{c} | " if $show_commit;
+ print "$c->{a} | ", format_svn_date($c->{t_utc}), ' | ';
+ my $nr_line = 0;
+
+ if (my $l = $c->{l}) {
+ while ($l->[$#$l] eq "\n" && $#$l > 0
+ && $l->[($#$l - 1)] eq "\n") {
+ pop @$l;
+ }
+ $nr_line = scalar @$l;
+ if (!$nr_line) {
+ print "1 line\n\n\n";
+ } else {
+ if ($nr_line == 1) {
+ $nr_line = '1 line';
+ } else {
+ $nr_line .= ' lines';
+ }
+ print $nr_line, "\n";
+ show_commit_changed_paths($c);
+ print "\n";
+ print $_ foreach @$l;
+ }
+ } else {
+ print "1 line\n";
+ show_commit_changed_paths($c);
+ print "\n";
+
+ }
+ foreach my $x (qw/raw stat diff/) {
+ if ($c->{$x}) {
+ print "\n";
+ print $_ foreach @{$c->{$x}}
+ }
+ }
+}
+
+sub cmd_show_log {
+ my (@args) = @_;
+ my ($r_min, $r_max);
+ my $r_last = -1; # prevent dupes
+ set_local_timezone();
+ if (defined $::_revision) {
+ if ($::_revision =~ /^(\d+):(\d+)$/) {
+ ($r_min, $r_max) = ($1, $2);
+ } elsif ($::_revision =~ /^\d+$/) {
+ $r_min = $r_max = $::_revision;
+ } else {
+ fatal "-r$::_revision is not supported, use ",
+ "standard 'git log' arguments instead";
+ }
+ }
+
+ config_pager();
+ @args = git_svn_log_cmd($r_min, $r_max, @args);
+ if (!@args) {
+ print commit_log_separator unless $incremental || $oneline;
+ return;
+ }
+ my $log = command_output_pipe(@args);
+ run_pager();
+ my (@k, $c, $d, $stat);
+ my $esc_color = qr/(?:\033\[(?:(?:\d+;)*\d*)?m)*/;
+ while (<$log>) {
+ if (/^${esc_color}commit (?:- )?($::sha1_short)/o) {
+ my $cmt = $1;
+ if ($c && cmt_showable($c) && $c->{r} != $r_last) {
+ $r_last = $c->{r};
+ process_commit($c, $r_min, $r_max, \@k) or
+ goto out;
+ }
+ $d = undef;
+ $c = { c => $cmt };
+ } elsif (/^${esc_color}author (.+) (\d+) ([\-\+]?\d+)$/o) {
+ get_author_info($c, $1, $2, $3);
+ } elsif (/^${esc_color}(?:tree|parent|committer) /o) {
+ # ignore
+ } elsif (/^${esc_color}:\d{6} \d{6} $::sha1_short/o) {
+ push @{$c->{raw}}, $_;
+ } elsif (/^${esc_color}[ACRMDT]\t/) {
+ # we could add $SVN->{svn_path} here, but that requires
+ # remote access at the moment (repo_path_split)...
+ s#^(${esc_color})([ACRMDT])\t#$1 $2 #o;
+ push @{$c->{changed}}, $_;
+ } elsif (/^${esc_color}diff /o) {
+ $d = 1;
+ push @{$c->{diff}}, $_;
+ } elsif ($d) {
+ push @{$c->{diff}}, $_;
+ } elsif (/^\ .+\ \|\s*\d+\ $esc_color[\+\-]*
+ $esc_color*[\+\-]*$esc_color$/x) {
+ $stat = 1;
+ push @{$c->{stat}}, $_;
+ } elsif ($stat && /^ \d+ files changed, \d+ insertions/) {
+ push @{$c->{stat}}, $_;
+ $stat = undef;
+ } elsif (/^${esc_color} (git-svn-id:.+)$/o) {
+ ($c->{url}, $c->{r}, undef) = ::extract_metadata($1);
+ } elsif (s/^${esc_color} //o) {
+ push @{$c->{l}}, $_;
+ }
+ }
+ if ($c && defined $c->{r} && $c->{r} != $r_last) {
+ $r_last = $c->{r};
+ process_commit($c, $r_min, $r_max, \@k);
+ }
+ if (@k) {
+ ($r_min, $r_max) = ($r_max, $r_min);
+ process_commit($_, $r_min, $r_max) foreach reverse @k;
+ }
+out:
+ close $log;
+ print commit_log_separator unless $incremental || $oneline;
+}
+
+sub cmd_blame {
+ my $path = pop;
+
+ config_pager();
+ run_pager();
+
+ my ($fh, $ctx, $rev);
+
+ if ($_git_format) {
+ ($fh, $ctx) = command_output_pipe('blame', @_, $path);
+ while (my $line = <$fh>) {
+ if ($line =~ /^\^?([[:xdigit:]]+)\s/) {
+ # Uncommitted edits show up as a rev ID of
+ # all zeros, which we can't look up with
+ # cmt_metadata
+ if ($1 !~ /^0+$/) {
+ (undef, $rev, undef) =
+ ::cmt_metadata($1);
+ $rev = '0' if (!$rev);
+ } else {
+ $rev = '0';
+ }
+ $rev = sprintf('%-10s', $rev);
+ $line =~ s/^\^?[[:xdigit:]]+(\s)/$rev$1/;
+ }
+ print $line;
+ }
+ } else {
+ ($fh, $ctx) = command_output_pipe('blame', '-p', @_, 'HEAD',
+ '--', $path);
+ my ($sha1);
+ my %authors;
+ my @buffer;
+ my %dsha; #distinct sha keys
+
+ while (my $line = <$fh>) {
+ push @buffer, $line;
+ if ($line =~ /^([[:xdigit:]]{40})\s\d+\s\d+/) {
+ $dsha{$1} = 1;
+ }
+ }
+
+ my $s2r = ::cmt_sha2rev_batch([keys %dsha]);
+
+ foreach my $line (@buffer) {
+ if ($line =~ /^([[:xdigit:]]{40})\s\d+\s\d+/) {
+ $rev = $s2r->{$1};
+ $rev = '0' if (!$rev)
+ }
+ elsif ($line =~ /^author (.*)/) {
+ $authors{$rev} = $1;
+ $authors{$rev} =~ s/\s/_/g;
+ }
+ elsif ($line =~ /^\t(.*)$/) {
+ printf("%6s %10s %s\n", $rev, $authors{$rev}, $1);
+ }
+ }
+ }
+ command_close_pipe($fh, $ctx);
+}
+
+1;
--
1.7.11.1
^ permalink raw reply related [flat|nested] 34+ messages in thread* Extract Git classes from git-svn (6/10) (was Re: Fix git-svn tests for SVN 1.7.5.)
2012-07-17 17:44 ` Jonathan Nieder
` (3 preceding siblings ...)
2012-07-17 23:13 ` Extract Git classes from git-svn (3/10) " Michael G Schwern
@ 2012-07-17 23:16 ` Michael G Schwern
2012-07-17 23:16 ` Extract Git classes from git-svn (7/10) " Michael G Schwern
` (5 subsequent siblings)
10 siblings, 0 replies; 34+ messages in thread
From: Michael G Schwern @ 2012-07-17 23:16 UTC (permalink / raw)
To: Jonathan Nieder; +Cc: git, gitster, robbat2, Eric Wong, Ben Walton
>From cb1a73929da15e87fa3dcc41c4cfa9ca592081fa Mon Sep 17 00:00:00 2001
From: "Michael G. Schwern" <schwern@pobox.com>
Date: Thu, 12 Jul 2012 17:14:24 -0700
Subject: [PATCH 07/11] Fix Git::SVN::Migration after its move.
Also...
* eliminate the big "import all the Git command functions" loop, nothing needs it
any more
* only load Git::SVN::Migration if we need it
---
git-svn.perl | 28 +++++++++++++++-------------
perl/Git/SVN/Migration.pm | 16 +++++++++++++++-
perl/Makefile | 1 +
t/Git-SVN/00compile.t | 3 ++-
4 files changed, 33 insertions(+), 15 deletions(-)
diff --git a/git-svn.perl b/git-svn.perl
index f2bf759..8b8607d 100755
--- a/git-svn.perl
+++ b/git-svn.perl
@@ -11,7 +11,6 @@ $AUTHOR = 'Eric Wong <normalperson@yhbt.net>';
$VERSION = '@@GIT_VERSION@@';
use Git::SVN;
-use Git::SVN::Log;
use Git::SVN::Utils qw(fatal can_compress);
@@ -77,24 +76,26 @@ use File::Spec;
use File::Find;
use Getopt::Long qw/:config gnu_getopt no_ignore_case auto_abbrev/;
use IPC::Open3;
-use Git;
+
+use Git qw(
+ git_cmd_try
+ command
+ command_oneline
+ command_noisy
+ command_output_pipe
+ command_close_pipe
+ command_bidi_pipe
+ command_close_bidi_pipe
+);
+
use Git::SVN::Editor qw//;
use Git::SVN::Fetcher qw//;
-use Git::SVN::Ra qw//;
+use Git::SVN::Log;
use Git::SVN::Prompt qw//;
+use Git::SVN::Ra qw//;
use Memoize; # core since 5.8.0, Jul 2002
BEGIN {
- # import functions from Git into our packages, en masse
- no strict 'refs';
- foreach (qw/command command_oneline command_noisy command_output_pipe
- command_input_pipe command_close_pipe
- command_bidi_pipe command_close_bidi_pipe/) {
- for my $package ( qw(Git::SVN::Migration),
- __PACKAGE__) {
- *{"${package}::$_"} = \&{"Git::$_"};
- }
- }
Memoize::memoize 'Git::config';
Memoize::memoize 'Git::config_bool';
}
@@ -365,6 +366,7 @@ if (defined $_authors_prog) {
}
unless ($cmd =~ /^(?:clone|init|multi-init|commit-diff)$/) {
+ require Git::SVN::Migration;
Git::SVN::Migration::migration_check();
}
Git::SVN::init_vars();
diff --git a/perl/Git/SVN/Migration.pm b/perl/Git/SVN/Migration.pm
index 082a788..b17fe00 100644
--- a/perl/Git/SVN/Migration.pm
+++ b/perl/Git/SVN/Migration.pm
@@ -32,12 +32,22 @@ package Git::SVN::Migration;
# rebuilding the .rev_db. Rebuilding the rev_db is not
# possible if noMetadata or useSvmProps are set; but should
# be no problem for users that use the (sensible) defaults.
+
use strict;
use warnings;
+
use Carp qw/croak/;
use File::Path qw/mkpath/;
use File::Basename qw/dirname basename/;
-use vars qw/$_minimize/;
+
+our $_minimize;
+
+use Git qw(
+ command
+ command_noisy
+ command_output_pipe
+ command_close_pipe
+);
sub migrate_from_v0 {
my $git_dir = $ENV{GIT_DIR};
@@ -146,6 +156,7 @@ sub migrate_from_v2 {
read_old_urls(\%l_map, '', "$ENV{GIT_DIR}/svn");
my $migrated = 0;
+ require Git::SVN;
foreach my $ref_id (sort keys %l_map) {
eval { Git::SVN->init($l_map{$ref_id}, '', undef, $ref_id) };
if ($@) {
@@ -157,6 +168,9 @@ sub migrate_from_v2 {
}
sub minimize_connections {
+ require Git::SVN;
+ require Git::SVN::Ra;
+
my $r = Git::SVN::read_all_remotes();
my $new_urls = {};
my $root_repos = {};
diff --git a/perl/Makefile b/perl/Makefile
index 2a4ca57..d6a0e84 100644
--- a/perl/Makefile
+++ b/perl/Makefile
@@ -31,6 +31,7 @@ modules += Git/SVN/Memoize/YAML
modules += Git/SVN/Fetcher
modules += Git/SVN/Editor
modules += Git/SVN/Log
+modules += Git/SVN/Migration
modules += Git/SVN/Prompt
modules += Git/SVN/Ra
modules += Git/SVN/Utils
diff --git a/t/Git-SVN/00compile.t b/t/Git-SVN/00compile.t
index 37626f4..1307b65 100644
--- a/t/Git-SVN/00compile.t
+++ b/t/Git-SVN/00compile.t
@@ -3,9 +3,10 @@
use strict;
use warnings;
-use Test::More tests => 4;
+use Test::More tests => 5;
require_ok 'Git::SVN';
require_ok 'Git::SVN::Utils';
require_ok 'Git::SVN::Ra';
require_ok 'Git::SVN::Log';
+require_ok 'Git::SVN::Migration';
--
1.7.11.1
^ permalink raw reply related [flat|nested] 34+ messages in thread* Extract Git classes from git-svn (7/10) (was Re: Fix git-svn tests for SVN 1.7.5.)
2012-07-17 17:44 ` Jonathan Nieder
` (4 preceding siblings ...)
2012-07-17 23:16 ` Extract Git classes from git-svn (6/10) " Michael G Schwern
@ 2012-07-17 23:16 ` Michael G Schwern
2012-07-17 23:17 ` Extract Git classes from git-svn (8/10) " Michael G Schwern
` (4 subsequent siblings)
10 siblings, 0 replies; 34+ messages in thread
From: Michael G Schwern @ 2012-07-17 23:16 UTC (permalink / raw)
To: Jonathan Nieder; +Cc: git, gitster, robbat2, Eric Wong, Ben Walton
>From 9ff49d9e91c9741d501620ac47f78d8ff8ef9983 Mon Sep 17 00:00:00 2001
From: "Michael G. Schwern" <schwern@pobox.com>
Date: Tue, 17 Jul 2012 15:51:53 -0700
Subject: [PATCH 08/11] Cut & paste Git::IndexInfo into its own file.
No other changes, those are next commit so they can be seen in the diff.
---
git-svn.perl | 32 --------------------------------
perl/Git/IndexInfo.pm | 33 +++++++++++++++++++++++++++++++++
2 files changed, 33 insertions(+), 32 deletions(-)
create mode 100644 perl/Git/IndexInfo.pm
diff --git a/git-svn.perl b/git-svn.perl
index 8b8607d..6632cfb 100755
--- a/git-svn.perl
+++ b/git-svn.perl
@@ -2045,38 +2045,6 @@ sub gc_directory {
}
-package Git::IndexInfo;
-use strict;
-use warnings;
-use Git qw/command_input_pipe command_close_pipe/;
-
-sub new {
- my ($class) = @_;
- my ($gui, $ctx) = command_input_pipe(qw/update-index -z --index-info/);
- bless { gui => $gui, ctx => $ctx, nr => 0}, $class;
-}
-
-sub remove {
- my ($self, $path) = @_;
- if (print { $self->{gui} } '0 ', 0 x 40, "\t", $path, "\0") {
- return ++$self->{nr};
- }
- undef;
-}
-
-sub update {
- my ($self, $mode, $hash, $path) = @_;
- if (print { $self->{gui} } $mode, ' ', $hash, "\t", $path, "\0") {
- return ++$self->{nr};
- }
- undef;
-}
-
-sub DESTROY {
- my ($self) = @_;
- command_close_pipe($self->{gui}, $self->{ctx});
-}
-
package Git::SVN::GlobSpec;
use strict;
use warnings;
diff --git a/perl/Git/IndexInfo.pm b/perl/Git/IndexInfo.pm
new file mode 100644
index 0000000..a43108c
--- /dev/null
+++ b/perl/Git/IndexInfo.pm
@@ -0,0 +1,33 @@
+package Git::IndexInfo;
+use strict;
+use warnings;
+use Git qw/command_input_pipe command_close_pipe/;
+
+sub new {
+ my ($class) = @_;
+ my ($gui, $ctx) = command_input_pipe(qw/update-index -z --index-info/);
+ bless { gui => $gui, ctx => $ctx, nr => 0}, $class;
+}
+
+sub remove {
+ my ($self, $path) = @_;
+ if (print { $self->{gui} } '0 ', 0 x 40, "\t", $path, "\0") {
+ return ++$self->{nr};
+ }
+ undef;
+}
+
+sub update {
+ my ($self, $mode, $hash, $path) = @_;
+ if (print { $self->{gui} } $mode, ' ', $hash, "\t", $path, "\0") {
+ return ++$self->{nr};
+ }
+ undef;
+}
+
+sub DESTROY {
+ my ($self) = @_;
+ command_close_pipe($self->{gui}, $self->{ctx});
+}
+
+1;
--
1.7.11.1
^ permalink raw reply related [flat|nested] 34+ messages in thread* Extract Git classes from git-svn (8/10) (was Re: Fix git-svn tests for SVN 1.7.5.)
2012-07-17 17:44 ` Jonathan Nieder
` (5 preceding siblings ...)
2012-07-17 23:16 ` Extract Git classes from git-svn (7/10) " Michael G Schwern
@ 2012-07-17 23:17 ` Michael G Schwern
2012-07-17 23:17 ` Extract Git classes from git-svn (9/10) " Michael G Schwern
` (3 subsequent siblings)
10 siblings, 0 replies; 34+ messages in thread
From: Michael G Schwern @ 2012-07-17 23:17 UTC (permalink / raw)
To: Jonathan Nieder; +Cc: git, gitster, robbat2, Eric Wong, Ben Walton
>From 4fd7b8574b32753dcf22ec0a592f13586b938689 Mon Sep 17 00:00:00 2001
From: "Michael G. Schwern" <schwern@pobox.com>
Date: Thu, 12 Jul 2012 17:20:02 -0700
Subject: [PATCH 09/11] Fix Git::IndexInfo so it compiles.
Only used by Git::SVN::Fetcher.
---
perl/Git/IndexInfo.pm | 2 ++
perl/Git/SVN/Fetcher.pm | 2 ++
perl/Makefile | 1 +
t/Git-SVN/00compile.t | 3 ++-
4 files changed, 7 insertions(+), 1 deletion(-)
diff --git a/perl/Git/IndexInfo.pm b/perl/Git/IndexInfo.pm
index a43108c..9e454be 100644
--- a/perl/Git/IndexInfo.pm
+++ b/perl/Git/IndexInfo.pm
@@ -1,6 +1,8 @@
package Git::IndexInfo;
+
use strict;
use warnings;
+
use Git qw/command_input_pipe command_close_pipe/;
sub new {
diff --git a/perl/Git/SVN/Fetcher.pm b/perl/Git/SVN/Fetcher.pm
index ef8e9ed..69486ef 100644
--- a/perl/Git/SVN/Fetcher.pm
+++ b/perl/Git/SVN/Fetcher.pm
@@ -10,6 +10,8 @@ use IO::File qw//;
use Git qw/command command_oneline command_noisy command_output_pipe
command_input_pipe command_close_pipe
command_bidi_pipe command_close_bidi_pipe/;
+use Git::IndexInfo;
+
BEGIN {
@ISA = qw(SVN::Delta::Editor);
}
diff --git a/perl/Makefile b/perl/Makefile
index d6a0e84..6c32918 100644
--- a/perl/Makefile
+++ b/perl/Makefile
@@ -26,6 +26,7 @@ instdir_SQ = $(subst ','\'',$(prefix)/lib)
modules += Git
modules += Git/I18N
+modules += Git/IndexInfo
modules += Git/SVN
modules += Git/SVN/Memoize/YAML
modules += Git/SVN/Fetcher
diff --git a/t/Git-SVN/00compile.t b/t/Git-SVN/00compile.t
index 1307b65..5419438 100644
--- a/t/Git-SVN/00compile.t
+++ b/t/Git-SVN/00compile.t
@@ -3,10 +3,11 @@
use strict;
use warnings;
-use Test::More tests => 5;
+use Test::More tests => 6;
require_ok 'Git::SVN';
require_ok 'Git::SVN::Utils';
require_ok 'Git::SVN::Ra';
require_ok 'Git::SVN::Log';
require_ok 'Git::SVN::Migration';
+require_ok 'Git::IndexInfo';
--
1.7.11.1
^ permalink raw reply related [flat|nested] 34+ messages in thread* Extract Git classes from git-svn (9/10) (was Re: Fix git-svn tests for SVN 1.7.5.)
2012-07-17 17:44 ` Jonathan Nieder
` (6 preceding siblings ...)
2012-07-17 23:17 ` Extract Git classes from git-svn (8/10) " Michael G Schwern
@ 2012-07-17 23:17 ` Michael G Schwern
2012-07-17 23:17 ` Extract Git classes from git-svn (10/10) " Michael G Schwern
` (2 subsequent siblings)
10 siblings, 0 replies; 34+ messages in thread
From: Michael G Schwern @ 2012-07-17 23:17 UTC (permalink / raw)
To: Jonathan Nieder; +Cc: git, gitster, robbat2, Eric Wong, Ben Walton
>From 368d6c7883080d85f82b1eae1815834e3d59ef5e Mon Sep 17 00:00:00 2001
From: "Michael G. Schwern" <schwern@pobox.com>
Date: Tue, 17 Jul 2012 15:54:33 -0700
Subject: [PATCH 10/11] Cut & paste Git::SVN::GlobSpec into its own file.
Fixes to make it work come next commit.
---
git-svn.perl | 58 -----------------------------------------------
perl/Git/SVN/GlobSpec.pm | 59 ++++++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 59 insertions(+), 58 deletions(-)
create mode 100644 perl/Git/SVN/GlobSpec.pm
diff --git a/git-svn.perl b/git-svn.perl
index 6632cfb..7b54f43 100755
--- a/git-svn.perl
+++ b/git-svn.perl
@@ -2045,64 +2045,6 @@ sub gc_directory {
}
-package Git::SVN::GlobSpec;
-use strict;
-use warnings;
-
-sub new {
- my ($class, $glob, $pattern_ok) = @_;
- my $re = $glob;
- $re =~ s!/+$!!g; # no need for trailing slashes
- my (@left, @right, @patterns);
- my $state = "left";
- my $die_msg = "Only one set of wildcard directories " .
- "(e.g. '*' or '*/*/*') is supported: '$glob'\n";
- for my $part (split(m|/|, $glob)) {
- if ($part =~ /\*/ && $part ne "*") {
- die "Invalid pattern in '$glob': $part\n";
- } elsif ($pattern_ok && $part =~ /[{}]/ &&
- $part !~ /^\{[^{}]+\}/) {
- die "Invalid pattern in '$glob': $part\n";
- }
- if ($part eq "*") {
- die $die_msg if $state eq "right";
- $state = "pattern";
- push(@patterns, "[^/]*");
- } elsif ($pattern_ok && $part =~ /^\{(.*)\}$/) {
- die $die_msg if $state eq "right";
- $state = "pattern";
- my $p = quotemeta($1);
- $p =~ s/\\,/|/g;
- push(@patterns, "(?:$p)");
- } else {
- if ($state eq "left") {
- push(@left, $part);
- } else {
- push(@right, $part);
- $state = "right";
- }
- }
- }
- my $depth = @patterns;
- if ($depth == 0) {
- die "One '*' is needed in glob: '$glob'\n";
- }
- my $left = join('/', @left);
- my $right = join('/', @right);
- $re = join('/', @patterns);
- $re = join('\/',
- grep(length, quotemeta($left), "($re)", quotemeta($right)));
- my $left_re = qr/^\/\Q$left\E(\/|$)/;
- bless { left => $left, right => $right, left_regex => $left_re,
- regex => qr/$re/, glob => $glob, depth => $depth }, $class;
-}
-
-sub full_path {
- my ($self, $path) = @_;
- return (length $self->{left} ? "$self->{left}/" : '') .
- $path . (length $self->{right} ? "/$self->{right}" : '');
-}
-
__END__
Data structures:
diff --git a/perl/Git/SVN/GlobSpec.pm b/perl/Git/SVN/GlobSpec.pm
new file mode 100644
index 0000000..96cfd98
--- /dev/null
+++ b/perl/Git/SVN/GlobSpec.pm
@@ -0,0 +1,59 @@
+package Git::SVN::GlobSpec;
+use strict;
+use warnings;
+
+sub new {
+ my ($class, $glob, $pattern_ok) = @_;
+ my $re = $glob;
+ $re =~ s!/+$!!g; # no need for trailing slashes
+ my (@left, @right, @patterns);
+ my $state = "left";
+ my $die_msg = "Only one set of wildcard directories " .
+ "(e.g. '*' or '*/*/*') is supported: '$glob'\n";
+ for my $part (split(m|/|, $glob)) {
+ if ($part =~ /\*/ && $part ne "*") {
+ die "Invalid pattern in '$glob': $part\n";
+ } elsif ($pattern_ok && $part =~ /[{}]/ &&
+ $part !~ /^\{[^{}]+\}/) {
+ die "Invalid pattern in '$glob': $part\n";
+ }
+ if ($part eq "*") {
+ die $die_msg if $state eq "right";
+ $state = "pattern";
+ push(@patterns, "[^/]*");
+ } elsif ($pattern_ok && $part =~ /^\{(.*)\}$/) {
+ die $die_msg if $state eq "right";
+ $state = "pattern";
+ my $p = quotemeta($1);
+ $p =~ s/\\,/|/g;
+ push(@patterns, "(?:$p)");
+ } else {
+ if ($state eq "left") {
+ push(@left, $part);
+ } else {
+ push(@right, $part);
+ $state = "right";
+ }
+ }
+ }
+ my $depth = @patterns;
+ if ($depth == 0) {
+ die "One '*' is needed in glob: '$glob'\n";
+ }
+ my $left = join('/', @left);
+ my $right = join('/', @right);
+ $re = join('/', @patterns);
+ $re = join('\/',
+ grep(length, quotemeta($left), "($re)", quotemeta($right)));
+ my $left_re = qr/^\/\Q$left\E(\/|$)/;
+ bless { left => $left, right => $right, left_regex => $left_re,
+ regex => qr/$re/, glob => $glob, depth => $depth }, $class;
+}
+
+sub full_path {
+ my ($self, $path) = @_;
+ return (length $self->{left} ? "$self->{left}/" : '') .
+ $path . (length $self->{right} ? "/$self->{right}" : '');
+}
+
+1;
--
1.7.11.1
^ permalink raw reply related [flat|nested] 34+ messages in thread* Extract Git classes from git-svn (10/10) (was Re: Fix git-svn tests for SVN 1.7.5.)
2012-07-17 17:44 ` Jonathan Nieder
` (7 preceding siblings ...)
2012-07-17 23:17 ` Extract Git classes from git-svn (9/10) " Michael G Schwern
@ 2012-07-17 23:17 ` Michael G Schwern
[not found] ` <5005F139.8050205@pobox.com>
2012-07-21 0:27 ` Fix git-svn tests for SVN 1.7.5 Ben Walton
10 siblings, 0 replies; 34+ messages in thread
From: Michael G Schwern @ 2012-07-17 23:17 UTC (permalink / raw)
To: Jonathan Nieder; +Cc: git, gitster, robbat2, Eric Wong, Ben Walton
>From 5152b76800f076ba0bd528664f62d3c67966fa4e Mon Sep 17 00:00:00 2001
From: "Michael G. Schwern" <schwern@pobox.com>
Date: Thu, 12 Jul 2012 17:25:25 -0700
Subject: [PATCH 11/11] Fix Git::SVN::GlobSpec so it works.
Only used in one place in Git::SVN, load it on demand.
That should be all the Git classes out of git-svn.
---
perl/Git/SVN.pm | 5 ++++-
perl/Git/SVN/GlobSpec.pm | 1 +
perl/Makefile | 1 +
t/Git-SVN/00compile.t | 3 ++-
4 files changed, 8 insertions(+), 2 deletions(-)
diff --git a/perl/Git/SVN.pm b/perl/Git/SVN.pm
index 02983d6..247ee1d 100644
--- a/perl/Git/SVN.pm
+++ b/perl/Git/SVN.pm
@@ -202,11 +202,14 @@ sub read_all_remotes {
. "must start with 'refs/'\n")
unless $remote_ref =~ m{^refs/};
$local_ref = uri_decode($local_ref);
+
+ require Git::SVN::GlobSpec;
my $rs = {
t => $t,
remote => $remote,
path => Git::SVN::GlobSpec->new($local_ref, 1),
- ref => Git::SVN::GlobSpec->new($remote_ref, 0) };
+ ref => Git::SVN::GlobSpec->new($remote_ref, 0)
+ };
if (length($rs->{ref}->{right}) != 0) {
die "The '*' glob character must be the last ",
"character of '$remote_ref'\n";
diff --git a/perl/Git/SVN/GlobSpec.pm b/perl/Git/SVN/GlobSpec.pm
index 96cfd98..fede3af 100644
--- a/perl/Git/SVN/GlobSpec.pm
+++ b/perl/Git/SVN/GlobSpec.pm
@@ -1,4 +1,5 @@
package Git::SVN::GlobSpec;
+
use strict;
use warnings;
diff --git a/perl/Makefile b/perl/Makefile
index 6c32918..aa4a28b 100644
--- a/perl/Makefile
+++ b/perl/Makefile
@@ -31,6 +31,7 @@ modules += Git/SVN
modules += Git/SVN/Memoize/YAML
modules += Git/SVN/Fetcher
modules += Git/SVN/Editor
+modules += Git/SVN/GlobSpec
modules += Git/SVN/Log
modules += Git/SVN/Migration
modules += Git/SVN/Prompt
diff --git a/t/Git-SVN/00compile.t b/t/Git-SVN/00compile.t
index 5419438..c92fee4 100644
--- a/t/Git-SVN/00compile.t
+++ b/t/Git-SVN/00compile.t
@@ -3,7 +3,7 @@
use strict;
use warnings;
-use Test::More tests => 6;
+use Test::More tests => 7;
require_ok 'Git::SVN';
require_ok 'Git::SVN::Utils';
@@ -11,3 +11,4 @@ require_ok 'Git::SVN::Ra';
require_ok 'Git::SVN::Log';
require_ok 'Git::SVN::Migration';
require_ok 'Git::IndexInfo';
+require_ok 'Git::SVN::GlobSpec';
--
1.7.11.1
^ permalink raw reply related [flat|nested] 34+ messages in thread[parent not found: <5005F139.8050205@pobox.com>]
* Re: Extract Git classes from git-svn (1/10) (was Re: Fix git-svn tests for SVN 1.7.5.)
[not found] ` <5005F139.8050205@pobox.com>
@ 2012-07-17 23:31 ` Jonathan Nieder
2012-07-18 5:49 ` Extract Git classes from git-svn (1/10) Junio C Hamano
0 siblings, 1 reply; 34+ messages in thread
From: Jonathan Nieder @ 2012-07-17 23:31 UTC (permalink / raw)
To: Michael G Schwern; +Cc: git, gitster, robbat2, Eric Wong, Ben Walton
Hi,
Michael G Schwern wrote:
> There's five classes, so this is ten patches. Let me go on record again to
> state that this one-inline-patch-per-email is a lot of busy work for me.
Well, there's no need to protest and go along with it if it's a bad
idea. It's just what we've found to be the easiest way in the past to
review and have a discussion about each patch, but if you know a
better way, I'm happy to hear for next time.
The mailing list archive at
http://news.gmane.org/gmane.comp.version-control.git might be useful
for seeing some examples of how it plays out in practice.
You mind find the "git send-email" tool to be helpful for automating
some of the tedious steps of sending a patch series out. It also does
some other nice things, like setting the subject lines in a more
useful way. :)
Thanks,
Jonathan
^ permalink raw reply [flat|nested] 34+ messages in thread* Re: Extract Git classes from git-svn (1/10)
2012-07-17 23:31 ` Extract Git classes from git-svn (1/10) " Jonathan Nieder
@ 2012-07-18 5:49 ` Junio C Hamano
2012-07-19 3:43 ` Thiago Farina
2012-07-24 22:38 ` Michael G Schwern
0 siblings, 2 replies; 34+ messages in thread
From: Junio C Hamano @ 2012-07-18 5:49 UTC (permalink / raw)
To: Michael G Schwern; +Cc: Jonathan Nieder, git, robbat2, Eric Wong, Ben Walton
Jonathan Nieder <jrnieder@gmail.com> writes:
> The mailing list archive at
> http://news.gmane.org/gmane.comp.version-control.git might be
> useful for seeing some examples of how it plays out in practice.
By allowing people to easily publish a completed work, and making it
easier for them to let others peek at their work, Git hosting
services like GitHub are wonderful. But I am not conviced that
quality code reviews like we do on the mailing list can be done with
existing Web based interface to a satisfactory degree.
Patches with proposed commit log messages are sent via e-mail,
people can review them and comment on them with quotes from the
relevant part of the patch. The review can even be made offline,
yet at the end, the list archive is an easy one-stop location you
need to go to see how the changes progressed, what the background
thinking was, etc. for all the changes that matter.
Look at recent ones (randomly, $gmane/199492, $gmane/199497,
$gmane/200750, $gmane/201477, $gmane/201434), and their re-rolls,
and admire how well the process works.
I've played with GitHub's in-line code comment interface, but
honestly, it is cumbersome to use, for one thing, but more
importantly, you have to click around various repositories of pull
requestors, dig around to see in-line comments, and I do not see how
we can keep a coherent "discussion" like we do on the mailing list.
There may be a hosting site with better code review features, but
all the code review of Git happens on this mailing list, and that is
not likely to change in the near future.
[Footnote]
$gmane stands for http://thread.gmane.org/gmane.comp.version-control.git/
in the above description.
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: Extract Git classes from git-svn (1/10)
2012-07-18 5:49 ` Extract Git classes from git-svn (1/10) Junio C Hamano
@ 2012-07-19 3:43 ` Thiago Farina
2012-07-24 22:38 ` Michael G Schwern
1 sibling, 0 replies; 34+ messages in thread
From: Thiago Farina @ 2012-07-19 3:43 UTC (permalink / raw)
To: Junio C Hamano
Cc: Michael G Schwern, Jonathan Nieder, git, robbat2, Eric Wong, Ben Walton
On Wed, Jul 18, 2012 at 2:49 AM, Junio C Hamano <gitster@pobox.com> wrote:
> There may be a hosting site with better code review features, but
> all the code review of Git happens on this mailing list, and that is
> not likely to change in the near future.
>
For me, you know, it's codereview, aka rietveld. [1]
[1] code.google.com/p/rietveld/
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: Extract Git classes from git-svn (1/10)
2012-07-18 5:49 ` Extract Git classes from git-svn (1/10) Junio C Hamano
2012-07-19 3:43 ` Thiago Farina
@ 2012-07-24 22:38 ` Michael G Schwern
2012-07-24 23:25 ` Jonathan Nieder
2012-07-25 2:55 ` Eric Wong
1 sibling, 2 replies; 34+ messages in thread
From: Michael G Schwern @ 2012-07-24 22:38 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Jonathan Nieder, git, robbat2, Eric Wong, Ben Walton
On 2012.7.17 10:49 PM, Junio C Hamano wrote:
> By allowing people to easily publish a completed work, and making it
> easier for them to let others peek at their work, Git hosting
> services like GitHub are wonderful. But I am not conviced that
> quality code reviews like we do on the mailing list can be done with
> existing Web based interface to a satisfactory degree.
In this instance, I was just using Github for repository storage. I was
hoping I could just submit a remote git repository and people would look at it
from there. No Github required.
I understand this makes things very convenient for you to review patches, let
me convey my POV...
After I'm exhausted from volunteering all the coding work, rather than
submitting a URL to a remote repository I find I have to learn new specialized
tools. It's extra learning and work, an extra step to screw up, and foreign
to me (even as a experienced git user). It is of little benefit to me as a
casual volunteer submitter.
I can see if you've been on the git mailing list for a while and have git-am
and all that set up, this system is great. But it comes at a cost which is
offloaded onto new and casual contributors.
> Patches with proposed commit log messages are sent via e-mail,
> people can review them and comment on them with quotes from the
> relevant part of the patch. The review can even be made offline,
> yet at the end, the list archive is an easy one-stop location you
> need to go to see how the changes progressed, what the background
> thinking was, etc. for all the changes that matter.
>
> Look at recent ones (randomly, $gmane/199492, $gmane/199497,
> $gmane/200750, $gmane/201477, $gmane/201434), and their re-rolls,
> and admire how well the process works.
>
> I've played with GitHub's in-line code comment interface, but
> honestly, it is cumbersome to use, for one thing, but more
> importantly, you have to click around various repositories of pull
> requestors, dig around to see in-line comments, and I do not see how
> we can keep a coherent "discussion" like we do on the mailing list.
>
> There may be a hosting site with better code review features, but
> all the code review of Git happens on this mailing list, and that is
> not likely to change in the near future.
Everything you've said above is correct... but it creates a procedure
optimized for the convenience of the long time reviewers at the expense of new
and casual submissions. I'm going to guess you live in email and have your
client setup to do fancy things to extract patches from your mailbox and the like.
This sort of specialized setup makes people bounce right off the submission
process. At OSCON I was asking around for help getting things setup so I
could submit patches here properly. As soon as they said "which mail daemon
are you running?", I said "stop! I don't want to know any more". I have too
many things to do to be fiddling with my mailer configuration just to submit
volunteer work in the right form (that said, I'm pleased as punch that
git-send-email now has instructions for sending via GMail). You're
volunteers, too. We're all volunteers, so a more balanced submission process
would be nice.
But since you brought Github up... (I get the impression its kind of a dirty
word around here)
As somebody who doesn't live in email anymore (once upon a time I did), I find
the Github Pull Request model to be an excellent... I'm not even going to use
the word "compromise" because I don't feel like either side has been
compromised... it's an excellent enhancement. The commits and conversations
about the commits are all there on one page. Looking at a commit is a click
away (I usually open them all in tabs at once, much faster). You can comment
on them as a whole or inline. Those comments appear BOTH in the commit AND in
the larger conversation on the pull request keeping it coherent, no clicking
around. And it has email mirroring. All that and its tracked and organized
in an issue tracker.
Here's an example that includes commits, discussion about the larger issue,
comments on commits, more commits based on those comments, and so on. As you
can see, the conversation is complete and coherent. It wasn't always this
way, but they're constantly improving.
https://github.com/schwern/test-more/pull/319
A concrete downside it is that it does not work offline. I agree that's a
problem. I don't think it's a veto. There are various work arounds which are
less complicated than your typical git-to-email-to-git setup. We can talk
about that you're interested.
I've gone all in on Github Pull Requests such that most of my projects don't
even have mailing lists (issues are used for discussion). This avoids a split
community between Github and the mailing list. And they have email mirroring,
so issue discussion can be done all in email (I prefer email for things that
involve push notification and replies).
Github has a nice API and it would be possible to create a Github Pull Request
<-> Mailing List gateway. Perl 5 uses something like that for bug reports.
All bug reports submitted via web or email all go into a bug tracker. All
discussion on the web is mirrored to a thread on the mailing list and vice
versa. Web users are happy. Mailing list users are happy. Everything is
archived and organized in the tracker.
If you were interested in going down this road, I'd be interested in helping.
Step one would be to have pull requests on Github posted to the mailing list.
Link to the pull request, links to each individual commit. Then those who
want to work on Github can do it. Step two would be to have activity on the
pull request mirrored back to the list, still a SMOP. Step three would be to
have replies on the list mirrored back into the Github discussion. It could
even submit the pull request with git-send-email and mirror individual replies
to patches back as comments on individual Github commits.
If all the clicking and opening tabs in a browser feels uncomfortable to
you... its something you learn like anything else. Less and less people are
comfortable in mail clients. Who is the system optimized for? It doesn't
have to be a zero sum game.
If you're interested, I'd be happy to help put something like this together if
it will break the "ease of review" vs "ease of submission" deadlock.
--
Don't try the paranormal until you know what's normal.
-- "Lords and Ladies" by Terry Prachett
^ permalink raw reply [flat|nested] 34+ messages in thread* Re: Extract Git classes from git-svn (1/10)
2012-07-24 22:38 ` Michael G Schwern
@ 2012-07-24 23:25 ` Jonathan Nieder
2012-07-25 2:55 ` Eric Wong
1 sibling, 0 replies; 34+ messages in thread
From: Jonathan Nieder @ 2012-07-24 23:25 UTC (permalink / raw)
To: Michael G Schwern; +Cc: Junio C Hamano, git, robbat2, Eric Wong, Ben Walton
Hi,
Michael G Schwern wrote:
> But since you brought Github up... (I get the impression its kind of a dirty
> word around here)
On the contrary, one of the main contributors is employed by github,
github hosts the git website, and all in all, github has done great
work.
Many people on the git list have also interacted with projects that
went all-in on github and other online code review tools. I imagine
experiences varied from person to person.
When I said that if you don't like emails I'd be open to other ideas
for how to review your code, I didn't mean Github's web interface. :)
It doesn't work for me. Maybe it works better for other people.
For reference, Github pull requests wouldn't be the right
apples-to-apples comparison to make, anyway. There are also pull
requests from time to time on this list --- see for example
http://thread.gmane.org/gmane.comp.version-control.git/201728
http://thread.gmane.org/gmane.comp.version-control.git/201186
http://thread.gmane.org/gmane.comp.version-control.git/200844
http://thread.gmane.org/gmane.comp.version-control.git/199577
http://thread.gmane.org/gmane.comp.version-control.git/193817
http://thread.gmane.org/gmane.comp.version-control.git/189178
http://thread.gmane.org/gmane.comp.version-control.git/187082
Those are for code that has already been reviewed.
Hoping that clarifies,
Jonathan
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: Extract Git classes from git-svn (1/10)
2012-07-24 22:38 ` Michael G Schwern
2012-07-24 23:25 ` Jonathan Nieder
@ 2012-07-25 2:55 ` Eric Wong
2012-07-25 5:37 ` Michael G Schwern
1 sibling, 1 reply; 34+ messages in thread
From: Eric Wong @ 2012-07-25 2:55 UTC (permalink / raw)
To: Michael G Schwern
Cc: Junio C Hamano, Jonathan Nieder, git, robbat2, Ben Walton
Michael G Schwern <schwern@pobox.com> wrote:
> On 2012.7.17 10:49 PM, Junio C Hamano wrote:
> > By allowing people to easily publish a completed work, and making it
> > easier for them to let others peek at their work, Git hosting
> > services like GitHub are wonderful. But I am not conviced that
> > quality code reviews like we do on the mailing list can be done with
> > existing Web based interface to a satisfactory degree.
>
> In this instance, I was just using Github for repository storage. I was
> hoping I could just submit a remote git repository and people would look at it
> from there. No Github required.
>
> I understand this makes things very convenient for you to review patches, let
> me convey my POV...
>
> After I'm exhausted from volunteering all the coding work, rather than
> submitting a URL to a remote repository I find I have to learn new specialized
> tools. It's extra learning and work, an extra step to screw up, and foreign
> to me (even as a experienced git user). It is of little benefit to me as a
> casual volunteer submitter.
Except git is also a "new specialized tool". Your examples are exactly
why I'm saddened many projects only adopted git, but not the workflow
which _built_ git (and Linux).
> I can see if you've been on the git mailing list for a while and have git-am
> and all that set up, this system is great. But it comes at a cost which is
> offloaded onto new and casual contributors.
Email is integral to Free/Open Source development and remains one of the
few things on the Internet not (yet) controlled by any central entity.
Once setup, the same email setup can work across all projects which use
email. These projects need not be hosted on the same websites/servers
at all.
> This sort of specialized setup makes people bounce right off the submission
> process. At OSCON I was asking around for help getting things setup so I
> could submit patches here properly. As soon as they said "which mail daemon
> are you running?", I said "stop! I don't want to know any more". I have too
> many things to do to be fiddling with my mailer configuration just to submit
> volunteer work in the right form (that said, I'm pleased as punch that
> git-send-email now has instructions for sending via GMail). You're
> volunteers, too. We're all volunteers, so a more balanced submission process
> would be nice.
How about we educate users about a proper email setup instead? If
they're capable of learning git, they're surely capable of setting up an
email client properly, and perhaps more projects can adopt an
email-centric workflow.
> But since you brought Github up... (I get the impression its kind of a dirty
> word around here)
(Not speaking for the git project) I'm entirely against the way GitHub
(or Ohloh or similar services) gamifies software development and tries
to tie a person to all their other projects.
Much of my code is public, but I am a private person. I want code to be
judged solely on its own merits of that code; not from what the author's
achieved or how "popular" the person might be in the development world.
Unfortunately, GitHub (and other social networks) is structured to
encourage that sort of thing (which I know is appealing to many).
For me, the whole social network followers/timeline thing also has a
_huge_ creepiness factor to it. How one prioritizes and spends time
between different different (especially unrelated) projects should be
nobody else's business.
I don't make it /easy/ for someone (e.g. Junio) to know I'm slacking
off on my git work to hack on ProjectNoOneUses :)
One could try using a different account for every project, but that's
also violating the terms of service.
> If all the clicking and opening tabs in a browser feels uncomfortable to
> you... its something you learn like anything else. Less and less people are
> comfortable in mail clients. Who is the system optimized for? It doesn't
> have to be a zero sum game.
(Still not speaking for others) I believe GUIs are (mostly) harmful.
Graphical browsers don't interact well with command-line tools.
Browsers have no concept of a working directory; I can't fire up a
browser tab/window for each project I work on and edit/apply patches
directly from the browser like I can with an MUA.
We need to figure out how to teach folks to use email (properly)
instead.
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: Extract Git classes from git-svn (1/10)
2012-07-25 2:55 ` Eric Wong
@ 2012-07-25 5:37 ` Michael G Schwern
2012-07-25 5:54 ` OT: mail-based interfaces and web-based interfaces (Re: Extract Git classes from git-svn (1/10)) Jonathan Nieder
2012-07-25 23:48 ` OT: mail-based interfaces and web-based interfaces (Re: Extract Eric Wong
0 siblings, 2 replies; 34+ messages in thread
From: Michael G Schwern @ 2012-07-25 5:37 UTC (permalink / raw)
To: Eric Wong; +Cc: Junio C Hamano, Jonathan Nieder, git, robbat2, Ben Walton
On 2012.7.24 7:55 PM, Eric Wong wrote:
>> After I'm exhausted from volunteering all the coding work, rather than
>> submitting a URL to a remote repository I find I have to learn new specialized
>> tools. It's extra learning and work, an extra step to screw up, and foreign
>> to me (even as a experienced git user). It is of little benefit to me as a
>> casual volunteer submitter.
>
> Except git is also a "new specialized tool". Your examples are exactly
> why I'm saddened many projects only adopted git, but not the workflow
> which _built_ git (and Linux).
There is an important difference between a tool which is useful for one or two
projects and one which is useful for a broad spectrum of projects. I learn
git once (or diff or bash or Perl or whatever) and I'm going to use it again
and again all over the place. I learn git-send-email and (if I'm not a kernel
developer) I'm going to use it on a handful of projects maybe. It's O(1) vs
O(n) effort.
Github also has broad spectrum utility. I learn how to fork and work with a
Github pull request once, and I can repeat that on thousands of different
projects of all different sorts of things.
This commonality of tools and techniques is very important to easing the on
ramp for new (to-your-project) developers.
>> I can see if you've been on the git mailing list for a while and have git-am
>> and all that set up, this system is great. But it comes at a cost which is
>> offloaded onto new and casual contributors.
>
> Email is integral to Free/Open Source development and remains one of the
> few things on the Internet not (yet) controlled by any central entity.
> Once setup, the same email setup can work across all projects which use
> email. These projects need not be hosted on the same websites/servers
> at all.
While I hear your concern about being centrally controlled, it is largely
irrelevant to the new user experience. And remaining independent does not
mean you can't use web tools. Be wary of a false dichotomy between Free and web.
"We use a mailing list" is by no means an indication of commonality. Every
project of the "send patches to the list" form has their own quirks and ways
of doing it. Usually they're not written down. This is what I've been
struggling with. I've been sending patches to mailing lists for decades and I
can tell you everybody does it differently. "Send a patch to the list" is one
of the steeper project on-ramps.
>> This sort of specialized setup makes people bounce right off the submission
>> process. At OSCON I was asking around for help getting things setup so I
>> could submit patches here properly. As soon as they said "which mail daemon
>> are you running?", I said "stop! I don't want to know any more". I have too
>> many things to do to be fiddling with my mailer configuration just to submit
>> volunteer work in the right form (that said, I'm pleased as punch that
>> git-send-email now has instructions for sending via GMail). You're
>> volunteers, too. We're all volunteers, so a more balanced submission process
>> would be nice.
>
> How about we educate users about a proper email setup instead? If
> they're capable of learning git, they're surely capable of setting up an
> email client properly, and perhaps more projects can adopt an
> email-centric workflow.
SubmittingPatches would be helped by that, particularly with a clear
step-by-step example of using git-send-email and all its numerous command line
switches.
I was showing Jonathan the guide I have for releasing Perl modules which is
both A) step-by-step and also B) covers the numerous little problems that are
usually only in somebody's head or scattered around the docs. It was built in
order to allow a person who had never released a module to release a module.
Then we watched just such a person follow the directions. As they asked
questions, struggled or made mistakes we filled in the gaps in the docs.
https://github.com/scrottie/autobox-Core/wiki/How-To-Make-A-Release
But this is also not about capability. Yes, people are capable of figuring
out git-send-email, but its Yet Another Special Thing to learn before they can
submit a patch and call their work done. Volunteers, especially brand new
ones, have only so much volunteerism to burn before they'll walk away. You
want them burning that on productive patching and contributions, not learning
specialty tools.
And, finally, the last thing most people want is more email. Seriously. It
sounds like you live in your mailer, but fewer and fewer people do that. Me?
I don't want to join another mailing list. My email management is a disaster!
What it comes down to is this: is it enough to contribute to git.git to know
how to work on git.git? Or do you also need to live in your mailer? Bolt on
that extra requirement and you lose a large swath of contributors.
>> But since you brought Github up... (I get the impression its kind of a dirty
>> word around here)
>
> (Not speaking for the git project) I'm entirely against the way GitHub
> (or Ohloh or similar services) gamifies software development and tries
> to tie a person to all their other projects.
>
> Much of my code is public, but I am a private person. I want code to be
> judged solely on its own merits of that code; not from what the author's
> achieved or how "popular" the person might be in the development world.
> Unfortunately, GitHub (and other social networks) is structured to
> encourage that sort of thing (which I know is appealing to many).
FWIW this aspect of Github is just a toy. For some people it acts as a
carrot, but nobody worth caring about takes it seriously. I wouldn't get too
worked up about it.
> For me, the whole social network followers/timeline thing also has a
> _huge_ creepiness factor to it. How one prioritizes and spends time
> between different different (especially unrelated) projects should be
> nobody else's business.
>
> I don't make it /easy/ for someone (e.g. Junio) to know I'm slacking
> off on my git work to hack on ProjectNoOneUses :)
>
> One could try using a different account for every project, but that's
> also violating the terms of service.
You want to know what's creepy? That you'd think people work like that.
It doesn't work out that way. People have far better things to do than stalk
your Github commits to see how you're spending your time. You're just not
that interesting. Neither am I!
(If I really wanted to I could just compile your activity from public list
archives and repositories, so you're really not broadcasting any less
information about yourself by staying off Github. But I wouldn't want to,
because you're just not that interesting and I have better things to do with
my time!)
Besides, Facebook is where all the stalking is at! ;)
>> If all the clicking and opening tabs in a browser feels uncomfortable to
>> you... its something you learn like anything else. Less and less people are
>> comfortable in mail clients. Who is the system optimized for? It doesn't
>> have to be a zero sum game.
>
> (Still not speaking for others) I believe GUIs are (mostly) harmful.
You can hate GUIs, but you should also realize that just about everyone out
there likes them, and they're not all morons. If you deny GUIs as a tool for
git.git developers because of your blind spot, then you're losing potential
contributors. More and more as time goes on.
Recognize it as a personal blind spot, don't punish your users because of it.
Me? I hate IDEs. Won't touch em. Do I really think emacs or vim are
superior and all those IDE users are deluded? HELL NO! I think I'm ignorant
and don't understand IDEs. I really don't know anything about them. I don't
even know what I don't know. I generally keep my mouth shut and listen rather
than blab a bunch of old guy ignorance.
> Graphical browsers don't interact well with command-line tools.
> Browsers have no concept of a working directory; I can't fire up a
> browser tab/window for each project I work on and edit/apply patches
> directly from the browser like I can with an MUA.
I'm not sure what you're talking about, or what sort of workflow you've got
going with your mailer, but I'm sure with just as much time and effort as
you've put into your CLI and MUA setup you can be just as efficient or moreso
GUI. One can do crazy shit in browsers these days either in Javascript or by
writing or using an add-on. But also, one doesn't have to do EVERYTHING with
a single tool. Amazing But True!
You should consider sitting down with somebody who works very differently from
how you do and see how they do it. You might learn something you don't know
you don't know!
And again, it *does not have to be zero sum*. It doesn't have to be email VS
GUI. You can have your cake and eat it too.
--
125. Two drink limit does not mean two kinds of drinks.
-- The 213 Things Skippy Is No Longer Allowed To Do In The U.S. Army
http://skippyslist.com/list/
^ permalink raw reply [flat|nested] 34+ messages in thread* OT: mail-based interfaces and web-based interfaces (Re: Extract Git classes from git-svn (1/10))
2012-07-25 5:37 ` Michael G Schwern
@ 2012-07-25 5:54 ` Jonathan Nieder
2012-07-25 6:20 ` Michael G Schwern
2012-07-25 23:48 ` OT: mail-based interfaces and web-based interfaces (Re: Extract Eric Wong
1 sibling, 1 reply; 34+ messages in thread
From: Jonathan Nieder @ 2012-07-25 5:54 UTC (permalink / raw)
To: Michael G Schwern; +Cc: Eric Wong, Junio C Hamano, git, robbat2, Ben Walton
Michael G Schwern wrote:
> And again, it *does not have to be zero sum*. It doesn't have to be email VS
> GUI. You can have your cake and eat it too.
I assume you're talking about web-based interfaces that have gateways
to email, that produce inboxes like this:
24 Jul 02:46 GitHub [github] msysgit/msysgit was forked by peters
23 Jul 10:27 GitHub [msysgit/git] ce8ebc: vcs-svn: rename check_o
23 Jul 10:01 GitHub [github] Comment created on issue 44 (new git
23 Jul 09:50 GitHub [github] Comment created on issue 44 (new git
23 Jul 09:33 GitHub [github] Comment created on issue 44 (new git
23 Jul 09:39 GitHub [github] Comment created on issue 24 (Long fi
23 Jul 09:31 GitHub [github] Comment created on issue 44 (new git
23 Jul 09:30 GitHub [github] Comment created on issue 24 (Long fi
22 Jul 23:57 GitHub [github] Comment created on issue 44 (new git
I call that pretending to have my cake, rather than having it. :)
Maybe some day someone will prove me wrong and make a nice web-based
tool that I don't even need to know about that mines project mailing
lists. If I have to tweak my subject lines a little to help it out,
that's fine with me. I think patchwork is supposed to work this way.
But unless we're talking about splitting the mailing list into a bunch
of mini mailing lists (like some bug trackers do), it doesn't change
anything fundamental, so I'm not sure why we're discussing this.
Ciao,
Jonathan
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: OT: mail-based interfaces and web-based interfaces (Re: Extract Git classes from git-svn (1/10))
2012-07-25 5:54 ` OT: mail-based interfaces and web-based interfaces (Re: Extract Git classes from git-svn (1/10)) Jonathan Nieder
@ 2012-07-25 6:20 ` Michael G Schwern
0 siblings, 0 replies; 34+ messages in thread
From: Michael G Schwern @ 2012-07-25 6:20 UTC (permalink / raw)
To: Jonathan Nieder; +Cc: Eric Wong, Junio C Hamano, git, robbat2, Ben Walton
On 2012.7.24 10:54 PM, Jonathan Nieder wrote:
>> And again, it *does not have to be zero sum*. It doesn't have to be email VS
>> GUI. You can have your cake and eat it too.
>
> I assume you're talking about web-based interfaces that have gateways
> to email, that produce inboxes like this:
>
> 24 Jul 02:46 GitHub [github] msysgit/msysgit was forked by peters
> 23 Jul 10:27 GitHub [msysgit/git] ce8ebc: vcs-svn: rename check_o
> 23 Jul 10:01 GitHub [github] Comment created on issue 44 (new git
> 23 Jul 09:50 GitHub [github] Comment created on issue 44 (new git
> 23 Jul 09:33 GitHub [github] Comment created on issue 44 (new git
> 23 Jul 09:39 GitHub [github] Comment created on issue 24 (Long fi
> 23 Jul 09:31 GitHub [github] Comment created on issue 44 (new git
> 23 Jul 09:30 GitHub [github] Comment created on issue 24 (Long fi
> 22 Jul 23:57 GitHub [github] Comment created on issue 44 (new git
>
> I call that pretending to have my cake, rather than having it. :)
That's kind of like pointing at RCS and saying "version control sucks and its
pointless to try and make it better!" Mail gateways built by web sites suck
because they live in the web browser and email is an after thought. Sound
familiar?
Here is a much better example of the RT mail gateway that Perl 5 development
uses. They're a dev community still centered around email, so it has to
integrate well.
http://www.nntp.perl.org/group/perl.perl5.porters/2012/07/msg189716.html
And the corresponding ticket in the tracker.
https://rt.perl.org/rt3/Public/Bug/Display.html?id=113684
The initial report comes in either via the web tracker or via a command line
program (perlbug) that sends an email to the list. Replies on either the
tracker or the mailing list are mirrored. Duplicates are detected etc...
That's the sort of mail gateways I'm used to.
> Maybe some day someone will prove me wrong and make a nice web-based
> tool that I don't even need to know about that mines project mailing
> lists. If I have to tweak my subject lines a little to help it out,
> that's fine with me. I think patchwork is supposed to work this way.
>
> But unless we're talking about splitting the mailing list into a bunch
> of mini mailing lists (like some bug trackers do), it doesn't change
> anything fundamental, so I'm not sure why we're discussing this.
I don't follow the bit about splitting the mailing list.
--
emacs -- THAT'S NO EDITOR... IT'S AN OPERATING SYSTEM!
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: OT: mail-based interfaces and web-based interfaces (Re: Extract
2012-07-25 5:37 ` Michael G Schwern
2012-07-25 5:54 ` OT: mail-based interfaces and web-based interfaces (Re: Extract Git classes from git-svn (1/10)) Jonathan Nieder
@ 2012-07-25 23:48 ` Eric Wong
2012-07-26 2:33 ` Michael G Schwern
1 sibling, 1 reply; 34+ messages in thread
From: Eric Wong @ 2012-07-25 23:48 UTC (permalink / raw)
To: Michael G Schwern
Cc: Junio C Hamano, Jonathan Nieder, git, robbat2, Ben Walton
Michael G Schwern <schwern@pobox.com> wrote:
> On 2012.7.24 7:55 PM, Eric Wong wrote:
> > Except git is also a "new specialized tool". Your examples are exactly
> > why I'm saddened many projects only adopted git, but not the workflow
> > which _built_ git (and Linux).
>
> Github also has broad spectrum utility. I learn how to fork and work with a
> Github pull request once, and I can repeat that on thousands of different
> projects of all different sorts of things.
>
> This commonality of tools and techniques is very important to easing the on
> ramp for new (to-your-project) developers.
I agree commonality is important.
But to me, it's not worth the cost of reliance on centralized,
single-vendor solutions. My distrust and dissatisfaction with
centralized/single-vendor solutions is the reason I'm involved with Free
Software (and decentralized version control) in the first place.
> > Email is integral to Free/Open Source development and remains one of the
> > few things on the Internet not (yet) controlled by any central entity.
> > Once setup, the same email setup can work across all projects which use
> > email. These projects need not be hosted on the same websites/servers
> > at all.
>
> While I hear your concern about being centrally controlled, it is largely
> irrelevant to the new user experience. And remaining independent does not
> mean you can't use web tools. Be wary of a false dichotomy between Free and web.
Of course, I'd be in favor of shifting development to something less
centralized than a mailing list. OStatus may become a good choice one
day, but the adoption/tools just aren't there right now.
> "We use a mailing list" is by no means an indication of commonality. Every
> project of the "send patches to the list" form has their own quirks and ways
> of doing it. Usually they're not written down. This is what I've been
> struggling with. I've been sending patches to mailing lists for decades and I
> can tell you everybody does it differently. "Send a patch to the list" is one
> of the steeper project on-ramps.
Sure, every project/culture has different norms. It's no big deal,
learn them, or avoid them.
Projects all have different coding styles, different conventions for
writing commit messages, changelogs and so on... I live with that and
I'll do my best to adjust my style/editor settings/grammar/vocabulary
accordingly for each project I contribute to.
I won't send patches to mailing lists of projects which prefer
patches/pull-requests on their web tracker, either. If the project is
important enough to me, I'll (grudgingly) use whatever tools/formats the
maintainers prefer.
> > How about we educate users about a proper email setup instead? If
> > they're capable of learning git, they're surely capable of setting up an
> > email client properly, and perhaps more projects can adopt an
> > email-centric workflow.
>
> SubmittingPatches would be helped by that, particularly with a clear
> step-by-step example of using git-send-email and all its numerous command line
> switches.
It's documented in the gitworkflows(7) manpage. They should probably be
linked somehow.
> And, finally, the last thing most people want is more email. Seriously. It
> sounds like you live in your mailer, but fewer and fewer people do that. Me?
> I don't want to join another mailing list. My email management is a disaster!
I live in my mailer/$EDITOR/shell and I'm happy with that.
The last thing I want is more cookies, non-Free JavaScript code, logins,
user tracking/profiling, memory/CPU usage. I doubt either of us will
get what we want, though :<
> What it comes down to is this: is it enough to contribute to git.git to know
> how to work on git.git? Or do you also need to live in your mailer? Bolt on
> that extra requirement and you lose a large swath of contributors.
We need to use something. Right now our choice of mailer is the best
choice for _existing_ contributors.
> > For me, the whole social network followers/timeline thing also has a
> > _huge_ creepiness factor to it. How one prioritizes and spends time
> > between different different (especially unrelated) projects should be
> > nobody else's business.
> >
> > I don't make it /easy/ for someone (e.g. Junio) to know I'm slacking
> > off on my git work to hack on ProjectNoOneUses :)
>
> You want to know what's creepy? That you'd think people work like that.
>
> It doesn't work out that way. People have far better things to do than stalk
> your Github commits to see how you're spending your time. You're just not
> that interesting. Neither am I!
>
> (If I really wanted to I could just compile your activity from public list
> archives and repositories, so you're really not broadcasting any less
> information about yourself by staying off Github. But I wouldn't want to,
> because you're just not that interesting and I have better things to do with
> my time!)
There's a big difference between making information easy-to-access vs.
merely possible-to-access.
For the most part, people don't care about me (which I'm glad for), but
it has creeped me out when somebody did (especially people I've never
interacted with before). People do often look to other well-known
developers as role models and GitHub's made it easier for folks who
welcome that attention.
> >> If all the clicking and opening tabs in a browser feels uncomfortable to
> >> you... its something you learn like anything else. Less and less people are
> >> comfortable in mail clients. Who is the system optimized for? It doesn't
> >> have to be a zero sum game.
> >
> > (Still not speaking for others) I believe GUIs are (mostly) harmful.
>
> You can hate GUIs, but you should also realize that just about everyone out
> there likes them, and they're not all morons. If you deny GUIs as a tool for
> git.git developers because of your blind spot, then you're losing potential
> contributors. More and more as time goes on.
I disagree, there's a healthy number of users learning to do more and
more on the command-line. There'll likely always be more GUI users, but
the command-line isn't going away.
> Recognize it as a personal blind spot, don't punish your users because of it.
Users of my software are already command-line users.
I'm certain of this because I don't develop GUI software :)
> > Graphical browsers don't interact well with command-line tools.
> > Browsers have no concept of a working directory; I can't fire up a
> > browser tab/window for each project I work on and edit/apply patches
> > directly from the browser like I can with an MUA.
>
> I'm not sure what you're talking about, or what sort of workflow you've got
> going with your mailer, but I'm sure with just as much time and effort as
> you've put into your CLI and MUA setup you can be just as efficient or moreso
> GUI. One can do crazy shit in browsers these days either in Javascript or by
> writing or using an add-on. But also, one doesn't have to do EVERYTHING with
> a single tool. Amazing But True!
I run my mailer from different machines which I usually control remotely
(on links insufficient for X11 forwarding, but enough for regular
ssh/mosh).
I write email/code with my $EDITOR of choice, and can pipe messages from
my mailer to arbitrary command-line programs (e.g. git am, patch, ssh).
My mailer lets me edit patches/commit messages as needed (fixing minor
grammar/spelling mistakes, adding Acked-by/Tested-by/Signed-off-by:
lines) before piping them. I use multiple tools, but I can do all this
without the overhead of switching terminals/windows (which can cause a
loss of mental focus).
A web browser is significantly clunkier and would require extra
steps/tabs/windows to put/run code where I want it.
One could develop/improve CLI programs for a REST API, too, but
right now that API is still owned and controlled by a central entity.
If the web-based API weren't centrally controlled, I'm not opposed to a
workflow which allows me to interact with the API via curl or
purpose-built CLI tools (with offline capability).
> You should consider sitting down with somebody who works very differently from
> how you do and see how they do it. You might learn something you don't know
> you don't know!
Sure I've tried, but I have yet to be impressed by anything I've seen
from GUI users. On the other hand, I have learned much from other
command-line users over the years (and developed my own workflows this
way).
> And again, it *does not have to be zero sum*. It doesn't have to be email VS
> GUI. You can have your cake and eat it too.
I agree we can do both, but we already have email.
I think the git project endorsing any /workflow/ based on a centralized,
non-Free service would be damaging to the viability of Free,
decentralized version control.
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: OT: mail-based interfaces and web-based interfaces (Re: Extract
2012-07-25 23:48 ` OT: mail-based interfaces and web-based interfaces (Re: Extract Eric Wong
@ 2012-07-26 2:33 ` Michael G Schwern
2012-07-26 2:47 ` Jonathan Nieder
2012-07-26 3:10 ` Eric Wong
0 siblings, 2 replies; 34+ messages in thread
From: Michael G Schwern @ 2012-07-26 2:33 UTC (permalink / raw)
To: Eric Wong; +Cc: Junio C Hamano, Jonathan Nieder, git, robbat2, Ben Walton
On 2012.7.25 4:48 PM, Eric Wong wrote:
> We need to use something. Right now our choice of mailer is the best
> choice for _existing_ contributors.
I believe this entire discussion can be reduced to that right there.
If your process is optimized for existing contributors, it will work well for
existing contributors, who will want to optimize it for themselves. Repeat.
If the main way you evaluate your process is asking "is this more convenient
for me" then you're probably in that spiral.
This creates a process very well tuned to the existing contributors, and its
very convenient for them. But the consequence is it becomes more and more
work for a new contributor to join.
Before talking about anything else, the existing contributors have to ask
themselves a simple question: Do we care about getting new contributors?
The answer can be "no" ("yes, but not if I'm inconvenienced" is a no). Maybe
you're happy with the people you've got. But there's no point in getting into
detail until that's settled.
That's mostly a rhetorical question. I want to wrap up the meta-discussion
and focus on getting patches in.
--
100. Claymore mines are not filled with yummy candy, and it is wrong
to tell new soldiers that they are.
-- The 213 Things Skippy Is No Longer Allowed To Do In The U.S. Army
http://skippyslist.com/list/
^ permalink raw reply [flat|nested] 34+ messages in thread* Re: OT: mail-based interfaces and web-based interfaces (Re: Extract
2012-07-26 2:33 ` Michael G Schwern
@ 2012-07-26 2:47 ` Jonathan Nieder
2012-07-26 3:10 ` Eric Wong
1 sibling, 0 replies; 34+ messages in thread
From: Jonathan Nieder @ 2012-07-26 2:47 UTC (permalink / raw)
To: Michael G Schwern; +Cc: Eric Wong, Junio C Hamano, git, robbat2, Ben Walton
Michael G Schwern wrote:
> Do we care about getting new contributors?
Not if the cost is too high, no. I personally do think it is
important to make life easy for new contributors, not because that
will somehow compel them to work for me for free, but because it's a
kind thing to do.
Does that mean that I'm going to start reviewing patches submitted as
virtual pieces of paper in Second Life if that is what is most
convenient for potential new contributors? No way. If I felt
compelled to do that, I'd become grouchy, and grouchily spending time
indulging people out of a perverse sense of obligation is not a kind
thing to do.
Jonathan
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: OT: mail-based interfaces and web-based interfaces (Re: Extract
2012-07-26 2:33 ` Michael G Schwern
2012-07-26 2:47 ` Jonathan Nieder
@ 2012-07-26 3:10 ` Eric Wong
1 sibling, 0 replies; 34+ messages in thread
From: Eric Wong @ 2012-07-26 3:10 UTC (permalink / raw)
To: Michael G Schwern
Cc: Junio C Hamano, Jonathan Nieder, git, robbat2, Ben Walton
Michael G Schwern <schwern@pobox.com> wrote:
> On 2012.7.25 4:48 PM, Eric Wong wrote:
> > We need to use something. Right now our choice of mailer is the best
> > choice for _existing_ contributors.
>
> I believe this entire discussion can be reduced to that right there.
>
> If your process is optimized for existing contributors, it will work well for
> existing contributors, who will want to optimize it for themselves. Repeat.
> If the main way you evaluate your process is asking "is this more convenient
> for me" then you're probably in that spiral.
>
> This creates a process very well tuned to the existing contributors, and its
> very convenient for them. But the consequence is it becomes more and more
> work for a new contributor to join.
The process is _not_ a lot of work. At least no more than any other
project: observe the regulars -> imitate the regulars
Many/most regular git contributors are not Linux kernel developers, yet
were able to quickly able to get up-to-speed with git. AFAIK, the Linux
kernel gets plenty of new contributors every year, too.
> Before talking about anything else, the existing contributors have to ask
> themselves a simple question: Do we care about getting new contributors?
Yes, if contributors are willing to learn/respect existing conventions.
We do take time to help new contributors out :)
For me, it's certainly "no" if there's any endorsement of
non-Free Software or centralized/commercial services involved.
> The answer can be "no" ("yes, but not if I'm inconvenienced" is a no). Maybe
> you're happy with the people you've got. But there's no point in getting into
> detail until that's settled.
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: Fix git-svn tests for SVN 1.7.5.
2012-07-17 17:44 ` Jonathan Nieder
` (9 preceding siblings ...)
[not found] ` <5005F139.8050205@pobox.com>
@ 2012-07-21 0:27 ` Ben Walton
10 siblings, 0 replies; 34+ messages in thread
From: Ben Walton @ 2012-07-21 0:27 UTC (permalink / raw)
To: Michael G Schwern; +Cc: Jonathan Nieder, git, gitster, robbat2, Eric Wong
Hi Michael,
> > I've fixed the git-svn tests for SVN 1.7 and tested with SVN 1.7.5.
>
> Thanks. git-svn is not maintained by Junio but by Eric and others on
> the list. I'm cc-ing Eric and Ben Walton so they can benefit from
> your work.
This is fantastic. It's been on my todo list but not a priority for
me. I'm glad you've taken the time to scratch this itch though.
I'll try to run through this series in the next few days and I can
also do some test builds on solaris to see how it plays there.
Thanks
-Ben
--
Ben Walton
Systems Programmer - CHASS
University of Toronto
C:416.407.5610 | W:416.978.4302
^ permalink raw reply [flat|nested] 34+ messages in thread