Git development
 help / color / mirror / Atom feed
* Re: [PATCH] make commit message a little more consistent and conforting
From: Shawn Pearce @ 2006-12-15 16:08 UTC (permalink / raw)
  To: Nicolas Pitre; +Cc: Andreas Ericsson, Junio C Hamano, Andy Parkins, git
In-Reply-To: <Pine.LNX.4.64.0612151032240.18171@xanadu.home>

Nicolas Pitre <nico@cam.org> wrote:
> On Fri, 15 Dec 2006, Shawn Pearce wrote:
> > Andreas Ericsson <ae@op5.se> wrote:
> > > diffstats can be huge though. I'd rather have those only with -v option.
> > 
> > But they are on by default for pull/merge, and disabled by -n.
> 
> And it is true that diffstat can be quite large.  I wouldn't mind the 
> diffstat to be added to the commit message summary in the text editor 
> though.  And displaying it when -v is used makes also a lot of sense.  
> But not by default please.

OK, two votes against diffstats by default in commit.  Since I
haven't written a patch for it yet consider it dropped.  ;-)

-- 

^ permalink raw reply

* Re: What's in git.git (stable)
From: Shawn Pearce @ 2006-12-15 16:12 UTC (permalink / raw)
  To: Nicolas Pitre; +Cc: Andreas Ericsson, Jakub Narebski, git
In-Reply-To: <Pine.LNX.4.64.0612151106580.18171@xanadu.home>

Nicolas Pitre <nico@cam.org> wrote:
> Wouldn't it be possible for aliases to be effective only when issued 
> from an interactive shell?  It is certainly true that aliases just make 
> no sense in a script.

Probably, but on Cygwin gitk needs to use 'git foo' to invoke a
command, even if 'git-foo' exists, because 'git-foo' might be a shell
script and the Cygwin wish process cannot execute shell scripts.

Worse it cannot pass down environment variables to git commands.
So it may be a little hard to tell in the git wrapper we are being
run from within gitk (or git-gui)...

-- 

^ permalink raw reply

* Re: What's in git.git (stable)
From: Andreas Ericsson @ 2006-12-15 16:13 UTC (permalink / raw)
  To: Nicolas Pitre; +Cc: Jakub Narebski, git
In-Reply-To: <Pine.LNX.4.64.0612151106580.18171@xanadu.home>

Nicolas Pitre wrote:
> On Fri, 15 Dec 2006, Andreas Ericsson wrote:
> 
>> Nicolas Pitre wrote:
>>> On Fri, 15 Dec 2006, Jakub Narebski wrote:
>>>
>>>> It would be nice to have some generic place in git config to specify
>>>> default options to git commands (at least for interactive shell). It
>>>> cannot be done using aliases. Perhaps defaults.<command> config variable?
>>> I would say the alias facility has to be fixed then.
>>>
>>> In bash you can alias "ls" to "ls -l" and it just works.
>>>
>> I think this is because git scripts that need a certain git command to work a
>> certain way don't want some alias to kick in and destroy things for them.
>> Shell-scripts would have the same problem if you alias "awk" to "grep" f.e.,
>> which is why prudent shell-scripters use the "unalias -a" thing.
> 
> Wouldn't it be possible for aliases to be effective only when issued 
> from an interactive shell?  It is certainly true that aliases just make 
> no sense in a script.
> 

Yes, but then aliases wouldn't work in one-liners, which would be a bit 
of a shame and pretty likely to cause some "interesting" bugreports. 
Perhaps an environment variable GIT_IGNORE_ALIAS=yes that git-scripts 
can set at the beginning of execution?

-- 
Andreas Ericsson                   andreas.ericsson@op5.se
OP5 AB                             www.op5.se

^ permalink raw reply

* Re: What's in git.git (stable)
From: Jakub Narebski @ 2006-12-15 16:16 UTC (permalink / raw)
  To: git
In-Reply-To: <20061214230307.GE26202@spearce.org>

Shawn Pearce wrote:

> Andy Parkins <andyparkins@gmail.com> wrote:
>>  * git-show-branch output is cryptic.
> 
> Agreed.  I still don't know how to read its output.  So I just
> don't use it.  Ever.  :-)

And the way it uses it's options is even more cryptic, and differs from
other similar commands. So I'd rather use qgit (or gitk, if gitk would
correct the error with remembering wrong window size).

-- 
Jakub Narebski
Warsaw, Poland
ShadeHawk on #git


^ permalink raw reply

* [PATCH] gitweb: Do not show difftree for merges in "commit" view
From: Jakub Narebski @ 2006-12-15 16:53 UTC (permalink / raw)
  To: git; +Cc: Shawn Pearce, Carl Worth, Jakub Narebski
In-Reply-To: <20061215160303.GG17860@spearce.org>

Do not show difftree against first parent for merges (commits with
more than one parent) in "commit" view, because it usually is
misleading.  git-show and git-whatchanged doesn't show diff for merges
either.

Signed-off-by: Jakub Narebski <jnareb@gmail.com>
---
 gitweb/gitweb.perl |   22 +++++++++++++++-------
 1 files changed, 15 insertions(+), 7 deletions(-)

diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index 040ee71..ebf35a1 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -3572,14 +3572,19 @@ sub git_commit {
 	my %cd = parse_date($co{'committer_epoch'}, $co{'committer_tz'});
 
 	my $parent = $co{'parent'};
+	my $parents = $co{'parents'};
 	if (!defined $parent) {
 		$parent = "--root";
 	}
-	open my $fd, "-|", git_cmd(), "diff-tree", '-r', "--no-commit-id",
-		@diff_opts, $parent, $hash, "--"
-		or die_error(undef, "Open git-diff-tree failed");
-	my @difftree = map { chomp; $_ } <$fd>;
-	close $fd or die_error(undef, "Reading git-diff-tree failed");
+	my @difftree;
+	if (@$parents <= 1) {
+		# difftree output is not printed for merges
+		open my $fd, "-|", git_cmd(), "diff-tree", '-r', "--no-commit-id",
+			@diff_opts, $parent, $hash, "--"
+				or die_error(undef, "Open git-diff-tree failed");
+		@difftree = map { chomp; $_ } <$fd>;
+		close $fd or die_error(undef, "Reading git-diff-tree failed");
+	}
 
 	# non-textual hash id's can be cached
 	my $expires;
@@ -3641,7 +3646,7 @@ sub git_commit {
 	}
 	print "</td>" .
 	      "</tr>\n";
-	my $parents = $co{'parents'};
+
 	foreach my $par (@$parents) {
 		print "<tr>" .
 		      "<td>parent</td>" .
@@ -3663,7 +3668,10 @@ sub git_commit {
 	git_print_log($co{'comment'});
 	print "</div>\n";
 
-	git_difftree_body(\@difftree, $hash, $parent);
+	if (@$parents <= 1) {
+		# do not output difftree/whatchanged for merges
+		git_difftree_body(\@difftree, $hash, $parent);
+	}
 
 	git_footer_html();
 }
-- 
1.4.4.1

^ permalink raw reply related

* Re: [RFC] Submodules in GIT
From: Torgil Svensson @ 2006-12-15 17:43 UTC (permalink / raw)
  To: Josef Weidendorfer; +Cc: R. Steve McKown, Linus Torvalds, git
In-Reply-To: <200612150007.44331.Josef.Weidendorfer@gmx.de>

On 12/15/06, Josef Weidendorfer <Josef.Weidendorfer@gmx.de> wrote:
> If you want to track build results for some source,
> why would you ever want these builds go out of sync with the source?

I don't, bad wording by me. That was the problem I wanted to address.


> Your example has the folling submodule dependence
> ("X ==> Y" means Y being a submodule of X):
>
>   App     ==>   Lib
>    ^             ^
>    |             |
>  AppBuild ==> LibBuild

In my example "AppBuild" and "LibBuild" were the same project but this
scenario is relevant as well.


> If we force submodules to be subdirectories of supermodules,
> Lib needlessly will have to appear two times in a checkout of
> AppBuild.
>
> However, there is nothing wrong with it. Yet, you perhaps want
> the 2 Lib submodules not to go out of sync. This easily
> can be done with symlinking the Lib checkouts. As they are submodules,
> everything should work fine.

This is interesting. In my notation:

/path/to/link/name -> <commit>/path/to/subtree

means that there is a link named "name" in the tree object for
"path/to/link". The link points to a "link object" specifying a
subtree or blob of the tree that is pointed to in a submodule commit.
This is not currently implemented but has at least the following
advantages:

1. You can access files in a submodule without fetching the whole
submodule (which may be very large). (App1 is only interested in
lib1.h, the rest is toally irrelevant)
2. Superproject can access referenced (linked) files in it's own
folder-structure without being forced a structure by the subproject.

If you do a symlink instead, doesn't you loose versioning information?
What happens with the symlinks if someone clones the superproject?


>
> Perhaps an option you want to have is to force a checkout
> of AppBuild to make these symlinking itself when it detects
> identical submodules links.
>
> Hmmm... the only problem with a symlink is that it can go wrong
> when moved. Unfortunately, I do not have a good solution for
> this. We can not make UNIX symlinks smart in any way.
> Hardlinking directories would be a solution, but that is not
> possible.
>

Wouldn't specifying the submodule path in the link object fit in well
here? Then each "link object" can represent a checked out tree from
the subproject in the superproject directory-structure.


> Another thing:
> With normal "$buildroot != $srcroot" environments, the source
> can not be a subdirectory of the build directory.

This is true for symlinks and would also be corrected if we have a
(sparse) submodule checkout there in it's place.


> BTW, build project commits probably should not depend on any
> history of other build commits.

Why? Can you give an example here.


> > Link: /headers/lib1.h -> <lib1-commit3>/src/lib1.h
> > Link: /bin/lib1.so -> <build1-commit>/i386/lib1/lib1.so
> > Link: /bin/app1 -> <build1-commit>/i386/app1/app1
> >
> >
> > <lib1-commit1>, <lib1-commit2> and <lib1-commit3> should be the same,
> > dictated by the app1 project.
>
> I do not see any problem here. Symlinks are stored in the git repository.
> As the AppBuild commit depends on App and LibBuild submodule commits, the
> symlinks always should be correct.

The main reason for these "links" are for versioning purposes: the
uniqe SHA1 of the "link" representing a tree/blob in a version of the
submodule should be "included" in the supermodules commit. Symlinks
won't give that at all.


> > How do the super-projects in this case get access to the blobs pointed
> > by the links - transparent or explicit in the build-process?
>
> Submodules should automatically be checked out when checking out the
> supermodule. So the blobs should already be there.
> Or do I miss something?

Probably not as that was a piece of the puzzle that I was missing.



^ permalink raw reply

* Re: svn versus git
From: Junio C Hamano @ 2006-12-15 18:14 UTC (permalink / raw)
  To: Andreas Ericsson; +Cc: git
In-Reply-To: <4582C233.1000706@op5.se>

Andreas Ericsson <ae@op5.se> writes:

>>> The number in front is octal mode of a file or directory. "blob"
>>> is a file (or symbolic link), "tree" is a directory, all of this
>>> can be found in git(7).
>>
>> I don't want to come through as rude, but that you can find the explanation
>> somewhere (and as an old(ish) Unix/git hand you know (or should be able to
>> guess easily) what it means) doesn't help the _newbie_ confronted with this
>> gibberish one iota.
>
> I think it would help if we could write out things ls-style though and
> I'm all for axing the SHA1 (I don't even think the SHA1 of specific
> files of a tree can be *used* for anything), so the above would be
>
> -rw-r--r-- Makefile
>
> which most newbies should grok fairly quickly.

'ls-tree' is a plumbing and existing scripts (not limited to
what are in git.git) use it to extract object names from
arbitrary trees.  It's output format will NOT change.

The honest position to take on "svn list" question is "we do not
have a counterpart for that command".

Now, I happen to think "getting list of all paths in one
particular revision" is not a very useful operation from the end
user's point of view.  One possible use is to get such a list
from multiple revisions and compare them with "comm -3", but
that is obviously useless in the context of git.  You can do
that with "diff --name-status --diff-filter=AD" without doing
ls-tree yourself.

But if "ls $commit" is still interesting at end-user level,
probably we would need a Porcelain to let users do so.

Johannes recently made

	$ git show $commit^{tree}

to do the name-only variant.

If people actually do "ls-tree --name-only" (or ls-tree
--like-ls-l) often, we might want to be even easier:

	$ git show --ls $commit

or even:

	$ git ls $commit

or even:

	[alias] ls = ls-tree --name-only


^ permalink raw reply

* Re: [PATCH] make commit message a little more consistent and conforting
From: Junio C Hamano @ 2006-12-15 18:14 UTC (permalink / raw)
  To: Nicolas Pitre; +Cc: Shawn Pearce, Andreas Ericsson, Andy Parkins, git
In-Reply-To: <Pine.LNX.4.64.0612151032240.18171@xanadu.home>

Nicolas Pitre <nico@cam.org> writes:

> So in short I do think there should be something shown after a 
> successful commit, and including the commit sha1 doesn't hurt.
> ...
> And it is true that diffstat can be quite large.  I wouldn't mind the 
> diffstat to be added to the commit message summary in the text editor 
> though.  And displaying it when -v is used makes also a lot of sense.  
> But not by default please.

I agree with everything you said in your message, including that
commit object name might help as a learning aid.

We could give something like this, though, if we wanted to:

	$ git commit
        4 files changed, 17 insertions(+), 10 deletions(-)
        mode change 100755 => 100644 test.sh


^ permalink raw reply

* Re: [PATCH] make commit message a little more consistent and conforting
From: Junio C Hamano @ 2006-12-15 18:21 UTC (permalink / raw)
  To: Shawn Pearce; +Cc: git
In-Reply-To: <20061215154025.GF17860@spearce.org>

Shawn Pearce <spearce@spearce.org> writes:

> I do the same (diff a lot before commit) and thus find commit
> outputting anything at all to be noisy and irritating.  Frankly
> the new
>
>   git-diff-tree --summary --root --no-commit-id HEAD
>
> that Junio put on the end is already irritating.
>
> But it was added to help users verify that commit did what they
> thought it would (see 61f5cb7f).

The credit should go Pasky's way.  Add/delete/mode are rarer
events and reminding them is sensible.

We do not show the 'summary' when we are concluding a merge that
conflicted.  Otherwise you will be seeing other people's huge
changes that was just brought in.

^ permalink raw reply

* Re: [RFC] requiring Perl SVN libraries for git-svn
From: Eric Wong @ 2006-12-15 18:44 UTC (permalink / raw)
  To: git
In-Reply-To: <20061213202142.GE8179@localdomain>

Eric Wong <normalperson@yhbt.net> wrote:
> Are there any git-svn users out there who would be seriously hurt if I
> dropped support for using the svn command-line client in git-svn?

One additional comment is that the dcommit command (much favored over
'git svn commit') does not currently work with the command-line client.

-- 

^ permalink raw reply

* Re: [RFC \ WISH] Add -o option to git-rev-list
From: Marco Costalba @ 2006-12-15 18:45 UTC (permalink / raw)
  To: Git Mailing List
  Cc: Josef Weidendorfer, Junio C Hamano, Alex Riesen, Shawn Pearce,
	Linus Torvalds
In-Reply-To: <Pine.LNX.4.64.0612111258050.3515@woody.osdl.org>

On 12/11/06, Linus Torvalds <torvalds@osdl.org> wrote:
>
>
> So I don't think there is a right answer here. I suspect QProcess does ok.
>

I have just pushed a patch (wiil be available in few hours I suspect)
to easily choose the data exchange facility with git-rev-list.

Default its temporary file based, but after uncommenting the
USE_QPROCESS define in dataloader.cpp and compile again, qgit will use
a QProcess based approach to connect with git-rev-list.

Only the low level connection is different in the two cases, _all_ the
rest, expecially data storing and parsing, is the same. If you take a
look at src/dataloader.cpp you will see that the difference between
QProcess and temp file code is minumum.

I've also tried to clearly report in comments the different approaches
and the data copy involved in both cases. I've searched a little bit
in QProcess sources too to get some additional info.


And finally these are mine warm and cold cache tests.

Load time test on Linux tree (44557 revs, 32167KB)
CPU Mobile Pentium 4 set at 1.2GHZ

To set data read interval in ms use (src/dataloader.cpp):
#define GUI_UPDATE_INTERVAL 500

Warmed-up cache
QProcess 7632ms (500ms data read interval)
QProcess 7972ms (100ms data read interval)

Temporary file 4408ms (500ms data read interval)
Temporary file 4591ms (100ms data read interval)


Cold cache
QProcess 25611ms (500ms data read interval)

File 22399ms (500ms data read interval)



^ permalink raw reply

* [RFC/PATCH] git-svn: convert to using Git.pm
From: Eric Wong @ 2006-12-15 18:59 UTC (permalink / raw)
  To: git; +Cc: Eric Wong

Thanks to Git.pm, I've been able to greatly reduce the amount
of extra work that needs to be done to manage input/output
pipes in Perl.

chomp usage has also been greatly reduced, too.

All tests (including full-svn-test) still pass, but this has
not been tested extensively in the real-world.

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

Junio: please don't apply this to master just yet.

 git-svn.perl |  328 ++++++++++++++++++++++++----------------------------------
 1 files changed, 135 insertions(+), 193 deletions(-)

diff --git a/git-svn.perl b/git-svn.perl
index 73ab8d8..f453c9a 100755
--- a/git-svn.perl
+++ b/git-svn.perl
@@ -46,6 +46,8 @@ use File::Copy qw/copy/;
 use POSIX qw/strftime/;
 use IPC::Open3;
 use Memoize;
+use Git qw/command command_oneline command_noisy
+           command_output_pipe command_input_pipe command_close_pipe/;
 memoize('revisions_eq');
 memoize('cmt_metadata');
 memoize('get_commit_time');
@@ -232,28 +234,27 @@ sub version {
 }
 
 sub rebuild {
-	if (quiet_run(qw/git-rev-parse --verify/,"refs/remotes/$GIT_SVN^0")) {
+	if (!verify_ref("refs/remotes/$GIT_SVN^0")) {
 		copy_remote_ref();
 	}
 	$SVN_URL = shift or undef;
 	my $newest_rev = 0;
 	if ($_upgrade) {
-		sys('git-update-ref',"refs/remotes/$GIT_SVN","$GIT_SVN-HEAD");
+		command_noisy('update-ref',"refs/remotes/$GIT_SVN","
+		              $GIT_SVN-HEAD");
 	} else {
 		check_upgrade_needed();
 	}
 
-	my $pid = open(my $rev_list,'-|');
-	defined $pid or croak $!;
-	if ($pid == 0) {
-		exec("git-rev-list","refs/remotes/$GIT_SVN") or croak $!;
-	}
+	my ($rev_list, $ctx) = command_output_pipe("rev-list",
+	                                           "refs/remotes/$GIT_SVN");
 	my $latest;
 	while (<$rev_list>) {
 		chomp;
 		my $c = $_;
 		croak "Non-SHA1: $c\n" unless $c =~ /^$sha1$/o;
-		my @commit = grep(/^git-svn-id: /,`git-cat-file commit $c`);
+		my @commit = grep(/^git-svn-id: /,
+		                  command(qw/cat-file commit/, $c));
 		next if (!@commit); # skip merges
 		my ($url, $rev, $uuid) = extract_metadata($commit[$#commit]);
 		if (!defined $rev || !$uuid) {
@@ -279,7 +280,7 @@ sub rebuild {
 		print "r$rev = $c\n";
 		$newest_rev = $rev if ($rev > $newest_rev);
 	}
-	close $rev_list or croak $?;
+	command_close_pipe($rev_list, $ctx);
 
 	goto out if $_use_lib;
 	if (!chdir $SVN_WC) {
@@ -287,7 +288,7 @@ sub rebuild {
 		chdir $SVN_WC or croak $!;
 	}
 
-	$pid = fork;
+	my $pid = fork;
 	defined $pid or croak $!;
 	if ($pid == 0) {
 		my @svn_up = qw(svn up);
@@ -323,10 +324,10 @@ sub init {
 
 	$SVN_URL = $url;
 	unless (-d $GIT_DIR) {
-		my @init_db = ('git-init-db');
+		my @init_db = ('init-db');
 		push @init_db, "--template=$_template" if defined $_template;
 		push @init_db, "--shared" if defined $_shared;
-		sys(@init_db);
+		command_noisy(@init_db);
 	}
 	setup_git_svn();
 }
@@ -335,9 +336,8 @@ sub fetch {
 	check_upgrade_needed();
 	$SVN_URL ||= file_to_s("$GIT_SVN_DIR/info/url");
 	my $ret = $_use_lib ? fetch_lib(@_) : fetch_cmd(@_);
-	if ($ret->{commit} && quiet_run(qw(git-rev-parse --verify
-						refs/heads/master^0))) {
-		sys(qw(git-update-ref refs/heads/master),$ret->{commit});
+	if ($ret->{commit} && !verify_ref('refs/heads/master^0')) {
+		command_noisy(qw(update-ref refs/heads/master),$ret->{commit});
 	}
 	return $ret;
 }
@@ -416,16 +416,16 @@ sub fetch_lib {
 	read_uuid();
 	if (defined $last_commit) {
 		unless (-e $GIT_SVN_INDEX) {
-			sys(qw/git-read-tree/, $last_commit);
+			command_noisy('read-tree', $last_commit);
 		}
-		chomp (my $x = `git-write-tree`);
-		my ($y) = (`git-cat-file commit $last_commit`
+		my $x = command_oneline('write-tree');
+		my ($y) = (command(qw/cat-file commit/, $last_commit)
 							=~ /^tree ($sha1)/m);
 		if ($y ne $x) {
 			unlink $GIT_SVN_INDEX or croak $!;
-			sys(qw/git-read-tree/, $last_commit);
+			command_noisy('read-tree', $last_commit);
 		}
-		chomp ($x = `git-write-tree`);
+		$x = command_oneline('write-tree');
 		if ($y ne $x) {
 			print STDERR "trees ($last_commit) $y != $x\n",
 				 "Something is seriously wrong...\n";
@@ -489,16 +489,15 @@ sub commit {
 	}
 	my @revs;
 	foreach my $c (@commits) {
-		chomp(my @tmp = safe_qx('git-rev-parse',$c));
+		my @tmp = command('rev-parse',$c);
 		if (scalar @tmp == 1) {
 			push @revs, $tmp[0];
 		} elsif (scalar @tmp > 1) {
-			push @revs, reverse (safe_qx('git-rev-list',@tmp));
+			push @revs, reverse(command('rev-list',@tmp));
 		} else {
 			die "Failed to rev-parse $c\n";
 		}
 	}
-	chomp @revs;
 	$_use_lib ? commit_lib(@revs) : commit_cmd(@revs);
 	print "Done committing ",scalar @revs," revisions to SVN\n";
 }
@@ -606,10 +605,10 @@ sub commit_lib {
 sub dcommit {
 	my $head = shift || 'HEAD';
 	my $gs = "refs/remotes/$GIT_SVN";
-	chomp(my @refs = safe_qx(qw/git-rev-list --no-merges/, "$gs..$head"));
+	my @refs = command(qw/rev-list --no-merges/, "$gs..$head");
 	my $last_rev;
 	foreach my $d (reverse @refs) {
-		if (quiet_run('git-rev-parse','--verify',"$d~1") != 0) {
+		if (!verify_ref("$d~1")) {
 			die "Commit $d\n",
 			    "has no parent commit, and therefore ",
 			    "nothing to diff against.\n",
@@ -633,7 +632,7 @@ sub dcommit {
 	}
 	return if $_dry_run;
 	fetch();
-	my @diff = safe_qx('git-diff-tree', $head, $gs);
+	my @diff = command('diff-tree', $head, $gs, '--');
 	my @finish;
 	if (@diff) {
 		@finish = qw/rebase/;
@@ -645,7 +644,7 @@ sub dcommit {
 		      "Resetting to the latest $gs\n";
 		@finish = qw/reset --mixed/;
 	}
-	sys('git', @finish, $gs);
+	command_noisy(@finish, $gs);
 }
 
 sub show_ignore {
@@ -689,7 +688,7 @@ sub graft_branches {
 
 	if (%$grafts) {
 		# temporarily disable our grafts file to make this idempotent
-		chomp($gr_sha1 = safe_qx(qw/git-hash-object -w/,$gr_file));
+		chomp($gr_sha1 = command(qw/hash-object -w/,$gr_file));
 		rename $gr_file, "$gr_file~$gr_sha1" or croak $!;
 	}
 
@@ -743,7 +742,7 @@ sub multi_init {
 	unless (-d $GIT_SVN_DIR) {
 		print "GIT_SVN_ID set to 'trunk' for $_trunk\n" if $ch_id;
 		init($_trunk);
-		sys('git-repo-config', 'svn.trunk', $_trunk);
+		command_noisy('repo-config', 'svn.trunk', $_trunk);
 	}
 	complete_url_ls_init($url, $_branches, '--branches/-b', '');
 	complete_url_ls_init($url, $_tags, '--tags/-t', 'tags/');
@@ -781,11 +780,8 @@ sub show_log {
 	}
 
 	config_pager();
-	my $pid = open(my $log,'-|');
-	defined $pid or croak $!;
-	if (!$pid) {
-		exec(git_svn_log_cmd($r_min,$r_max), @args) or croak $!;
-	}
+	@args = (git_svn_log_cmd($r_min, $r_max), @args);
+	my $log = command_output_pipe(@args);
 	run_pager();
 	my (@k, $c, $d);
 
@@ -832,7 +828,7 @@ sub show_log {
 		process_commit($_, $r_min, $r_max) foreach reverse @k;
 	}
 out:
-	close $log;
+	eval { command_close_pipe($log) };
 	print '-' x72,"\n" unless $_incremental || $_oneline;
 }
 
@@ -912,7 +908,7 @@ sub cmt_showable {
 	return 1 if defined $c->{r};
 	if ($c->{l} && $c->{l}->[-1] eq "...\n" &&
 				$c->{a_raw} =~ /\@([a-f\d\-]+)>$/) {
-		my @msg = safe_qx(qw/git-cat-file commit/, $c->{c});
+		my @msg = command(qw/cat-file commit/, $c->{c});
 		shift @msg while ($msg[0] ne "\n");
 		shift @msg;
 		@{$c->{l}} = grep !/^git-svn-id: /, @msg;
@@ -961,7 +957,7 @@ sub log_use_color {
 
 sub git_svn_log_cmd {
 	my ($r_min, $r_max) = @_;
-	my @cmd = (qw/git-log --abbrev-commit --pretty=raw
+	my @cmd = (qw/log --abbrev-commit --pretty=raw
 			--default/, "refs/remotes/$GIT_SVN");
 	push @cmd, '-r' unless $_non_recursive;
 	push @cmd, qw/--raw --name-status/ if $_verbose;
@@ -1071,7 +1067,7 @@ sub complete_url_ls_init {
 	waitpid $pid, 0;
 	croak $? if $?;
 	my ($n) = ($switch =~ /^--(\w+)/);
-	sys('git-repo-config', "svn.$n", $var);
+	command_noisy('repo-config', "svn.$n", $var);
 }
 
 sub common_prefix {
@@ -1103,11 +1099,8 @@ sub graft_tree_joins {
 
 	git_svn_each(sub {
 		my $i = shift;
-		defined(my $pid = open my $fh, '-|') or croak $!;
-		if (!$pid) {
-			exec qw/git-rev-list --pretty=raw/,
-					"refs/remotes/$i" or croak $!;
-		}
+		my @args = (qw/rev-list --pretty=raw/, "refs/remotes/$i");
+		my ($fh, $ctx) = command_output_pipe(@args);
 		while (<$fh>) {
 			next unless /^commit ($sha1)$/o;
 			my $c = $1;
@@ -1130,9 +1123,7 @@ sub graft_tree_joins {
 
 			foreach my $p (@{$tree_map{$t}}) {
 				next if $p eq $c;
-				my $mb = eval {
-					safe_qx('git-merge-base', $c, $p)
-				};
+				my $mb = eval { command('merge-base', $c, $p) };
 				next unless ($@ || $?);
 				if (defined $r_a) {
 					# see if SVN says it's a relative
@@ -1161,7 +1152,7 @@ sub graft_tree_joins {
 				# what should we do when $ct == $s ?
 			}
 		}
-		close $fh or croak $?;
+		command_close_pipe($fh, $ctx);
 	});
 }
 
@@ -1297,6 +1288,11 @@ sub read_uuid {
 	}
 }
 
+sub verify_ref {
+	my ($ref) = @_;
+	eval { command_oneline([ 'rev-parse', $ref ], { STDERR => 0 }) };
+}
+
 sub quiet_run {
 	my $pid = fork;
 	defined $pid or croak $!;
@@ -1379,13 +1375,14 @@ sub assert_svn_wc_clean {
 sub get_tree_from_treeish {
 	my ($treeish) = @_;
 	croak "Not a sha1: $treeish\n" unless $treeish =~ /^$sha1$/o;
-	chomp(my $type = `git-cat-file -t $treeish`);
+	my $type = command_oneline(qw/cat-file -t/, $treeish);
 	my $expected;
 	while ($type eq 'tag') {
-		chomp(($treeish, $type) = `git-cat-file tag $treeish`);
+		($treeish, $type) = command(qw/cat-file tag/, $treeish);
 	}
 	if ($type eq 'commit') {
-		$expected = (grep /^tree /,`git-cat-file commit $treeish`)[0];
+		$expected = (grep /^tree /, command(qw/cat-file commit/,
+		                                    $treeish))[0];
 		($expected) = ($expected =~ /^tree ($sha1)$/);
 		die "Unable to get tree from $treeish\n" unless $expected;
 	} elsif ($type eq 'tree') {
@@ -1407,7 +1404,7 @@ sub assert_tree {
 	}
 	my $old_index = set_index($tmpindex);
 	index_changes(1);
-	chomp(my $tree = `git-write-tree`);
+	my $tree = command_oneline('write-tree');
 	restore_index($old_index);
 	if ($tree ne $expected) {
 		croak "Tree mismatch, Got: $tree, Expected: $expected\n";
@@ -1415,8 +1412,20 @@ sub assert_tree {
 	unlink $tmpindex;
 }
 
-sub parse_diff_tree {
-	my $diff_fh = shift;
+sub get_diff {
+	my ($from, $treeish) = @_;
+	assert_tree($from);
+	print "diff-tree $from $treeish\n";
+	my @diff_tree = qw(diff-tree -z -r);
+	if ($_cp_similarity) {
+		push @diff_tree, "-C$_cp_similarity";
+	} else {
+		push @diff_tree, '-C';
+	}
+	push @diff_tree, '--find-copies-harder' if $_find_copies_harder;
+	push @diff_tree, "-l$_l" if defined $_l;
+	push @diff_tree, $from, $treeish;
+	my ($diff_fh, $ctx) = command_output_pipe(@diff_tree);
 	local $/ = "\0";
 	my $state = 'meta';
 	my @mods;
@@ -1452,8 +1461,7 @@ sub parse_diff_tree {
 			croak "Error parsing $_\n";
 		}
 	}
-	close $diff_fh or croak $?;
-
+	command_close_pipe($diff_fh, $ctx);
 	return \@mods;
 }
 
@@ -1547,26 +1555,6 @@ sub precommit_check {
 }
 
 
-sub get_diff {
-	my ($from, $treeish) = @_;
-	assert_tree($from);
-	print "diff-tree $from $treeish\n";
-	my $pid = open my $diff_fh, '-|';
-	defined $pid or croak $!;
-	if ($pid == 0) {
-		my @diff_tree = qw(git-diff-tree -z -r);
-		if ($_cp_similarity) {
-			push @diff_tree, "-C$_cp_similarity";
-		} else {
-			push @diff_tree, '-C';
-		}
-		push @diff_tree, '--find-copies-harder' if $_find_copies_harder;
-		push @diff_tree, "-l$_l" if defined $_l;
-		exec(@diff_tree, $from, $treeish) or croak $!;
-	}
-	return parse_diff_tree($diff_fh);
-}
-
 sub svn_checkout_tree {
 	my ($from, $treeish) = @_;
 	my $mods = get_diff($from->{commit}, $treeish);
@@ -1676,14 +1664,10 @@ sub get_commit_message {
 	my %log_msg = ( msg => '' );
 	open my $msg, '>', $commit_msg or croak $!;
 
-	chomp(my $type = `git-cat-file -t $commit`);
+	my $type = command_oneline(qw/cat-file -t/, $commit);
 	if ($type eq 'commit' || $type eq 'tag') {
-		my $pid = open my $msg_fh, '-|';
-		defined $pid or croak $!;
-
-		if ($pid == 0) {
-			exec('git-cat-file', $type, $commit) or croak $!;
-		}
+		my ($msg_fh, $ctx) = command_output_pipe('cat-file',
+		                                         $type, $commit);
 		my $in_msg = 0;
 		while (<$msg_fh>) {
 			if (!$in_msg) {
@@ -1695,7 +1679,7 @@ sub get_commit_message {
 				print $msg $_ or croak $!;
 			}
 		}
-		close $msg_fh or croak $?;
+		command_close_pipe($msg_fh, $ctx);
 	}
 	close $msg or croak $!;
 
@@ -1771,13 +1755,8 @@ sub svn_commit_tree {
 }
 
 sub rev_list_raw {
-	my (@args) = @_;
-	my $pid = open my $fh, '-|';
-	defined $pid or croak $!;
-	if (!$pid) {
-		exec(qw/git-rev-list --pretty=raw/, @args) or croak $!;
-	}
-	return { fh => $fh, t => { } };
+	my ($fh, $c) = command_output_pipe(qw/rev-list --pretty=raw/, @_);
+	return { fh => $fh, ctx => $c, t => { } };
 }
 
 sub next_rev_list_entry {
@@ -1799,6 +1778,7 @@ sub next_rev_list_entry {
 			$x->{m} .= $_;
 		}
 	}
+	command_close_pipe($fh, $rl->{ctx});
 	return ($x != $rl->{t}) ? $x : undef;
 }
 
@@ -1924,15 +1904,10 @@ sub sys { system(@_) == 0 or croak $? }
 sub do_update_index {
 	my ($z_cmd, $cmd, $no_text_base) = @_;
 
-	my $z = open my $p, '-|';
-	defined $z or croak $!;
-	unless ($z) { exec @$z_cmd or croak $! }
+	my ($p, $pctx) = command_output_pipe(@$z_cmd);
 
-	my $pid = open my $ui, '|-';
-	defined $pid or croak $!;
-	unless ($pid) {
-		exec('git-update-index',"--$cmd",'-z','--stdin') or croak $!;
-	}
+	my ($ui, $uctx) = command_input_pipe('update-index',
+	                                     "--$cmd",'-z','--stdin');
 	local $/ = "\0";
 	while (my $x = <$p>) {
 		chomp $x;
@@ -1956,7 +1931,8 @@ sub do_update_index {
 		}
 		print $ui $x,"\0";
 	}
-	close $ui or croak $?;
+	command_close_pipe($p, $pctx);
+	command_close_pipe($ui, $uctx);
 }
 
 sub index_changes {
@@ -1968,10 +1944,10 @@ sub index_changes {
 		close $fd or croak $!;
 	}
 	my $no_text_base = shift;
-	do_update_index([qw/git-diff-files --name-only -z/],
+	do_update_index([qw/diff-files --name-only -z/],
 			'remove',
 			$no_text_base);
-	do_update_index([qw/git-ls-files -z --others/,
+	do_update_index([qw/ls-files -z --others/,
 				"--exclude-from=$GIT_SVN_DIR/info/exclude"],
 			'add',
 			$no_text_base);
@@ -2004,10 +1980,10 @@ sub assert_revision_unknown {
 
 sub trees_eq {
 	my ($x, $y) = @_;
-	my @x = safe_qx('git-cat-file','commit',$x);
-	my @y = safe_qx('git-cat-file','commit',$y);
-	if (($y[0] ne $x[0]) || $x[0] !~ /^tree $sha1\n$/
-				|| $y[0] !~ /^tree $sha1\n$/) {
+	my @x = command(qw/cat-file commit/,$x);
+	my @y = command(qw/cat-file commit/,$y);
+	if (($y[0] ne $x[0]) || $x[0] ne "tree $sha1"
+		             || $y[0] ne "tree $sha1") {
 		print STDERR "Trees not equal: $y[0] != $x[0]\n";
 		return 0
 	}
@@ -2039,33 +2015,24 @@ sub git_commit {
 	if (!defined $tree) {
 		my $index = set_index($GIT_SVN_INDEX);
 		index_changes();
-		chomp($tree = `git-write-tree`);
+		$tree = command_oneline('write-tree');
 		croak $? if $?;
 		restore_index($index);
 	}
 
 	# just in case we clobber the existing ref, we still want that ref
 	# as our parent:
-	open my $null, '>', '/dev/null' or croak $!;
-	open my $stderr, '>&', \*STDERR or croak $!;
-	open STDERR, '>&', $null or croak $!;
-	if (my $cur = eval { safe_qx('git-rev-parse',
-	                             "refs/remotes/$GIT_SVN^0") }) {
+	if (my $cur = verify_ref("refs/remotes/$GIT_SVN^0")) {
 		chomp $cur;
 		push @tmp_parents, $cur;
 	}
-	open STDERR, '>&', $stderr or croak $!;
-	close $stderr or croak $!;
-	close $null or croak $!;
 
 	if (exists $tree_map{$tree}) {
 		foreach my $p (@{$tree_map{$tree}}) {
 			my $skip;
 			foreach (@tmp_parents) {
 				# see if a common parent is found
-				my $mb = eval {
-					safe_qx('git-merge-base', $_, $p)
-				};
+				my $mb = eval { command('merge-base', $_, $p) };
 				next if ($@ || $?);
 				$skip = 1;
 				last;
@@ -2107,7 +2074,7 @@ sub git_commit {
 	if ($commit !~ /^$sha1$/o) {
 		die "Failed to commit, invalid sha1: $commit\n";
 	}
-	sys('git-update-ref',"refs/remotes/$GIT_SVN",$commit);
+	command_noisy('update-ref',"refs/remotes/$GIT_SVN",$commit);
 	revdb_set($REVDB, $log_msg->{revision}, $commit);
 
 	# this output is read via pipe, do not change:
@@ -2119,7 +2086,8 @@ sub git_commit {
 sub check_repack {
 	if ($_repack && (--$_repack_nr == 0)) {
 		$_repack_nr = $_repack;
-		sys("git repack $_repack_flags");
+		# repack doesn't use any arguments with spaces in them, does it?
+		command_noisy('repack', split(/\s+/, $_repack_flags));
 	}
 }
 
@@ -2238,20 +2206,11 @@ sub check_upgrade_needed {
 		open my $fh, '>>',$REVDB or croak $!;
 		close $fh;
 	}
-	my $old = eval {
-		my $pid = open my $child, '-|';
-		defined $pid or croak $!;
-		if ($pid == 0) {
-			close STDERR;
-			exec('git-rev-parse',"$GIT_SVN-HEAD") or croak $!;
-		}
-		my @ret = (<$child>);
-		close $child or croak $?;
-		die $? if $?; # just in case close didn't error out
-		return wantarray ? @ret : join('',@ret);
+	return unless eval {
+		command([qw/rev-parse --verify/,"$GIT_SVN-HEAD^0"],
+		        {STDERR => 0});
 	};
-	return unless $old;
-	my $head = eval { safe_qx('git-rev-parse',"refs/remotes/$GIT_SVN") };
+	my $head = eval { command('rev-parse',"refs/remotes/$GIT_SVN") };
 	if ($@ || !$head) {
 		print STDERR "Please run: $0 rebuild --upgrade\n";
 		exit 1;
@@ -2263,12 +2222,8 @@ sub check_upgrade_needed {
 sub map_tree_joins {
 	my %seen;
 	foreach my $br (@_branch_from) {
-		my $pid = open my $pipe, '-|';
-		defined $pid or croak $!;
-		if ($pid == 0) {
-			exec(qw(git-rev-list --topo-order --pretty=raw), $br)
-								or croak $!;
-		}
+		my $pipe = command_output_pipe(qw/rev-list
+		                            --topo-order --pretty=raw/, $br);
 		while (<$pipe>) {
 			if (/^commit ($sha1)$/o) {
 				my $commit = $1;
@@ -2284,7 +2239,7 @@ sub map_tree_joins {
 				$seen{$commit} = 1;
 			}
 		}
-		close $pipe; # we could be breaking the pipe early
+		eval { command_close_pipe($pipe) };
 	}
 }
 
@@ -2296,7 +2251,7 @@ sub load_all_refs {
 
 	# don't worry about rev-list on non-commit objects/tags,
 	# it shouldn't blow up if a ref is a blob or tree...
-	chomp(@_branch_from = `git-rev-parse --symbolic --all`);
+	@_branch_from = command(qw/rev-parse --symbolic --all/);
 }
 
 # '<svn username> = real-name <email address>' mapping based on git-svnimport:
@@ -2330,7 +2285,7 @@ sub svn_propget_base {
 
 sub git_svn_each {
 	my $sub = shift;
-	foreach (`git-rev-parse --symbolic --all`) {
+	foreach (command(qw/rev-parse --symbolic --all/)) {
 		next unless s#^refs/remotes/##;
 		chomp $_;
 		next unless -f "$GIT_DIR/svn/$_/info/url";
@@ -2371,7 +2326,7 @@ sub migration_check {
 				"$GIT_SVN_DIR\n\t(required for this version ",
 				"($VERSION) of git-svn) does not.\n";
 
-	foreach my $x (`git-rev-parse --symbolic --all`) {
+	foreach my $x (command(qw/rev-parse --symbolic --all/)) {
 		next unless $x =~ s#^refs/remotes/##;
 		chomp $x;
 		next unless -f "$GIT_DIR/$x/info/url";
@@ -2476,11 +2431,7 @@ sub write_grafts {
 		my $p = $grafts->{$c};
 		my %x; # real parents
 		delete $p->{$c}; # commits are not self-reproducing...
-		my $pid = open my $ch, '-|';
-		defined $pid or croak $!;
-		if (!$pid) {
-			exec(qw/git-cat-file commit/, $c) or croak $!;
-		}
+		my $ch = command_output_pipe(qw/cat-file commit/, $c);
 		while (<$ch>) {
 			if (/^parent ($sha1)/) {
 				$x{$1} = $p->{$1} = 1;
@@ -2488,7 +2439,7 @@ sub write_grafts {
 				last unless /^\S/;
 			}
 		}
-		close $ch; # breaking the pipe
+		eval { command_close_pipe($ch) }; # breaking the pipe
 
 		# if real parents are the only ones in the grafts, drop it
 		next if join(' ',sort keys %$p) eq join(' ',sort keys %x);
@@ -2500,7 +2451,7 @@ sub write_grafts {
 			next if $del{$i} || $p->{$i} == 2;
 			foreach my $j (@jp) {
 				next if $i eq $j || $del{$j} || $p->{$j} == 2;
-				$mb = eval { safe_qx('git-merge-base',$i,$j) };
+				$mb = eval { command('merge-base', $i, $j) };
 				next unless $mb;
 				chomp $mb;
 				next if $x{$mb};
@@ -2571,15 +2522,12 @@ sub extract_metadata {
 
 sub cmt_metadata {
 	return extract_metadata((grep(/^git-svn-id: /,
-		safe_qx(qw/git-cat-file commit/, shift)))[-1]);
+		command(qw/cat-file commit/, shift)))[-1]);
 }
 
 sub get_commit_time {
 	my $cmt = shift;
-	defined(my $pid = open my $fh, '-|') or croak $!;
-	if (!$pid) {
-		exec qw/git-rev-list --pretty=raw -n1/, $cmt or croak $!;
-	}
+	my $fh = command_output_pipe(qw/rev-list --pretty=raw -n1/, $cmt);
 	while (<$fh>) {
 		/^committer\s(?:.+) (\d+) ([\-\+]?\d+)$/ or next;
 		my ($s, $tz) = ($1, $2);
@@ -2588,7 +2536,7 @@ sub get_commit_time {
 		} elsif ($tz =~ s/^\-//) {
 			$s -= tz_to_s_offset($tz);
 		}
-		close $fh;
+		eval { command_close_pipe($fh) };
 		return $s;
 	}
 	die "Can't get commit time for commit: $cmt\n";
@@ -2748,7 +2696,6 @@ sub libsvn_load {
 		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.
@@ -2965,7 +2912,7 @@ sub libsvn_get_file {
 	my $mode = exists $props->{'svn:executable'} ? '100755' : '100644';
 	if (exists $props->{'svn:special'}) {
 		$mode = '120000';
-		my $link = `git-cat-file blob $hash`;
+		my $link = `git-cat-file blob $hash`; # no chomping symlinks
 		$link =~ s/^link // or die "svn:special file with contents: <",
 						$link, "> is not understood\n";
 		defined($pid = open3($in, $out, '>&STDERR',
@@ -3069,19 +3016,17 @@ sub libsvn_log_entry {
 sub process_rm {
 	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 $!;
-		if (!$pid) {
-			exec(qw/git-ls-tree -r --name-only -z/,
-				$last_commit,'--',$f) or croak $!;
-		}
+	if (command('ls-tree',$last_commit,'--',$f) =~ /^040000 tree/) {
+		my ($ls, $ctx) = command_output_pipe(qw/ls-tree
+		                                     -r --name-only -z/,
+				                     $last_commit,'--',$f);
 		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 $?;
+		command_close_pipe($ls, $ctx);
 		return $SVN::Node::dir;
 	} else {
 		print $gui '0 ',0 x 40,"\t",$f,"\0" or croak $!;
@@ -3112,7 +3057,7 @@ sub libsvn_fetch_delta {
 
 sub libsvn_fetch_full {
 	my ($last_commit, $paths, $rev, $author, $date, $msg) = @_;
-	open my $gui, '| git-update-index -z --index-info' or croak $!;
+	my ($gui, $ctx) = command_input_pipe(qw/update-index -z --index-info/);
 	my %amr;
 	my $ut = { empty => {}, dir_prop => {}, file_prop => {} };
 	my $p = $SVN->{svn_path};
@@ -3166,20 +3111,14 @@ sub libsvn_fetch_full {
 		%{$ut->{dir_prop}->{''}} = %$props;
 		$pool->clear;
 	}
-	close $gui or croak $?;
+	command_close_pipe($gui, $ctx);
 	libsvn_log_entry($rev, $author, $date, $msg, [$last_commit], $ut);
 }
 
 sub svn_grab_base_rev {
-	defined(my $pid = open my $fh, '-|') or croak $!;
-	if (!$pid) {
-		open my $null, '>', '/dev/null' or croak $!;
-		open STDERR, '>&', $null or croak $!;
-		exec qw/git-rev-parse --verify/,"refs/remotes/$GIT_SVN^0"
-								or croak $!;
-	}
-	chomp(my $c = do { local $/; <$fh> });
-	close $fh;
+	my $c = eval { command_oneline([qw/rev-parse --verify/,
+	                                "refs/remotes/$GIT_SVN^0"],
+				        { STDERR => 0 }) };
 	if (defined $c && length $c) {
 		my ($url, $rev, $uuid) = cmt_metadata($c);
 		return ($rev, $c) if defined $rev;
@@ -3358,7 +3297,7 @@ sub libsvn_find_parent_branch {
 	if (revisions_eq($branch_from, $r0, $r)) {
 		unlink $GIT_SVN_INDEX;
 		print STDERR "Found branch parent: ($GIT_SVN) $parent\n";
-		sys(qw/git-read-tree/, $parent);
+		command_noisy('read-tree', $parent);
 		unless (libsvn_can_do_switch()) {
 			return libsvn_fetch_full($parent, $paths, $rev,
 						$author, $date, $msg);
@@ -3367,7 +3306,7 @@ sub libsvn_find_parent_branch {
 		# included with SVN 1.4.2 (the latest version at the moment),
 		# so we can't rely on it.
 		my $ra = libsvn_connect("$url/$branch_from");
-		my $ed = SVN::Git::Fetcher->new({c => $parent, q => $_q});
+		my $ed = SVN::Git::Fetcher->new({c => $parent, q => $_q });
 		my $pool = SVN::Pool->new;
 		my $reporter = $ra->do_switch($rev, '', 1, $SVN->{url},
 		                              $ed, $pool);
@@ -3413,9 +3352,10 @@ sub libsvn_new_tree {
 		$ut = $ed;
 	} else {
 		$ut = { empty => {}, dir_prop => {}, file_prop => {} };
-		open my $gui, '| git-update-index -z --index-info' or croak $!;
+	        my ($gui, $ctx) = command_input_pipe(qw/update-index
+	                                             -z --index-info/);
 		libsvn_traverse($gui, '', $SVN->{svn_path}, $rev, undef, $ut);
-		close $gui or croak $?;
+		command_close_pipe($gui, $ctx);
 	}
 	libsvn_log_entry($rev, $author, $date, $msg, [], $ut);
 }
@@ -3487,7 +3427,7 @@ sub libsvn_commit_cb {
 		my $log = libsvn_log_entry($rev,$committer,$date,$msg);
 		$log->{tree} = get_tree_from_treeish($c);
 		my $cmt = git_commit($log, $cmt_last, $c);
-		my @diff = safe_qx('git-diff-tree', $cmt, $c);
+		my @diff = command('diff-tree', $cmt, $c);
 		if (@diff) {
 			print STDERR "Trees differ: $cmt $c\n",
 					join('',@diff),"\n";
@@ -3579,8 +3519,8 @@ sub revdb_get {
 sub copy_remote_ref {
 	my $origin = $_cp_remote ? $_cp_remote : 'origin';
 	my $ref = "refs/remotes/$GIT_SVN";
-	if (safe_qx('git-ls-remote', $origin, $ref)) {
-		sys(qw/git fetch/, $origin, "$ref:$ref");
+	if (command('ls-remote', $origin, $ref)) {
+		command_noisy('fetch', $origin, "$ref:$ref");
 	} elsif ($_cp_remote && !$_upgrade) {
 		die "Unable to find remote reference: ",
 				"refs/remotes/$GIT_SVN on $origin\n";
@@ -3592,14 +3532,14 @@ use strict;
 use warnings;
 use Carp qw/croak/;
 use IO::File qw//;
+use Git qw/command command_oneline command_noisy
+           command_output_pipe command_input_pipe command_close_pipe/;
 
 # 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};
 	$self->{q} = $git_svn->{q};
 	$self->{empty} = {};
@@ -3607,6 +3547,8 @@ sub new {
 	$self->{file_prop} = {};
 	$self->{absent_dir} = {};
 	$self->{absent_file} = {};
+	($self->{gui}, $self->{ctx}) = command_input_pipe(
+	                                     qw/update-index -z --index-info/);
 	require Digest::MD5;
 	$self;
 }
@@ -3629,7 +3571,7 @@ sub delete_entry {
 
 sub open_file {
 	my ($self, $path, $pb, $rev) = @_;
-	my ($mode, $blob) = (safe_qx('git-ls-tree',$self->{c},'--',$path)
+	my ($mode, $blob) = (command('ls-tree', $self->{c}, '--',$path)
 	                     =~ /^(\d{6}) blob ([a-f\d]{40})\t/);
 	unless (defined $mode && defined $blob) {
 		die "$path was not found in commit $self->{c} (r$rev)\n";
@@ -3764,13 +3706,13 @@ sub close_file {
 
 sub abort_edit {
 	my $self = shift;
-	close $self->{gui};
+	eval { command_close_pipe($self->{gui}, $self->{ctx}) };
 	$self->SUPER::abort_edit(@_);
 }
 
 sub close_edit {
 	my $self = shift;
-	close $self->{gui} or croak $!;
+	command_close_pipe($self->{gui}, $self->{ctx});
 	$self->{git_commit_ok} = 1;
 	$self->SUPER::close_edit(@_);
 }
@@ -3781,6 +3723,8 @@ use strict;
 use warnings;
 use Carp qw/croak/;
 use IO::File;
+use Git qw/command command_oneline command_noisy
+           command_output_pipe command_input_pipe command_close_pipe/;
 
 sub new {
 	my $class = shift;
@@ -3830,10 +3774,8 @@ sub rmdirs {
 	delete $rm->{''}; # we never delete the url we're tracking
 	return unless %$rm;
 
-	defined(my $pid = open my $fh,'-|') or croak $!;
-	if (!$pid) {
-		exec qw/git-ls-tree --name-only -r -z/, $self->{c} or croak $!;
-	}
+	my ($fh, $ctx) = command_output_pipe(
+	                           qw/ls-tree --name-only -r -z/, $self->{c});
 	local $/ = "\0";
 	while (<$fh>) {
 		chomp;
@@ -3842,11 +3784,11 @@ sub rmdirs {
 			delete $rm->{join '/', @dn};
 		}
 		unless (%$rm) {
-			close $fh;
+			eval { command_close_pipe($fh) };
 			return;
 		}
 	}
-	close $fh;
+	command_close_pipe($fh, $ctx);
 
 	my ($r, $p, $bat) = ($self->{r}, $self->{pool}, $self->{bat});
 	foreach my $d (sort { $b =~ tr#/#/# <=> $a =~ tr#/#/# } keys %$rm) {
-- 
1.4.4.2.g0817

^ permalink raw reply related

* Re: [RFC \ WISH] Add -o option to git-rev-list
From: Linus Torvalds @ 2006-12-15 19:20 UTC (permalink / raw)
  To: Marco Costalba
  Cc: Git Mailing List, Josef Weidendorfer, Junio C Hamano, Alex Riesen,
	Shawn Pearce
In-Reply-To: <e5bfff550612151045q5782e1f2j8686ccab24dbf566@mail.gmail.com>



On Fri, 15 Dec 2006, Marco Costalba wrote:
> 
> Warmed-up cache
> QProcess 7632ms (500ms data read interval)
> QProcess 7972ms (100ms data read interval)

Why do you even bother posting numbers, when multiple people have told you 
that the numbers you post are meaningless?

As long as you throttle the writer by not reading data in a timely fashion 
(by using poll() or select() in the main loop and reading when it's 
available), and you continue to talk about "data read intervals", all your 
numbers are CRAP.

OF COURSE a temp-file will work better, because then you basically have an 
infinite buffer, and data read intervals don't _matter_. But we've tried 
to tell you that this is not because temp-files are better, but because 
your reader artificially throttles the writer for a pipe.


^ permalink raw reply

* Re: [RFC/PATCH] git-svn: convert to using Git.pm
From: Junio C Hamano @ 2006-12-15 19:47 UTC (permalink / raw)
  To: Eric Wong; +Cc: git
In-Reply-To: <11662091941543-git-send-email-normalperson@yhbt.net>

Eric Wong <normalperson@yhbt.net> writes:

> Thanks to Git.pm, I've been able to greatly reduce the amount
> of extra work that needs to be done to manage input/output
> pipes in Perl.
>
> chomp usage has also been greatly reduced, too.
>
> All tests (including full-svn-test) still pass, but this has
> not been tested extensively in the real-world.
>
> Signed-off-by: Eric Wong <normalperson@yhbt.net>
> ---
>
> Junio: please don't apply this to master just yet.

Will queue for 'next'.  Thanks for the warning.

^ permalink raw reply

* Re: [PATCH] make commit message a little more consistent and conforting
From: Nicolas Pitre @ 2006-12-15 20:13 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Shawn Pearce, Andreas Ericsson, Andy Parkins, git
In-Reply-To: <7vvekdvxeo.fsf@assigned-by-dhcp.cox.net>

On Fri, 15 Dec 2006, Junio C Hamano wrote:

> Nicolas Pitre <nico@cam.org> writes:
> 
> > So in short I do think there should be something shown after a 
> > successful commit, and including the commit sha1 doesn't hurt.
> > ...
> > And it is true that diffstat can be quite large.  I wouldn't mind the 
> > diffstat to be added to the commit message summary in the text editor 
> > though.  And displaying it when -v is used makes also a lot of sense.  
> > But not by default please.
> 
> I agree with everything you said in your message, including that
> commit object name might help as a learning aid.
> 
> We could give something like this, though, if we wanted to:
> 
> 	$ git commit
>         4 files changed, 17 insertions(+), 10 deletions(-)
>         mode change 100755 => 100644 test.sh

Actually that would really be nice to have all the time for the
diff --summary output whenever --stat is not provided.  Or maybe a 
--shortstat option.

What about this (on top of my previous patch):

Signed-off-by: Nicolas Pitre <nico@cam.org>

---

diff --git a/Documentation/diff-options.txt b/Documentation/diff-options.txt
index 9cdd171..f12082e 100644
--- a/Documentation/diff-options.txt
+++ b/Documentation/diff-options.txt
@@ -21,6 +21,11 @@
 	deleted lines in decimal notation and pathname without
 	abbreviation, to make it more machine friendly.
 
+--shortstat::
+	Output only the last line of the --stat format containing total
+	number of modified files, as well as number of added and deleted
+	lines.
+
 --summary::
 	Output a condensed summary of extended header information
 	such as creations, renames and mode changes.
diff --git a/diff.c b/diff.c
index 0b284b3..d754280 100644
--- a/diff.c
+++ b/diff.c
@@ -809,6 +809,35 @@ static void show_stats(struct diffstat_t* data, struct diff_options *options)
 	       set, total_files, adds, dels, reset);
 }
 
+static void show_shortstats(struct diffstat_t* data)
+{
+	int i, adds = 0, dels = 0, total_files = data->nr;
+
+	if (data->nr == 0)
+		return;
+
+	for (i = 0; i < data->nr; i++) {
+		if (!data->files[i]->is_binary &&
+		    !data->files[i]->is_unmerged) {
+			int added = data->files[i]->added;
+			int deleted= data->files[i]->deleted;
+			if (!data->files[i]->is_renamed &&
+			    (added + deleted == 0)) {
+				total_files--;
+			} else {
+				adds += added;
+				dels += deleted;
+			}
+		}
+		free(data->files[i]->name);
+		free(data->files[i]);
+	}
+	free(data->files);
+
+	printf(" %d files changed, %d insertions(+), %d deletions(-)\n",
+	       total_files, adds, dels);
+}
+
 static void show_numstat(struct diffstat_t* data, struct diff_options *options)
 {
 	int i;
@@ -1767,6 +1796,7 @@ int diff_setup_done(struct diff_options *options)
 		options->output_format &= ~(DIFF_FORMAT_RAW |
 					    DIFF_FORMAT_NUMSTAT |
 					    DIFF_FORMAT_DIFFSTAT |
+					    DIFF_FORMAT_SHORTSTAT |
 					    DIFF_FORMAT_SUMMARY |
 					    DIFF_FORMAT_PATCH);
 
@@ -1777,6 +1807,7 @@ int diff_setup_done(struct diff_options *options)
 	if (options->output_format & (DIFF_FORMAT_PATCH |
 				      DIFF_FORMAT_NUMSTAT |
 				      DIFF_FORMAT_DIFFSTAT |
+				      DIFF_FORMAT_SHORTSTAT |
 				      DIFF_FORMAT_SUMMARY |
 				      DIFF_FORMAT_CHECKDIFF))
 		options->recursive = 1;
@@ -1868,6 +1899,9 @@ int diff_opt_parse(struct diff_options *options, const char **av, int ac)
 	else if (!strcmp(arg, "--numstat")) {
 		options->output_format |= DIFF_FORMAT_NUMSTAT;
 	}
+	else if (!strcmp(arg, "--shortstat")) {
+		options->output_format |= DIFF_FORMAT_SHORTSTAT;
+	}
 	else if (!strncmp(arg, "--stat", 6)) {
 		char *end;
 		int width = options->stat_width;
@@ -2642,7 +2676,7 @@ void diff_flush(struct diff_options *options)
 		separator++;
 	}
 
-	if (output_format & (DIFF_FORMAT_DIFFSTAT|DIFF_FORMAT_NUMSTAT)) {
+	if (output_format & (DIFF_FORMAT_DIFFSTAT|DIFF_FORMAT_SHORTSTAT|DIFF_FORMAT_NUMSTAT)) {
 		struct diffstat_t diffstat;
 
 		memset(&diffstat, 0, sizeof(struct diffstat_t));
@@ -2656,6 +2690,8 @@ void diff_flush(struct diff_options *options)
 			show_numstat(&diffstat, options);
 		if (output_format & DIFF_FORMAT_DIFFSTAT)
 			show_stats(&diffstat, options);
+		else if (output_format & DIFF_FORMAT_SHORTSTAT)
+			show_shortstats(&diffstat);
 		separator++;
 	}
 
diff --git a/diff.h b/diff.h
index 101b2b5..eff4455 100644
--- a/diff.h
+++ b/diff.h
@@ -29,6 +29,7 @@ typedef void (*diff_format_fn_t)(struct diff_queue_struct *q,
 #define DIFF_FORMAT_NUMSTAT	0x0004
 #define DIFF_FORMAT_SUMMARY	0x0008
 #define DIFF_FORMAT_PATCH	0x0010
+#define DIFF_FORMAT_SHORTSTAT	0x0020
 
 /* These override all above */
 #define DIFF_FORMAT_NAME	0x0100
diff --git a/git-commit.sh b/git-commit.sh
index 395bcd2..b9e49ea 100755
--- a/git-commit.sh
+++ b/git-commit.sh
@@ -629,7 +629,7 @@ then
 	if test -z "$quiet"
 	then
 		echo "Created${initial_commit:+ initial} commit $commit"
-		git-diff-tree --summary --root --no-commit-id HEAD
+		git-diff-tree --shortstat --summary --root --no-commit-id HEAD
 	fi
 fi

^ permalink raw reply related

* Re: svn versus git
From: Johannes Schindelin @ 2006-12-15 20:15 UTC (permalink / raw)
  To: Nguyen Thai Ngoc Duy; +Cc: git
In-Reply-To: <fcaeb9bf0612150726o40527552l8b3564ddcc3adb94@mail.gmail.com>

Hi,

On Fri, 15 Dec 2006, Nguyen Thai Ngoc Duy wrote:

> About adding index support to git-show, yes it's really messy. index
> doesn't have tree objects.

Insofar, it is not messy: git-show only shows _objects_. For example, "git 
show :README" works as expected if you have a file called "README" in the 
index...

Ciao,
Dscho

^ permalink raw reply

* Re: svn versus git
From: Johannes Schindelin @ 2006-12-15 20:19 UTC (permalink / raw)
  To: Nguyen Thai Ngoc Duy; +Cc: git
In-Reply-To: <Pine.LNX.4.63.0612152115000.3635@wbgn013.biozentrum.uni-wuerzburg.de>

Hi,

On Fri, 15 Dec 2006, Johannes Schindelin wrote:

> On Fri, 15 Dec 2006, Nguyen Thai Ngoc Duy wrote:
> 
> > About adding index support to git-show, yes it's really messy. index
> > doesn't have tree objects.
> 
> Insofar, it is not messy: git-show only shows _objects_. For example, "git 
> show :README" works as expected if you have a file called "README" in the 
> index...

Note: this is not completely true. The index contains cache_trees. But 
they do not need to be up-to-date, which would mean that "git show :./" 
would have to remake the cache_tree, and _then_ show it.

Ciao,
Dscho

^ permalink raw reply

* Re: [RFC \ WISH] Add -o option to git-rev-list
From: Marco Costalba @ 2006-12-15 20:41 UTC (permalink / raw)
  To: Linus Torvalds
  Cc: Git Mailing List, Josef Weidendorfer, Junio C Hamano, Alex Riesen,
	Shawn Pearce
In-Reply-To: <Pine.LNX.4.64.0612151118270.3849@woody.osdl.org>

On 12/15/06, Linus Torvalds <torvalds@osdl.org> wrote:
>
>
> On Fri, 15 Dec 2006, Marco Costalba wrote:
> >
> > Warmed-up cache
> > QProcess 7632ms (500ms data read interval)
> > QProcess 7972ms (100ms data read interval)
>
> Why do you even bother posting numbers, when multiple people have told you
> that the numbers you post are meaningless?
>
> As long as you throttle the writer by not reading data in a timely fashion
> (by using poll() or select() in the main loop and reading when it's
> available), and you continue to talk about "data read intervals", all your
> numbers are CRAP.
>

QProcess implementation is like this FWIK (from
qt-x11-free-3.3.4/src/kernel/qprocess_unix.cpp):

The SIGCHLD handler writes to a socket to tell the manager that
something happened. Then in  QProcessManager::sigchldHnd() data is read by

process->socketRead( proc->socketStdout );


In QProcess::socketRead() a call to C function read() is done to get
4KB of data:

const int basize = 4096;
QByteArray *ba = new QByteArray( basize );
n = ::read( fd, ba->data(), basize );


The pointer ba is then appended to a pointer list.

This happens _ALWAYS_ indipendently of _how_ the application calls the
Qt library for reading.

When application read recieved data with QProcess::readStdout(), then
data stored in buffers pointed by the pointer list is memcpy() to a
temporary array that is the return value of QProcess::readStdout().
See in qt-x11-free-3.3.4/src/kernel/qprocess.cpp:

QByteArray QProcess::readStdout()
{
    if ( readStdoutCalled ) {
	return QByteArray();
    }
    readStdoutCalled = TRUE;
    QMembuf *buf = membufStdout();
    readStdoutCalled = FALSE;

    return buf->readAll();  // here a memcpy() is involved
}


So it' true that we use a timeout, but only to trigger a memcpy() from
Qt internal library buffers that, instead are feeded as soon as
possible by Qt implementation.

IOW the timely select() is already done by Qt library. We just read
what has been already received and stored.

   Marco


^ permalink raw reply

* [PATCH] gitweb: Add "next" link to commit view
From: Jakub Narebski @ 2006-12-15 20:57 UTC (permalink / raw)
  To: git; +Cc: Jakub Narebski

Add a kind of "next" view in the bottom part of navigation bar for
"commit" view, similar to what was added for "commitdiff" view in
commit 151602df00b8e5c5b4a8193f59a94b85f9b5aebc
  'gitweb: Add "next" link to commitdiff view'

For "commit" view for single parent commit:
  (parent: _commit_)
For "commit" view for merge (multi-parent) commit:
  (merge: _commit_ _commit_ ...)
For "commit" view for root (parentless) commit
  (initial)
where _link_ denotes hyperlink.  SHA1 of commit is shortened
to 7 characters on display.

While at it, remove leftovers from commit cae1862a by Petr Baudis:
  'gitweb: More per-view navigation bar links'
namely the "blame" link if there exist $file_name and commit has a
parent; it was added in git_commit probably by mistake.  The rest
of what mentioned commit added for git_commit was removed in
commit 6e0e92fda893311ff5af91836e5007bf6bbd4a21 by Luben Tuikov:
  'gitweb: Do not print "log" and "shortlog" redundantly in commit view'
(which should have probably removed also this "blame" link removed now).


Signed-off-by: Jakub Narebski <jnareb@gmail.com>
---
By the way, the history shown above was found using git 'pickaxe',
namely
  $ git log -p -S'my @views_nav = ();' -- gitweb/
which found the first commit. I have noticed that some parts of
what this patch added to git_commit were removed later:
  $ git log -p -S'push @views_nav,' -- gitweb/
found the second commit mentioned.

The first part could be found also by git-blame command (or
any other annotate/blame command in any other SCM), but to
notice the second...


 gitweb/gitweb.perl |   38 +++++++++++++++++++++++++++++---------
 1 files changed, 29 insertions(+), 9 deletions(-)

diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index ebf35a1..902c514 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -3571,8 +3571,34 @@ sub git_commit {
 	my %ad = parse_date($co{'author_epoch'}, $co{'author_tz'});
 	my %cd = parse_date($co{'committer_epoch'}, $co{'committer_tz'});
 
-	my $parent = $co{'parent'};
-	my $parents = $co{'parents'};
+	my $parent  = $co{'parent'};
+	my $parents = $co{'parents'}; # listref
+
+	# we need to prepare $formats_nav before any parameter munging
+	my $formats_nav;
+	if (!defined $parent) {
+		# --root commitdiff
+		$formats_nav .= '(initial)';
+	} elsif (@$parents == 1) {
+		# single parent commit
+		$formats_nav .=
+			'(parent: ' .
+			$cgi->a({-href => href(action=>"commit",
+			                       hash=>$parent)},
+			        esc_html(substr($parent, 0, 7))) .
+			')';
+	} else {
+		# merge commit
+		$formats_nav .=
+			'(merge: ' .
+			join(' ', map {
+				$cgi->a({-href => href(action=>"commitdiff",
+				                       hash=>$_)},
+				        esc_html(substr($_, 0, 7)));
+			} @$parents ) .
+			')';
+	}
+
 	if (!defined $parent) {
 		$parent = "--root";
 	}
@@ -3596,16 +3622,10 @@ sub git_commit {
 
 	my $have_snapshot = gitweb_have_snapshot();
 
-	my @views_nav = ();
-	if (defined $file_name && defined $co{'parent'}) {
-		push @views_nav,
-			$cgi->a({-href => href(action=>"blame", hash_parent=>$parent, file_name=>$file_name)},
-			        "blame");
-	}
 	git_header_html(undef, $expires);
 	git_print_page_nav('commit', '',
 	                   $hash, $co{'tree'}, $hash,
-	                   join (' | ', @views_nav));
+	                   $formats_nav);
 
 	if (defined $co{'parent'}) {
 		git_print_header_div('commitdiff', esc_html($co{'title'}) . $ref, $hash);
-- 
1.4.4.1

^ permalink raw reply related

* Re: Avoiding uninteresting merges in Cairo
From: Junio C Hamano @ 2006-12-15 21:03 UTC (permalink / raw)
  To: Carl Worth; +Cc: git, Shawn Pearce
In-Reply-To: <87tzzx4zm7.wl%cworth@cworth.org>

Carl Worth <cworth@cworth.org> writes:

> On Thu, 14 Dec 2006 21:06:29 -0500, Shawn Pearce wrote:
>> ...
>> Cairo has seriously been using `reset --hard HEAD^` as part of its
>> workflow since April?
> ...
> Also, I don't actually need this. I don't use the "reset --hard"
> workflow suggested in the mail above. I always obtain remote changes
> with "git fetch" and then examine things locally and decide to either
> merge (or fast forward) with "git pull", (though maybe I'll start
> using "git merge" now), or else to use "git rebase" to avoid the noisy
> merge commits.

I wish I have more time to keep track of "recommended workflows"
that projects using git give to its developers.

I think what you recommend makes quite a lot of sense, for
very centralized setting, and would leave much more usable
history than the simple-minded way cvs-migration suggests.



^ permalink raw reply

* Re: [RFC \ WISH] Add -o option to git-rev-list
From: Marco Costalba @ 2006-12-15 21:04 UTC (permalink / raw)
  To: Linus Torvalds
  Cc: Git Mailing List, Josef Weidendorfer, Junio C Hamano, Alex Riesen,
	Shawn Pearce
In-Reply-To: <e5bfff550612151241pd11f49eqd1666ce0b33855b6@mail.gmail.com>

On 12/15/06, Marco Costalba <mcostalba@gmail.com> wrote:
>
> IOW the timely select() is already done by Qt library. We just read
> what has been already received and stored.
>

Try to set the timer to a very high value so that never triggers and
there is only one read call at the end, after git-rev-list exited,
when there is a forced call to the data read function.

Times seems to get even better! This is because timely read of the
pipe is _already_ done inside Qt library, not by qgit application.


^ permalink raw reply

* Re: [RFC] requiring Perl SVN libraries for git-svn
From: Seth Falcon @ 2006-12-15 21:06 UTC (permalink / raw)
  To: git
In-Reply-To: <20061215184424.GA1442@localdomain>

Eric Wong <normalperson@yhbt.net> writes:

> Eric Wong <normalperson@yhbt.net> wrote:
>> Are there any git-svn users out there who would be seriously hurt if I
>> dropped support for using the svn command-line client in git-svn?
>
> One additional comment is that the dcommit command (much favored over
> 'git svn commit') does not currently work with the command-line
> client.

Which IMO, suggests that you should drop command-line support asap.
Those wanting command-line support are most likely to be beginning
users who almost certainly want to be using dcommit.

(that makes $0.04)


^ permalink raw reply

* [PATCH] gitweb: Add some mod_perl specific support
From: Jakub Narebski @ 2006-12-15 21:18 UTC (permalink / raw)
  To: git; +Cc: Jakub Narebski

Add $r variable which holds Apache::RequestRec if script is run under
mod_perl (if $ENV{MOD_PERL} is defined). It is used as argument to
constructor of CGI object (needs CGI module version at least 2.93).
It is needed for further mod_perl support, for example adding
headers using Apache::RequestRec methods instead of making Apache
to have to parse headers (to add it's own HTTP headers like Server:
header).

Following advice from CGI(3pm) man page, precompile all CGI routines
for mod_perl.

Use $r->path_info() instead of $ENV{"PATH_INFO"}.

All this makes gitweb slightly faster under mod_perl (436 +/-  23.9 ms
for summary of git.git before, 429 +/- 12.0 ms after, according to
'ab -n 10 -k "http://localhost/perl/gitweb/gitweb.cgi?p=git.git"').

Signed-off-by: Jakub Narebski <jnareb@gmail.com>
---
This is the same patch as previous
  'gitweb: Sprinkle some mod_perl goodies'

 gitweb/gitweb.perl |   11 +++++++++--
 1 files changed, 9 insertions(+), 2 deletions(-)

diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index 902c514..7df253c 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -18,11 +18,18 @@ use File::Find qw();
 use File::Basename qw(basename);
 binmode STDOUT, ':utf8';
 
-our $cgi = new CGI;
+# mod_perl request
+my $r;
+$r = shift @_ if $ENV{MOD_PERL};
+
+our $cgi = new CGI($r);
 our $version = "++GIT_VERSION++";
 our $my_url = $cgi->url();
 our $my_uri = $cgi->url(-absolute => 1);
 
+# speeding up mod_perl and FastCGI (later)
+$cgi->compile() if $r;
+
 # core git executable to use
 # this can just be "git" if your webserver has a sensible PATH
 our $GIT = "++GIT_BINDIR++/git";
@@ -364,7 +371,7 @@ if (defined $searchtype) {
 # now read PATH_INFO and use it as alternative to parameters
 sub evaluate_path_info {
 	return if defined $project;
-	my $path_info = $ENV{"PATH_INFO"};
+	my $path_info = $r ? $r->path_info() : $ENV{"PATH_INFO"};
 	return if !$path_info;
 	$path_info =~ s,^/+,,;
 	return if !$path_info;
-- 
1.4.4.1

^ permalink raw reply related

* Re: [DRAFT 2] Branching and merging with git
From: Jakub Narebski @ 2006-12-15 21:38 UTC (permalink / raw)
  To: git
In-Reply-To: <20061120235136.4841.qmail@science.horizon.com>

linux@horizon.com wrote:

> I tried to incorporate all the suggestions.  There are still a few things
> I have to research, and now I'm worried it's getting too long.  Sigh.

Tutorials can (and usually are) be long, don't worry.
 

Could you resend this as patch creating Documentation/tutorial-3.txt
This way it would be in the repository, and people would be able to correct
this (I guess that it at first would appear in 'next' branch)...
-- 
Jakub Narebski
Warsaw, Poland
ShadeHawk on #git


^ permalink raw reply

* Re: [DRAFT 2] Branching and merging with git
From: J. Bruce Fields @ 2006-12-15 21:41 UTC (permalink / raw)
  To: Jakub Narebski; +Cc: git
In-Reply-To: <elv4f5$kcj$1@sea.gmane.org>

On Fri, Dec 15, 2006 at 10:38:05PM +0100, Jakub Narebski wrote:
> linux@horizon.com wrote:
> 
> > I tried to incorporate all the suggestions.  There are still a few things
> > I have to research, and now I'm worried it's getting too long.  Sigh.
> 
> Tutorials can (and usually are) be long, don't worry.
>  
> 
> Could you resend this as patch creating Documentation/tutorial-3.txt
> This way it would be in the repository, and people would be able to correct
> this (I guess that it at first would appear in 'next' branch)...

Yes please.  Even if it has some problems or isn't really a perfect fit
in the current tutorial sequence, we can fix that later.


^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox