Git development
 help / color / mirror / Atom feed
* Re: [PATCH 01/12] Introduce Git.pm (v4)
From: Junio C Hamano @ 2006-06-24  2:46 UTC (permalink / raw)
  To: Petr Baudis; +Cc: git
In-Reply-To: <20060624023429.32751.80619.stgit@machine.or.cz>

Just to let you know, I already have v3 in my tree which is
merged in "pu".  With the two fixes I sent you earlier I think
it is in a good shape to be cooked in "next".

I do not think I'd have trouble applying this new series (I
would probably start from "master" to apply it and perhaps merge
or --squash merge it onto pb/gitpm topic branch that has v3 with
the two fixes I sent you separately) but we will see soon
enough.

^ permalink raw reply

* Re: [Patch] trap: exit: invalid signal specification
From: Junio C Hamano @ 2006-06-24  2:50 UTC (permalink / raw)
  To: caglar; +Cc: git
In-Reply-To: <200606240410.18334.caglar@pardus.org.tr>

> diff -ur git-1.4.0.orig/git-clone.sh git-1.4.0/git-clone.sh
> --- git-1.4.0.orig/git-clone.sh 2006-06-10 22:41:54.000000000 +0300
> +++ git-1.4.0/git-clone.sh      2006-06-24 03:54:49.000000000 +0300
> @@ -205,7 +205,7 @@
>  [ -e "$dir" ] && echo "$dir already exists." && usage
>  mkdir -p "$dir" &&
>  D=$(cd "$dir" && pwd) &&
> -trap 'err=$?; cd ..; rm -r "$D"; exit $err' 0
> +trap 'err=$?; cd ..; rm -r "$D"; EXIT $err' 0
>  case "$bare" in
>  yes)

I am not quite sure what to make out this...  Do you mean your
shell does not like the command "exit" spelled in lowercase
under Turkic locale?

^ permalink raw reply

* Re: [PATCH 0/5] Rework diff options
From: Junio C Hamano @ 2006-06-24  2:51 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: Timo Hirvonen, git
In-Reply-To: <Pine.LNX.4.63.0606240201580.29667@wbgn013.biozentrum.uni-wuerzburg.de>

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

> On Fri, 23 Jun 2006, Junio C Hamano wrote:
>
>> I personally feel that the benefit of being able to make sure you 
>> covered everything outweighs the size of initial diff.
>
> IMHO the difficulty of finding bugs is proportional to the square of the 
> diff size, while the number of people willing to review it is proportional 
> to its square root. So, if it is not difficult (which it is not at all in 
> this case), I politely ask to cut the patch size down.

Yeah, your "renaming in private while you are making sure but
rename them back" suggestion actually would work in this case.

^ permalink raw reply

* Re: [PATCH 01/12] Introduce Git.pm (v4)
From: Petr Baudis @ 2006-06-24  3:14 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7vr71f5kzs.fsf@assigned-by-dhcp.cox.net>

Dear diary, on Sat, Jun 24, 2006 at 04:46:15AM CEST, I got a letter
where Junio C Hamano <junkio@cox.net> said that...
> Just to let you know, I already have v3 in my tree which is
> merged in "pu".  With the two fixes I sent you earlier I think
> it is in a good shape to be cooked in "next".

Ah, yes, could you please apply your fixes as well? :-)

> I do not think I'd have trouble applying this new series (I
> would probably start from "master" to apply it and perhaps merge
> or --squash merge it onto pb/gitpm topic branch that has v3 with
> the two fixes I sent you separately) but we will see soon
> enough.

I noticed v3 in pu but thought that it should be trivial to just throw
that away from pu and take v4 instead - I'm sorry if that turns out to
be an issue.

-- 
				Petr "Pasky" Baudis
Stuff: http://pasky.or.cz/
A person is just about as big as the things that make them angry.

^ permalink raw reply

* commit triggers.
From: Dave Jones @ 2006-06-24  3:29 UTC (permalink / raw)
  To: git

I've grepped around, and come up with nothing, so hopefully
I'm not overlooking some already-implemented feature
(though it'd be great if I am).  How much work is involved in
creating a mechanism where some scripts living in say .git/triggers/
get executed on commits ?

The idea behind this stems from some scripts I've been running
periodically against an mbox of the daily kernel commits, which
greps for common bugs; kmalloc(GFP, size) instead of kmalloc(size,GFP),
memsets with reversed 2nd/3rd args etc etc.

It'd be useful I think to have a way to hook up such a script
to git's commit process that aborts a commit if a script returns
a hit, forcing the user to fix up the mistake before committing
it to the world.

thoughts ?

		Dave

-- 
http://www.codemonkey.org.uk

^ permalink raw reply

* [PATCH] cvsimport - streamline temp index file creation and avoid creating empty tmpfiles
From: Martin Langhoff @ 2006-06-24  5:09 UTC (permalink / raw)
  To: git, junkio, Johannes.Schindelin; +Cc: Martin Langhoff
In-Reply-To: <Pine.LNX.4.63.0606231811200.29667@wbgn013.biozentrum.uni-wuerzburg.de>

On 6/24/06, Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:
> It seems that git-cvsimport makes a temporary file of size 0, which cannot
> get mmap()ed, because it has size 0.

This switch to tmpnam() avoids creating the tmpfile in the first place and
streamlines the code. This handling of tmpfiles is slightly safer, but there
is an inherent race condition.

---
NOTE: (a) I cannot reproduce the problem and (b) this is only lightly tested,
if trivial.

However, this switch to tmpnam() avoids creating the tmpfile in the first place
and streamlines the code. This usage of tempfiles is open to a race condition
if someone could guess the name returned by tmpnam, but even this is safer than
what we did before, which was creating a file, closing the fh and then
clobbering it from git-read-tree.

And if someone can guess the name that tmpnam() returns their magic is strong
enough that they'll go for more interesting targets.
Signed-off-by: Martin Langhoff <martin@catalyst.net.nz>
---
 git-cvsimport.perl |   20 +++++---------------
 1 files changed, 5 insertions(+), 15 deletions(-)

diff --git a/git-cvsimport.perl b/git-cvsimport.perl
index f3daa6c..d961b7b 100644
--- a/git-cvsimport.perl
+++ b/git-cvsimport.perl
@@ -17,7 +17,7 @@ use strict;
 use warnings;
 use Getopt::Std;
 use File::Spec;
-use File::Temp qw(tempfile);
+use File::Temp qw(tempfile tmpnam);
 use File::Path qw(mkpath);
 use File::Basename qw(basename dirname);
 use Time::Local;
@@ -467,12 +467,8 @@ my $orig_git_index;
 $orig_git_index = $ENV{GIT_INDEX_FILE} if exists $ENV{GIT_INDEX_FILE};
 
 my %index; # holds filenames of one index per branch
-{   # init with an index for origin
-    my ($fh, $fn) = tempfile('gitXXXXXX', SUFFIX => '.idx',
-			     DIR => File::Spec->tmpdir());
-    close ($fh);
-    $index{$opt_o} = $fn;
-}
+$index{$opt_o} = tmpnam();
+
 $ENV{GIT_INDEX_FILE} = $index{$opt_o};
 unless(-d $git_dir) {
 	system("git-init-db");
@@ -502,10 +498,7 @@ unless(-d $git_dir) {
 
 	# populate index
 	unless ($index{$last_branch}) {
-	    my ($fh, $fn) = tempfile('gitXXXXXX', SUFFIX => '.idx',
-				     DIR => File::Spec->tmpdir());
-	    close ($fh);
-	    $index{$last_branch} = $fn;
+	    $index{$last_branch} = tmpnam();
 	}
 	$ENV{GIT_INDEX_FILE} = $index{$last_branch};
 	system('git-read-tree', $last_branch);
@@ -818,10 +811,7 @@ while(<CVS>) {
 		if(($ancestor || $branch) ne $last_branch) {
 			print "Switching from $last_branch to $branch\n" if $opt_v;
 			unless ($index{$branch}) {
-			    my ($fh, $fn) = tempfile('gitXXXXXX', SUFFIX => '.idx',
-						     DIR => File::Spec->tmpdir());
-			    close ($fh);
-			    $index{$branch} = $fn;
+			    $index{$branch} = tmpnam();
 			    $ENV{GIT_INDEX_FILE} = $index{$branch};
 			    system("git-read-tree", $branch);
 			    die "read-tree failed: $?\n" if $?;
-- 
1.4.0.gcda2

^ permalink raw reply related

* Re: commit triggers.
From: Junio C Hamano @ 2006-06-24  6:43 UTC (permalink / raw)
  To: Dave Jones; +Cc: git
In-Reply-To: <20060624032908.GH19461@redhat.com>

Dave Jones <davej@redhat.com> writes:

> I've grepped around, and come up with nothing, so hopefully
> I'm not overlooking some already-implemented feature
> (though it'd be great if I am).  How much work is involved in
> creating a mechanism where some scripts living in say .git/triggers/
> get executed on commits ?

Perhaps "grep hooks/ git-commit.sh"?

The hooks we have right now are more interested in preventing
you from making crappy commits, so we might not have a
post-commit hook you can use for notification purposes, but if
that is the case I am open to suggestions to add one.  I think
post-commit hook is already there, though.

^ permalink raw reply

* Re: x86 asm SHA1 (draft)
From: linux @ 2006-06-24  1:22 UTC (permalink / raw)
  To: git, junkio; +Cc: linux
In-Reply-To: <7vzmg376ee.fsf@assigned-by-dhcp.cox.net>

> The series to revamp SHA1 is good but judging the merit of each
> is outside my expertise, so I'd appreciate help to evaluate
> these changes.  For example, I cannot choose between competing
> three implementations for ppc without having access to a variety
> of ppc machines, and even if I did, ppc is not what I normally
> use, so incentive to try picking the best one for everybody is
> relatively low on my part.

Well, I'm not sure it's worth this much trouble.  Both of my PPC
implementations are smaller and faster than the current one,
so that's a pretty easy decision.  The difference between them
is 2-3%, which is, I think, not enough to be worth the maintenance
burden of a run-time decision infrastructure.  Just pick either one
and call it a day.

> The only external interface for the set of SHA1 implementation
> alternatives to the outside world is a well established SHA_CTX
> type, and three functions SHA1_Init(), SHA1_Update() and
> SHA1_Final(), and the alternative implementations are supposed
> to be drop-in replaceable.

I'd prefer it it was an *opaque* SHA_CTX type.  Sometimes you can achieve
some useful benefits by rearranging the fields.  For example, keeping the
64-bit length field as a native-order 64-bit number when appropriate.
And sometimes it's useful to have the full 80-word W[] array, while
other implementations don't want it.

> We probably would want to collect the benchmark results from
> popular platforms, have a summary to help people to choose a
> sensible one in the toplevel INSTALL file, and include the raw
> numbers in Documentation/technical/sha1-implementations.txt.

Not that numbers are bad, but I think that until there's a real
need for more than a single good-enough version per instruction set,
this is excessive.  Does hashing even show up on a profile?

^ permalink raw reply

* Re: [PATCH 2/5] Rework diff options
From: Junio C Hamano @ 2006-06-24  6:55 UTC (permalink / raw)
  To: Timo Hirvonen; +Cc: git
In-Reply-To: <20060624005252.c694e421.tihirvon@gmail.com>

Timo Hirvonen <tihirvon@gmail.com> writes:

> diff --git a/builtin-diff.c b/builtin-diff.c
> index 99a2f76..372894a 100644
> --- a/builtin-diff.c
> +++ b/builtin-diff.c
> @@ -56,15 +56,15 @@ static int builtin_diff_files(struct rev
>  	    3 < revs->max_count)
>  		usage(builtin_diff_usage);
>  	if (revs->max_count < 0 &&
> -	    (revs->diffopt.output_format == DIFF_FORMAT_PATCH))
> +	    (revs->diffopt.output_fmt & OUTPUT_FMT_PATCH))
>  		revs->combine_merges = revs->dense_combined_merges = 1;
>  	/*
>  	 * Backward compatibility wart - "diff-files -s" used to
>  	 * defeat the common diff option "-s" which asked for
> -	 * DIFF_FORMAT_NO_OUTPUT.
> +	 * OUTPUT_FMT_NONE
>  	 */
> -	if (revs->diffopt.output_format == DIFF_FORMAT_NO_OUTPUT)
> -		revs->diffopt.output_format = DIFF_FORMAT_RAW;
> +	if (revs->diffopt.output_fmt & OUTPUT_FMT_NONE)
> +		revs->diffopt.output_fmt = OUTPUT_FMT_RAW;
>  	return run_diff_files(revs, silent);
>  }

We do not have to remove this now, but I think we can remove
this "backward compatibility wart" sometime in the next round;
Cogito has been fixed not to use this.

> @@ -110,7 +110,7 @@ static int builtin_diff_b_f(struct rev_i
>  	while (1 < argc) {
>  		const char *arg = argv[1];
>  		if (!strcmp(arg, "--raw"))
> -			revs->diffopt.output_format = DIFF_FORMAT_RAW;
> +			revs->diffopt.output_fmt |= OUTPUT_FMT_RAW;
>  		else
>  			usage(builtin_diff_usage);
>  		argv++; argc--;

I see later in the series you taught low-level diff_opt_parse
about --raw switch, which is good.

> diff --git a/builtin-log.c b/builtin-log.c
> index 5a8a50b..e4a6385 100644
> --- a/builtin-log.c
> +++ b/builtin-log.c
> @@ -26,8 +26,8 @@ static int cmd_log_wc(int argc, const ch
>  	if (rev->always_show_header) {
>  		if (rev->diffopt.pickaxe || rev->diffopt.filter) {
>  			rev->always_show_header = 0;
> -			if (rev->diffopt.output_format == DIFF_FORMAT_RAW)
> -				rev->diffopt.output_format = DIFF_FORMAT_NO_OUTPUT;
> +			if (rev->diffopt.output_fmt & OUTPUT_FMT_RAW)
> +				rev->diffopt.output_fmt |= OUTPUT_FMT_NONE;
>  		}
>  	}

The original code is saying "For git-log command (i.e. when
always-show-header is on), if the command line did not override
but ended up asking for diff only because it wanted to do -S or
--diff-filter, do not show any diff" which is quite an opaque
logic.

> diff --git a/combine-diff.c b/combine-diff.c
> index 64b20cc..d0d8d01 100644
> --- a/combine-diff.c
> +++ b/combine-diff.c
> @@ -878,13 +867,13 @@ void diff_tree_combined(const unsigned c
>  			num_paths++;
>  	}
>  	if (num_paths) {
> -		if (opt->with_raw) {
> -			int saved_format = opt->output_format;
> -			opt->output_format = DIFF_FORMAT_RAW;
> +		if (opt->output_fmt & OUTPUT_FMT_RAW) {
> +			int saved_fmt = opt->output_fmt;
> +			opt->output_fmt |= OUTPUT_FMT_RAW;
>  			for (p = paths; p; p = p->next) {
>  				show_combined_diff(p, num_parent, dense, rev);
>  			}
> -			opt->output_format = saved_format;
> +			opt->output_fmt = saved_fmt;
>  			putchar(opt->line_termination);
>  		}
>  		for (p = paths; p; p = p->next) {

The code needs to run show_combined_diff twice, once for raw
output and another for normal output, since the processing
needed to be done in show_combined_diff are quite different
between the case where you emit raw diff (in which case you do
not combine at the hunk level) and normal diff (in which case
you do).  Since your modified show_combined_diff checks FMT_RAW
first, I suspect it would not make any difference if it is ORing
OUTPUT_FMT_RAW into output_fmt or assignment.  In fact, since
the bit is already on when the if () statement is taken, I think
you can lose the whole saved_fmt business, and just say
something like this here:

		if (opt->output_fmt & OUTPUT_FMT_RAW) {
			for (p = paths; p; p = p->next) {
				show_combined_diff(p, num_parent, dense, rev);
			}
			putchar(opt->line_termination);
		}

But I think the other side needs to drop OUTPUT_FMT_RAW bit,
since around ll. 817 in combine-diff.c you show_raw_diff() if
OUTPUT_FMT_RAW bit is on regardless of the OUTPUT_FMT_PATCH bit.

> diff --git a/diff.c b/diff.c
> index 1db0285..6eb7db0 100644
> --- a/diff.c
> +++ b/diff.c
> @@ -203,7 +203,7 @@ static void emit_rewrite_diff(const char
>  static int fill_mmfile(mmfile_t *mf, struct diff_filespec *one)
>  {
>  	if (!DIFF_FILE_VALID(one)) {
> -		mf->ptr = ""; /* does not matter */
> +		mf->ptr = (char *)""; /* does not matter */
>  		mf->size = 0;
>  		return 0;
>  	}
> @@ -395,7 +395,7 @@ static void show_stats(struct diffstat_t
>  	}
>  
>  	for (i = 0; i < data->nr; i++) {
> -		char *prefix = "";
> +		const char *prefix = "";
>  		char *name = data->files[i]->name;
>  		int added = data->files[i]->added;
>  		int deleted = data->files[i]->deleted;
> @@ -917,7 +917,7 @@ int diff_populate_filespec(struct diff_f
>  			err_empty:
>  				err = -1;
>  			empty:
> -				s->data = "";
> +				s->data = (char *)"";
>  				s->size = 0;
>  				return err;
>  			}
> @@ -1408,7 +1411,7 @@ int diff_setup_done(struct diff_options 
>  	return 0;
>  }
>  
> -int opt_arg(const char *arg, int arg_short, const char *arg_long, int *val)
> +static int opt_arg(const char *arg, int arg_short, const char *arg_long, int *val)
>  {
>  	char c, *eq;
>  	int len;
> @@ -1720,16 +1722,12 @@ static void diff_flush_raw(struct diff_f
>  		free((void*)path_two);
>  }
>  
> -static void diff_flush_name(struct diff_filepair *p,
> -			    int inter_name_termination,
> -			    int line_termination)
> +static void diff_flush_name(struct diff_filepair *p, int line_termination)
>  {
>  	char *path = p->two->path;
>  
>  	if (line_termination)
>  		path = quote_one(p->two->path);
> -	else
> -		path = p->two->path;
>  	printf("%s%c", path, line_termination);
>  	if (p->two->path != path)
>  		free(path);

These are all good changes but I would have liked to see them as
a separate series.

> @@ -1371,23 +1371,26 @@ int diff_setup_done(struct diff_options 
>  	    (0 <= options->rename_limit && !options->detect_rename))
>  		return -1;
>  
> +	if (options->output_fmt & OUTPUT_FMT_NONE)
> +		options->output_fmt = 0;
> +
> +	if (options->output_fmt & (OUTPUT_FMT_NAME |
> +				   OUTPUT_FMT_CHECKDIFF |
> +				   OUTPUT_FMT_NONE))
> +		options->output_fmt &= ~(OUTPUT_FMT_RAW |
> +					 OUTPUT_FMT_DIFFSTAT |
> +					 OUTPUT_FMT_SUMMARY |
> +					 OUTPUT_FMT_PATCH);
> +

Maybe doing the same for --name-status?  I wonder if the --name,
--name-status and --check should be mutually exclusive.  What
happens when you specify more than one of them?

> +/* Same as output_fmt = 0 but we know that -s flag was given
> + * and we should not give default value to output_fmt.
> + */
> +#define OUTPUT_FMT_NONE		0x0080

This is a very good comment to tell the reader what is going on.
I appreciate it.

> diff --git a/revision.c b/revision.c
> index b963f2a..4ad2272 100644
> --- a/revision.c
> +++ b/revision.c
> @@ -852,8 +852,8 @@ int setup_revisions(int argc, const char
>  	if (revs->combine_merges) {
>  		revs->ignore_merges = 0;
>  		if (revs->dense_combined_merges &&
> -		    (revs->diffopt.output_format != DIFF_FORMAT_DIFFSTAT))
> -			revs->diffopt.output_format = DIFF_FORMAT_PATCH;
> +		   !(revs->diffopt.output_fmt & OUTPUT_FMT_DIFFSTAT))
> +			revs->diffopt.output_fmt |= OUTPUT_FMT_PATCH;
>  	}
>  	revs->diffopt.abbrev = revs->abbrev;
>  	diff_setup_done(&revs->diffopt);

This tells it to default to patch format unless we are asked to
do diffstat only, in which case we just show stat without patch.
The new logic seems to be fishy.

Whew, now it's quite a lot of changes.  How to proceed from
here?  My rather selfish preference is:

 - "git-merge not add -p when asking for --summary --stat" is an
   obvious fix that is independently applicable to "master", so
   I took it already.

 - could I ask you to redo a patch to do only the clean-up part
   first, so that I can accept it for either "next" or "master".

 - Then after I take the clean-up, could you rebase four
   remainder patches ("Rework diff options" to "Add --patch
   option for diff-*") on the result?  The patches this round
   are already split quite well in that the first one does the
   enum to bit conversion and the latter three cleans things up
   (all of which I like a lot).  As Johannes suggested, it might
   be easier to review if they reused the same preprocessor
   symbols instead of renaming them.  I'd take them for "next".

^ permalink raw reply

* Re: [PATCH] cvsimport - streamline temp index file creation and avoid creating empty tmpfiles
From: Junio C Hamano @ 2006-06-24  6:59 UTC (permalink / raw)
  To: Martin Langhoff; +Cc: git, Johannes.Schindelin
In-Reply-To: <11511257501323-git-send-email-martin@catalyst.net.nz>

Martin Langhoff <martin@catalyst.net.nz> writes:

> On 6/24/06, Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:
>> It seems that git-cvsimport makes a temporary file of size 0, which cannot
>> get mmap()ed, because it has size 0.
>
> This switch to tmpnam() avoids creating the tmpfile in the first place and
> streamlines the code. This handling of tmpfiles is slightly safer, but there
> is an inherent race condition.
>
> ---
> NOTE: (a) I cannot reproduce the problem and (b) this is only lightly tested,
> if trivial.

Thanks both.

I'd take this to "next" after I hear from somebody, most likely
Johannes, who had trouble with the code earlier that the problem
is fixed.

^ permalink raw reply

* Re: x86 asm SHA1 (draft)
From: Junio C Hamano @ 2006-06-24  7:03 UTC (permalink / raw)
  To: linux; +Cc: git
In-Reply-To: <20060624012202.4822.qmail@science.horizon.com>

linux@horizon.com writes:

> Well, I'm not sure it's worth this much trouble.  Both of my PPC
> implementations are smaller and faster than the current one,
> so that's a pretty easy decision.  The difference between them
> is 2-3%, which is, I think, not enough to be worth the maintenance
> burden of a run-time decision infrastructure.  Just pick either one
> and call it a day.
>...
> Not that numbers are bad, but I think that until there's a real
> need for more than a single good-enough version per instruction set,
> this is excessive.

OK.  I somehow got an impression that your two versions had
quite different performance characteristics on G4 and G5 and
there was a real choice.  If they are between a few per-cent,
then I agree it is not worth doing at all.

^ permalink raw reply

* Re: [PATCH] rebase --merge: fix for rebasing more than 7 commits.
From: Junio C Hamano @ 2006-06-24  7:09 UTC (permalink / raw)
  To: Eric Wong; +Cc: git
In-Reply-To: <20060622110941.GA32261@hand.yhbt.net>

Eric Wong <normalperson@yhbt.net> writes:

> Junio C Hamano <junkio@cox.net> wrote:
>> Junio C Hamano <junkio@cox.net> writes:
>> 
>> >  * I wanted to raise my confidence level in the new rebase --merge
>> >    code, so I did a little exercise which resulted in finding this
>> >    buglet.
>> >...
>> >    So the exercise went like this:
>> >...
>> >   With this fix, the above works beautifully.  I am reasonably
>> >   happy with this shiny new toy.  Good job, Eric! and thanks.
>
> :)  Thanks for the extra QA and fix.

Another thing I noticed is while rebasing onto the mainline that
has accepted a few of the patches from the topic.  The original
rebase with "git am -3" logic notices that the patch has already
been applied and drops that commit, which is rather nice, but
the new "rebase --merge" logic barfs when git-commit notices
there is nothing to commit.  I think you could before calling
git-commit check if the git-merge-$strategy gave you the tree
identical to the HEAD tree, and simply skip it (maybe after
giving the user "patch already applied"message).

^ permalink raw reply

* From b65bc21e7d8dc8cafc70dfa6354cb66b8874b2d9 Mon Sep 17 00:00:00 2001, [PATCH] Makefile: add framework to verify and bench sha1 implementations.
From: Junio C Hamano, Junio C Hamano @ 2006-06-24  7:59 UTC (permalink / raw)
  To: git; +Cc: linux
In-Reply-To: <7vfyhv11ej.fsf@assigned-by-dhcp.cox.net>

With this, you can say

	MOZILLA_SHA1=YesPlease make check-sha1 

to see how fast your favorite SHA-1 implementation is and if it
fails with small to huge inputs.

Signed-off-by: Junio C Hamano <junkio@cox.net>
---

 * As the maintainer, I do need people to see breakage before it
   happens, without having access to all the platforms, hence
   this.

 Makefile     |    6 ++++
 test-sha1.c  |   24 +++++++++++++++++
 test-sha1.sh |   83 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 113 insertions(+), 0 deletions(-)

diff --git a/Makefile b/Makefile
index e29e3fa..ea0044d 100644
--- a/Makefile
+++ b/Makefile
@@ -636,6 +636,12 @@ test-delta$X: test-delta.c diff-delta.o 
 test-dump-cache-tree$X: dump-cache-tree.o $(GITLIBS)
 	$(CC) $(ALL_CFLAGS) -o $@ $(ALL_LDFLAGS) $(filter %.o,$^) $(LIBS)
 
+test-sha1$X: test-sha1.o $(GITLIBS)
+	$(CC) $(ALL_CFLAGS) -o $@ $(ALL_LDFLAGS) $(filter %.o,$^) $(LIBS)
+
+check-sha1:: test-sha1$X
+	./test-sha1.sh
+
 check:
 	for i in *.c; do sparse $(ALL_CFLAGS) $(SPARSE_FLAGS) $$i || exit; done
 
diff --git a/test-sha1.c b/test-sha1.c
new file mode 100644
index 0000000..2efc315
--- /dev/null
+++ b/test-sha1.c
@@ -0,0 +1,24 @@
+#include "cache.h"
+
+int main(int ac, char **av)
+{
+	SHA_CTX ctx;
+	unsigned char sha1[20];
+
+	SHA1_Init(&ctx);
+
+	while (1) {
+		ssize_t sz;
+		char buffer[8192];
+		sz = xread(0, buffer, sizeof(buffer));
+		if (sz == 0)
+			break;
+		if (sz < 0)
+			die("test-sha1: %s", strerror(errno));
+		SHA1_Update(&ctx, buffer, sz);
+	}
+	SHA1_Final(sha1, &ctx);
+	puts(sha1_to_hex(sha1));
+	exit(0);
+}
+
diff --git a/test-sha1.sh b/test-sha1.sh
new file mode 100755
index 0000000..01bbb57
--- /dev/null
+++ b/test-sha1.sh
@@ -0,0 +1,83 @@
+#!/bin/sh
+
+dd if=/dev/zero bs=1048576 count=100 2>/dev/null |
+/usr/bin/time ./test-sha1 >/dev/null
+
+while read expect cnt pfx
+do
+	case "$expect" in '#'*) continue ;; esac
+	actual=`
+		{
+			test -z "$pfx" || echo "$pfx"
+			dd if=/dev/zero bs=1048576 count=$cnt 2>/dev/null |
+			tr '[\0]' '[g]'
+		} | ./test-sha1
+	`
+	if test "$expect" = "$actual"
+	then
+		echo "OK: $expect $cnt $pfx"
+	else
+		echo >&2 "OOPS: $cnt"
+		echo >&2 "expect: $expect"
+		echo >&2 "actual: $actual"
+		exit 1
+	fi
+done <<EOF
+da39a3ee5e6b4b0d3255bfef95601890afd80709 0
+3f786850e387550fdab836ed7e6dc881de23001b 0 a
+5277cbb45a15902137d332d97e89cf8136545485 0 ab
+03cfd743661f07975fa2f1220c5194cbaff48451 0 abc
+3330b4373640f9e4604991e73c7e86bfd8da2dc3 0 abcd
+ec11312386ad561674f724b8cca7cf1796e26d1d 0 abcde
+bdc37c074ec4ee6050d68bc133c6b912f36474df 0 abcdef
+69bca99b923859f2dc486b55b87f49689b7358c7 0 abcdefg
+e414af7161c9554089f4106d6f1797ef14a73666 0 abcdefgh
+0707f2970043f9f7c22029482db27733deaec029 0 abcdefghi
+a4dd8aa74a5636728fe52451636e2e17726033aa 1
+9986b45e2f4d7086372533bb6953a8652fa3644a 1 frotz
+23d8d4f788e8526b4877548a32577543cbaaf51f 10
+8cd23f822ab44c7f481b8c92d591f6d1fcad431c 10 frotz
+f3b5604a4e604899c1233edb3bf1cc0ede4d8c32 512
+b095bd837a371593048136e429e9ac4b476e1bb3 512 frotz
+08fa81d6190948de5ccca3966340cc48c10cceac 1200 xyzzy
+e33a291f42c30a159733dd98b8b3e4ff34158ca0 4090 4G
+#a3bf783bc20caa958f6cb24dd140a7b21984838d 9999 nitfol
+EOF
+
+exit
+
+# generating test vectors
+# inputs are number of megabytes followed by some random string to prefix.
+
+while read cnt pfx
+do
+	actual=`
+		{
+			test -z "$pfx" || echo "$pfx"
+			dd if=/dev/zero bs=1048576 count=$cnt 2>/dev/null |
+			tr '[\0]' '[g]'
+		} | sha1sum |
+		sed -e 's/ .*//'
+	`
+	echo "$actual $cnt $pfx"
+done <<EOF
+0
+0 a
+0 ab
+0 abc
+0 abcd
+0 abcde
+0 abcdef
+0 abcdefg
+0 abcdefgh
+0 abcdefghi
+1
+1 frotz
+10
+10 frotz
+512
+512 frotz
+1200 xyzzy
+4090 4G
+9999 nitfol
+EOF
-- 
1.4.1.rc1.g0fca

^ permalink raw reply related

* Re: [PATCH 01/12] Introduce Git.pm (v4)
From: Junio C Hamano @ 2006-06-24  8:33 UTC (permalink / raw)
  To: Petr Baudis; +Cc: git
In-Reply-To: <7vr71f5kzs.fsf@assigned-by-dhcp.cox.net>

Junio C Hamano <junkio@cox.net> writes:

> Just to let you know, I already have v3 in my tree which is
> merged in "pu".  With the two fixes I sent you earlier I think
> it is in a good shape to be cooked in "next".
>
> I do not think I'd have trouble applying this new series (I
> would probably start from "master" to apply it and perhaps merge
> or --squash merge it onto pb/gitpm topic branch that has v3 with
> the two fixes I sent you separately) but we will see soon
> enough.

EEEEEEEEEK.

git-fmt-merge-msg failed and git-pull did not notice it and went
ahead to call git-merge with an empty log message.  This screwed
up my tree rather big way.

The reason it failed?  Well, it could not find Git.pm because
the changes to fmt-merge-msg was done for distros not for people
who install under their home directories.

Now, I am quite unhappy about the situation (and it is not your
fault).  "git pull" is something almost everybody uses, and
having the series means they would need to make sure whereever
Git.pm is installed is on their PERL5LIB as things currently
stand.

In the case of git-merge-recursive.py, Fredrik does

	sys.path.append('''@@GIT_PYTHON_PATH@@''')

and while running test this invalid path is overridden by having
PYTHONPATH environment variable.  Since it is "append", the test
picks up the freshly built version, not the version from the
previous install (i.e. PYTHONPATH environment variable wins).

So you would probably want to have something like

	use lib '@@GIT_PERL_PATH@@';

inside git-fmt-merge-msg.perl, which will be turned into

	use lib '/home/junio/some/where/you/install/pm';

in git-fmt-merge-msg and things might start working.

... gitster mumbles something in his mouth, and in a puff of
smoke Merlyn appears and solves all our Perl problems ;-) ...

^ permalink raw reply

* Re: [PATCH 07/12] Git.pm: Better error handling
From: Junio C Hamano @ 2006-06-24  8:37 UTC (permalink / raw)
  To: Petr Baudis; +Cc: git
In-Reply-To: <20060624023442.32751.28342.stgit@machine.or.cz>

Petr Baudis <pasky@suse.cz> writes:

> +int
> +error_xs(const char *err, va_list params)
> +{

You said in git-compat-util.h that set_error_routine takes a
function that returns void, so this gives unnecessary type
clash.

--------------------------------
In file included from /usr/lib/perl/5.8/CORE/perl.h:756,
                 from Git.xs:15:
/usr/lib/perl/5.8/CORE/embed.h:4193:1: warning: "die" redefined
Git.xs:11:1: warning: this is the location of the previous definition
Git.xs: In function 'boot_Git':
Git.xs:57: warning: passing argument 1 of 'set_error_routine' from incompatible pointer type
Git.xs:58: warning: passing argument 1 of 'set_die_routine' makes qualified function pointer from unqualified
--------------------------------

Other troubles I saw with the v4 series while compiling:

--------------------------------
usage.c:35: warning: initialization makes qualified function pointer from unqualified
usage.c:36: warning: initialization makes qualified function pointer from unqualified

I'd fix it with this

diff --git a/usage.c b/usage.c
index b781b00..52c2e96 100644
--- a/usage.c
+++ b/usage.c
@@ -12,19 +12,19 @@ static void report(const char *prefix, c
 	fputs("\n", stderr);
 }
 
-void usage_builtin(const char *err)
+static NORETURN void usage_builtin(const char *err)
 {
 	fprintf(stderr, "usage: %s\n", err);
 	exit(129);
 }
 
-void die_builtin(const char *err, va_list params)
+static NORETURN void die_builtin(const char *err, va_list params)
 {
 	report("fatal: ", err, params);
 	exit(128);
 }
 
-void error_builtin(const char *err, va_list params)
+static void error_builtin(const char *err, va_list params)
 {
 	report("error: ", err, params);
 }

--------------------------------

(cd perl && /usr/bin/perl Makefile.PL \
                PREFIX="/home/junio/git-test" \
                DEFINE="-O2 -Wall -Wdeclaration-after-statement
                -g -DSHA1_HEADER='<openssl/sha.h>'
                -DGIT_VERSION=\\\"1.4.1.rc1.gab0df\\\"" \
                LIBS="libgit.a xdiff/lib.a -lz  -lcrypto")
Unrecognized argument in LIBS ignored: 'libgit.a'
Unrecognized argument in LIBS ignored: 'xdiff/lib.a'

Do you need to pass LIBS, and if so maybe this is not a way
Makefile.PL expects it to be passed perhaps?

--------------------------------
Makefile out-of-date with respect to Makefile.PL
Cleaning current config before rebuilding Makefile...
make -f Makefile.old clean > /dev/null 2>&1
/usr/bin/perl Makefile.PL "PREFIX=/home/junio/git-test" "DEFINE=-O2 -Wall -Wdeclaration-after-statement -g -DSHA1_HEADER='<openssl/sha.h>'  -DGIT_VERSION=\"1.4.1.rc1.gab0df\"" "LIBS=libgit.a xdiff/lib.a -lz  -lcrypto"
Unrecognized argument in LIBS ignored: 'libgit.a'
Unrecognized argument in LIBS ignored: 'xdiff/lib.a'
Writing Makefile for Git
==> Your Makefile has been rebuilt. <==
==> Please rerun the make command.  <==
false
make[1]: *** [Makefile] Error 1
--------------------------------

The latter is what Perl's build mechanism does so it is not
strictly your fault, but it nevertheless is irritating that we
have to say make clean twice.

^ permalink raw reply related

* Re: A series file for git?
From: Junio C Hamano @ 2006-06-24  9:01 UTC (permalink / raw)
  To: Eric W. Biederman; +Cc: Linus Torvalds, Git Mailing List
In-Reply-To: <m1odwkyuf5.fsf_-_@ebiederm.dsl.xmission.com>

ebiederm@xmission.com (Eric W. Biederman) writes:

> I was using:
> 	git-rev-list $revargs | tac > list
> 	for sha1 in $(cat list); do git-cherry-pick -r $sha1 ; done
>...
> - Keeping patches in git and just remembering their sha1 is nice
>   but it has the downside that it can be really easy to loose
>   the sha1, and thus the patch.  So some sort of history mechanism
>   so you can get back to where you were would be nice.

Actually, the $revargs above is composed of your branch names
(e.g. "^master this-topic that-topic"), so as long as you do not
lose these branches they are protected.
>
> - This is a similar technique to topic branches.  However in some
>   of the more interesting cases a topic branch can't be used because
>   you have a whole series of little changes, that allow depend on
>   each other.  So topic branches need not apply.

Sorry I fail to see why.  A series of little changes that depend
on each other would be a string of pearls on a topic branch in
the simplest case, or a handful of topic branches that
occasionally merge with each other if you want to get fancier.
Cherry-picking from a DAG of the latter kind with your rev-list
piped to tac is no different than cherry-picking from a simple
straight line of the former kind, isn't it?

> - One of the places where we currently uses series files
>   (patch-scripts && quilt), and heavy cherry picking is for patch
>   aging.  That is letting patches sit in a testing branch for 
>   a while so people have a chance to test and look at them.

I agree that patch aging and updating does not mesh well with
how git inherently works, as git regards commits immutable
objects.  Even then, I use my "pu" branch (and topics that
hasn't merged to "next" but in "pu") pretty much as patch aging
area and I regularly do "git commit --amend" to update them.
This however is cumbersome with core git tools alone, and I
suspect is better done with StGIT.

> If we create a meta data branch with just the series file
> we can remove the risk of loosing things, as we always
> have a path back to the old history if we want it.

I am not sure about that.  What does the series file contain,
and what other things the meta data branch contain?  If you are
listing commit SHA1 in the series file, you _do_ have the risk
of losing things -- git-fsck-objects needs to look at such blob
object and consider that as the root of reachability chain; to
me that seems too specialized hack.

I have a feeling that I really should study how well StGIT does
things before talking about this further.  It may suit your
needs perfectly.  What do you feel is lacking in StGIT that you
need?

^ permalink raw reply

* [PATCH] git-repack -- respect -q and be quiet
From: Martin Langhoff @ 2006-06-24  9:41 UTC (permalink / raw)
  To: git, junkio; +Cc: Martin Langhoff

git-repack was passing the -q along to pack-objects but ignoring it
itself. Correct the oversight.
Signed-off-by: Martin Langhoff <martin@catalyst.net.nz>
---
 git-repack.sh |    5 +++--
 1 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/git-repack.sh b/git-repack.sh
index 4fb3f26..eb75c8c 100755
--- a/git-repack.sh
+++ b/git-repack.sh
@@ -49,8 +49,9 @@ name=$(git-rev-list --objects --all $rev
 if [ -z "$name" ]; then
 	echo Nothing new to pack.
 else
-	echo "Pack pack-$name created."
-
+	if test "$quiet" != '-q'; then
+	    echo "Pack pack-$name created."
+	fi
 	mkdir -p "$PACKDIR" || exit
 
 	mv .tmp-pack-$name.pack "$PACKDIR/pack-$name.pack" &&
-- 
1.4.1.rc1.g59c8

^ permalink raw reply related

* Re: [PATCH] cvsimport - streamline temp index file creation and avoid creating empty tmpfiles
From: Johannes Schindelin @ 2006-06-24  9:50 UTC (permalink / raw)
  To: Martin Langhoff; +Cc: git, junkio
In-Reply-To: <11511257501323-git-send-email-martin@catalyst.net.nz>

Hi,

On Sat, 24 Jun 2006, Martin Langhoff wrote:

> On 6/24/06, Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:
> > It seems that git-cvsimport makes a temporary file of size 0, which cannot
> > get mmap()ed, because it has size 0.
> 
> This switch to tmpnam() avoids creating the tmpfile in the first place and
> streamlines the code. This handling of tmpfiles is slightly safer, but there
> is an inherent race condition.

Thank you. This fixes the error.

HOWEVER, it does not fix the main problem: when I try to git-cvsimport, 
there is no index for that branch yet, since I used to git-cvsimport with 
the old cvsimport.

Now, when cvsimport sees there is no index, it evidently assumes that the 
current state is an empty tree, which is *not* true.

The effect is: the first commit removes all files from the tree which were 
not touched by the cvs commit. Bad.

> This usage of tempfiles is open to a race condition

I would not care too strongly about that. Eventually, I really would like 
this file to reside in $GIT_DIR, not /tmp, but whatever. That is not my 
biggest concern right now. That I cannot update since June 18th, however, 
is.

Ciao,
Dscho

^ permalink raw reply

* PPC SHA-1 Updates in "pu"
From: Junio C Hamano @ 2006-06-24 10:03 UTC (permalink / raw)
  To: git
In-Reply-To: <7vfyhv11ej.fsf@assigned-by-dhcp.cox.net>

Junio C Hamano <junkio@cox.net> writes:

> linux@horizon.com writes:
>
>> Well, I'm not sure it's worth this much trouble.  Both of my PPC
>> implementations are smaller and faster than the current one,
>> so that's a pretty easy decision.  The difference between them
>> is 2-3%, which is, I think, not enough to be worth the maintenance
>> burden of a run-time decision infrastructure.  Just pick either one
>> and call it a day.
>>...
>> Not that numbers are bad, but I think that until there's a real
>> need for more than a single good-enough version per instruction set,
>> this is excessive.
>
> OK.  I somehow got an impression that your two versions had
> quite different performance characteristics on G4 and G5 and
> there was a real choice.  If they are between a few per-cent,
> then I agree it is not worth doing at all.

If somebody has time and inclination, please try updated PPC SHA-1
from linux@horizon.com that is in "pu" (say make check-sha1) and
report impressions.

The first line from ./test-sha1.sh is the time output to hash 100MB
and there should be bunch of OK output to verify the code hashes
things correctly for inputs of various sizes.

^ permalink raw reply

* Re: [PATCH] cvsimport - streamline temp index file creation and avoid creating empty tmpfiles
From: Junio C Hamano @ 2006-06-24 10:05 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: git, Martin Langhoff
In-Reply-To: <Pine.LNX.4.63.0606241145280.29667@wbgn013.biozentrum.uni-wuerzburg.de>

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

> I would not care too strongly about that. Eventually, I really would like 
> this file to reside in $GIT_DIR, not /tmp, but whatever. That is not my 
> biggest concern right now. That I cannot update since June 18th, however, 
> is.

Would reverting 8f732649 in the meantime be an option for you?

^ permalink raw reply

* Re: [PATCH] cvsimport - streamline temp index file creation and avoid creating empty tmpfiles
From: Martin Langhoff @ 2006-06-24 10:08 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: Martin Langhoff, git, junkio
In-Reply-To: <Pine.LNX.4.63.0606241145280.29667@wbgn013.biozentrum.uni-wuerzburg.de>

On 6/24/06, Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:
> Thank you. This fixes the error.

Your welcome!

> HOWEVER, it does not fix the main problem: when I try to git-cvsimport,
> there is no index for that branch yet, since I used to git-cvsimport with
> the old cvsimport.
>
> Now, when cvsimport sees there is no index, it evidently assumes that the
> current state is an empty tree, which is *not* true.
>
> The effect is: the first commit removes all files from the tree which were
> not touched by the cvs commit. Bad.

I don't quite understand. No it shouldn't be the case -- it should
create the index using git-read-tree based on the tip of the branch.
Right after the call to tmpnam() the code looks like

 $index{$branch} = tmpnam();
 $ENV{GIT_INDEX_FILE} = $index{$branch};
 system("git-read-tree", $branch);
 die "read-tree failed: $?\n" if $?;

> > This usage of tempfiles is open to a race condition
>
> I would not care too strongly about that. Eventually, I really would like
> this file to reside in $GIT_DIR, not /tmp, but whatever. That is not my
> biggest concern right now. That I cannot update since June 18th, however,
> is.

It's worrying me too. Running some tests now...




martin

^ permalink raw reply

* [PATCH] diff --color: use $GIT_DIR/config
From: Junio C Hamano @ 2006-06-24 11:12 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: git, Linus Torvalds
In-Reply-To: <Pine.LNX.4.64.0606230756050.6483@g5.osdl.org>

This lets you use something like this in your $GIT_DIR/config
file.

	[diff]
		color = auto

	[diff.color]
		new = blue
		old = yellow
		frag = reverse

When diff.color is set to "auto", colored diff is enabled when
the standard output is the terminal.  Other choices are "always",
and "never".  Usual boolean true/false can also be used.

The colormap entries can specify colors for the following slots:

	plain	- lines that appear in both old and new file (context)
	meta	- diff --git header and extended git diff headers
	frag	- @@ -n,m +l,k @@ lines (hunk header)
	old	- lines deleted from old file
	new	- lines added to new file

The following color names can be used:

	normal, bold, dim, l, blink, reverse, reset,
	black, red, green, yellow, blue, magenta, cyan,
	white

Signed-off-by: Junio C Hamano <junkio@cox.net>

---

 Linus Torvalds <torvalds@osdl.org> writes:

 > Which just means that we should have some way to let people set their own 
 > colors.

 At the source level it is easy to specify color and attribute
 combinations (e.g. "bold|red") by having the preprocessor
 concatenate string literals, but I was too lazy to allow that
 from the configuration level.  People might want to have that,
 though.

 BTW, while doing this, I noticed that the patch does not do the
 color output for combined diffs.  Care to look into it after
 Timo's output format series settles?

 cache.h |    1 -
 diff.c  |   94 ++++++++++++++++++++++++++++++++++++++++++++++++++++++---------
 2 files changed, 80 insertions(+), 15 deletions(-)

diff --git a/cache.h b/cache.h
index efeafea..3502fee 100644
--- a/cache.h
+++ b/cache.h
@@ -181,7 +181,6 @@ extern int assume_unchanged;
 extern int prefer_symlink_refs;
 extern int log_all_ref_updates;
 extern int warn_ambiguous_refs;
-extern int diff_rename_limit_default;
 extern int shared_repository;
 extern const char *apply_default_whitespace;
 
diff --git a/diff.c b/diff.c
index 1db0285..33c8c57 100644
--- a/diff.c
+++ b/diff.c
@@ -13,17 +13,8 @@ #include "xdiff-interface.h"
 
 static int use_size_cache;
 
-int diff_rename_limit_default = -1;
-
-int git_diff_config(const char *var, const char *value)
-{
-	if (!strcmp(var, "diff.renamelimit")) {
-		diff_rename_limit_default = git_config_int(var, value);
-		return 0;
-	}
-
-	return git_default_config(var, value);
-}
+static int diff_rename_limit_default = -1;
+static int diff_use_color_default = 0;
 
 enum color_diff {
 	DIFF_RESET = 0,
@@ -51,9 +42,6 @@ #define COLOR_MAGENTA "\033[35m"
 #define COLOR_CYAN    "\033[36m"
 #define COLOR_WHITE   "\033[37m"
 
-#define COLOR_CYANBG  "\033[46m"
-#define COLOR_GRAYBG  "\033[47m"	// Good for xterm
-
 static const char *diff_colors[] = {
 	[DIFF_RESET]    = COLOR_RESET,
 	[DIFF_PLAIN]    = COLOR_NORMAL,
@@ -63,6 +51,83 @@ static const char *diff_colors[] = {
 	[DIFF_FILE_NEW] = COLOR_GREEN,
 };
 
+static int parse_diff_color_slot(const char *var, int ofs)
+{
+	if (!strcasecmp(var+ofs, "plain"))
+		return DIFF_PLAIN;
+	if (!strcasecmp(var+ofs, "meta"))
+		return DIFF_METAINFO;
+	if (!strcasecmp(var+ofs, "frag"))
+		return DIFF_FRAGINFO;
+	if (!strcasecmp(var+ofs, "old"))
+		return DIFF_FILE_OLD;
+	if (!strcasecmp(var+ofs, "new"))
+		return DIFF_FILE_NEW;
+	die("bad config variable '%s'", var);
+}
+
+static const char *parse_diff_color_value(const char *value, const char *var)
+{
+	if (!strcasecmp(value, "normal"))
+		return COLOR_NORMAL;
+	if (!strcasecmp(value, "bold"))
+		return COLOR_BOLD;
+	if (!strcasecmp(value, "dim"))
+		return COLOR_DIM;
+	if (!strcasecmp(value, "ul"))
+		return COLOR_UL;
+	if (!strcasecmp(value, "blink"))
+		return COLOR_BLINK;
+	if (!strcasecmp(value, "reverse"))
+		return COLOR_REVERSE;
+	if (!strcasecmp(value, "reset"))
+		return COLOR_RESET;
+	if (!strcasecmp(value, "black"))
+		return COLOR_BLACK;
+	if (!strcasecmp(value, "red"))
+		return COLOR_RED;
+	if (!strcasecmp(value, "green"))
+		return COLOR_GREEN;
+	if (!strcasecmp(value, "yellow"))
+		return COLOR_YELLOW;
+	if (!strcasecmp(value, "blue"))
+		return COLOR_BLUE;
+	if (!strcasecmp(value, "magenta"))
+		return COLOR_MAGENTA;
+	if (!strcasecmp(value, "cyan"))
+		return COLOR_CYAN;
+	if (!strcasecmp(value, "white"))
+		return COLOR_WHITE;
+	die("bad config value '%s' for variable '%s'", value, var);
+}
+
+int git_diff_config(const char *var, const char *value)
+{
+	if (!strcmp(var, "diff.renamelimit")) {
+		diff_rename_limit_default = git_config_int(var, value);
+		return 0;
+	}
+	if (!strcmp(var, "diff.color")) {
+		if (!value)
+			diff_use_color_default = 1; /* bool */
+		else if (!strcasecmp(value, "auto"))
+			diff_use_color_default = isatty(1);
+		else if (!strcasecmp(value, "never"))
+			diff_use_color_default = 0;
+		else if (!strcasecmp(value, "always"))
+			diff_use_color_default = 1;
+		else
+			diff_use_color_default = git_config_bool(var, value);
+		return 0;
+	}
+	if (!strncmp(var, "diff.color.", 11)) {
+		int slot = parse_diff_color_slot(var, 11);
+		diff_colors[slot] = parse_diff_color_value(value, var);
+		return 0;
+	}
+	return git_default_config(var, value);
+}
+
 static char *quote_one(const char *str)
 {
 	int needlen;
@@ -1362,6 +1427,7 @@ void diff_setup(struct diff_options *opt
 
 	options->change = diff_change;
 	options->add_remove = diff_addremove;
+	options->color_diff = diff_use_color_default;
 }
 
 int diff_setup_done(struct diff_options *options)
-- 
1.4.1.rc1.ga77b7

^ permalink raw reply related

* [PATCH] cvsimport: setup indexes correctly for ancestors and incremental imports
From: Martin Langhoff @ 2006-06-24 11:13 UTC (permalink / raw)
  To: git, junkio, Johannes.Schindelin; +Cc: Martin Langhoff

Two bugs had slipped in the "keep one index per branch during import"
patch. Both incremental imports and new branches would see an
empty tree for their initial commit. Now we cover all the relevant
cases, checking whether we actually need to setup the index before
preparing the actual commit, and doing it.

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

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

diff --git a/git-cvsimport.perl b/git-cvsimport.perl
old mode 100644
new mode 100755
index d961b7b..1c1fd02
--- a/git-cvsimport.perl
+++ b/git-cvsimport.perl
@@ -813,11 +813,26 @@ while(<CVS>) {
 			unless ($index{$branch}) {
 			    $index{$branch} = tmpnam();
 			    $ENV{GIT_INDEX_FILE} = $index{$branch};
-			    system("git-read-tree", $branch);
+			}
+			if ($ancestor) {
+			    system("git-read-tree", $ancestor);
 			    die "read-tree failed: $?\n" if $?;
 			} else {
+			    unless ($index{$branch}) {
+				$index{$branch} = tmpnam();
+				$ENV{GIT_INDEX_FILE} = $index{$branch};
+				system("git-read-tree", $branch);
+				die "read-tree failed: $?\n" if $?;
+			    }
+			}    
+		} else {
+			# just in case
+			unless ($index{$branch}) {
+			    $index{$branch} = tmpnam();
 			    $ENV{GIT_INDEX_FILE} = $index{$branch};
-		        }
+			    system("git-read-tree", $branch);
+			    die "read-tree failed: $?\n" if $?;
+			}
 		}
 		$last_branch = $branch if $branch ne $last_branch;
 		$state = 9;
-- 
1.4.1.rc1.g59c8

^ permalink raw reply related

* Re: [PATCH] cvsimport - streamline temp index file creation and avoid creating empty tmpfiles
From: Martin Langhoff @ 2006-06-24 11:16 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Johannes Schindelin, git, Martin Langhoff
In-Reply-To: <7vslluyika.fsf@assigned-by-dhcp.cox.net>

Johannes, Junio,

I've managed to repro the problem -- which was totally reproduceable,
I was just testing the wrong version of the script. The problem was
quite obvious: when running an incremental, the first head would not
get the index created properly. Even worse, when forking a new branch,
the index would be empty too.

Fixed both cases and posted separately. Thanks for the sharp eyes, and
sorry about the bug!


martin

^ permalink raw reply

* Re: [PATCH 01/12] Introduce Git.pm (v4)
From: Petr Baudis @ 2006-06-24 11:16 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7vu06bymtr.fsf@assigned-by-dhcp.cox.net>

Dear diary, on Sat, Jun 24, 2006 at 10:33:52AM CEST, I got a letter
where Junio C Hamano <junkio@cox.net> said that...
> The reason it failed?  Well, it could not find Git.pm because
> the changes to fmt-merge-msg was done for distros not for people
> who install under their home directories.

I don't understand what are you trying to say here...

> Now, I am quite unhappy about the situation (and it is not your
> fault).  "git pull" is something almost everybody uses, and
> having the series means they would need to make sure whereever
> Git.pm is installed is on their PERL5LIB as things currently
> stand.

...because well, they do:

$(patsubst %.perl,%,$(SCRIPT_PERL)) : % : %.perl
	rm -f $@ $@+
	sed -e '1s|#!.*perl\(.*\)|#!$(PERL_PATH_SQ)\1 -I'"$$(make -s -C perl instlibdir)"'|' \
	    -e 's/@@GIT_VERSION@@/$(GIT_VERSION)/g' \
	    $@.perl >$@+
	chmod +x $@+
	mv $@+ $@

(This is also why I was a bit confused by your make test patch - it does
not "fix" anything per se since no tests directly use Git.pm.)

-- 
				Petr "Pasky" Baudis
Stuff: http://pasky.or.cz/
A person is just about as big as the things that make them angry.

^ 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