[PATCH 0/8] Speed up git-svn

[PATCH 0/8] Speed up git-svn

[Michele Ballabio]

This is a respin of the patches by Adam Roben that follow
the ones that are now in "pu".


Adam Roben (6):
  Move git-hash-object tests from t5303 to t1007
  Add more tests for git hash-object
  git-hash-object: Add --stdin-paths option
  Git.pm: Add command_bidi_pipe and command_close_bidi_pipe
  Git.pm: Add hash_and_insert_object and cat_blob
  git-svn: Speed up fetch

These patches have the changes suggested in the old thread to
remove some warnings and fix the tests, plus formatting changes in
the tests. The patches on Git.pm and git-svn.perl are the same as
before, patches 2 and 3 are the ones I have touched.


Michele Ballabio (2):
  builtin-cat-file.c: use parse_options()
  change quoting in test t1006-cat-file.sh

These are independent changes, but are always to be applied on top
of what's in "pu" (on top of ar/batch-cat, actually). The first
tries to simplify the code about the option parsing. The second
one is only for readability.


 Documentation/git-hash-object.txt |    5 +-
 builtin-cat-file.c                |  119 ++++++++-------------
 git-svn.perl                      |   42 ++++----
 hash-object.c                     |   45 ++++++++-
 perl/Git.pm                       |  208 ++++++++++++++++++++++++++++++++++++-
 t/t1006-cat-file.sh               |    6 +-
 t/t1007-hash-object.sh            |  133 +++++++++++++++++++++++
 t/t5303-hash-object.sh            |   35 ------
 8 files changed, 456 insertions(+), 137 deletions(-)
 create mode 100755 t/t1007-hash-object.sh
 delete mode 100755 t/t5303-hash-object.sh

[PATCH 1/8] Move git-hash-object tests from t5303 to t1007

[Michele Ballabio]

From: Adam Roben <aroben@apple.com>

This is a more appropriate location according to t/README.

Signed-off-by: Adam Roben <aroben@apple.com>
---
 t/{t5303-hash-object.sh => t1007-hash-object.sh} |    0 
 1 files changed, 0 insertions(+), 0 deletions(-)
 rename t/{t5303-hash-object.sh => t1007-hash-object.sh} (100%)

diff --git a/t/t5303-hash-object.sh b/t/t1007-hash-object.sh
similarity index 100%
rename from t/t5303-hash-object.sh
rename to t/t1007-hash-object.sh
-- 
1.5.5.1

[PATCH 2/8] Add more tests for git hash-object

[Michele Ballabio]

From: Adam Roben <aroben@apple.com>

Signed-off-by: Adam Roben <aroben@apple.com>
---
 t/t1007-hash-object.sh |  120 +++++++++++++++++++++++++++++++++++++-----------
 1 files changed, 93 insertions(+), 27 deletions(-)

diff --git a/t/t1007-hash-object.sh b/t/t1007-hash-object.sh
index 543c078..2019ea7 100755
--- a/t/t1007-hash-object.sh
+++ b/t/t1007-hash-object.sh
@@ -4,32 +4,98 @@ test_description=git-hash-object
 
 . ./test-lib.sh
 
-test_expect_success \
-    'git hash-object -w --stdin saves the object' \
-    'obname=$(echo foo | git hash-object -w --stdin) &&
-    obpath=$(echo $obname | sed -e "s/\(..\)/\1\//") &&
-    test -r .git/objects/"$obpath" &&
-    rm -f .git/objects/"$obpath"'
-    
-test_expect_success \
-    'git hash-object --stdin -w saves the object' \
-    'obname=$(echo foo | git hash-object --stdin -w) &&
-    obpath=$(echo $obname | sed -e "s/\(..\)/\1\//") &&
-    test -r .git/objects/"$obpath" &&
-    rm -f .git/objects/"$obpath"'    
-
-test_expect_success \
-    'git hash-object --stdin file1 <file0 first operates on file0, then file1' \
-    'echo foo > file1 &&
-    obname0=$(echo bar | git hash-object --stdin) &&
-    obname1=$(git hash-object file1) &&
-    obname0new=$(echo bar | git hash-object --stdin file1 | sed -n -e 1p) &&
-    obname1new=$(echo bar | git hash-object --stdin file1 | sed -n -e 2p) &&
-    test "$obname0" = "$obname0new" &&
-    test "$obname1" = "$obname1new"'
-
-test_expect_success \
-    'git hash-object refuses multiple --stdin arguments' \
-    '! git hash-object --stdin --stdin < file1'
+echo_without_newline() {
+	printf '%s' "$*"
+}
+
+test_blob_does_not_exist() {
+	test_expect_success 'blob does not exist in database' "
+		test_must_fail git cat-file blob $1
+	"
+}
+
+test_blob_exists() {
+	test_expect_success 'blob exists in database' "
+		git cat-file blob $1
+	"
+}
+
+hello_content="Hello World"
+hello_sha1=5e1c309dae7f45e0f39b1bf3ac3cd9db12e7d689
+
+example_content="This is an example"
+example_sha1=ddd3f836d3e3fbb7ae289aa9ae83536f76956399
+
+setup_repo() {
+	echo_without_newline "$hello_content" > hello
+	echo_without_newline "$example_content" > example
+}
+
+test_repo=test
+push_repo() {
+	test_create_repo $test_repo
+	cd $test_repo
+
+	setup_repo
+}
+
+pop_repo() {
+	cd ..
+	rm -rf $test_repo
+}
+
+setup_repo
+
+# Argument checking
+
+test_expect_success "multiple '--stdin's are rejected" '
+	test_must_fail git hash-object --stdin --stdin < example
+'
+
+# Behavior
+
+push_repo
+
+test_expect_success 'hash a file' '
+	test $hello_sha1 = $(git hash-object hello)
+'
+
+test_blob_does_not_exist $hello_sha1
+
+test_expect_success 'hash from stdin' '
+	test $example_sha1 = $(git hash-object --stdin < example)
+'
+
+test_blob_does_not_exist $example_sha1
+
+test_expect_success 'hash a file and write to database' '
+	test $hello_sha1 = $(git hash-object -w hello)
+'
+
+test_blob_exists $hello_sha1
+
+test_expect_success 'git hash-object --stdin file1 <file0 first operates on file0, then file1' '
+	echo foo > file1 &&
+	obname0=$(echo bar | git hash-object --stdin) &&
+	obname1=$(git hash-object file1) &&
+	obname0new=$(echo bar | git hash-object --stdin file1 | sed -n -e 1p) &&
+	obname1new=$(echo bar | git hash-object --stdin file1 | sed -n -e 2p) &&
+	test "$obname0" = "$obname0new" &&
+	test "$obname1" = "$obname1new"
+'
+
+pop_repo
+
+for args in "-w --stdin" "--stdin -w"; do
+	push_repo
+
+	test_expect_success "hash from stdin and write to database ($args)" '
+		test $example_sha1 = $(git hash-object $args < example)
+	'
+
+	test_blob_exists $example_sha1
+
+	pop_repo
+done
 
 test_done
-- 
1.5.5.1

[PATCH 3/8] git-hash-object: Add --stdin-paths option

[Michele Ballabio]

From: Adam Roben <aroben@apple.com>

This allows multiple paths to be specified on stdin.

Signed-off-by: Adam Roben <aroben@apple.com>
---
 Documentation/git-hash-object.txt |    5 +++-
 hash-object.c                     |   45 ++++++++++++++++++++++++++++++++++++-
 t/t1007-hash-object.sh            |   32 ++++++++++++++++++++++++++
 3 files changed, 80 insertions(+), 2 deletions(-)

diff --git a/Documentation/git-hash-object.txt b/Documentation/git-hash-object.txt
index 33030c0..99a2143 100644
--- a/Documentation/git-hash-object.txt
+++ b/Documentation/git-hash-object.txt
@@ -8,7 +8,7 @@ git-hash-object - Compute object ID and optionally creates a blob from a file
 
 SYNOPSIS
 --------
-'git-hash-object' [-t <type>] [-w] [--stdin] [--] <file>...
+'git-hash-object' [-t <type>] [-w] [--stdin | --stdin-paths] [--] <file>...
 
 DESCRIPTION
 -----------
@@ -32,6 +32,9 @@ OPTIONS
 --stdin::
 	Read the object from standard input instead of from a file.
 
+--stdin-paths::
+	Read file names from stdin instead of from the command-line.
+
 Author
 ------
 Written by Junio C Hamano <junkio@cox.net>
diff --git a/hash-object.c b/hash-object.c
index 61e7160..0a7ac2f 100644
--- a/hash-object.c
+++ b/hash-object.c
@@ -6,6 +6,7 @@
  */
 #include "cache.h"
 #include "blob.h"
+#include "quote.h"
 
 static void hash_object(const char *path, enum object_type type, int write_object)
 {
@@ -20,6 +21,7 @@ static void hash_object(const char *path, enum object_type type, int write_objec
 		    ? "Unable to add %s to database"
 		    : "Unable to hash %s", path);
 	printf("%s\n", sha1_to_hex(sha1));
+	maybe_flush_or_die(stdout, "hash to stdout");
 }
 
 static void hash_stdin(const char *type, int write_object)
@@ -30,8 +32,27 @@ static void hash_stdin(const char *type, int write_object)
 	printf("%s\n", sha1_to_hex(sha1));
 }
 
+static void hash_stdin_paths(const char *type, int write_objects)
+{
+	struct strbuf buf, nbuf;
+
+	strbuf_init(&buf, 0);
+	strbuf_init(&nbuf, 0);
+	while (strbuf_getline(&buf, stdin, '\n') != EOF) {
+		if (buf.buf[0] == '"') {
+			strbuf_reset(&nbuf);
+			if (unquote_c_style(&nbuf, buf.buf, NULL))
+				die("line is badly quoted");
+			strbuf_swap(&buf, &nbuf);
+		}
+		hash_object(buf.buf, type_from_string(type), write_objects);
+	}
+	strbuf_release(&buf);
+	strbuf_release(&nbuf);
+}
+
 static const char hash_object_usage[] =
-"git-hash-object [-t <type>] [-w] [--stdin] <file>...";
+"git-hash-object [ [-t <type>] [-w] [--stdin] <file>... | --stdin-paths < <list-of-paths> ]";
 
 int main(int argc, char **argv)
 {
@@ -42,6 +63,7 @@ int main(int argc, char **argv)
 	int prefix_length = -1;
 	int no_more_flags = 0;
 	int hashstdin = 0;
+	int stdin_paths = 0;
 
 	git_config(git_default_config);
 
@@ -65,7 +87,19 @@ int main(int argc, char **argv)
 			}
 			else if (!strcmp(argv[i], "--help"))
 				usage(hash_object_usage);
+			else if (!strcmp(argv[i], "--stdin-paths")) {
+				if (hashstdin) {
+					error("Can't use --stdin-paths with --stdin");
+					usage(hash_object_usage);
+				}
+				stdin_paths = 1;
+
+			}
 			else if (!strcmp(argv[i], "--stdin")) {
+				if (stdin_paths) {
+					error("Can't use %s with --stdin-paths", argv[i]);
+					usage(hash_object_usage);
+				}
 				if (hashstdin)
 					die("Multiple --stdin arguments are not supported");
 				hashstdin = 1;
@@ -76,6 +110,11 @@ int main(int argc, char **argv)
 		else {
 			const char *arg = argv[i];
 
+			if (stdin_paths) {
+				error("Can't specify files (such as \"%s\") with --stdin-paths", arg);
+				usage(hash_object_usage);
+			}
+
 			if (hashstdin) {
 				hash_stdin(type, write_object);
 				hashstdin = 0;
@@ -87,6 +126,10 @@ int main(int argc, char **argv)
 			no_more_flags = 1;
 		}
 	}
+
+	if (stdin_paths)
+		hash_stdin_paths(type, write_object);
+
 	if (hashstdin)
 		hash_stdin(type, write_object);
 	return 0;
diff --git a/t/t1007-hash-object.sh b/t/t1007-hash-object.sh
index 2019ea7..0526295 100755
--- a/t/t1007-hash-object.sh
+++ b/t/t1007-hash-object.sh
@@ -52,6 +52,15 @@ test_expect_success "multiple '--stdin's are rejected" '
 	test_must_fail git hash-object --stdin --stdin < example
 '
 
+test_expect_success "Can't use --stdin and --stdin-paths together" '
+	test_must_fail git hash-object --stdin --stdin-paths &&
+	test_must_fail git hash-object --stdin-paths --stdin
+'
+
+test_expect_success "Can't pass filenames as arguments with --stdin-paths" '
+	test_must_fail git hash-object --stdin-paths hello < example
+'
+
 # Behavior
 
 push_repo
@@ -98,4 +107,27 @@ for args in "-w --stdin" "--stdin -w"; do
 	pop_repo
 done
 
+filenames="hello
+example"
+
+sha1s="$hello_sha1
+$example_sha1"
+
+test_expect_success "hash two files with names on stdin" '
+	test "$sha1s" = "$(echo_without_newline "$filenames" | git hash-object --stdin-paths)"
+'
+
+for args in "-w --stdin-paths" "--stdin-paths -w"; do
+	push_repo
+
+	test_expect_success "hash two files with names on stdin and write to database ($args)" '
+		test "$sha1s" = "$(echo_without_newline "$filenames" | git hash-object $args)"
+	'
+
+	test_blob_exists $hello_sha1
+	test_blob_exists $example_sha1
+
+	pop_repo
+done
+
 test_done
-- 
1.5.5.1

[PATCH 4/8] Git.pm: Add command_bidi_pipe and command_close_bidi_pipe

[Michele Ballabio]

From: Adam Roben <aroben@apple.com>

command_bidi_pipe hands back the stdin and stdout file handles from the
executed command. command_close_bidi_pipe closes these handles and terminates
the process.

Signed-off-by: Adam Roben <aroben@apple.com>
---
 perl/Git.pm |   56 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 56 insertions(+), 0 deletions(-)

diff --git a/perl/Git.pm b/perl/Git.pm
index 2e7f896..d766974 100644
--- a/perl/Git.pm
+++ b/perl/Git.pm
@@ -51,6 +51,7 @@ require Exporter;
 # Methods which can be called as standalone functions as well:
 @EXPORT_OK = qw(command command_oneline command_noisy
                 command_output_pipe command_input_pipe command_close_pipe
+                command_bidi_pipe command_close_bidi_pipe
                 version exec_path hash_object git_cmd_try);
 
 
@@ -92,6 +93,7 @@ increate nonwithstanding).
 use Carp qw(carp croak); # but croak is bad - throw instead
 use Error qw(:try);
 use Cwd qw(abs_path);
+use IPC::Open2 qw(open2);
 
 }
 
@@ -375,6 +377,60 @@ sub command_close_pipe {
 	_cmd_close($fh, $ctx);
 }
 
+=item command_bidi_pipe ( COMMAND [, ARGUMENTS... ] )
+
+Execute the given C<COMMAND> in the same way as command_output_pipe()
+does but return both an input pipe filehandle and an output pipe filehandle.
+
+The function will return return C<($pid, $pipe_in, $pipe_out, $ctx)>.
+See C<command_close_bidi_pipe()> for details.
+
+=cut
+
+sub command_bidi_pipe {
+	my ($pid, $in, $out);
+	$pid = open2($in, $out, 'git', @_);
+	return ($pid, $in, $out, join(' ', @_));
+}
+
+=item command_close_bidi_pipe ( PID, PIPE_IN, PIPE_OUT [, CTX] )
+
+Close the C<PIPE_IN> and C<PIPE_OUT> as returned from C<command_bidi_pipe()>,
+checking whether the command finished successfully. The optional C<CTX>
+argument is required if you want to see the command name in the error message,
+and it is the fourth value returned by C<command_bidi_pipe()>.  The call idiom
+is:
+
+	my ($pid, $in, $out, $ctx) = $r->command_bidi_pipe('cat-file --batch-check');
+	print "000000000\n" $out;
+	while (<$in>) { ... }
+	$r->command_close_bidi_pipe($pid, $in, $out, $ctx);
+
+Note that you should not rely on whatever actually is in C<CTX>;
+currently it is simply the command name but in future the context might
+have more complicated structure.
+
+=cut
+
+sub command_close_bidi_pipe {
+	my ($pid, $in, $out, $ctx) = @_;
+	foreach my $fh ($in, $out) {
+		unless (close $fh) {
+			if ($!) {
+				carp "error closing pipe: $!";
+			} elsif ($? >> 8) {
+				throw Git::Error::Command($ctx, $? >>8);
+			}
+		}
+	}
+
+	waitpid $pid, 0;
+
+	if ($? >> 8) {
+		throw Git::Error::Command($ctx, $? >>8);
+	}
+}
+
 
 =item command_noisy ( COMMAND [, ARGUMENTS... ] )
 
-- 
1.5.5.1

[PATCH 5/8] Git.pm: Add hash_and_insert_object and cat_blob

[Michele Ballabio]

From: Adam Roben <aroben@apple.com>

These functions are more efficient ways of executing `git hash-object -w` and
`git cat-file blob` when you are dealing with many files/objects.

Signed-off-by: Adam Roben <aroben@apple.com>
---
 perl/Git.pm |  152 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
 1 files changed, 150 insertions(+), 2 deletions(-)

diff --git a/perl/Git.pm b/perl/Git.pm
index d766974..6ba8ee5 100644
--- a/perl/Git.pm
+++ b/perl/Git.pm
@@ -39,6 +39,10 @@ $VERSION = '0.01';
   my $lastrev = $repo->command_oneline( [ 'rev-list', '--all' ],
                                         STDERR => 0 );
 
+  my $sha1 = $repo->hash_and_insert_object('file.txt');
+  my $tempfile = tempfile();
+  my $size = $repo->cat_blob($sha1, $tempfile);
+
 =cut
 
 
@@ -218,7 +222,6 @@ sub repository {
 	bless $self, $class;
 }
 
-
 =back
 
 =head1 METHODS
@@ -734,6 +737,147 @@ sub hash_object {
 }
 
 
+=item hash_and_insert_object ( FILENAME )
+
+Compute the SHA1 object id of the given C<FILENAME> and add the object to the
+object database.
+
+The function returns the SHA1 hash.
+
+=cut
+
+# TODO: Support for passing FILEHANDLE instead of FILENAME
+sub hash_and_insert_object {
+	my ($self, $filename) = @_;
+
+	carp "Bad filename \"$filename\"" if $filename =~ /[\r\n]/;
+
+	$self->_open_hash_and_insert_object_if_needed();
+	my ($in, $out) = ($self->{hash_object_in}, $self->{hash_object_out});
+
+	unless (print $out $filename, "\n") {
+		$self->_close_hash_and_insert_object();
+		throw Error::Simple("out pipe went bad");
+	}
+
+	chomp(my $hash = <$in>);
+	unless (defined($hash)) {
+		$self->_close_hash_and_insert_object();
+		throw Error::Simple("in pipe went bad");
+	}
+
+	return $hash;
+}
+
+sub _open_hash_and_insert_object_if_needed {
+	my ($self) = @_;
+
+	return if defined($self->{hash_object_pid});
+
+	($self->{hash_object_pid}, $self->{hash_object_in},
+	 $self->{hash_object_out}, $self->{hash_object_ctx}) =
+		command_bidi_pipe(qw(hash-object -w --stdin-paths));
+}
+
+sub _close_hash_and_insert_object {
+	my ($self) = @_;
+
+	return unless defined($self->{hash_object_pid});
+
+	my @vars = map { 'hash_object_' . $_ } qw(pid in out ctx);
+
+	command_close_bidi_pipe($self->{@vars});
+	delete $self->{@vars};
+}
+
+=item cat_blob ( SHA1, FILEHANDLE )
+
+Prints the contents of the blob identified by C<SHA1> to C<FILEHANDLE> and
+returns the number of bytes printed.
+
+=cut
+
+sub cat_blob {
+	my ($self, $sha1, $fh) = @_;
+
+	$self->_open_cat_blob_if_needed();
+	my ($in, $out) = ($self->{cat_blob_in}, $self->{cat_blob_out});
+
+	unless (print $out $sha1, "\n") {
+		$self->_close_cat_blob();
+		throw Error::Simple("out pipe went bad");
+	}
+
+	my $description = <$in>;
+	if ($description =~ / missing$/) {
+		carp "$sha1 doesn't exist in the repository";
+		return 0;
+	}
+
+	if ($description !~ /^[0-9a-fA-F]{40} \S+ (\d+)$/) {
+		carp "Unexpected result returned from git cat-file";
+		return 0;
+	}
+
+	my $size = $1;
+
+	my $blob;
+	my $bytesRead = 0;
+
+	while (1) {
+		my $bytesLeft = $size - $bytesRead;
+		last unless $bytesLeft;
+
+		my $bytesToRead = $bytesLeft < 1024 ? $bytesLeft : 1024;
+		my $read = read($in, $blob, $bytesToRead, $bytesRead);
+		unless (defined($read)) {
+			$self->_close_cat_blob();
+			throw Error::Simple("in pipe went bad");
+		}
+
+		$bytesRead += $read;
+	}
+
+	# Skip past the trailing newline.
+	my $newline;
+	my $read = read($in, $newline, 1);
+	unless (defined($read)) {
+		$self->_close_cat_blob();
+		throw Error::Simple("in pipe went bad");
+	}
+	unless ($read == 1 && $newline eq "\n") {
+		$self->_close_cat_blob();
+		throw Error::Simple("didn't find newline after blob");
+	}
+
+	unless (print $fh $blob) {
+		$self->_close_cat_blob();
+		throw Error::Simple("couldn't write to passed in filehandle");
+	}
+
+	return $size;
+}
+
+sub _open_cat_blob_if_needed {
+	my ($self) = @_;
+
+	return if defined($self->{cat_blob_pid});
+
+	($self->{cat_blob_pid}, $self->{cat_blob_in},
+	 $self->{cat_blob_out}, $self->{cat_blob_ctx}) =
+		command_bidi_pipe(qw(cat-file --batch));
+}
+
+sub _close_cat_blob {
+	my ($self) = @_;
+
+	return unless defined($self->{cat_blob_pid});
+
+	my @vars = map { 'cat_blob_' . $_ } qw(pid in out ctx);
+
+	command_close_bidi_pipe($self->{@vars});
+	delete $self->{@vars};
+}
 
 =back
 
@@ -951,7 +1095,11 @@ sub _cmd_close {
 }
 
 
-sub DESTROY { }
+sub DESTROY {
+	my ($self) = @_;
+	$self->_close_hash_and_insert_object();
+	$self->_close_cat_blob();
+}
 
 
 # Pipe implementation for ActiveState Perl.
-- 
1.5.5.1

[PATCH 6/8] git-svn: Speed up fetch

[Michele Ballabio]

From: Adam Roben <aroben@apple.com>

We were spending a lot of time forking/execing git-cat-file and
git-hash-object. We now maintain a global Git repository object in order to use
Git.pm's more efficient hash_and_insert_object and cat_blob methods.

Signed-off-by: Adam Roben <aroben@apple.com>
---
 git-svn.perl |   42 ++++++++++++++++++++----------------------
 1 files changed, 20 insertions(+), 22 deletions(-)

diff --git a/git-svn.perl b/git-svn.perl
index b70f8ef..33e9266 100755
--- a/git-svn.perl
+++ b/git-svn.perl
@@ -4,7 +4,7 @@
 use warnings;
 use strict;
 use vars qw/	$AUTHOR $VERSION
-		$sha1 $sha1_short $_revision
+		$sha1 $sha1_short $_revision $_repository
 		$_q $_authors %users/;
 $AUTHOR = 'Eric Wong <normalperson@yhbt.net>';
 $VERSION = '@@GIT_VERSION@@';
@@ -220,6 +220,7 @@ unless ($cmd && $cmd =~ /(?:clone|init|multi-init)$/) {
 		}
 		$ENV{GIT_DIR} = $git_dir;
 	}
+	$_repository = Git->repository(Repository => $ENV{GIT_DIR});
 }
 
 my %opts = %{$cmd{$cmd}->[2]} if (defined $cmd);
@@ -301,6 +302,7 @@ sub do_git_init_db {
 			}
 		}
 		command_noisy(@init_db);
+		$_repository = Git->repository(Repository => ".git");
 	}
 	my $set;
 	my $pfx = "svn-remote.$Git::SVN::default_repo_id";
@@ -317,6 +319,7 @@ sub init_subdir {
 	mkpath([$repo_path]) unless -d $repo_path;
 	chdir $repo_path or die "Couldn't chdir to $repo_path: $!\n";
 	$ENV{GIT_DIR} = '.git';
+	$_repository = Git->repository(Repository => $ENV{GIT_DIR});
 }
 
 sub cmd_clone {
@@ -3017,6 +3020,7 @@ use vars qw/@ISA/;
 use strict;
 use warnings;
 use Carp qw/croak/;
+use File::Temp qw/tempfile/;
 use IO::File qw//;
 
 # file baton members: path, mode_a, mode_b, pool, fh, blob, base
@@ -3172,14 +3176,9 @@ sub apply_textdelta {
 	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 $?;
+		print $base 'link ' if ($fb->{mode_a} == 120000);
+		my $size = $::_repository->cat_blob($fb->{blob}, $base);
+		die "Failed to read object $fb->{blob}" unless $size;
 
 		if (defined $exp) {
 			seek $base, 0, 0 or croak $!;
@@ -3220,14 +3219,18 @@ sub close_file {
 				sysseek($fh, 0, 0) or croak $!;
 			}
 		}
-		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 $!;
+
+		my ($tmp_fh, $tmp_filename) = File::Temp::tempfile(UNLINK => 1);
+		my $result;
+		while ($result = sysread($fh, my $string, 1024)) {
+			syswrite($tmp_fh, $string, $result);
 		}
-		chomp($hash = do { local $/; <$out> });
-		close $out or croak $!;
+		defined $result or croak $!;
+		close $tmp_fh or croak $!;
+
 		close $fh or croak $!;
+
+		$hash = $::_repository->hash_and_insert_object($tmp_filename);
 		$hash =~ /^[a-f\d]{40}$/ or die "not a sha1: $hash\n";
 		close $fb->{base} or croak $!;
 	} else {
@@ -3553,13 +3556,8 @@ sub chg_file {
 	} elsif ($m->{mode_a} =~ /^120/ && $m->{mode_b} !~ /^120/) {
 		$self->change_file_prop($fbat,'svn:special',undef);
 	}
-	defined(my $pid = fork) or croak $!;
-	if (!$pid) {
-		open STDOUT, '>&', $fh or croak $!;
-		exec qw/git-cat-file blob/, $m->{sha1_b} or croak $!;
-	}
-	waitpid $pid, 0;
-	croak $? if $?;
+	my $size = $::_repository->cat_blob($m->{sha1_b}, $fh);
+	croak "Failed to read object $m->{sha1_b}" unless $size;
 	$fh->flush == 0 or croak $!;
 	seek $fh, 0, 0 or croak $!;
 
-- 
1.5.5.1

[PATCH 7/8] builtin-cat-file.c: use parse_options()

[Michele Ballabio]

This simplifies the option parsing.

Signed-off-by: Michele Ballabio <barra_cuda@katamail.com>
---
 builtin-cat-file.c |  119 ++++++++++++++++++++--------------------------------
 1 files changed, 46 insertions(+), 73 deletions(-)

diff --git a/builtin-cat-file.c b/builtin-cat-file.c
index b4d0c25..5ef15a4 100644
--- a/builtin-cat-file.c
+++ b/builtin-cat-file.c
@@ -8,6 +8,10 @@
 #include "tag.h"
 #include "tree.h"
 #include "builtin.h"
+#include "parse-options.h"
+
+#define BATCH 1
+#define BATCH_CHECK 2
 
 static void pprint_tag(const unsigned char *sha1, const char *buf, unsigned long size)
 {
@@ -158,7 +162,7 @@ static int batch_one_object(const char *obj_name, int print_contents)
 		return 0;
 	}
 
-	if (print_contents)
+	if (print_contents == BATCH)
 		contents = read_sha1_file(sha1, &type, &size);
 	else
 		type = sha1_object_info(sha1, &size);
@@ -169,7 +173,7 @@ static int batch_one_object(const char *obj_name, int print_contents)
 	printf("%s %s %lu\n", sha1_to_hex(sha1), typename(type), size);
 	fflush(stdout);
 
-	if (print_contents) {
+	if (print_contents == BATCH) {
 		write_or_die(1, contents, size);
 		printf("\n");
 		fflush(stdout);
@@ -192,89 +196,58 @@ static int batch_objects(int print_contents)
 	return 0;
 }
 
-static const char cat_file_usage[] = "git-cat-file [ [-t|-s|-e|-p|<type>] <sha1> | [--batch|--batch-check] < <list_of_sha1s> ]";
+static const char * const cat_file_usage[] = {
+	"git-cat-file [-t|-s|-e|-p|<type>] <sha1>",
+	"git-cat-file [--batch|--batch-check] < <list_of_sha1s>",
+	NULL
+};
 
 int cmd_cat_file(int argc, const char **argv, const char *prefix)
 {
-	int i, opt = 0, batch = 0, batch_check = 0;
+	int opt = 0, batch = 0;
 	const char *exp_type = NULL, *obj_name = NULL;
 
-	git_config(git_default_config);
-
-	for (i = 1; i < argc; ++i) {
-		const char *arg = argv[i];
-		int is_batch = 0, is_batch_check = 0;
-
-		is_batch = !strcmp(arg, "--batch");
-		if (!is_batch)
-			is_batch_check = !strcmp(arg, "--batch-check");
-
-		if (is_batch || is_batch_check) {
-			if (opt) {
-				error("git-cat-file: Can't use %s with -%c", arg, opt);
-				usage(cat_file_usage);
-			} else if (exp_type) {
-				error("git-cat-file: Can't use %s when a type (\"%s\") is specified", arg, exp_type);
-				usage(cat_file_usage);
-			} else if (obj_name) {
-				error("git-cat-file: Can't use %s when an object (\"%s\") is specified", arg, obj_name);
-				usage(cat_file_usage);
-			}
-
-			if ((is_batch && batch_check) || (is_batch_check && batch)) {
-				error("git-cat-file: Can't use %s with %s", arg, is_batch ? "--batch-check" : "--batch");
-				usage(cat_file_usage);
-			}
-
-			if (is_batch)
-				batch = 1;
-			else
-				batch_check = 1;
+	const struct option options[] = {
+		OPT_GROUP("<type> can be one of: blob, tree, commit, tag"),
+		OPT_SET_INT('t', NULL, &opt, "show object type", 't'),
+		OPT_SET_INT('s', NULL, &opt, "show object size", 's'),
+		OPT_SET_INT('e', NULL, &opt,
+			    "exit with zero when there's no error", 'e'),
+		OPT_SET_INT('p', NULL, &opt, "pretty-print object's content", 'p'),
+		OPT_SET_INT(0, "batch", &batch,
+			    "show info and content of objects feeded on stdin", BATCH),
+		OPT_SET_INT(0, "batch-check", &batch,
+			    "show info about objects feeded on stdin",
+			    BATCH_CHECK),
+		OPT_END()
+	};
 
-			continue;
-		}
-
-		if (!strcmp(arg, "-t") || !strcmp(arg, "-s") || !strcmp(arg, "-e") || !strcmp(arg, "-p")) {
-			if (batch || batch_check) {
-				error("git-cat-file: Can't use %s with %s", arg, batch ? "--batch" : "--batch-check");
-				usage(cat_file_usage);
-			}
-
-			exp_type = arg;
-			opt = exp_type[1];
-			continue;
-		}
-
-		if (arg[0] == '-')
-			usage(cat_file_usage);
-
-		if (!exp_type) {
-			if (batch || batch_check) {
-				error("git-cat-file: Can't specify a type (\"%s\") with %s", arg, batch ? "--batch" : "--batch-check");
-				usage(cat_file_usage);
-			}
-
-			exp_type = arg;
-			continue;
-		}
+	git_config(git_default_config);
 
-		if (obj_name)
-			usage(cat_file_usage);
+	if (argc != 3 && argc != 2)
+		usage_with_options(cat_file_usage, options);
 
-		// We should have hit one of the earlier if (batch || batch_check) cases before
-		// getting here.
-		assert(!batch);
-		assert(!batch_check);
+	argc = parse_options(argc, argv, options, cat_file_usage, 0);
 
-		obj_name = arg;
-		break;
+	if (opt) {
+		if (argc == 1)
+			obj_name = argv[0];
+		else
+			usage_with_options(cat_file_usage, options);
+	}
+	if (!opt && !batch) {
+		if (argc == 2) {
+			exp_type = argv[0];
+			obj_name = argv[1];
+		} else
+			usage_with_options(cat_file_usage, options);
+	}
+	if (batch && (opt || argc)) {
+		usage_with_options(cat_file_usage, options);
 	}
 
-	if (batch || batch_check)
+	if (batch)
 		return batch_objects(batch);
 
-	if (!exp_type || !obj_name)
-		usage(cat_file_usage);
-
 	return cat_one_file(opt, exp_type, obj_name);
 }
-- 
1.5.5.1

[PATCH 8/8] change quoting in test t1006-cat-file.sh

[Michele Ballabio]

Signed-off-by: Michele Ballabio <barra_cuda@katamail.com>
---
 t/t1006-cat-file.sh |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/t/t1006-cat-file.sh b/t/t1006-cat-file.sh
index d569676..cb1fbe5 100755
--- a/t/t1006-cat-file.sh
+++ b/t/t1006-cat-file.sh
@@ -199,9 +199,9 @@ $tag_content
 deadbeef missing
  missing"
 
-test_expect_success \
-    "--batch with multiple sha1s gives correct format" \
-    "test \"\$(maybe_remove_timestamp \"$batch_output\" 1)\" = \"\$(maybe_remove_timestamp \"\$(echo_without_newline \"$batch_input\" | git cat-file --batch)\" 1)\""
+test_expect_success '--batch with multiple sha1s gives correct format' '
+	test "$(maybe_remove_timestamp "$batch_output" 1)" = "$(maybe_remove_timestamp "$(echo_without_newline "$batch_input" | git cat-file --batch)" 1)"
+'
 
 batch_check_input="$hello_sha1
 $tree_sha1
-- 
1.5.5.1

Re: [PATCH 0/8] Speed up git-svn

[Michele Ballabio]

Output of "git diff -w" against the old version by Adam Roben of
patches 2 and 3:

diff --git a/hash-object.c b/hash-object.c
index 1b39162..0a7ac2f 100644
--- a/hash-object.c
+++ b/hash-object.c
@@ -6,6 +6,7 @@
  */
 #include "cache.h"
 #include "blob.h"
+#include "quote.h"
 
 static void hash_object(const char *path, enum object_type type, int write_object)
 {
@@ -31,7 +32,7 @@ static void hash_stdin(const char *type, int write_object)
 	printf("%s\n", sha1_to_hex(sha1));
 }
 
-static int hash_stdin_paths(const char *type, int write_objects)
+static void hash_stdin_paths(const char *type, int write_objects)
 {
 	struct strbuf buf, nbuf;
 
@@ -127,7 +128,7 @@ int main(int argc, char **argv)
 	}
 
 	if (stdin_paths)
-		return hash_stdin_paths(type, write_object);
+		hash_stdin_paths(type, write_object);
 
 	if (hashstdin)
 		hash_stdin(type, write_object);
diff --git a/t/t1007-hash-object.sh b/t/t1007-hash-object.sh
index dbb4129..0526295 100755
--- a/t/t1007-hash-object.sh
+++ b/t/t1007-hash-object.sh
@@ -4,23 +4,20 @@ test_description=git-hash-object
 
 . ./test-lib.sh
 
-function echo_without_newline()
-{
-    echo "$@\c"
+echo_without_newline() {
+	printf '%s' "$*"
 }
 
-function test_blob_does_not_exist()
-{
-    test_expect_success \
-        "blob does not exist in database" \
-        "test_must_fail git cat-file blob $1"
+test_blob_does_not_exist() {
+	test_expect_success 'blob does not exist in database' "
+		test_must_fail git cat-file blob $1
+	"
 }
 
-function test_blob_exists()
-{
-    test_expect_success \
-        "blob exists in database" \
-        "git cat-file blob $1"
+test_blob_exists() {
+	test_expect_success 'blob exists in database' "
+		git cat-file blob $1
+	"
 }
 
 hello_content="Hello World"
@@ -29,23 +26,20 @@ hello_sha1=5e1c309dae7f45e0f39b1bf3ac3cd9db12e7d689
 example_content="This is an example"
 example_sha1=ddd3f836d3e3fbb7ae289aa9ae83536f76956399
 
-function setup_repo()
-{
+setup_repo() {
     echo_without_newline "$hello_content" > hello
     echo_without_newline "$example_content" > example
 }
 
 test_repo=test
-function push_repo()
-{
+push_repo() {
     test_create_repo $test_repo
     cd $test_repo
 
     setup_repo
 }
 
-function pop_repo()
-{
+pop_repo() {
     cd ..
     rm -rf $test_repo
 }
@@ -54,59 +48,59 @@ setup_repo
 
 # Argument checking
 
-test_expect_success \
-    "multiple '--stdin's are rejected" \
-    "test_must_fail git hash-object --stdin --stdin < example"
+test_expect_success "multiple '--stdin's are rejected" '
+	test_must_fail git hash-object --stdin --stdin < example
+'
 
-test_expect_success \
-    "Can't use --stdin and --stdin-paths together" \
-    "test_must_fail git hash-object --stdin --stdin-paths &&
-     test_must_fail git hash-object --stdin-paths --stdin"
+test_expect_success "Can't use --stdin and --stdin-paths together" '
+	test_must_fail git hash-object --stdin --stdin-paths &&
+	test_must_fail git hash-object --stdin-paths --stdin
+'
 
-test_expect_success \
-    "Can't pass filenames as arguments with --stdin-paths" \
-    "test_must_fail git hash-object --stdin-paths hello < example"
+test_expect_success "Can't pass filenames as arguments with --stdin-paths" '
+	test_must_fail git hash-object --stdin-paths hello < example
+'
 
 # Behavior
 
 push_repo
 
-    test_expect_success \
-        "hash a file" \
-        "test $hello_sha1 = \$(git hash-object hello)"
+test_expect_success 'hash a file' '
+	test $hello_sha1 = $(git hash-object hello)
+'
 
     test_blob_does_not_exist $hello_sha1
 
-    test_expect_success \
-        "hash from stdin" \
-        "test $example_sha1 = \$(git hash-object --stdin < example)"
+test_expect_success 'hash from stdin' '
+	test $example_sha1 = $(git hash-object --stdin < example)
+'
 
     test_blob_does_not_exist $example_sha1
 
-    test_expect_success \
-        "hash a file and write to database" \
-        "test $hello_sha1 = \$(git hash-object -w hello)"
+test_expect_success 'hash a file and write to database' '
+	test $hello_sha1 = $(git hash-object -w hello)
+'
 
     test_blob_exists $hello_sha1
 
-    test_expect_success \
-        'git hash-object --stdin file1 <file0 first operates on file0, then file1' \
-        'echo foo > file1 &&
+test_expect_success 'git hash-object --stdin file1 <file0 first operates on file0, then file1' '
+	echo foo > file1 &&
         obname0=$(echo bar | git hash-object --stdin) &&
         obname1=$(git hash-object file1) &&
         obname0new=$(echo bar | git hash-object --stdin file1 | sed -n -e 1p) &&
         obname1new=$(echo bar | git hash-object --stdin file1 | sed -n -e 2p) &&
         test "$obname0" = "$obname0new" &&
-        test "$obname1" = "$obname1new"'
+	test "$obname1" = "$obname1new"
+'
 
 pop_repo
 
 for args in "-w --stdin" "--stdin -w"; do
     push_repo
 
-        test_expect_success \
-            "hash from stdin and write to database ($args)" \
-            "test $example_sha1 = \$(git hash-object $args < example)"
+	test_expect_success "hash from stdin and write to database ($args)" '
+		test $example_sha1 = $(git hash-object $args < example)
+	'
 
         test_blob_exists $example_sha1
 
@@ -119,16 +113,16 @@ example"
 sha1s="$hello_sha1
 $example_sha1"
 
-test_expect_success \
-    "hash two files with names on stdin" \
-    "test \"$sha1s\" = \"\$(echo_without_newline \"$filenames\" | git hash-object --stdin-paths)\""
+test_expect_success "hash two files with names on stdin" '
+	test "$sha1s" = "$(echo_without_newline "$filenames" | git hash-object --stdin-paths)"
+'
 
 for args in "-w --stdin-paths" "--stdin-paths -w"; do
     push_repo
 
-        test_expect_success \
-            "hash two files with names on stdin and write to database ($args)" \
-            "test \"$sha1s\" = \"\$(echo_without_newline \"$filenames\" | git hash-object $args)\""
+	test_expect_success "hash two files with names on stdin and write to database ($args)" '
+		test "$sha1s" = "$(echo_without_newline "$filenames" | git hash-object $args)"
+	'
 
         test_blob_exists $hello_sha1
         test_blob_exists $example_sha1