Git development
 help / color / mirror / Atom feed
* Re: [PATCH 0/2] tagsize < 8kb restriction
From: Linus Torvalds @ 2006-05-23  0:02 UTC (permalink / raw)
  To: Sean; +Cc: Junio C Hamano, BjEngelmann, git
In-Reply-To: <BAYC1-PASMTP1164FE2A24B4D1B4C0A607AE9A0@CEZ.ICE>



On Mon, 22 May 2006, Sean wrote:
> What seems to becoming clear as more people find new ways to use
> git is that many of them would be well served by having a solid
> infrastructure to handle metadata.  Consider the case above: _git_
> itself doesn't need a structural reference, but users and external
> applications definitely need to be able to lookup which metadata
> is associated with any given commit.  Having a git standard for
> this type of data would help.  Tags already do this, so they're
> likely to be used and abused in ways not initially envisioned,
> just because git doesn't have another such facility.

I definitely think we should allow arbitrary tags.

That said, I think that what you actually want to do may be totally 
different.

If _each_ commit has some extra information associated with it, you don't 
want to create a tag that points to the commit, you more likely want to 
create an object that is indexed by the commit ID rather than the other 
way around.

IOW, I _think_ that what you described would be that if you have the 
commit ID, you want to find the data based on that ID. No?

And that you can do quite easily, while _also_ using git to distribute the 
extra per-commit meta-data. Just create a separate branch that has the 
data indexed by commit ID. That could be as simple as having one file per 
commit (using, perhaps, a similar directory layout as the .git/objects/ 
directory itself), and then you could do something like

	# Get the SHA1 of the named commit
	commit=$(git-rev-parse --verify "$cmitname"^0)

	# turn it into a filename (slash between two first chars and the rest)
	filename=$(echo $commit | sed 's:^\(..\)\(.*\):\1/\2:')

	# look it up in the "annotations" branch
	git cat-file blob "annotations:$filename"

which gets the data from the "annotations" branch, indexed by the SHA1 
name.

Now, everybody can track your "annotations" branch using git, and get your 
per-commit annotations for the main branch.

See?

The real advantage of tags is that you can use them for the SHA1 
expressions, and follow them automatically. If that's what you want (ie 
you don't want to index things by the commit SHA1, but by some external 
name, like the name the commit had in some other repository), then by all 
means use tags. But if you just want to associate some data with each 
commit, the above "separate branch for annotations" approach is much more 
efficient.

		Linus

^ permalink raw reply

* cogito testsuite failure - git-read-tree too careful
From: Pavel Roskin @ 2006-05-23  0:28 UTC (permalink / raw)
  To: Petr Baudis, git

Hello, Petr!

The testsuite for cogito fails in t9300-seek.sh for up-to-date master
branches of git and Cogito.

$ ./t9300-seek.sh -v -i
* expecting success: cg-seek cbd273f56aecaaf28b857ae74da77cbb11a4d659
Warning: uncommitted local changes, trying to bring them along
fatal: Entry 'newdir/newfile' not uptodate. Cannot merge.
cg-seek: cbd273f56aecaaf28b857ae74da77cbb11a4d659: bad commit
* FAIL 21: seeking to the first commit
        cg-seek cbd273f56aecaaf28b857ae74da77cbb11a4d659

As I understand it, "git-read-tree -m" in cg-Xlib refuses to merge if
there are local changes.  This was likely caused by commit
fcc387db9bc453dc7e07a262873481af2ee9e5c8:

read-tree -m -u: do not overwrite or remove untracked working tree
files.
    
I guess git-read-tree should be using "--reset" or something to restore
the original behavior.

The "tutorial" testsuite also fails:

Should not be doing an Octopus.
No merge strategy handled the merge.
Merging 5de8995e58b4b478dff476788c3607ed5021fc24 ->
ba8b9edd80500d60d68a6630ee415a3e710f6db2
        to a60f36f73018dc1959d8d2cbd28271f93ee5f686 ...
fatal: Untracked working tree file 'stack.h' would be overwritten by
merge.
cg-merge: git-read-tree failed (merge likely blocked by local changes)
162
?
Unexpected error 4 on line 242

In this case, we may want cg-merge to fail, because it's wrong to
overwrite local files without backing them up.

-- 
Regards,
Pavel Roskin

^ permalink raw reply

* [PATCH] Avoid segfault in diff --stat rename output.
From: Sean @ 2006-05-23  0:36 UTC (permalink / raw)
  To: Torgil Svensson; +Cc: git
In-Reply-To: <e7bda7770605221609h7c18c2ccpe92db34050d46f9f@mail.gmail.com>


Signed-off-by: Sean Estabrooks <seanlkml@sympatico.ca>
---
 diff.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

On Tue, 23 May 2006 01:09:43 +0200
"Torgil Svensson" <torgil.svensson@gmail.com> wrote:

> Hi
> 
> It seems like git-diff-tree has some problems with moved files:
> 
> $ git-diff-tree -p --stat --summary -M
> 348f179e3195448cea49c98a79cce8c7f446ce26
> 343ca16424ba031b37e4df49afddaee098a8f347 | wc -l
> *** glibc detected *** free(): invalid pointer: 0x12ecbbf0 ***
> 6101


diff --git a/diff.c b/diff.c
index 7f35e59..a7bb9b9 100644
--- a/diff.c
+++ b/diff.c
@@ -237,7 +237,7 @@ static char *pprint_rename(const char *a
 		if (a_midlen < 0) a_midlen = 0;
 		if (b_midlen < 0) b_midlen = 0;
 
-		name = xmalloc(len_a + len_b - pfx_length - sfx_length + 7);
+		name = xmalloc(pfx_length + a_midlen + b_midlen + sfx_length + 7);
 		sprintf(name, "%.*s{%.*s => %.*s}%s",
 			pfx_length, a,
 			a_midlen, a + pfx_length,
-- 
1.3.GIT

^ permalink raw reply related

* Re: [PATCH] Avoid segfault in diff --stat rename output.
From: Junio C Hamano @ 2006-05-23  1:02 UTC (permalink / raw)
  To: Sean; +Cc: git
In-Reply-To: <BAYC1-PASMTP115C9137E5BDABD705881BAE9B0@CEZ.ICE>

Sean <seanlkml@sympatico.ca> writes:

> Signed-off-by: Sean Estabrooks <seanlkml@sympatico.ca>
> ---
>  diff.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/diff.c b/diff.c
> index 7f35e59..a7bb9b9 100644
> --- a/diff.c
> +++ b/diff.c
> @@ -237,7 +237,7 @@ static char *pprint_rename(const char *a
>  		if (a_midlen < 0) a_midlen = 0;
>  		if (b_midlen < 0) b_midlen = 0;
>  
> -		name = xmalloc(len_a + len_b - pfx_length - sfx_length + 7);
> +		name = xmalloc(pfx_length + a_midlen + b_midlen + sfx_length + 7);
>  		sprintf(name, "%.*s{%.*s => %.*s}%s",

Obviously correct given what the sprintf() that immediately
follows does.  Sheesh, what was I smoking back then.  *BLUSH*

Thanks.

^ permalink raw reply

* Re: [PATCH] git status: ignore empty directories (because they cannot be added)
From: Junio C Hamano @ 2006-05-23  1:11 UTC (permalink / raw)
  To: Matthias Lederhofer; +Cc: git
In-Reply-To: <E1FiHXS-0008MC-LB@moooo.ath.cx>

Matthias Lederhofer <matled@gmx.net> writes:

> and a new option -u / --untracked-files to show files in untracked
> directories.
>
> ---
> A few things I'm not sure about:
> - Should there be another option to disable --no-empty-directory?

I am not sure about this.  We used to show everything in a
directory full of untracked directory, which was distracting and
that was the reason we added --directory there.  Maybe it would
be less confusing if we just updated the message

	    print "#\n# Untracked files:\n";
	    print "#   (use \"git add\" to add to commit)\n";
	    print "#\n";

to say "use 'git add' on these files and files in these
directories you wish to add", or something silly like that,
without this patch?

^ permalink raw reply

* Re: [PATCH] Change GIT-VERSION-GEN to call git commands with "git" not "git-".
From: Junio C Hamano @ 2006-05-23  1:20 UTC (permalink / raw)
  To: git
In-Reply-To: <BAYC1-PASMTP09B22AA86724B4F2C01F7FAE9A0@CEZ.ICE>

Sean <seanlkml@sympatico.ca> writes:

> GIT-VERSION-GEN can incorrectly return a default version of
> "v1.3.GIT" because it tries to execute git commands using the
> "git-cmd" format that expects all git commands to be in the $PATH.
> Convert these to  "git cmd" format so that a proper answer is
> returned even when the git commands have been moved out of the
> $PATH and into a $gitexecdir.

IIRC, the reason we spelled it as "git-describe"
with a dash is ancient git wrapper said "not a git command" when
given "describe" which it did not understand without failing.

I think it has been long enough since we introduced "git describe",
so this would be OK. 

^ permalink raw reply

* Re: [PATCH] cvsimport: introduce -L<imit> option to workaround memory leaks
From: Linus Torvalds @ 2006-05-23  2:28 UTC (permalink / raw)
  To: Martin Langhoff
  Cc: Git Mailing List, Junio C Hamano, Johannes.Schindelin, spyderous,
	smurf
In-Reply-To: <11482978883713-git-send-email-martin@catalyst.net.nz>



This stupid patch on top of yours seems to make git happier. It's 
disgusting, I know, but it just repacks things every kilo-commit.

I actually think that I found a real ext3 performance bug from trying to 
determine why git sometimes slows down ridiculously when the tree has been 
allowed to go too long without a repack.

		Linus

---
diff --git a/git-cvsimport.perl b/git-cvsimport.perl
index fb56278..c141f5e 100755
--- a/git-cvsimport.perl
+++ b/git-cvsimport.perl
@@ -853,10 +853,14 @@ #	VERSION:1.96->1.96.2.1
 	} elsif($state == 9 and /^\s*$/) {
 		$state = 10;
 	} elsif(($state == 9 or $state == 10) and /^-+$/) {
-		if ($opt_L && $commitcount++ >= $opt_L) {
+		$commitcount++;
+		if ($opt_L && $commitcount > $opt_L) {
 			last;
 		}
 		commit();
+		if (($commitcount & 1023) == 0) {
+			system("git repack -a -d");
+		}
 		$state = 1;
 	} elsif($state == 11 and /^-+$/) {
 		$state = 1;

^ permalink raw reply related

* Re: [PATCH] cvsimport: introduce -L<imit> option to workaround memory leaks
From: Martin Langhoff (CatalystIT) @ 2006-05-23  3:15 UTC (permalink / raw)
  To: Linus Torvalds
  Cc: Git Mailing List, Junio C Hamano, Johannes.Schindelin, spyderous,
	smurf
In-Reply-To: <Pine.LNX.4.64.0605221926270.3697@g5.osdl.org>

Linus Torvalds wrote:

> 
> This stupid patch on top of yours seems to make git happier. It's 
> disgusting, I know, but it just repacks things every kilo-commit.
> 
> I actually think that I found a real ext3 performance bug from trying to 
> determine why git sometimes slows down ridiculously when the tree has been 
> allowed to go too long without a repack.
> 

Acked (in case anyone cares for such an obvious one), and thanks! I 
thought of doing that last night together with that exact patch, but I 
was focussing on the leak.

cheers,


m
-- 
-----------------------------------------------------------------------
Martin @ Catalyst .Net .NZ  Ltd, PO Box 11-053, Manners St,  Wellington
WEB: http://catalyst.net.nz/           PHYS: Level 2, 150-154 Willis St
OFFICE: +64(4)916-7224                              MOB: +64(21)364-017
       Make things as simple as possible, but no simpler - Einstein
-----------------------------------------------------------------------

^ permalink raw reply

* Re: Git 1.3.2 on Solaris
From: Stefan Pfetzing @ 2006-05-23  3:20 UTC (permalink / raw)
  To: Git Mailing List
In-Reply-To: <6471.1147883724@lotus.CS.Berkeley.EDU>

Hi Jason,

2006/5/17, Jason Riedy <ejr@eecs.berkeley.edu>:
> And pkgsrc itself works just fine without the silly g prefix,
> or at least does for me as a mere user (and as well as it does
> work).  But if you intend on adding the package upstream, it'll
> need something to cope with the g.  And pkgsrc handles local
> patches...

Well I had some problems on NetBSD without the g prefix for the
gnu coreutils - since then I always used that prefix.

But now I have a completely different problem with the tests on
solaris. It seems on solaris access() always returns 0 if a file is
existant and the effective uid is 0.

so:
--- snip ---
#include <stdio.h>
#include <unistd.h>

int
main (int argc, char **argv)
{
  printf ("access: %d\n", access("/etc/motd", X_OK));
  return 0;
}
--- snap ---

will return 0 on solaris - when run as root, even though /etc/motd
is not executeable. This seems to break hooks on Solaris - but
I'm not sure if this is only a Solaris Express bug. (I have no Solaris
10 system to verify it)

bye

Stefan

-- 
       http://www.dreamind.de/
Oroborus and Debian GNU/Linux Developer.

^ permalink raw reply

* Re: Git 1.3.2 on Solaris
From: Jason Riedy @ 2006-05-23  4:51 UTC (permalink / raw)
  To: Stefan Pfetzing; +Cc: Git Mailing List
In-Reply-To: <f3d7535d0605222020j2d581bd9j602752659a4b3ac2@mail.gmail.com>

And "Stefan Pfetzing" writes:
 -   printf ("access: %d\n", access("/etc/motd", X_OK));
[...]
 - will return 0 on solaris - when run as root, even though /etc/motd
 - is not executeable.

This is explicitly allowed by the SUS, even for non-root:
  http://www.opengroup.org/onlinepubs/000095399/functions/access.html
For non-root, some ACL systems could allow you to execute
the file even if there are no execute bits.  What a joy
ACLs are.  Or NFS uid mappings could play tricks on you,
or...  And as you've noticed, this kills [ -x ].  (Failing
to run the hooks in receive-pack.c is noisy but not fatal.
It's the shell scripts that stop.)

I think you're stuck.  To disable the hooks for all possible
users, OSes, file systems, etc., you need to remove them.

Or just don't run as root, and hope that the OS isn't 
completely insane.

BTW, ERR_RUN_COMMAND_EXEC is never returned.  Any failure
to exec will produce an exit code of 128 from die.  This
will be an issue when commit becomes a built-in, right?

Jason

^ permalink raw reply

* Re: [PATCH] git status: ignore empty directories (because they cannot be added)
From: Matthias Lederhofer @ 2006-05-23  5:35 UTC (permalink / raw)
  To: git
In-Reply-To: <7vu07h8rzr.fsf@assigned-by-dhcp.cox.net>

> > and a new option -u / --untracked-files to show files in untracked
> > directories.
> >
> > ---
> > A few things I'm not sure about:
> > - Should there be another option to disable --no-empty-directory?
>
> I am not sure about this.  We used to show everything in a
> directory full of untracked directory, which was distracting and
> that was the reason we added --directory there.  Maybe it would
> be less confusing if we just updated the message
>
>           print "#\n# Untracked files:\n";
>           print "#   (use \"git add\" to add to commit)\n";
>           print "#\n";
>
> to say "use 'git add' on these files and files in these
> directories you wish to add", or something silly like that,
> without this patch?

I do not know if I understand you correctly, do you want to leave out
the -u part of the patch or the whole patch?

Well, anyway, here the reasons for this patch:
- Working in a git repository with a lot of empty directories is
  annoying, because all of them show up in git status even though they
  cannot be added. With --no-empty-directories they are hidden.
- If there is a directory which may be added because it is quite
  useful to have the -u option to see what is in there to add (without
  using ls path/to/directory).

^ permalink raw reply

* Re: irc usage..
From: Jeff King @ 2006-05-23  6:52 UTC (permalink / raw)
  To: Martin Langhoff; +Cc: Junio C Hamano, Matthias Urlichs, git
In-Reply-To: <46a038f90605221615j59583bcdqf128bab31603148e@mail.gmail.com>

On Tue, May 23, 2006 at 11:15:07AM +1200, Martin Langhoff wrote:

> >I think cvsimport predates that option, but these days that loop
> >can be optimized by feeding --index-info from standard input.
> Oh, yep, that'd be a good addition. I think we can also cut down on

This patch is relatively simple, and I'll post it in a moment.

I also made a few other cleanups to commit() which apply on top of that;
I'll post it also.

> - Stop abusing globals in commit() -- pass the commit data as parameters.

Some of the globals actually get modified in commit() (e.g., @old and
@new get cleared).  So we need to either pass them in as references or
remember to do that cleanup each time it is called (which is really only
twice, I think).

> Will be trying to do those things in the next few days, don't mind if
> someone jumps in as well.

I can look at the line/block CVS file slurping, but not tonight.

-Peff

^ permalink raw reply

* Re: irc usage..
From: Jeff King @ 2006-05-23  6:58 UTC (permalink / raw)
  To: Martin Langhoff, Junio C Hamano, Matthias Urlichs, git
In-Reply-To: <20060523065232.GA6180@coredump.intra.peff.net>

>From nobody Mon Sep 17 00:00:00 2001
From: Jeff King <peff@peff.net>
Date: Tue, 23 May 2006 01:16:07 -0400
Subject: [PATCH 1/2] cvsimport: use git-update-index --index-info

This should reduce the number of git-update-index forks required per
commit. We now do adds/removes in one call, and we are no longer forced to
deal with argv limitations.

---

cb6452bbfda9c52ad8dbeaac6e3440ae61099a05
 git-cvsimport.perl |   36 +++++++++++++-----------------------
 1 files changed, 13 insertions(+), 23 deletions(-)

cb6452bbfda9c52ad8dbeaac6e3440ae61099a05
diff --git a/git-cvsimport.perl b/git-cvsimport.perl
index d257e66..4efb0a5 100755
--- a/git-cvsimport.perl
+++ b/git-cvsimport.perl
@@ -565,29 +565,19 @@ my($patchset,$date,$author_name,$author_
 my(@old,@new,@skipped);
 sub commit {
 	my $pid;
-	while(@old) {
-		my @o2;
-		if(@old > 55) {
-			@o2 = splice(@old,0,50);
-		} else {
-			@o2 = @old;
-			@old = ();
-		}
-		system("git-update-index","--force-remove","--",@o2);
-		die "Cannot remove files: $?\n" if $?;
-	}
-	while(@new) {
-		my @n2;
-		if(@new > 12) {
-			@n2 = splice(@new,0,10);
-		} else {
-			@n2 = @new;
-			@new = ();
-		}
-		system("git-update-index","--add",
-			(map { ('--cacheinfo', @$_) } @n2));
-		die "Cannot add files: $?\n" if $?;
-	}
+
+      	open(my $fh, '|-', qw(git-update-index --index-info))
+		or die "unable to open git-update-index: $!";
+	print $fh 
+		(map { "0 0000000000000000000000000000000000000000\t$_\n" }
+			@old),
+		(map { '100' . sprintf('%o', $_->[0]) . " $_->[1]\t$_->[2]\n" }
+			@new)
+		or die "unable to write to git-update-index: $!";
+	close $fh
+		or die "unable to write to git-update-index: $!";
+	$? and die "git-update-index reported error: $?";
+	@old = @new = ();
 
 	$pid = open(C,"-|");
 	die "Cannot fork: $!" unless defined $pid;
-- 
1.3.3.gcb64-dirty

^ permalink raw reply related

* [PATCH 2/2] cvsimport: cleanup commit function
From: Jeff King @ 2006-05-23  7:00 UTC (permalink / raw)
  To: Martin Langhoff, Junio C Hamano, Matthias Urlichs, git
In-Reply-To: <20060523065232.GA6180@coredump.intra.peff.net>

This change attempts to clean up the commit function to make it a bit
easier to read (or at least the first half of it). It also improves
robustness and performance. Specifically:
  - report get_headref errors on opening ref unless the error is ENOENT
  - use regex to check for sha1 instead of length
  - use lexically scoped filehandles which get cleaned up automagically
  - check for error on both 'print' and 'close' (since output is buffered)
  - avoid "fork, do some perl, then exec" in commit(). It's not necessary,
    and we probably end up COW'ing parts of the perl process. Plus the code
    is much smaller because we can use open2()
  - avoid calling strftime over and over (mainly a readability cleanup)

---

I know this patch is quite large. I can try to split it if you want, but
I suspect it's not worth the effort (either you like refactoring or you
don't :) ).

9dc9f05ab5e1cbd8765238e7b1da0addd6f4296a
 git-cvsimport.perl |  150 ++++++++++++++++++++++------------------------------
 1 files changed, 64 insertions(+), 86 deletions(-)

9dc9f05ab5e1cbd8765238e7b1da0addd6f4296a
diff --git a/git-cvsimport.perl b/git-cvsimport.perl
index 4efb0a5..f8feb52 100755
--- a/git-cvsimport.perl
+++ b/git-cvsimport.perl
@@ -23,7 +23,7 @@ use File::Basename qw(basename dirname);
 use Time::Local;
 use IO::Socket;
 use IO::Pipe;
-use POSIX qw(strftime dup2);
+use POSIX qw(strftime dup2 :errno_h);
 use IPC::Open2;
 
 $SIG{'PIPE'}="IGNORE";
@@ -429,22 +429,25 @@ sub getwd() {
 	return $pwd;
 }
 
+sub is_sha1 {
+	my $s = shift;
+	return $s =~ /^[a-zA-Z0-9]{40}$/;
+}
 
-sub get_headref($$) {
+sub get_headref ($$) {
     my $name    = shift;
     my $git_dir = shift; 
-    my $sha;
     
-    if (open(C,"$git_dir/refs/heads/$name")) {
-	chomp($sha = <C>);
-	close(C);
-	length($sha) == 40
-	    or die "Cannot get head id for $name ($sha): $!\n";
+    my $f = "$git_dir/refs/heads/$name";
+    if(open(my $fh, $f)) {
+      	    chomp(my $r = <$fh>);
+	    is_sha1($r) or die "Cannot get head id for $name ($r): $!";
+	    return $r;
     }
-    return $sha;
+    die "unable to open $f: $!" unless $! == POSIX::ENOENT;
+    return undef;
 }
 
-
 -d $git_tree
 	or mkdir($git_tree,0777)
 	or die "Could not create $git_tree: $!";
@@ -561,90 +564,67 @@ #---------------------
 
 my $state = 0;
 
-my($patchset,$date,$author_name,$author_email,$branch,$ancestor,$tag,$logmsg);
-my(@old,@new,@skipped);
-sub commit {
-	my $pid;
-
+sub update_index (\@\@) {
+	my $old = shift;
+	my $new = shift;
       	open(my $fh, '|-', qw(git-update-index --index-info))
 		or die "unable to open git-update-index: $!";
 	print $fh 
 		(map { "0 0000000000000000000000000000000000000000\t$_\n" }
-			@old),
+			@$old),
 		(map { '100' . sprintf('%o', $_->[0]) . " $_->[1]\t$_->[2]\n" }
-			@new)
+			@$new)
 		or die "unable to write to git-update-index: $!";
 	close $fh
 		or die "unable to write to git-update-index: $!";
 	$? and die "git-update-index reported error: $?";
-	@old = @new = ();
+}
 
-	$pid = open(C,"-|");
-	die "Cannot fork: $!" unless defined $pid;
-	unless($pid) {
-		exec("git-write-tree");
-		die "Cannot exec git-write-tree: $!\n";
-	}
-	chomp(my $tree = <C>);
-	length($tree) == 40
-		or die "Cannot get tree id ($tree): $!\n";
-	close(C)
+sub write_tree () {
+	open(my $fh, '-|', qw(git-write-tree))
+		or die "unable to open git-write-tree: $!";
+	chomp(my $tree = <$fh>);
+	is_sha1($tree)
+		or die "Cannot get tree id ($tree): $!";
+	close($fh)
 		or die "Error running git-write-tree: $?\n";
 	print "Tree ID $tree\n" if $opt_v;
+	return $tree;
+}
 
-	my $parent = "";
-	if(open(C,"$git_dir/refs/heads/$last_branch")) {
-		chomp($parent = <C>);
-		close(C);
-		length($parent) == 40
-			or die "Cannot get parent id ($parent): $!\n";
-		print "Parent ID $parent\n" if $opt_v;
-	}
-
-	my $pr = IO::Pipe->new() or die "Cannot open pipe: $!\n";
-	my $pw = IO::Pipe->new() or die "Cannot open pipe: $!\n";
-	$pid = fork();
-	die "Fork: $!\n" unless defined $pid;
-	unless($pid) {
-		$pr->writer();
-		$pw->reader();
-		open(OUT,">&STDOUT");
-		dup2($pw->fileno(),0);
-		dup2($pr->fileno(),1);
-		$pr->close();
-		$pw->close();
-
-		my @par = ();
-		@par = ("-p",$parent) if $parent;
-
-		# loose detection of merges
-		# based on the commit msg
-		foreach my $rx (@mergerx) {
-			if ($logmsg =~ $rx) {
-				my $mparent = $1;
-				if ($mparent eq 'HEAD') { $mparent = $opt_o };
-				if ( -e "$git_dir/refs/heads/$mparent") {
-					$mparent = get_headref($mparent, $git_dir);
-					push @par, '-p', $mparent;
-					print OUT "Merge parent branch: $mparent\n" if $opt_v;
-				}
-			}
+my($patchset,$date,$author_name,$author_email,$branch,$ancestor,$tag,$logmsg);
+my(@old,@new,@skipped);
+sub commit {
+	update_index(@old, @new);
+	@old = @new = ();
+	my $tree = write_tree();
+	my $parent = get_headref($last_branch, $git_dir);
+	print "Parent ID " . ($parent ? $parent : "(empty)") . "\n" if $opt_v;
+
+	my @commit_args;
+	push @commit_args, ("-p", $parent) if $parent;
+
+	# loose detection of merges
+	# based on the commit msg
+	foreach my $rx (@mergerx) {
+		next unless $logmsg =~ $rx && $1;
+		my $mparent = $1 eq 'HEAD' ? $opt_o : $1;
+		if(my $sha1 = get_headref($mparent, $git_dir)) {
+			push @commit_args, '-p', $mparent;
+			print "Merge parent branch: $mparent\n" if $opt_v;
 		}
-
-		exec("env",
-			"GIT_AUTHOR_NAME=$author_name",
-			"GIT_AUTHOR_EMAIL=$author_email",
-			"GIT_AUTHOR_DATE=".strftime("+0000 %Y-%m-%d %H:%M:%S",gmtime($date)),
-			"GIT_COMMITTER_NAME=$author_name",
-			"GIT_COMMITTER_EMAIL=$author_email",
-			"GIT_COMMITTER_DATE=".strftime("+0000 %Y-%m-%d %H:%M:%S",gmtime($date)),
-			"git-commit-tree", $tree,@par);
-		die "Cannot exec git-commit-tree: $!\n";
-
-		close OUT;
 	}
-	$pw->writer();
-	$pr->reader();
+
+	my $commit_date = strftime("+0000 %Y-%m-%d %H:%M:%S",gmtime($date));
+	my $pid = open2(my $commit_read, my $commit_write,
+		'env',
+		"GIT_AUTHOR_NAME=$author_name",
+		"GIT_AUTHOR_EMAIL=$author_email",
+		"GIT_AUTHOR_DATE=$commit_date",
+		"GIT_COMMITTER_NAME=$author_name",
+		"GIT_COMMITTER_EMAIL=$author_email",
+		"GIT_COMMITTER_DATE=$commit_date",
+		'git-commit-tree', $tree, @commit_args);
 
 	# compatibility with git2cvs
 	substr($logmsg,32767) = "" if length($logmsg) > 32767;
@@ -656,16 +636,14 @@ sub commit {
 	    @skipped = ();
 	}
 
-	print $pw "$logmsg\n"
+	print($commit_write "$logmsg\n") && close($commit_write)
 		or die "Error writing to git-commit-tree: $!\n";
-	$pw->close();
 
-	print "Committed patch $patchset ($branch ".strftime("%Y-%m-%d %H:%M:%S",gmtime($date)).")\n" if $opt_v;
-	chomp(my $cid = <$pr>);
-	length($cid) == 40
-		or die "Cannot get commit id ($cid): $!\n";
+	print "Committed patch $patchset ($branch $commit_date)\n" if $opt_v;
+	chomp(my $cid = <$commit_read>);
+	is_sha1($cid) or die "Cannot get commit id ($cid): $!\n";
 	print "Commit ID $cid\n" if $opt_v;
-	$pr->close();
+	close($commit_read);
 
 	waitpid($pid,0);
 	die "Error running git-commit-tree: $?\n" if $?;
-- 
1.3.3.gcb64-dirty

^ permalink raw reply related

* [PATCH 1/2] cvsimport: use git-update-index --index-info
From: Jeff King @ 2006-05-23  7:01 UTC (permalink / raw)
  To: Martin Langhoff, Junio C Hamano, Matthias Urlichs, git
In-Reply-To: <20060523065810.GB6180@coredump.intra.peff.net>

This should reduce the number of git-update-index forks required per
commit. We now do adds/removes in one call, and we are no longer forced to
deal with argv limitations.

---

Oops, apparently using a mail reader is too challenging for me. Here's a
repost with the headers correctly merged.

cb6452bbfda9c52ad8dbeaac6e3440ae61099a05
 git-cvsimport.perl |   36 +++++++++++++-----------------------
 1 files changed, 13 insertions(+), 23 deletions(-)

cb6452bbfda9c52ad8dbeaac6e3440ae61099a05
diff --git a/git-cvsimport.perl b/git-cvsimport.perl
index d257e66..4efb0a5 100755
--- a/git-cvsimport.perl
+++ b/git-cvsimport.perl
@@ -565,29 +565,19 @@ my($patchset,$date,$author_name,$author_
 my(@old,@new,@skipped);
 sub commit {
 	my $pid;
-	while(@old) {
-		my @o2;
-		if(@old > 55) {
-			@o2 = splice(@old,0,50);
-		} else {
-			@o2 = @old;
-			@old = ();
-		}
-		system("git-update-index","--force-remove","--",@o2);
-		die "Cannot remove files: $?\n" if $?;
-	}
-	while(@new) {
-		my @n2;
-		if(@new > 12) {
-			@n2 = splice(@new,0,10);
-		} else {
-			@n2 = @new;
-			@new = ();
-		}
-		system("git-update-index","--add",
-			(map { ('--cacheinfo', @$_) } @n2));
-		die "Cannot add files: $?\n" if $?;
-	}
+
+      	open(my $fh, '|-', qw(git-update-index --index-info))
+		or die "unable to open git-update-index: $!";
+	print $fh 
+		(map { "0 0000000000000000000000000000000000000000\t$_\n" }
+			@old),
+		(map { '100' . sprintf('%o', $_->[0]) . " $_->[1]\t$_->[2]\n" }
+			@new)
+		or die "unable to write to git-update-index: $!";
+	close $fh
+		or die "unable to write to git-update-index: $!";
+	$? and die "git-update-index reported error: $?";
+	@old = @new = ();
 
 	$pid = open(C,"-|");
 	die "Cannot fork: $!" unless defined $pid;
-- 
1.3.3.gcb64-dirty

^ permalink raw reply related

* Re: [PATCH 2/2] cvsimport: cleanup commit function
From: Jeff King @ 2006-05-23  7:13 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7v4pzh6wtr.fsf@assigned-by-dhcp.cox.net>

[cc'd to list to get reactions on open2]

On Tue, May 23, 2006 at 12:10:08AM -0700, Junio C Hamano wrote:

> > +	return $s =~ /^[a-zA-Z0-9]{40}$/;
> [0-9a-f] (We always do lowercase).

Er, yes, that was a complete think-o on my part.

> Hmm.  I personally do not have problems with open2, but folks on
> some other platforms might.  I'll see how the list audience
> sounds.

FWIW, it was already being used in git-cvsimport.

-Peff

^ permalink raw reply

* [PATCH 2/2] cvsimport: cleanup commit function
From: Jeff King @ 2006-05-23  7:27 UTC (permalink / raw)
  To: git; +Cc: martin, junkio
In-Reply-To: <1148369266352-git-send-email-1>

This change attempts to clean up the commit function to make it a bit
easier to read (or at least the first half of it). It also improves
robustness and performance. Specifically:
  - report get_headref errors on opening ref unless the error is ENOENT
  - use regex to check for sha1 instead of length
  - use lexically scoped filehandles which get cleaned up automagically
  - check for error on both 'print' and 'close' (since output is buffered)
  - avoid "fork, do some perl, then exec" in commit(). It's not necessary,
    and we probably end up COW'ing parts of the perl process. Plus the code
    is much smaller because we can use open2()
  - avoid calling strftime over and over (mainly a readability cleanup)

---

This is a repost with some minor fixups from Junio (and based off of the
fixed 1/2 patch).

3408c8d8364f816a7c4a34a03045f466bf028540
 git-cvsimport.perl |  150 ++++++++++++++++++++++------------------------------
 1 files changed, 64 insertions(+), 86 deletions(-)

3408c8d8364f816a7c4a34a03045f466bf028540
diff --git a/git-cvsimport.perl b/git-cvsimport.perl
index a65bea6..219f6dc 100755
--- a/git-cvsimport.perl
+++ b/git-cvsimport.perl
@@ -23,7 +23,7 @@ use File::Basename qw(basename dirname);
 use Time::Local;
 use IO::Socket;
 use IO::Pipe;
-use POSIX qw(strftime dup2);
+use POSIX qw(strftime dup2 :errno_h);
 use IPC::Open2;
 
 $SIG{'PIPE'}="IGNORE";
@@ -429,22 +429,25 @@ sub getwd() {
 	return $pwd;
 }
 
+sub is_sha1 {
+	my $s = shift;
+	return $s =~ /^[a-f0-9]{40}$/;
+}
 
-sub get_headref($$) {
+sub get_headref ($$) {
     my $name    = shift;
     my $git_dir = shift; 
-    my $sha;
     
-    if (open(C,"$git_dir/refs/heads/$name")) {
-	chomp($sha = <C>);
-	close(C);
-	length($sha) == 40
-	    or die "Cannot get head id for $name ($sha): $!\n";
+    my $f = "$git_dir/refs/heads/$name";
+    if(open(my $fh, $f)) {
+      	    chomp(my $r = <$fh>);
+	    is_sha1($r) or die "Cannot get head id for $name ($r): $!";
+	    return $r;
     }
-    return $sha;
+    die "unable to open $f: $!" unless $! == POSIX::ENOENT;
+    return undef;
 }
 
-
 -d $git_tree
 	or mkdir($git_tree,0777)
 	or die "Could not create $git_tree: $!";
@@ -561,90 +564,67 @@ #---------------------
 
 my $state = 0;
 
-my($patchset,$date,$author_name,$author_email,$branch,$ancestor,$tag,$logmsg);
-my(@old,@new,@skipped);
-sub commit {
-	my $pid;
-
+sub update_index (\@\@) {
+	my $old = shift;
+	my $new = shift;
 	open(my $fh, '|-', qw(git-update-index -z --index-info))
 		or die "unable to open git-update-index: $!";
 	print $fh 
 		(map { "0 0000000000000000000000000000000000000000\t$_\0" }
-			@old),
+			@$old),
 		(map { '100' . sprintf('%o', $_->[0]) . " $_->[1]\t$_->[2]\0" }
-			@new)
+			@$new)
 		or die "unable to write to git-update-index: $!";
 	close $fh
 		or die "unable to write to git-update-index: $!";
 	$? and die "git-update-index reported error: $?";
-	@old = @new = ();
+}
 
-	$pid = open(C,"-|");
-	die "Cannot fork: $!" unless defined $pid;
-	unless($pid) {
-		exec("git-write-tree");
-		die "Cannot exec git-write-tree: $!\n";
-	}
-	chomp(my $tree = <C>);
-	length($tree) == 40
-		or die "Cannot get tree id ($tree): $!\n";
-	close(C)
+sub write_tree () {
+	open(my $fh, '-|', qw(git-write-tree))
+		or die "unable to open git-write-tree: $!";
+	chomp(my $tree = <$fh>);
+	is_sha1($tree)
+		or die "Cannot get tree id ($tree): $!";
+	close($fh)
 		or die "Error running git-write-tree: $?\n";
 	print "Tree ID $tree\n" if $opt_v;
+	return $tree;
+}
 
-	my $parent = "";
-	if(open(C,"$git_dir/refs/heads/$last_branch")) {
-		chomp($parent = <C>);
-		close(C);
-		length($parent) == 40
-			or die "Cannot get parent id ($parent): $!\n";
-		print "Parent ID $parent\n" if $opt_v;
-	}
-
-	my $pr = IO::Pipe->new() or die "Cannot open pipe: $!\n";
-	my $pw = IO::Pipe->new() or die "Cannot open pipe: $!\n";
-	$pid = fork();
-	die "Fork: $!\n" unless defined $pid;
-	unless($pid) {
-		$pr->writer();
-		$pw->reader();
-		open(OUT,">&STDOUT");
-		dup2($pw->fileno(),0);
-		dup2($pr->fileno(),1);
-		$pr->close();
-		$pw->close();
-
-		my @par = ();
-		@par = ("-p",$parent) if $parent;
-
-		# loose detection of merges
-		# based on the commit msg
-		foreach my $rx (@mergerx) {
-			if ($logmsg =~ $rx) {
-				my $mparent = $1;
-				if ($mparent eq 'HEAD') { $mparent = $opt_o };
-				if ( -e "$git_dir/refs/heads/$mparent") {
-					$mparent = get_headref($mparent, $git_dir);
-					push @par, '-p', $mparent;
-					print OUT "Merge parent branch: $mparent\n" if $opt_v;
-				}
-			}
+my($patchset,$date,$author_name,$author_email,$branch,$ancestor,$tag,$logmsg);
+my(@old,@new,@skipped);
+sub commit {
+	update_index(@old, @new);
+	@old = @new = ();
+	my $tree = write_tree();
+	my $parent = get_headref($last_branch, $git_dir);
+	print "Parent ID " . ($parent ? $parent : "(empty)") . "\n" if $opt_v;
+
+	my @commit_args;
+	push @commit_args, ("-p", $parent) if $parent;
+
+	# loose detection of merges
+	# based on the commit msg
+	foreach my $rx (@mergerx) {
+		next unless $logmsg =~ $rx && $1;
+		my $mparent = $1 eq 'HEAD' ? $opt_o : $1;
+		if(my $sha1 = get_headref($mparent, $git_dir)) {
+			push @commit_args, '-p', $mparent;
+			print "Merge parent branch: $mparent\n" if $opt_v;
 		}
-
-		exec("env",
-			"GIT_AUTHOR_NAME=$author_name",
-			"GIT_AUTHOR_EMAIL=$author_email",
-			"GIT_AUTHOR_DATE=".strftime("+0000 %Y-%m-%d %H:%M:%S",gmtime($date)),
-			"GIT_COMMITTER_NAME=$author_name",
-			"GIT_COMMITTER_EMAIL=$author_email",
-			"GIT_COMMITTER_DATE=".strftime("+0000 %Y-%m-%d %H:%M:%S",gmtime($date)),
-			"git-commit-tree", $tree,@par);
-		die "Cannot exec git-commit-tree: $!\n";
-
-		close OUT;
 	}
-	$pw->writer();
-	$pr->reader();
+
+	my $commit_date = strftime("+0000 %Y-%m-%d %H:%M:%S",gmtime($date));
+	my $pid = open2(my $commit_read, my $commit_write,
+		'env',
+		"GIT_AUTHOR_NAME=$author_name",
+		"GIT_AUTHOR_EMAIL=$author_email",
+		"GIT_AUTHOR_DATE=$commit_date",
+		"GIT_COMMITTER_NAME=$author_name",
+		"GIT_COMMITTER_EMAIL=$author_email",
+		"GIT_COMMITTER_DATE=$commit_date",
+		'git-commit-tree', $tree, @commit_args);
 
 	# compatibility with git2cvs
 	substr($logmsg,32767) = "" if length($logmsg) > 32767;
@@ -656,16 +636,14 @@ sub commit {
 	    @skipped = ();
 	}
 
-	print $pw "$logmsg\n"
+	print($commit_write "$logmsg\n") && close($commit_write)
 		or die "Error writing to git-commit-tree: $!\n";
-	$pw->close();
 
-	print "Committed patch $patchset ($branch ".strftime("%Y-%m-%d %H:%M:%S",gmtime($date)).")\n" if $opt_v;
-	chomp(my $cid = <$pr>);
-	length($cid) == 40
-		or die "Cannot get commit id ($cid): $!\n";
+	print "Committed patch $patchset ($branch $commit_date)\n" if $opt_v;
+	chomp(my $cid = <$commit_read>);
+	is_sha1($cid) or die "Cannot get commit id ($cid): $!\n";
 	print "Commit ID $cid\n" if $opt_v;
-	$pr->close();
+	close($commit_read);
 
 	waitpid($pid,0);
 	die "Error running git-commit-tree: $?\n" if $?;
-- 
1.3.3.g3408

^ permalink raw reply related

* [PATCH 1/2] cvsimport: use git-update-index --index-info
From: Jeff King @ 2006-05-23  7:27 UTC (permalink / raw)
  To: git; +Cc: martin, junkio
In-Reply-To: <20060523070007.GC6180@coredump.intra.peff.net>

This should reduce the number of git-update-index forks required per
commit. We now do adds/removes in one call, and we are no longer forced to
deal with argv limitations.

---

This is a repost using -z/NUL instead of line feeds.

d82d215430ae5e79210f73a31f5f8a053f36c27f
 git-cvsimport.perl |   36 +++++++++++++-----------------------
 1 files changed, 13 insertions(+), 23 deletions(-)

d82d215430ae5e79210f73a31f5f8a053f36c27f
diff --git a/git-cvsimport.perl b/git-cvsimport.perl
index d257e66..a65bea6 100755
--- a/git-cvsimport.perl
+++ b/git-cvsimport.perl
@@ -565,29 +565,19 @@ my($patchset,$date,$author_name,$author_
 my(@old,@new,@skipped);
 sub commit {
 	my $pid;
-	while(@old) {
-		my @o2;
-		if(@old > 55) {
-			@o2 = splice(@old,0,50);
-		} else {
-			@o2 = @old;
-			@old = ();
-		}
-		system("git-update-index","--force-remove","--",@o2);
-		die "Cannot remove files: $?\n" if $?;
-	}
-	while(@new) {
-		my @n2;
-		if(@new > 12) {
-			@n2 = splice(@new,0,10);
-		} else {
-			@n2 = @new;
-			@new = ();
-		}
-		system("git-update-index","--add",
-			(map { ('--cacheinfo', @$_) } @n2));
-		die "Cannot add files: $?\n" if $?;
-	}
+
+	open(my $fh, '|-', qw(git-update-index -z --index-info))
+		or die "unable to open git-update-index: $!";
+	print $fh 
+		(map { "0 0000000000000000000000000000000000000000\t$_\0" }
+			@old),
+		(map { '100' . sprintf('%o', $_->[0]) . " $_->[1]\t$_->[2]\0" }
+			@new)
+		or die "unable to write to git-update-index: $!";
+	close $fh
+		or die "unable to write to git-update-index: $!";
+	$? and die "git-update-index reported error: $?";
+	@old = @new = ();
 
 	$pid = open(C,"-|");
 	die "Cannot fork: $!" unless defined $pid;
-- 
1.3.3.g3408

^ permalink raw reply related

* [PATCH] cvsimport: introduce _fetchfile() method and used a 1M buffer to read()
From: Martin Langhoff @ 2006-05-23  8:08 UTC (permalink / raw)
  To: git; +Cc: Martin Langhoff

File retrieval from the socket is now moved to _fetchfile() and we now
cap reads at 1MB. This should limit the memory growth of the cvsimport
process.

Signed-off-by: Martin Langhoff <martin@catalyst.net.nz>

---

 git-cvsimport.perl |   36 +++++++++++++++++++-----------------
 1 files changed, 19 insertions(+), 17 deletions(-)

c06eea55b2b061abe6c09fa0737d6bb87afa9d39
diff --git a/git-cvsimport.perl b/git-cvsimport.perl
index 5b68671..ace7087 100755
--- a/git-cvsimport.perl
+++ b/git-cvsimport.perl
@@ -315,15 +315,7 @@ sub _line {
 			chomp $cnt;
 			die "Duh: Filesize $cnt" if $cnt !~ /^\d+$/;
 			$line="";
-			$res=0;
-			while($cnt) {
-				my $buf;
-				my $num = $self->{'socketi'}->read($buf,$cnt);
-				die "Server: Filesize $cnt: $num: $!\n" if not defined $num or $num<=0;
-				print $fh $buf;
-				$res += $num;
-				$cnt -= $num;
-			}
+			$res = $self->_fetchfile($fh, $cnt);
 		} elsif($line =~ s/^ //) {
 			print $fh $line;
 			$res += length($line);
@@ -335,14 +327,7 @@ sub _line {
 			chomp $cnt;
 			die "Duh: Mbinary $cnt" if $cnt !~ /^\d+$/ or $cnt<1;
 			$line="";
-			while($cnt) {
-				my $buf;
-				my $num = $self->{'socketi'}->read($buf,$cnt);
-				die "S: Mbinary $cnt: $num: $!\n" if not defined $num or $num<=0;
-				print $fh $buf;
-				$res += $num;
-				$cnt -= $num;
-			}
+			$res += $self->_fetchfile($fh, $cnt);
 		} else {
 			chomp $line;
 			if($line eq "ok") {
@@ -384,6 +369,23 @@ sub file {
 
 	return ($name, $res);
 }
+sub _fetchfile {
+	my ($self, $fh, $cnt) = @_;
+	my $res;
+	my $bufsize = 1024 * 1024;
+	while($cnt) {
+	    if ($bufsize > $cnt) {
+		$bufsize = $cnt;
+	    }
+	    my $buf;      
+	    my $num = $self->{'socketi'}->read($buf,$bufsize);
+	    die "Server: Filesize $cnt: $num: $!\n" if not defined $num or $num<=0;
+	    print $fh $buf;
+	    $res += $num;
+	    $cnt -= $num;
+	}
+	return $res;
+}
 
 
 package main;
-- 
1.3.2.g82000

^ permalink raw reply related

* [PATCH] Add git-quiltimport to .gitignore.
From: Peter Eriksen @ 2006-05-23  8:10 UTC (permalink / raw)
  To: git

From: Peter Eriksen <s022018@student.dtu.dk>
Date: Mon, 22 May 2006 15:46:25 +0200

Signed-off-by: Peter Eriksen <s022018@student.dtu.dk>


---

24b65a30015aeddc9e1e90432e34b144cf8a3f30
 .gitignore |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

24b65a30015aeddc9e1e90432e34b144cf8a3f30
diff --git a/.gitignore b/.gitignore
index b5959d6..199cc31 100644
--- a/.gitignore
+++ b/.gitignore
@@ -77,6 +77,7 @@ git-prune
 git-prune-packed
 git-pull
 git-push
+git-quiltimport
 git-read-tree
 git-rebase
 git-receive-pack
-- 
1.3.3.g288c

^ permalink raw reply related

* [PATCH] Set the executable bit on gitMergeCommon.py.
From: Peter Eriksen @ 2006-05-23  8:12 UTC (permalink / raw)
  To: git

From: Peter Eriksen <s022018@student.dtu.dk>
Date: Mon, 22 May 2006 15:35:42 +0200

Signed-off-by: Peter Eriksen <s022018@student.dtu.dk>


---

6465b4c8cc760102ed5771ccded2c02904539475
 0 files changed, 0 insertions(+), 0 deletions(-)
 mode change 100644 => 100755 gitMergeCommon.py

6465b4c8cc760102ed5771ccded2c02904539475
diff --git a/gitMergeCommon.py b/gitMergeCommon.py
old mode 100644
new mode 100755
-- 
1.3.3.g288c

^ permalink raw reply

* Re: [PATCH 2/2] cvsimport: cleanup commit function
From: Martin Langhoff @ 2006-05-23  8:13 UTC (permalink / raw)
  To: Martin Langhoff, Junio C Hamano, Matthias Urlichs, git
In-Reply-To: <20060523070007.GC6180@coredump.intra.peff.net>

Jeff,

good stuff -- aiming at exactly the things that had been nagging me.
Some minor notes on top of what junio's mentioned...

> +    die "unable to open $f: $!" unless $! == POSIX::ENOENT;
> +    return undef;

Heh. Is that the return of the living dead?

> +sub update_index (\@\@) {
> +       my $old = shift;
> +       my $new = shift;

Would it not make more sense to just pass them as plain parameters?

> +       print "Committed patch $patchset ($branch $commit_date)\n" if

Given that we have that -- should we remember it and avoid re-reading
the headref from disk? A %seenheads cache would save us 99.9% of the
hassle.

In related news, I've dealt with file reads from the socket being
memorybound. Should merge ok.

cheers,


martin

^ permalink raw reply

* Make more commands builtin
From: Peter Eriksen @ 2006-05-23  8:23 UTC (permalink / raw)
  To: git


 Makefile                               |   26 +++++++++++++++-----------
 apply.c => builtin-apply.c             |    3 ++-
 commit-tree.c => builtin-commit-tree.c |    3 ++-
 diff-files.c => builtin-diff-files.c   |    3 ++-
 diff-index.c => builtin-diff-index.c   |    3 ++-
 diff-stages.c => builtin-diff-stages.c |    3 ++-
 diff-tree.c => builtin-diff-tree.c     |    3 ++-
 ls-files.c => builtin-ls-files.c       |    3 ++-
 ls-tree.c => builtin-ls-tree.c         |    3 ++-
 read-tree.c => builtin-read-tree.c     |    3 ++-
 show-branch.c => builtin-show-branch.c |    3 ++-
 tar-tree.c => builtin-tar-tree.c       |    3 ++-
 builtin.h                              |   12 ++++++++++++
 git.c                                  |   13 ++++++++++++-

I've tried to follow the trend of making commands builtin.
All patches have the same form.  This is my first use
of git-send-email, so this might come out wrong.

Peter Eriksen <s022018@student.dtu.dk>

^ permalink raw reply

* Re: [PATCH 2/2] cvsimport: cleanup commit function
From: Junio C Hamano @ 2006-05-23  8:24 UTC (permalink / raw)
  To: git
In-Reply-To: <46a038f90605230113x2f6b0e4bq5a2ea97308b495e0@mail.gmail.com>

"Martin Langhoff" <martin.langhoff@gmail.com> writes:

> Jeff,
>
> good stuff -- aiming at exactly the things that had been nagging me.
> Some minor notes on top of what junio's mentioned...
>
>> +    die "unable to open $f: $!" unless $! == POSIX::ENOENT;
>> +    return undef;
>
> Heh. Is that the return of the living dead?

Note the trailing "unless" there.

>> +sub update_index (\@\@) {
>> +       my $old = shift;
>> +       my $new = shift;
>
> Would it not make more sense to just pass them as plain parameters?

Meaning...?  Perl5 can pass only one flat array, so the above is
a standard way to pass two arrays.

>> +       print "Committed patch $patchset ($branch $commit_date)\n" if
>
> Given that we have that -- should we remember it and avoid re-reading
> the headref from disk? A %seenheads cache would save us 99.9% of the
> hassle.
>
> In related news, I've dealt with file reads from the socket being
> memorybound. Should merge ok.

Merged OK, and I think your last suggestion makes sense.  I'll
go to bed after pushing out Jeff's two patches and yours.

^ permalink raw reply

* Make more commands builtin
From: Peter Eriksen @ 2006-05-23  8:31 UTC (permalink / raw)
  To: git

 Makefile                               |   26 +++++++++++++++-----------
 apply.c => builtin-apply.c             |    3 ++-
 commit-tree.c => builtin-commit-tree.c |    3 ++-
 diff-files.c => builtin-diff-files.c   |    3 ++-
 diff-index.c => builtin-diff-index.c   |    3 ++-
 diff-stages.c => builtin-diff-stages.c |    3 ++-
 diff-tree.c => builtin-diff-tree.c     |    3 ++-
 ls-files.c => builtin-ls-files.c       |    3 ++-
 ls-tree.c => builtin-ls-tree.c         |    3 ++-
 read-tree.c => builtin-read-tree.c     |    3 ++-
 show-branch.c => builtin-show-branch.c |    3 ++-
 tar-tree.c => builtin-tar-tree.c       |    3 ++-
 builtin.h                              |   12 ++++++++++++
 git.c                                  |   13 ++++++++++++-

I've tried to follow the trend of making commands builtin.
All patches have the same form.  This is my second use
of git-send-email, so this might come out wrong.

Peter Eriksen <s022018@student.dtu.dk>

^ 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