git-svn: why fetching files is so slow

git-svn: why fetching files is so slow

[Pazu]

... compared to the standalone svn client. I'm working with repositories over
the internet, using not-so-fast links, but still, a svn checkout takes somewhere
around 5 to 10 minutes, while git-svn fetch takes at least 10 times that just to
fetch the initial revision. Later fetches also take *a lot* more time than a svn
update would.

Cheers,

-- Pazu

Re: git-svn: why fetching files is so slow

[Seth Falcon]

Pazu <pazu@pazu.com.br> writes:
> ... compared to the standalone svn client. I'm working with repositories over
> the internet, using not-so-fast links, but still, a svn checkout takes somewhere
> around 5 to 10 minutes, while git-svn fetch takes at least 10 times that just to
> fetch the initial revision. Later fetches also take *a lot* more time than a svn
> update would.

[warning: I _think_ this is how it works, but not 100% sure]
When you use git-svn to fetch from an svn repository, you make a
separate request for each commit that occurred on the remote svn
repos.  When you use the svn client, it only needs to compute and
download one delta .

If you are not already using the Perl SVN bindings (you will need to
build svn from source), you should give them a try.  They are much
faster.

My experience has often been the opposite, but I think that is because
I work with an svn repository where I track a directory that has many
many subdirs.  The svn working copy traversal is so slow that even
with the extra network overhead, git + git-svn ends up being faster
for fetch (and much faster for any local operation).

Re: git-svn: why fetching files is so slow

[Eric Wong]

Pazu <pazu@pazu.com.br> wrote:
> ... compared to the standalone svn client. I'm working with repositories over
> the internet, using not-so-fast links, but still, a svn checkout takes somewhere
> around 5 to 10 minutes, while git-svn fetch takes at least 10 times that just to
> fetch the initial revision. Later fetches also take *a lot* more time than a svn
> update would.

git-svn transfers full files, and not deltas.  I'll hopefully have a
chance to look into improving the situation for slow links this weekend.

-- 

Re: git-svn: why fetching files is so slow

[Pazu]

Eric Wong <normalperson <at> yhbt.net> writes:

> git-svn transfers full files, and not deltas.  I'll hopefully have a
> chance to look into improving the situation for slow links this weekend.

Yes, but why would that make fetching the first revision slower? In this
situation, both svn and git-svn would have to fetch full files. Maybe git-svn
isn't using gzip compression or http pipelining?

-- Pazu

Re: git-svn: why fetching files is so slow

[Eric Wong]

Pazu <pazu@pazu.com.br> wrote:
> Eric Wong <normalperson <at> yhbt.net> writes:
> 
> > git-svn transfers full files, and not deltas.  I'll hopefully have a
> > chance to look into improving the situation for slow links this weekend.
> 
> Yes, but why would that make fetching the first revision slower? In this
> situation, both svn and git-svn would have to fetch full files. Maybe git-svn
> isn't using gzip compression or http pipelining?

Even for the initial transfer, the tree is bundled into one big delta
(at least over https).

-- 

Re: git-svn: why fetching files is so slow

[Junio C Hamano]

Eric Wong <normalperson@yhbt.net> writes:

> Pazu <pazu@pazu.com.br> wrote:
>> Eric Wong <normalperson <at> yhbt.net> writes:
>> 
>> > git-svn transfers full files, and not deltas.  I'll hopefully have a
>> > chance to look into improving the situation for slow links this weekend.
>> 
>> Yes, but why would that make fetching the first revision slower? In this
>> situation, both svn and git-svn would have to fetch full files. Maybe git-svn
>> isn't using gzip compression or http pipelining?
>
> Even for the initial transfer, the tree is bundled into one big delta
> (at least over https).

Do you mean that "one big delta" saves duplicates across copies
inside the tree (e.g. svn tags and branches can be expressed as
a mostly identical copies of each other), or do you mean "one
full file at a time" requests are killing us, compared to a such
single transfer of "one big delta"?

Re: git-svn: why fetching files is so slow

[Eric Wong]

Junio C Hamano <junkio@cox.net> wrote:
> Eric Wong <normalperson@yhbt.net> writes:
> 
> > Pazu <pazu@pazu.com.br> wrote:
> >> Eric Wong <normalperson <at> yhbt.net> writes:
> >> 
> >> > git-svn transfers full files, and not deltas.  I'll hopefully have a
> >> > chance to look into improving the situation for slow links this weekend.
> >> 
> >> Yes, but why would that make fetching the first revision slower? In this
> >> situation, both svn and git-svn would have to fetch full files. Maybe git-svn
> >> isn't using gzip compression or http pipelining?
> >
> > Even for the initial transfer, the tree is bundled into one big delta
> > (at least over https).
> 
> Do you mean that "one big delta" saves duplicates across copies
> inside the tree (e.g. svn tags and branches can be expressed as
> a mostly identical copies of each other), or do you mean "one
> full file at a time" requests are killing us, compared to a such
> single transfer of "one big delta"?

One full file at a time requests are definitely killing us (over slow
links, at least).  I'm not sure how/if duplicates inside a requested
tree are optimized on the server side.

-- 

[PATCH 1/2] git-svn: enable delta transfers during fetches when using SVN:: libs

[Eric Wong]

This should drastically reduce bandwidth used for network
transfers.  This is not enabled for file:// repositories by
default because of the increased CPU usage and I/O needed.

GIT_SVN_DELTA_FETCH may be set to a true value to enable or
false (0) to disable delta transfers regardless of the
repository type.

Signed-off-by: Eric Wong <normalperson@yhbt.net>
---
 git-svn.perl |  194 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++--
 1 files changed, 189 insertions(+), 5 deletions(-)

diff --git a/git-svn.perl b/git-svn.perl
index d5d9c49..9b86d91 100755
--- a/git-svn.perl
+++ b/git-svn.perl
@@ -68,7 +68,7 @@ my ($_revision,$_stdin,$_no_ignore_ext,$
 	$_limit, $_verbose, $_incremental, $_oneline, $_l_fmt, $_show_commit,
 	$_version, $_upgrade, $_authors, $_branch_all_refs, @_opt_m,
 	$_merge, $_strategy, $_dry_run, $_ignore_nodate, $_non_recursive,
-	$_username, $_config_dir, $_no_auth_cache);
+	$_username, $_config_dir, $_no_auth_cache, $_xfer_delta);
 my (@_branch_from, %tree_map, %users, %rusers, %equiv);
 my ($_svn_co_url_revs, $_svn_pg_peg_revs);
 my @repo_path_split_cache;
@@ -2675,6 +2675,9 @@ sub libsvn_load {
 		require SVN::Ra;
 		require SVN::Delta;
 		push @SVN::Git::Editor::ISA, 'SVN::Delta::Editor';
+		push @SVN::Git::Fetcher::ISA, 'SVN::Delta::Editor';
+		*SVN::Git::Fetcher::process_rm = *process_rm;
+		*SVN::Git::Fetcher::safe_qx = *safe_qx;
 		my $kill_stupid_warnings = $SVN::Node::none.$SVN::Node::file.
 					$SVN::Node::dir.$SVN::Node::unknown.
 					$SVN::Node::none.$SVN::Node::file.
@@ -2827,6 +2830,13 @@ sub libsvn_connect {
 	                      config => $config,
 	                      pool => SVN::Pool->new,
 	                      auth_provider_callbacks => $callbacks);
+
+	my $df = $ENV{GIT_SVN_DELTA_FETCH};
+	if (defined $df) {
+		$_xfer_delta = $df;
+	} else {
+		$_xfer_delta = ($url =~ m#^file://#) ? undef : 1;
+	}
 	$ra->{svn_path} = $url;
 	$ra->{repos_root} = $ra->get_repos_root;
 	$ra->{svn_path} =~ s#^\Q$ra->{repos_root}\E/*##;
@@ -2915,6 +2925,24 @@ sub process_rm {
 }
 
 sub libsvn_fetch {
+	$_xfer_delta ? libsvn_fetch_delta(@_) : libsvn_fetch_full(@_);
+}
+
+sub libsvn_fetch_delta {
+	my ($last_commit, $paths, $rev, $author, $date, $msg) = @_;
+	my $pool = SVN::Pool->new;
+	my $ed = SVN::Git::Fetcher->new({ c => $last_commit, ra => $SVN,
+	                                  paths => $paths });
+	my $reporter = $SVN->do_update($rev, '', 1, $ed, $pool);
+	my @lock = $SVN::Core::VERSION ge '1.2.0' ? (undef) : ();
+	my (undef, $last_rev, undef) = cmt_metadata($last_commit);
+	$reporter->set_path('', $last_rev, 0, @lock, $pool);
+	$reporter->finish_report($pool);
+	$pool->clear;
+	libsvn_log_entry($rev, $author, $date, $msg, [$last_commit]);
+}
+
+sub libsvn_fetch_full {
 	my ($last_commit, $paths, $rev, $author, $date, $msg) = @_;
 	open my $gui, '| git-update-index -z --index-info' or croak $!;
 	my @amr;
@@ -3133,7 +3161,11 @@ sub libsvn_find_parent_branch {
 		unlink $GIT_SVN_INDEX;
 		print STDERR "Found branch parent: ($GIT_SVN) $parent\n";
 		sys(qw/git-read-tree/, $parent);
-		return libsvn_fetch($parent, $paths, $rev,
+		# I can't seem to get do_switch() to work correctly with
+		# the SWIG interface (TypeError when passing switch_url...),
+		# so we'll unconditionally bypass the delta interface here
+		# for now
+		return libsvn_fetch_full($parent, $paths, $rev,
 					$author, $date, $msg);
 	}
 	print STDERR "Nope, branch point not imported or unknown\n";
@@ -3153,9 +3185,19 @@ sub libsvn_new_tree {
 		return $log_entry;
 	}
 	my ($paths, $rev, $author, $date, $msg) = @_;
-	open my $gui, '| git-update-index -z --index-info' or croak $!;
-	libsvn_traverse($gui, '', $SVN->{svn_path}, $rev);
-	close $gui or croak $?;
+	if ($_xfer_delta) {
+		my $pool = SVN::Pool->new;
+		my $ed = SVN::Git::Fetcher->new({paths => $paths, ra => $SVN});
+		my $reporter = $SVN->do_update($rev, '', 1, $ed, $pool);
+		my @lock = $SVN::Core::VERSION ge '1.2.0' ? (undef) : ();
+		$reporter->set_path('', $rev, 1, @lock, $pool);
+		$reporter->finish_report($pool);
+		$pool->clear;
+	} else {
+		open my $gui, '| git-update-index -z --index-info' or croak $!;
+		libsvn_traverse($gui, '', $SVN->{svn_path}, $rev);
+		close $gui or croak $?;
+	}
 	return libsvn_log_entry($rev, $author, $date, $msg);
 }
 
@@ -3325,6 +3367,148 @@ sub copy_remote_ref {
 				"refs/remotes/$GIT_SVN on $origin\n";
 	}
 }
+package SVN::Git::Fetcher;
+use vars qw/@ISA/;
+use strict;
+use warnings;
+use Carp qw/croak/;
+use IO::File qw//;
+
+# file baton members: path, mode_a, mode_b, pool, fh, blob, base
+sub new {
+	my ($class, $git_svn) = @_;
+	my $self = SVN::Delta::Editor->new;
+	bless $self, $class;
+	open my $gui, '| git-update-index -z --index-info' or croak $!;
+	$self->{gui} = $gui;
+	$self->{c} = $git_svn->{c} if exists $git_svn->{c};
+	if (my $p = $git_svn->{paths} && $git_svn->{ra}) {
+		my $s = $git_svn->{ra}->{svn_path};
+		$s = length $s ? qr#^/\Q$s\E/# : qr#^/#;
+		$self->{paths} = { map { my $x = $_;
+		                         $x =~ s/$s//;
+		                         $x => $p->{$_} } keys %$p };
+	}
+	require Digest::MD5;
+	$self;
+}
+
+sub delete_entry {
+	my ($self, $path, $rev, $pb) = @_;
+	process_rm($self->{gui}, $self->{c}, $path);
+	undef;
+}
+
+sub open_file {
+	my ($self, $path, $pb, $rev) = @_;
+	my ($mode, $blob) = (safe_qx('git-ls-tree',$self->{c},'--',$path)
+	                     =~ /^(\d{6}) blob ([a-f\d]{40})\t/);
+	{ path => $path, mode_a => $mode, mode_b => $mode, blob => $blob,
+	  pool => SVN::Pool->new };
+}
+
+sub add_file {
+	my ($self, $path, $pb, $cp_path, $cp_rev) = @_;
+	{ path => $path, mode_a => 100644, mode_b => 100644,
+	  pool => SVN::Pool->new };
+}
+
+sub change_file_prop {
+	my ($self, $fb, $prop, $value) = @_;
+	if ($prop eq 'svn:executable') {
+		if ($fb->{mode_b} != 120000) {
+			$fb->{mode_b} = defined $value ? 100755 : 100644;
+		}
+	} elsif ($prop eq 'svn:special') {
+		$fb->{mode_b} = defined $value ? 120000 : 100644;
+	}
+	undef;
+}
+
+sub apply_textdelta {
+	my ($self, $fb, $exp) = @_;
+	my $fh = IO::File->new_tmpfile;
+	$fh->autoflush(1);
+	# $fh gets auto-closed() by SVN::TxDelta::apply(),
+	# (but $base does not,) so dup() it for reading in close_file
+	open my $dup, '<&', $fh or croak $!;
+	my $base = IO::File->new_tmpfile;
+	$base->autoflush(1);
+	if ($fb->{blob}) {
+		defined (my $pid = fork) or croak $!;
+		if (!$pid) {
+			open STDOUT, '>&', $base or croak $!;
+			print STDOUT 'link ' if ($fb->{mode_a} == 120000);
+			exec qw/git-cat-file blob/, $fb->{blob} or croak $!;
+		}
+		waitpid $pid, 0;
+		croak $? if $?;
+
+		if (defined $exp) {
+			seek $base, 0, 0 or croak $!;
+			my $md5 = Digest::MD5->new;
+			$md5->addfile($base);
+			my $got = $md5->hexdigest;
+			die "Checksum mismatch: $fb->{path} $fb->{blob}\n",
+			    "expected: $exp\n",
+			    "     got: $got\n" if ($got ne $exp);
+		}
+	}
+	seek $base, 0, 0 or croak $!;
+	$fb->{fh} = $dup;
+	$fb->{base} = $base;
+	[ SVN::TxDelta::apply($base, $fh, undef, $fb->{path}, $fb->{pool}) ];
+}
+
+sub close_file {
+	my ($self, $fb, $exp) = @_;
+	my $hash;
+	my $path = $fb->{path};
+	if (my $fh = $fb->{fh}) {
+		seek($fh, 0, 0) or croak $!;
+		my $md5 = Digest::MD5->new;
+		$md5->addfile($fh);
+		my $got = $md5->hexdigest;
+		die "Checksum mismatch: $path\n",
+		    "expected: $exp\n    got: $got\n" if ($got ne $exp);
+		seek($fh, 0, 0) or croak $!;
+		if ($fb->{mode_b} == 120000) {
+			read($fh, my $buf, 5) == 5 or croak $!;
+			$buf eq 'link ' or die "$path has mode 120000",
+			                       "but is not a link\n";
+		}
+		defined(my $pid = open my $out,'-|') or die "Can't fork: $!\n";
+		if (!$pid) {
+			open STDIN, '<&', $fh or croak $!;
+			exec qw/git-hash-object -w --stdin/ or croak $!;
+		}
+		chomp($hash = do { local $/; <$out> });
+		close $out or croak $!;
+		close $fh or croak $!;
+		$hash =~ /^[a-f\d]{40}$/ or die "not a sha1: $hash\n";
+		close $fb->{base} or croak $!;
+	} else {
+		$hash = $fb->{blob} or die "no blob information\n";
+	}
+	$fb->{pool}->clear;
+	my $gui = $self->{gui};
+	print $gui "$fb->{mode_b} $hash\t$path\0" or croak $!;
+	print "\t", $self->{paths}->{$path}->action,
+	      "\t$path\n" if defined $self->{paths}->{$path};
+	undef;
+}
+
+sub abort_edit {
+	my $self = shift;
+	close $self->{gui};
+	$self->SUPER::abort_edit(@_);
+}
+
+sub close_edit {
+	my $self = shift;
+	close $self->{gui} or croak;
+	$self->SUPER::close_edit(@_);
+}
 
 package SVN::Git::Editor;
 use vars qw/@ISA/;
-- 
1.4.4.1.g22a08

[PATCH 2/2] git-svn: update tests for recent changes

[Eric Wong]

* Enable test for delta transfers in full-svn-test.

* Run tests against the root of the repository so we won't have
  to revisit 308906fa6e98132cab839a4f42701386fba368ef and
  efe4631def181d32f932672a7ea31e52ee0ab308 again.
  The graft-branches test still runs as before.

Signed-off-by: Eric Wong <normalperson@yhbt.net>
---
 t/Makefile                        |    3 ++-
 t/lib-git-svn.sh                  |    2 +-
 t/t9100-git-svn-basic.sh          |    5 +++++
 t/t9103-git-svn-graft-branches.sh |    2 ++
 4 files changed, 10 insertions(+), 2 deletions(-)

diff --git a/t/Makefile b/t/Makefile
index 8983509..0abe66d 100644
--- a/t/Makefile
+++ b/t/Makefile
@@ -27,8 +27,9 @@ clean:
 
 # we can test NO_OPTIMIZE_COMMITS independently of LC_ALL
 full-svn-test:
+	$(MAKE) $(TSVN) GIT_SVN_NO_LIB=0 GIT_SVN_DELTA_FETCH=1 \
+					GIT_SVN_NO_OPTIMIZE_COMMITS=1 LC_ALL=C
 	$(MAKE) $(TSVN) GIT_SVN_NO_LIB=1 GIT_SVN_NO_OPTIMIZE_COMMITS=1 LC_ALL=C
-	$(MAKE) $(TSVN) GIT_SVN_NO_LIB=0 GIT_SVN_NO_OPTIMIZE_COMMITS=1 LC_ALL=C
 	$(MAKE) $(TSVN) GIT_SVN_NO_LIB=1 GIT_SVN_NO_OPTIMIZE_COMMITS=0 \
 							LC_ALL=en_US.UTF-8
 	$(MAKE) $(TSVN) GIT_SVN_NO_LIB=0 GIT_SVN_NO_OPTIMIZE_COMMITS=0 \
diff --git a/t/lib-git-svn.sh b/t/lib-git-svn.sh
index 29a1e72..63c6703 100644
--- a/t/lib-git-svn.sh
+++ b/t/lib-git-svn.sh
@@ -45,6 +45,6 @@ else
 	svnadmin create "$svnrepo"
 fi
 
-svnrepo="file://$svnrepo/test-git-svn"
+svnrepo="file://$svnrepo"
 
 
diff --git a/t/t9100-git-svn-basic.sh b/t/t9100-git-svn-basic.sh
index 34a3ccd..f9de232 100755
--- a/t/t9100-git-svn-basic.sh
+++ b/t/t9100-git-svn-basic.sh
@@ -228,6 +228,11 @@ tree 56a30b966619b863674f5978696f4a3594f
 tree d667270a1f7b109f5eb3aaea21ede14b56bfdd6e
 tree 8f51f74cf0163afc9ad68a4b1537288c4558b5a4
 EOF
+
+if test -z "$GIT_SVN_NO_LIB" || test "$GIT_SVN_NO_LIB" -eq 0; then
+	echo tree 4b825dc642cb6eb9a060e54bf8d69288fbee4904 >> expected
+fi
+
 test_expect_success "$name" "diff -u a expected"
 
 test_done
diff --git a/t/t9103-git-svn-graft-branches.sh b/t/t9103-git-svn-graft-branches.sh
index cc62d4e..293b98f 100755
--- a/t/t9103-git-svn-graft-branches.sh
+++ b/t/t9103-git-svn-graft-branches.sh
@@ -1,6 +1,8 @@
 test_description='git-svn graft-branches'
 . ./lib-git-svn.sh
 
+svnrepo="$svnrepo/test-git-svn"
+
 test_expect_success 'initialize repo' "
 	mkdir import &&
 	cd import &&
-- 
1.4.4.1.g22a08

[PATCH 1.1/2] git-svn: fix output reporting from the delta fetcher

[Eric Wong]

There was nothing printed in the code originally because I left
out a pair of parentheses.  Nevertheless, the affected code has
been replaced with a more efficient version that respects the -q
flag as well as requiring less bandwidth.

We save some bandwidth by not requesting changed paths
information when calling get_log() since we're using the delta
fetcher.

Signed-off-by: Eric Wong <normalperson@yhbt.net>
---
 git-svn.perl |   31 +++++++++++++------------------
 1 files changed, 13 insertions(+), 18 deletions(-)

diff --git a/git-svn.perl b/git-svn.perl
index 9b86d91..1a675c4 100755
--- a/git-svn.perl
+++ b/git-svn.perl
@@ -2906,7 +2906,7 @@ sub libsvn_log_entry {
 }
 
 sub process_rm {
-	my ($gui, $last_commit, $f) = @_;
+	my ($gui, $last_commit, $f, $q) = @_;
 	# remove entire directories.
 	if (safe_qx('git-ls-tree',$last_commit,'--',$f) =~ /^040000 tree/) {
 		defined(my $pid = open my $ls, '-|') or croak $!;
@@ -2917,10 +2917,13 @@ sub process_rm {
 		local $/ = "\0";
 		while (<$ls>) {
 			print $gui '0 ',0 x 40,"\t",$_ or croak $!;
+			print "\tD\t$_\n" unless $q;
 		}
+		print "\tD\t$f/\n" unless $q;
 		close $ls or croak $?;
 	} else {
 		print $gui '0 ',0 x 40,"\t",$f,"\0" or croak $!;
+		print "\tD\t$f\n" unless $q;
 	}
 }
 
@@ -2931,8 +2934,7 @@ sub libsvn_fetch {
 sub libsvn_fetch_delta {
 	my ($last_commit, $paths, $rev, $author, $date, $msg) = @_;
 	my $pool = SVN::Pool->new;
-	my $ed = SVN::Git::Fetcher->new({ c => $last_commit, ra => $SVN,
-	                                  paths => $paths });
+	my $ed = SVN::Git::Fetcher->new({ c => $last_commit, q => $_q });
 	my $reporter = $SVN->do_update($rev, '', 1, $ed, $pool);
 	my @lock = $SVN::Core::VERSION ge '1.2.0' ? (undef) : ();
 	my (undef, $last_rev, undef) = cmt_metadata($last_commit);
@@ -2956,8 +2958,7 @@ sub libsvn_fetch_full {
 			$f =~ s#^/##;
 		}
 		if ($m =~ /^[DR]$/) {
-			print "\t$m\t$f\n" unless $_q;
-			process_rm($gui, $last_commit, $f);
+			process_rm($gui, $last_commit, $f, $_q);
 			next if $m eq 'D';
 			# 'R' can be file replacements, too, right?
 		}
@@ -3174,6 +3175,7 @@ sub libsvn_find_parent_branch {
 
 sub libsvn_get_log {
 	my ($ra, @args) = @_;
+	$args[4] = 0 if $_xfer_delta && ! $_follow_parent;
 	if ($SVN::Core::VERSION le '1.2.0') {
 		splice(@args, 3, 1);
 	}
@@ -3187,7 +3189,7 @@ sub libsvn_new_tree {
 	my ($paths, $rev, $author, $date, $msg) = @_;
 	if ($_xfer_delta) {
 		my $pool = SVN::Pool->new;
-		my $ed = SVN::Git::Fetcher->new({paths => $paths, ra => $SVN});
+		my $ed = SVN::Git::Fetcher->new({q => $_q});
 		my $reporter = $SVN->do_update($rev, '', 1, $ed, $pool);
 		my @lock = $SVN::Core::VERSION ge '1.2.0' ? (undef) : ();
 		$reporter->set_path('', $rev, 1, @lock, $pool);
@@ -3382,20 +3384,14 @@ sub new {
 	open my $gui, '| git-update-index -z --index-info' or croak $!;
 	$self->{gui} = $gui;
 	$self->{c} = $git_svn->{c} if exists $git_svn->{c};
-	if (my $p = $git_svn->{paths} && $git_svn->{ra}) {
-		my $s = $git_svn->{ra}->{svn_path};
-		$s = length $s ? qr#^/\Q$s\E/# : qr#^/#;
-		$self->{paths} = { map { my $x = $_;
-		                         $x =~ s/$s//;
-		                         $x => $p->{$_} } keys %$p };
-	}
+	$self->{q} = $git_svn->{q};
 	require Digest::MD5;
 	$self;
 }
 
 sub delete_entry {
 	my ($self, $path, $rev, $pb) = @_;
-	process_rm($self->{gui}, $self->{c}, $path);
+	process_rm($self->{gui}, $self->{c}, $path, $self->{q});
 	undef;
 }
 
@@ -3404,13 +3400,13 @@ sub open_file {
 	my ($mode, $blob) = (safe_qx('git-ls-tree',$self->{c},'--',$path)
 	                     =~ /^(\d{6}) blob ([a-f\d]{40})\t/);
 	{ path => $path, mode_a => $mode, mode_b => $mode, blob => $blob,
-	  pool => SVN::Pool->new };
+	  pool => SVN::Pool->new, action => 'M' };
 }
 
 sub add_file {
 	my ($self, $path, $pb, $cp_path, $cp_rev) = @_;
 	{ path => $path, mode_a => 100644, mode_b => 100644,
-	  pool => SVN::Pool->new };
+	  pool => SVN::Pool->new, action => 'A' };
 }
 
 sub change_file_prop {
@@ -3493,8 +3489,7 @@ sub close_file {
 	$fb->{pool}->clear;
 	my $gui = $self->{gui};
 	print $gui "$fb->{mode_b} $hash\t$path\0" or croak $!;
-	print "\t", $self->{paths}->{$path}->action,
-	      "\t$path\n" if defined $self->{paths}->{$path};
+	print "\t$fb->{action}\t$path\n" if $fb->{action} && ! $self->{q};
 	undef;
 }
 
-- 
1.4.4.1.g22a08

[PATCH 1.2/2 (fixed)] git-svn: fix output reporting from the delta fetcher

[Eric Wong]

There was nothing printed in the code originally because I left
out a pair of parentheses.  Nevertheless, the affected code has
been replaced with a more efficient version that respects the -q
flag as well as requiring less bandwidth.

We save some bandwidth by not requesting changed paths
information when calling get_log() since we're using the delta
fetcher.

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

Oops, the previous one (1.1) was broken with the the graft branches test
This one fixes it.

 git-svn.perl |   35 +++++++++++++++--------------------
 1 files changed, 15 insertions(+), 20 deletions(-)

diff --git a/git-svn.perl b/git-svn.perl
index 9b86d91..7942bba 100755
--- a/git-svn.perl
+++ b/git-svn.perl
@@ -1152,7 +1152,7 @@ sub graft_file_copy_lib {
 	while (1) {
 		my $pool = SVN::Pool->new;
 		libsvn_get_log(libsvn_dup_ra($SVN), [$path],
-		               $min, $max, 0, 1, 1,
+		               $min, $max, 0, 2, 1,
 			sub {
 				libsvn_graft_file_copies($grafts, $tree_paths,
 							$path, @_);
@@ -2906,7 +2906,7 @@ sub libsvn_log_entry {
 }
 
 sub process_rm {
-	my ($gui, $last_commit, $f) = @_;
+	my ($gui, $last_commit, $f, $q) = @_;
 	# remove entire directories.
 	if (safe_qx('git-ls-tree',$last_commit,'--',$f) =~ /^040000 tree/) {
 		defined(my $pid = open my $ls, '-|') or croak $!;
@@ -2917,10 +2917,13 @@ sub process_rm {
 		local $/ = "\0";
 		while (<$ls>) {
 			print $gui '0 ',0 x 40,"\t",$_ or croak $!;
+			print "\tD\t$_\n" unless $q;
 		}
+		print "\tD\t$f/\n" unless $q;
 		close $ls or croak $?;
 	} else {
 		print $gui '0 ',0 x 40,"\t",$f,"\0" or croak $!;
+		print "\tD\t$f\n" unless $q;
 	}
 }
 
@@ -2931,8 +2934,7 @@ sub libsvn_fetch {
 sub libsvn_fetch_delta {
 	my ($last_commit, $paths, $rev, $author, $date, $msg) = @_;
 	my $pool = SVN::Pool->new;
-	my $ed = SVN::Git::Fetcher->new({ c => $last_commit, ra => $SVN,
-	                                  paths => $paths });
+	my $ed = SVN::Git::Fetcher->new({ c => $last_commit, q => $_q });
 	my $reporter = $SVN->do_update($rev, '', 1, $ed, $pool);
 	my @lock = $SVN::Core::VERSION ge '1.2.0' ? (undef) : ();
 	my (undef, $last_rev, undef) = cmt_metadata($last_commit);
@@ -2956,8 +2958,7 @@ sub libsvn_fetch_full {
 			$f =~ s#^/##;
 		}
 		if ($m =~ /^[DR]$/) {
-			print "\t$m\t$f\n" unless $_q;
-			process_rm($gui, $last_commit, $f);
+			process_rm($gui, $last_commit, $f, $_q);
 			next if $m eq 'D';
 			# 'R' can be file replacements, too, right?
 		}
@@ -3098,7 +3099,7 @@ sub revisions_eq {
 		# should be OK to use Pool here (r1 - r0) should be small
 		my $pool = SVN::Pool->new;
 		libsvn_get_log($SVN, [$path], $r0, $r1,
-				0, 1, 1, sub {$nr++}, $pool);
+				0, 0, 1, sub {$nr++}, $pool);
 		$pool->clear;
 	} else {
 		my ($url, undef) = repo_path_split($SVN_URL);
@@ -3174,6 +3175,7 @@ sub libsvn_find_parent_branch {
 
 sub libsvn_get_log {
 	my ($ra, @args) = @_;
+	$args[4]-- if $args[4] && $_xfer_delta && ! $_follow_parent;
 	if ($SVN::Core::VERSION le '1.2.0') {
 		splice(@args, 3, 1);
 	}
@@ -3187,7 +3189,7 @@ sub libsvn_new_tree {
 	my ($paths, $rev, $author, $date, $msg) = @_;
 	if ($_xfer_delta) {
 		my $pool = SVN::Pool->new;
-		my $ed = SVN::Git::Fetcher->new({paths => $paths, ra => $SVN});
+		my $ed = SVN::Git::Fetcher->new({q => $_q});
 		my $reporter = $SVN->do_update($rev, '', 1, $ed, $pool);
 		my @lock = $SVN::Core::VERSION ge '1.2.0' ? (undef) : ();
 		$reporter->set_path('', $rev, 1, @lock, $pool);
@@ -3382,20 +3384,14 @@ sub new {
 	open my $gui, '| git-update-index -z --index-info' or croak $!;
 	$self->{gui} = $gui;
 	$self->{c} = $git_svn->{c} if exists $git_svn->{c};
-	if (my $p = $git_svn->{paths} && $git_svn->{ra}) {
-		my $s = $git_svn->{ra}->{svn_path};
-		$s = length $s ? qr#^/\Q$s\E/# : qr#^/#;
-		$self->{paths} = { map { my $x = $_;
-		                         $x =~ s/$s//;
-		                         $x => $p->{$_} } keys %$p };
-	}
+	$self->{q} = $git_svn->{q};
 	require Digest::MD5;
 	$self;
 }
 
 sub delete_entry {
 	my ($self, $path, $rev, $pb) = @_;
-	process_rm($self->{gui}, $self->{c}, $path);
+	process_rm($self->{gui}, $self->{c}, $path, $self->{q});
 	undef;
 }
 
@@ -3404,13 +3400,13 @@ sub open_file {
 	my ($mode, $blob) = (safe_qx('git-ls-tree',$self->{c},'--',$path)
 	                     =~ /^(\d{6}) blob ([a-f\d]{40})\t/);
 	{ path => $path, mode_a => $mode, mode_b => $mode, blob => $blob,
-	  pool => SVN::Pool->new };
+	  pool => SVN::Pool->new, action => 'M' };
 }
 
 sub add_file {
 	my ($self, $path, $pb, $cp_path, $cp_rev) = @_;
 	{ path => $path, mode_a => 100644, mode_b => 100644,
-	  pool => SVN::Pool->new };
+	  pool => SVN::Pool->new, action => 'A' };
 }
 
 sub change_file_prop {
@@ -3493,8 +3489,7 @@ sub close_file {
 	$fb->{pool}->clear;
 	my $gui = $self->{gui};
 	print $gui "$fb->{mode_b} $hash\t$path\0" or croak $!;
-	print "\t", $self->{paths}->{$path}->action,
-	      "\t$path\n" if defined $self->{paths}->{$path};
+	print "\t$fb->{action}\t$path\n" if $fb->{action} && ! $self->{q};
 	undef;
 }
 
-- 
1.4.4.1.g22a08

Re: [PATCH 1.2/2 (fixed)] git-svn: fix output reporting from the delta fetcher

[Pazu]

I've tried applying the patch, and running init'ing a new git-svn
repository for my project. The initial commit was fetched OK (and very
fast!), but now I get the following error trying to fetch any later
revision:

Error from SVN, (200003): Incomplete data: Delta source ended unexpectedly
 at /opt/local/lib/perl5/vendor_perl/5.8.8/darwin-2level/SVN/Ra.pm line 157
512 at /Users/pazu/bin/git-svn line 448
        main::fetch_lib() called at /Users/pazu/bin/git-svn line 319
        main::fetch() called at /Users/pazu/bin/git-svn line 178

I'm on Mac OS X 10.4.8 (Intel), with perl 5.8.8 compiled from
macports, and SVN perl bindings from subversion 1.4.0, also compiled
from macports. git was built from 'master' plus your patches.

Re: [PATCH 1.2/2 (fixed)] git-svn: fix output reporting from the delta fetcher

[Pazu]

On 11/28/06, Pazu <pazu@pazu.com.br> wrote:

> Error from SVN, (200003): Incomplete data: Delta source ended unexpectedly
>  at /opt/local/lib/perl5/vendor_perl/5.8.8/darwin-2level/SVN/Ra.pm line 157
> 512 at /Users/pazu/bin/git-svn line 448
>         main::fetch_lib() called at /Users/pazu/bin/git-svn line 319
>         main::fetch() called at /Users/pazu/bin/git-svn line 178

I've added some debug statements to git-svn and found more
information. Apparently, this error is happening because
SVN::Git::Fetcher is trying to open a file that hasn't been fetched by
git-svn yet. Now, *why* this is happening, I don't know. Maybe
something happened while fetching the initial revision.

Here's the output from my "debug" run:

Creating fetcher for revision 9785a6fc2d61a6f9b702bb8e2dd81b11415e6e92
Starting update on revision 11159
Opening CamelEar/config/glconfig.xml@11143
Error from SVN, (200003): Incomplete data: Delta source ended unexpectedly

Here, SVN::Git::Fetcher received a request to open
CamelEar/config/glconfig.xml, but:

mini:~/devel/camel-git pazu$ git-ls-tree
9785a6fc2d61a6f9b702bb8e2dd81b11415e6e92
040000 tree 9a02a43ec34e88d4cee64d322354a49c8f6063e2    BungeIntegrationService
040000 tree 6e0bc09356f480f8b6ec782420c4af322ed3bc0a
BungeIntegrationServiceClient
040000 tree 5da3e715525c9b436fdf9a9dfb4de2bffd8e5ff3    CamelService
040000 tree a2d36b9cc469a52aea4f2422ffab30b9a1ee67de    CamelServiceClient
040000 tree 3d550fad4dcf09a8d45c5e5808dac4883854872c    CamelServiceTests
040000 tree 1fcd2416b4c8494c066abc38051db5244ee801cd    CamelWeb
040000 tree d2e30726674b22e06b4ec07aa68b49f925609c85    Reports
040000 tree 007c4ded31cf16418a7fb0b2ffbe2b796e7ce11e    Servers
100644 blob da2d34be7d44fff5cf70702edc61fd8b6057a14a    _root.iml
100644 blob 6219148365613fd6195b8558376455c3379bbd6e    bg-cam.ipr
100644 blob 25d3fbab0971940cf2cdbe399fd2b79cb21de1b5    build.xml
100644 blob abc91fc1b268a3610a927125b88c1ad05c095d95    checkout-svn-build.sh
100644 blob 475a5dc68c8e0eaaac3a182c078c04b947825469    cvs-checkout.xml

Notice that there's no "CamelEar" directory. For some reason, it
wasn't fetched in the initial revision. Now, just to make sure this
isn't svn fault:

mini:~/devel/camel-git pazu$ svn ls -r11143
https://tech.bga.bunge.com/BungeHomeExt/GLS/trunk/java/bg-cam
.cvsignore
BungeIntegrationEar/
BungeIntegrationService/
BungeIntegrationServiceClient/
CamelEar/
CamelService/
CamelServiceClient/
CamelServiceTests/
CamelWeb/
Reports/
Servers/
Servers_W6/
_root.iml
bg-cam.ipr
build.ws.properties.example
build.xml
checkout-build.sh
checkout-svn-build.sh
cvs-checkout.xml
svn-checkout.xml

So yeah, CamelEar was supposed to exist in this revision, but for some
reason, git-svn missed it. I'll keep trying to find why that happened
-- my bets are on a broken network connection during the initial
fetch, with git-svn accepting what it got so far as the initial
revision, instead of reporting the broken connection.

Re: [PATCH 1.2/2 (fixed)] git-svn: fix output reporting from the delta fetcher

[Seth Falcon]

Pazu <pazu@pazu.com.br> writes:
> Notice that there's no "CamelEar" directory. For some reason, it
> wasn't fetched in the initial revision. Now, just to make sure this
> isn't svn fault:
>
> mini:~/devel/camel-git pazu$ svn ls -r11143
> https://tech.bga.bunge.com/BungeHomeExt/GLS/trunk/java/bg-cam
> .cvsignore
> BungeIntegrationEar/
> BungeIntegrationService/
> BungeIntegrationServiceClient/
> CamelEar/

Is CamelEar an empty directory (or was it an empty directory in the
first fetch) by any chance?

I think that presently git-svn does not create empty dirs when pulling
from svn.  It would be nice to have such directories created since
some projects will expect the empty dir to be there (no need to track
it in git, IMO).

Re: [PATCH 1.2/2 (fixed)] git-svn: fix output reporting from the delta fetcher

[Pazu]

On 11/28/06, Seth Falcon <sethfalcon@gmail.com> wrote:

> Is CamelEar an empty directory (or was it an empty directory in the
> first fetch) by any chance?

No, neither. The directory isn't empty on the requested revision, nor
in the initial revision. Anyways…

I've tried to init and fetch the repository again, and guess what, it
worked. So my bet that this was caused by a broken connection during
the initial fetch was probably right -- I'll try to "force" a broken
connection and see what happens…

Hmmm.…

mini:/tmp pazu$ git-svn init
https://tech.bga.bunge.com/BungeHomeExt/GLS/trunk/java/bg-cam
mini:/tmp pazu$ git-svn fetch
        A       bg-cam.ipr
        A       CamelWeb/CamelWeb.iml
        A       CamelWeb/.classpath
        A       CamelWeb/JavaSource/.emptyDir
        A       CamelWeb/.serverPreference
        A       CamelWeb/.website-config
        ...
        <<Here I turned AirPort off>>
Committing initial tree d14cd2aca9a6f15fdc8875212776c6a7cd111341
r8618 = 5a994a730da8dc8141fd116100a773061b7d4212
Creating fetcher for revision 5a994a730da8dc8141fd116100a773061b7d4212
Starting update on revision 9236
Opening CamelService/ejbModule/com/bunge/logistics/elevator/service/util/ElevatorConstants.java@8618
Error from SVN, (200003): Incomplete data: Delta source ended unexpectedly

When I deactivated AirPort, git-svn hang for a long while, but it
finally timed out and just saved what it got so far as the initial
revision. Then when git-svn tried to fetch the next revision, it ended
up trying to open a file that wasn't checked out during the (broken)
initial fetch.

Well, it seems I've found the problem. Unfortunately I have no idea
how to fix that… I'll read some subversion books and source code to
figure that out.

Re: [PATCH 1.2/2 (fixed)] git-svn: fix output reporting from the delta fetcher

[Eric Wong]

Seth Falcon <sethfalcon@gmail.com> wrote:
> Pazu <pazu@pazu.com.br> writes:
> > Notice that there's no "CamelEar" directory. For some reason, it
> > wasn't fetched in the initial revision. Now, just to make sure this
> > isn't svn fault:
> >
> > mini:~/devel/camel-git pazu$ svn ls -r11143
> > https://tech.bga.bunge.com/BungeHomeExt/GLS/trunk/java/bg-cam
> > .cvsignore
> > BungeIntegrationEar/
> > BungeIntegrationService/
> > BungeIntegrationServiceClient/
> > CamelEar/
> 
> Is CamelEar an empty directory (or was it an empty directory in the
> first fetch) by any chance?
> 
> I think that presently git-svn does not create empty dirs when pulling
> from svn.  It would be nice to have such directories created since
> some projects will expect the empty dir to be there (no need to track
> it in git, IMO).

Git itself cannot easily track empty directories (at least as far as
update-index and checkout) goes.

What I *can* do is run mktree and to force the creation of tree objects
with a 4b825dc642cb6eb9a060e54bf8d69288fbee4904 (empty) sub tree and run
commit-tree on it, but checkout/checkout-index would still need to be
modified to support it.

Is that something the git community wants?

-- 

Re: [PATCH 1.2/2 (fixed)] git-svn: fix output reporting from the delta fetcher

[Pazu]

On 11/28/06, Eric Wong <normalperson@yhbt.net> wrote:

> Git itself cannot easily track empty directories (at least as far as
> update-index and checkout) goes.
> [...]
> Is that something the git community wants?

No, I guess not. I detailed the real problem in my previous message,
and it had nothing to do with empty directories, but with git-svn
recording broken revisions from svn. Did you get it, or Trogdor ate my
email?

Re: [PATCH 1.2/2 (fixed)] git-svn: fix output reporting from the delta fetcher

[Eric Wong]

Pazu <pazu@pazu.com.br> wrote:
> On 11/28/06, Eric Wong <normalperson@yhbt.net> wrote:
> 
> >Git itself cannot easily track empty directories (at least as far as
> >update-index and checkout) goes.
> >[...]
> >Is that something the git community wants?
> 
> No, I guess not. I detailed the real problem in my previous message,
> and it had nothing to do with empty directories, but with git-svn
> recording broken revisions from svn. Did you get it, or Trogdor ate my
> email?

Oops, I didn't notice the part about git-svn continuing despite a failed
connection.  Thanks for poking me again.
I'll look into how/if abort_edit/close_edit is called and how to deal
with a failed network connection.

-- 

git-svn and empty directories in svn (was: [PATCH 1.2/2 (fixed)] git-svn: fix output reporting from the delta fetcher)

[Seth Falcon]

Hi Eric, and list,

Eric Wong <normalperson@yhbt.net> writes:
> Seth Falcon <sethfalcon@gmail.com> wrote:
>> I think that presently git-svn does not create empty dirs when pulling
>> from svn.  It would be nice to have such directories created since
>> some projects will expect the empty dir to be there (no need to track
>> it in git, IMO).
>
> Git itself cannot easily track empty directories (at least as far as
> update-index and checkout) goes.
>
> What I *can* do is run mktree and to force the creation of tree objects
> with a 4b825dc642cb6eb9a060e54bf8d69288fbee4904 (empty) sub tree and run
> commit-tree on it, but checkout/checkout-index would still need to be
> modified to support it.
>
> Is that something the git community wants?

I recently encountered a situation where code wasn't working for me
because git-svn didn't create an empty dir that is present in svn.

I'm not trying to argue for the sense/anti-sense of tracking empty
dirs in an scm, but I think this is an issue worth addressing in some
fashion.  Here's why.

I think there are many potential git users out there who are currently
svn users.  And git-svn is a really nice way to get started, but this
sort of stumbling block could really turn people off.  For example, it
made me look pretty dumb when I carelessly complained to my colleague
about his code not working and then it turns out to be because my
super-advanced scm tool "messed things up".

One simple thing (I think it would be simple) is that git-svn could
issue a loud warning when it encounters an empty directory that it is
going to ignore.

I don't understand the implications adding the tracking of empty dirs
to git.  I suspect it has been discussed before, but haven't yet gone
fishing in the list archives.  I imagine it would make the argument
easier for folks wanting to switch a project from svn to git if this
wasn't one of the differences.  For good or bad, I've often heard this
svn feature as a motivator to switch from cvs.


+ seth

Re: git-svn and empty directories in svn (was: [PATCH 1.2/2 (fixed)] git-svn: fix output reporting from the delta fetcher)

[Eric Wong]

Seth Falcon <sethfalcon@gmail.com> wrote:
> Hi Eric, and list,
> 
> Eric Wong <normalperson@yhbt.net> writes:
> > Seth Falcon <sethfalcon@gmail.com> wrote:
> >> I think that presently git-svn does not create empty dirs when pulling
> >> from svn.  It would be nice to have such directories created since
> >> some projects will expect the empty dir to be there (no need to track
> >> it in git, IMO).
> >
> > Git itself cannot easily track empty directories (at least as far as
> > update-index and checkout) goes.
> >
> > What I *can* do is run mktree and to force the creation of tree objects
> > with a 4b825dc642cb6eb9a060e54bf8d69288fbee4904 (empty) sub tree and run
> > commit-tree on it, but checkout/checkout-index would still need to be
> > modified to support it.
> >
> > Is that something the git community wants?
> 
> I recently encountered a situation where code wasn't working for me
> because git-svn didn't create an empty dir that is present in svn.
> 
> I'm not trying to argue for the sense/anti-sense of tracking empty
> dirs in an scm, but I think this is an issue worth addressing in some
> fashion.  Here's why.
> 
> I think there are many potential git users out there who are currently
> svn users.  And git-svn is a really nice way to get started, but this
> sort of stumbling block could really turn people off.  For example, it
> made me look pretty dumb when I carelessly complained to my colleague
> about his code not working and then it turns out to be because my
> super-advanced scm tool "messed things up".
> 
> One simple thing (I think it would be simple) is that git-svn could
> issue a loud warning when it encounters an empty directory that it is
> going to ignore.
> 
> I don't understand the implications adding the tracking of empty dirs
> to git.  I suspect it has been discussed before, but haven't yet gone
> fishing in the list archives.  I imagine it would make the argument
> easier for folks wanting to switch a project from svn to git if this
> wasn't one of the differences.  For good or bad, I've often heard this
> svn feature as a motivator to switch from cvs.

I agree that missing empty directories when tracking foreign SVN repos
is annoying.  I've looked into this a bit more; but ended up
trying to reinvent the index :x

Since git-svn misses some other stuff (many property settings,
externals) I'll be working on an internal logging format that can help
track those things.  It'd be nice to have a command like git svn
checkout which works like git checkout; but empty directories are
created.

-- 

Re: git-svn and empty directories in svn

[Steven Grimm]

Eric Wong wrote:
> Since git-svn misses some other stuff (many property settings,
> externals) I'll be working on an internal logging format that can help
> track those things.  It'd be nice to have a command like git svn
> checkout which works like git checkout; but empty directories are
> created.

Presumably once the submodule support is worked out, svn externals could 
be represented as git-svn-managed submodules, yes?

In fact, I'd go so far as to say it should be a design goal of the 
submodule support: you should be able to indicate somehow that a 
submodule is a git clone of some non-git resource, and anything that 
iterates through the submodules (e.g. to freshen them from their 
respective origins) should know how to run git-svn or whatever so it's 
all seamlessly integrated. I suppose that's a special case of making 
git-svn and friends more tightly integrated with git in general; if the 
git "push" and "fetch" commands know to run git-svn instead of talking 
to a remote git repository, then it might Just Work for submodules.

Independent of the supermodule being managed by git-svn, the "my 
software depends on externally-managed code" problem that submodules are 
attempting to address would be solved a lot more comprehensively if the 
remote code base could be an svn repository and git knew enough to run 
git-svn as appropriate to keep it fresh. (Not just svn, of course; any 
foreign CM system that has an equivalent of git-svn should work.) It'd 
be pretty cool to have a supermodule that tied together some 
native-git-managed code, a couple of external svn repositories, and a 
CVS tree or two, all under a single umbrella with the details 
automatically taken care of by default.

Re: git-svn and empty directories in svn

[Eric Wong]

Steven Grimm <koreth@midwinter.com> wrote:
> Eric Wong wrote:
> >Since git-svn misses some other stuff (many property settings,
> >externals) I'll be working on an internal logging format that can help
> >track those things.  It'd be nice to have a command like git svn
> >checkout which works like git checkout; but empty directories are
> >created.
> 
> Presumably once the submodule support is worked out, svn externals could 
> be represented as git-svn-managed submodules, yes?

Yes, that is the plan.  Better integration with native git remotes is
very much on the roadmap for git-svn.  I'll have to work on cleaning up
how multi-fetch works internally, however.

--