Git development
 help / color / mirror / Atom feed
* [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

* [PATCH] diff --color: use reset sequence when we mean reset.
From: Junio C Hamano @ 2006-06-24 11:23 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: git
In-Reply-To: <Pine.LNX.4.64.0606221301500.5498@g5.osdl.org>


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

 Linus Torvalds <torvalds@osdl.org> writes:

 >  - always reset the color _before_ printing out the newline.
 >
 >    This is actually important. You (and Johannes) didn't see it, because 
 >    it only matters if you set the background, but if you don't do this, 
 >    you get some random and funky behaviour if you pick a color with a 
 >    non-default background (which still potentially has problems with tabs 
 >    etc, but less so).

 Doh.

 I think you did not see it until you tried "git diff" with
 a stat-dirty but otherwise unmodified file.

 diff.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/diff.c b/diff.c
index 33c8c57..549f4e0 100644
--- a/diff.c
+++ b/diff.c
@@ -681,7 +681,7 @@ static void builtin_diff(const char *nam
 	const char *lbl[2];
 	char *a_one, *b_two;
 	const char *set = get_color(o->color_diff, DIFF_METAINFO);
-	const char *reset = get_color(o->color_diff, DIFF_PLAIN);
+	const char *reset = get_color(o->color_diff, DIFF_RESET);
 
 	a_one = quote_two("a/", name_a);
 	b_two = quote_two("b/", name_b);
-- 
1.4.1.rc1.ga77b7

^ permalink raw reply related

* Re: [PATCH] cvsimport - streamline temp index file creation and avoid creating empty tmpfiles
From: Junio C Hamano @ 2006-06-24 11:28 UTC (permalink / raw)
  To: Martin Langhoff; +Cc: Johannes Schindelin, git
In-Reply-To: <46a038f90606240416n563288f5q99a5ac81723776c3@mail.gmail.com>

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

> 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.  Will be in "next".

^ permalink raw reply

* Re: [PATCH 2/5] Rework diff options
From: Timo Hirvonen @ 2006-06-24 11:29 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7vodwj11qa.fsf@assigned-by-dhcp.cox.net>

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

> > 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.

I'll just remove this change from the fixed patch 2/5.  New version of
the patch 3/5 should then fix this logic.

> > @@ -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?

Will fix.  Originally I made --name-status imply --name-only but changed
it and forgot to fix this.

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

I'll just make it die() then. If it breaks something then the code is
really dumb anyway.

> > 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.

If we first initialize it to 0 instead of DIFF_FORMAT_RAW and after
command line flags have been parsed, if it still is 0, then default to
DIFF_FORMAT_PATCH.

>  - 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".

Yes, all this makes sense.

-- 
http://onion.dynserv.net/~timo/

^ permalink raw reply

* [RFC] git-fetch - repack in the background after fetching
From: Martin Langhoff @ 2006-06-24 11:30 UTC (permalink / raw)
  To: git, junkio; +Cc: Martin Langhoff

Check whether we have a large set of unpacked objects and repack
after the fetch, but don't for the user to wait for us. Conditional
on core.autorepack =! no.

Having ' handle concurrent pruning of packed objects'
(637cdd9d1d997fca34a1fc668fed1311e30fe95f) from Jeff King it should
be safe to repack and prune in the background.

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

---

This is a follow up to a similar patch earlier
http://www.gelato.unsw.edu.au/archives/git/0605/21401.html -- is there 
interest in making GIT more friendly to users who don't know or care
about packing and repacking their repos?

I loathe to do this conditionally only on the count of unpacked
objects. If there's a quick'n'dirty way of asking portably whether
the machine is busy or otherwise resource-constrained (ie: on battery)
it should use it to avoid running repack at inconvenient times.

---
 git-fetch.sh |    9 +++++++++
 1 files changed, 9 insertions(+), 0 deletions(-)

diff --git a/git-fetch.sh b/git-fetch.sh
index 48818f8..7211318 100755
--- a/git-fetch.sh
+++ b/git-fetch.sh
@@ -427,3 +427,12 @@ case ",$update_head_ok,$orig_head," in
 	fi
 	;;
 esac
+
+if test "$(git-repo-config --get core.autorepack)" != 'no'
+then
+	if test $(git rev-list --unpacked --all | wc -l) -gt 1000
+	then
+		echo "Repacking in the background"
+		nice git repack -a -d -q &
+	fi
+fi
-- 
1.4.1.rc1.g59c8

^ permalink raw reply related

* Re: [PATCH 01/12] Introduce Git.pm (v4)
From: Petr Baudis @ 2006-06-24 11:52 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <20060624111657.GR21864@pasky.or.cz>

Dear diary, on Sat, Jun 24, 2006 at 01:16:57PM CEST, I got a letter
where Petr Baudis <pasky@suse.cz> said that...
> $(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.)

And this makes the Perl scripts work even without make install:

Signed-off-by: Petr Baudis <pasky@suse.cz>

diff --git a/Makefile b/Makefile
index 7842195..d614f18 100644
--- a/Makefile
+++ b/Makefile
@@ -509,7 +509,9 @@ common-cmds.h: Documentation/git-*.txt
 
 $(patsubst %.perl,%,$(SCRIPT_PERL)) : % : %.perl
 	rm -f $@ $@+
-	sed -e '1s|#!.*perl\(.*\)|#!$(PERL_PATH_SQ)\1 -I'"$$(make -s -C perl instlibdir)"'|' \
+	sed -e '1s|#!.*perl|#!$(PERL_PATH_SQ)|' \
+	    -e '2i\
+	        use lib qw ('"$$(make -s -C perl instlibdir)"' '"$$(pwd)"'/perl/blib/lib);' \
 	    -e 's/@@GIT_VERSION@@/$(GIT_VERSION)/g' \
 	    $@.perl >$@+
 	chmod +x $@+
diff --git a/perl/Makefile.PL b/perl/Makefile.PL
index 54e8b20..2cbd227 100644
--- a/perl/Makefile.PL
+++ b/perl/Makefile.PL
@@ -2,6 +2,9 @@ use ExtUtils::MakeMaker;
 
 sub MY::postamble {
 	return <<'MAKE_FRAG';
+all::
+	cp blib/arch/auto/Git/* blib/lib/auto/Git/
+
 instlibdir:
 	@echo $(INSTALLSITELIB)
 

-- 
				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 related

* Re: [PATCH 01/12] Introduce Git.pm (v4)
From: Junio C Hamano @ 2006-06-24 11:57 UTC (permalink / raw)
  To: Petr Baudis; +Cc: git
In-Reply-To: <20060624111657.GR21864@pasky.or.cz>

Petr Baudis <pasky@suse.cz> writes:

> (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.)

You are right.

You do not want to be testing installed version, but the one
freshly built, so the patch does not have any effect, except for
one case: testing before installing Git.pm for the first time
anywhere yet.  -I prepends the directory to the search path, so
we are not testing the freshly built copy at all.

Is there a way from the environment to override this behaviour,
so that we can run the tests properly?  I think PERL5LIB and
PERLLIB are defeated by having -I there (that's why I said I
liked what Fredrik did with his Python script, which appends the
final installed location to the search path).  I think unshift
into @INC by hand (i.e. without even using use lib "$path")
would do what we want, but I feel that is a bit too ugly just 
for the testing X-<.

I suspect we would need to think this a bit more... sigh.

> ...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 $@+ $@

I'll need to look at why it fails for me, but the above seems to
be doing the right thing, from a superficial look at least.

git-fmt-merge-msg substituted like the above begins with:

	#!/usr/bin/perl -w -I/home/junio/git-pu/share/perl/5.8.8

because my $(prefix) is /home/junio/git-pu/ when building from "pu"
branch.  Then it goes on to create ~/git-pu/{lib,share}/perl/5.8.8
and does this:

make -C perl install
make[1]: Entering directory `/opt/git/git.git/perl'
Installing /home/junio/git-pu/lib/perl/5.8.8/auto/Git/Git.so
Installing /home/junio/git-pu/lib/perl/5.8.8/auto/Git/Git.bs
Files found in blib/arch: installing files in blib/lib into architecture dependent library tree
Installing /home/junio/git-pu/lib/perl/5.8.8/Git.pm
Installing /home/junio/git-pu/lib/perl/5.8.8/Error.pm
Installing /home/junio/git-pu/man/man3/Error.3pm
Installing /home/junio/git-pu/man/man3/Git.3pm
Writing /home/junio/git-pu/lib/perl/5.8.8/auto/Git/.packlist
Appending installation info to /home/junio/git-pu/lib/perl/5.8.8/perllocal.pod

It appears that this is needed perhaps?

diff --git a/perl/Makefile.PL b/perl/Makefile.PL
index 54e8b20..92c140d 100644
--- a/perl/Makefile.PL
+++ b/perl/Makefile.PL
@@ -3,7 +3,7 @@ use ExtUtils::MakeMaker;
 sub MY::postamble {
 	return <<'MAKE_FRAG';
 instlibdir:
-	@echo $(INSTALLSITELIB)
+	@echo $(INSTALLSITEARCH)
 
 MAKE_FRAG
 }

^ permalink raw reply related

* On boolean configuration variables...
From: Junio C Hamano @ 2006-06-24 12:28 UTC (permalink / raw)
  To: git; +Cc: Johannes Schindelin

Boolean configuration variables in $GIT_DIR/config are a bit
strange.

	[bool]
        	var1
                var2 =
                var3 = true
                var4 = yes
                var5 = 1
                var6 = 2
                var7 = false
                var8 = no
                var9 = 0

var1, var3, var5, and var6 are "true"; var2, var7 and var9 are
"false".  var4 and var8 are syntax errors.

Currently "git repo-config --bool --get bool.var1" returns
"false", which is fixed by the attached patch, but I am
wondering if it is a good idea to allow "yes" and "no" as well.

-- >8 --
[PATCH] repo-config: fix printing of bool

When a bool variable appears without any value, it means true.
However, replacing the NULL value with an empty string, an earlier
commit f067a13745fbeae1aa357876348a00e5edd0a629 broke show-config.

Signed-off-by: Junio C Hamano <junkio@cox.net>
---
 repo-config.c |    9 +++------
 1 files changed, 3 insertions(+), 6 deletions(-)

diff --git a/repo-config.c b/repo-config.c
index ab8f1af..743f02b 100644
--- a/repo-config.c
+++ b/repo-config.c
@@ -29,16 +29,13 @@ static int show_config(const char* key_,
 	const char *vptr = value;
 	int dup_error = 0;
 
-	if (value_ == NULL)
-		value_ = "";
-
 	if (!use_key_regexp && strcmp(key_, key))
 		return 0;
 	if (use_key_regexp && regexec(key_regexp, key_, 0, NULL, 0))
 		return 0;
 	if (regexp != NULL &&
 			 (do_not_match ^
-			  regexec(regexp, value_, 0, NULL, 0)))
+			  regexec(regexp, (value_?value_:""), 0, NULL, 0)))
 		return 0;
 
 	if (show_keys)
@@ -46,11 +43,11 @@ static int show_config(const char* key_,
 	if (seen && !do_all)
 		dup_error = 1;
 	if (type == T_INT)
-		sprintf(value, "%d", git_config_int(key_, value_));
+		sprintf(value, "%d", git_config_int(key_, value_?value_:""));
 	else if (type == T_BOOL)
 		vptr = git_config_bool(key_, value_) ? "true" : "false";
 	else
-		vptr = value_;
+		vptr = value_?value_:"";
 	seen++;
 	if (dup_error) {
 		error("More than one value for the key %s: %s",
-- 
1.4.1.rc1.ga77b7

^ permalink raw reply related

* Ocupacion temporal para un trabajo a tiempo parcial
From: Diann @ 2006-06-25  0:36 UTC (permalink / raw)
  To: git

Hola!.

Este correo electrónico le muestra una oferta de trabajo, que podría ser interesante a usted.

Gerente financiero situado en su país! Trabajo en Internet con buen sueldo!
GoldLeader Inc. busca a personas enérgicas y responsables para completar el
puesto de encargado de deudores de media jornada.
Como encargado de deudores, usted será el responsable de procesar y facilitar
las transferencias de fondos iniciadas por nuestros clientes bajo la supervisión del gerente regional.

 Ofrecemos:

- Ventajas buenas (más de 1000 $ por semana);

- Contrato legal;

 Se precisa puntualidad, capacidades directivas y responsabilidad.

Usted también recibirá instrucciones detalladas para acciones
subsecuentes de nuestro gerente, con información sobre como recibir/transferir el dinero.

 
1. Ser capaz de comprobar su correo electrónico varias veces por día

2. Ser capaz de responder a correos electrónico inmediatamente

3. Ser capaz de trabajar horas extra si es necesario

4. Ser responsable y trabajador

5. Hablar ingles

6. Tener más de 21 años;

7. Debería tener una cuenta bancaria personal

Para información adicionales y preguntas sobre el puesto de trabajo,
por favor envíe sus datos de contacto a  career@goldleader.biz

NO SON VENTAS!!! NO SON LLAMADAS!!!
USTED NO NECESITA DINERO PARA COMENZAR!!!
 
Gracias por su atencion.
Con respeto,
Departamento de personal Goldleader Inc.
http://www.goldleader.biz

^ permalink raw reply

* Re: [Patch] trap: exit: invalid signal specification
From: S.Çağlar Onur @ 2006-06-24 12:54 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7vejxf5ktc.fsf@assigned-by-dhcp.cox.net>

[-- Attachment #1: Type: text/plain, Size: 1232 bytes --]

Cumartesi 24 Haziran 2006 05:50 tarihinde, Junio C Hamano şunları yazmıştı: 
> 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?

Sorry to not clear enough previously, for Turkic locales (tr_TR, az_AZ etc.) 
upper(i) != I. More detailed analysis can be found at 
http://www.i18nguy.com/unicode/turkish-i18n.html, "Why Applications Fail With 
The Turkish Language" section. This is the main reason of this problem. 

According to its man page signals defined with uppercase letters but also 
different trap implementations may permit lowercase signal names as an 
extension. 

As an example bash (v. 3.1.17) permits lowercase signal names but it converts 
this lowercase signal names into uppercase ones while interpreting the 
script. But for our "Turkish has 4 letter "I"s" problem this convert to 
uppercase one process fails but for bash invalid signal names not be 
considered a syntax error and do not cause the shell to abort. 

Yours
-- 
S.Çağlar Onur <caglar@pardus.org.tr>
http://cekirdek.pardus.org.tr/~caglar/

Linux is like living in a teepee. No Windows, no Gates and an Apache in house!

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

^ permalink raw reply

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

> 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.

My apologies for being unclear.

The place where a noticeable (if not disastrous) difference can appear
is x86, which has a lot more models with "interesting" performance
characteristics.  In particular, Intel is fond of building CPUs with a
very small "sweet spot".

The openssl SHA1 code had to be reworked to not suck on a P4, with the
resultant performance change:

#               compared with original  compared with Intel cc
#               assembler impl.         generated code
# Pentium       -16%                    +48%
# PIII/AMD      +8%                     +16%
# P4            +85%(!)                 +45%

The original code had the most popular round (what I call
ROUND_MIX(F2,...))) implemented as follows, with single-uop
instructions (no load+op) scheduled for the Pentium pipeline:
(A..E are working variables, S and T are temps)

	movl    16(%esp),S	U  \
        movl    24(%esp),T	 V  \
        xorl    S,T		U    \
        movl    48(%esp),S	 V    > "MIX", pentium-optimized
        xorl    S,T		U    /
        movl    4(%esp),S	 V  /
        xorl    S,T		U  /
        movl	B,S		 V
	roll	$1,T		U	Rotate of mix (SHA0 -> SHA1 fix)
	xor	C,S		 V
	mov	T,16(%esp)	U	Store back W[i]
	xor	D,S		 V	Finish computing F(B,C,D) = B^C^D
	lea	K(T,E),E	U	Add K and W[i] to E
	mov	A,T		 V
	roll	$5,T		UV
	rorl	$1,B		U
	add	S,E		 V
	rorl	$1,B		U
	add	T,E		 V

While the P4-optimized version goes:
	movl	B,S
	movl	16(%esp),T
	rorl	$2,B
	xorl	24(%esp),T
	xorl	C,S
	xorl	48(%esp),T
	xorl	D,S		This is F(B,C,D) = B^C^D
	xorl	4(%esp),T
	roll	$1,T		Rotate of mix (SHA0 -> SHA1 fix)
	addl	S,E
	movl	T,16(%esp)
	movl	A,S
	roll	$5,S
	lea	K(E,T),E
	add	S,E

(The original code actually rotates the working variables around 6
registers, not 5, but I've rearranged the last couple of instructions
to rotate around 5.)

^ permalink raw reply

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

Dear diary, on Sat, Jun 24, 2006 at 01:57:31PM CEST, I got a letter
where Junio C Hamano <junkio@cox.net> said that...
> Petr Baudis <pasky@suse.cz> writes:
> 
> > (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.)
> 
> You are right.
> 
> You do not want to be testing installed version, but the one
> freshly built, so the patch does not have any effect, except for
> one case: testing before installing Git.pm for the first time
> anywhere yet.  -I prepends the directory to the search path, so
> we are not testing the freshly built copy at all.
> 
> Is there a way from the environment to override this behaviour,
> so that we can run the tests properly?  I think PERL5LIB and
> PERLLIB are defeated by having -I there (that's why I said I
> liked what Fredrik did with his Python script, which appends the
> final installed location to the search path).  I think unshift
> into @INC by hand (i.e. without even using use lib "$path")
> would do what we want, but I feel that is a bit too ugly just 
> for the testing X-<.

PERL5LIB and use lib at the same time works for me. Anyway, with the
second patch I've sent things should work well even if you don't have
Git.pm installed anywhere yet.

> diff --git a/perl/Makefile.PL b/perl/Makefile.PL
> index 54e8b20..92c140d 100644
> --- a/perl/Makefile.PL
> +++ b/perl/Makefile.PL
> @@ -3,7 +3,7 @@ use ExtUtils::MakeMaker;
>  sub MY::postamble {
>  	return <<'MAKE_FRAG';
>  instlibdir:
> -	@echo $(INSTALLSITELIB)
> +	@echo $(INSTALLSITEARCH)
>  
>  MAKE_FRAG
>  }

Oh, yes; that line came from the time when we had no .xs yet. It is not
visible here since both arch-specific and non-arch-specific libraries
get installed to ~/lib.

-- 
				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

* Re: [PATCH] cvsimport: setup indexes correctly for ancestors and   incremental imports
From: Sergey Vlasov @ 2006-06-24 13:06 UTC (permalink / raw)
  To: Martin Langhoff; +Cc: git, junkio, Johannes.Schindelin
In-Reply-To: <11511475882820-git-send-email-martin@catalyst.net.nz>

[-- Attachment #1: Type: text/plain, Size: 1850 bytes --]

On Sat, 24 Jun 2006 23:13:08 +1200 Martin Langhoff wrote:

> 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}) {

This seems to be dead code - even if $index{$branch} was not set, it
will be set inside "unless ($index{$branch})" above.  Or there is
another bug here?

> +				$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;

[-- Attachment #2: Type: application/pgp-signature, Size: 190 bytes --]

^ permalink raw reply

* Re: From b65bc21e7d8dc8cafc70dfa6354cb66b8874b2d9 Mon Sep 17 00:00:00 2001 [PATCH] Makefile: add framework to verify and bench sha1 implementations.
From: linux @ 2006-06-24  9:29 UTC (permalink / raw)
  To: git, junkio; +Cc: linux
In-Reply-To: <7v3bdv0xyd.fsf_-_@assigned-by-dhcp.cox.net>

Nice work, but I might point out that the original PPC SHA bug was hashing
more than 0.5G of contiguous data in a *single* call to SHA1_Update,
while your test program works with 8K buffers.

^ permalink raw reply

* Re: [PATCH 07/12] Git.pm: Better error handling
From: Petr Baudis @ 2006-06-24 13:17 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7vmzc3ymnu.fsf@assigned-by-dhcp.cox.net>

Dear diary, on Sat, Jun 24, 2006 at 10:37:25AM CEST, I got a letter
where Junio C Hamano <junkio@cox.net> said that...
> 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
> --------------------------------

Oh, I forgot to fix it in the .xs. :-(

> 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
...

Ah, so THAT's where you put the NORETURN thing. ;-)

> --------------------------------
> 
> (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?

It is harmless, but this should fix it:

Signed-off-by: Petr Baudis <pasky@suse.cz>

diff --git a/Makefile b/Makefile
index d614f18..91bef4e 100644
--- a/Makefile
+++ b/Makefile
@@ -230,7 +230,8 @@ BUILTIN_OBJS = \
 	builtin-update-ref.o
 
 GITLIBS = $(LIB_FILE) $(XDIFF_LIB)
-LIBS = $(GITLIBS) -lz
+
+EXTLIBS = -lz
 
 #
 # Platform specific tweaks
@@ -380,14 +381,14 @@ ifdef NEEDS_LIBICONV
 	else
 		ICONV_LINK =
 	endif
-	LIBS += $(ICONV_LINK) -liconv
+	EXTLIBS += $(ICONV_LINK) -liconv
 endif
 ifdef NEEDS_SOCKET
-	LIBS += -lsocket
+	EXTLIBS += -lsocket
 	SIMPLE_LIB += -lsocket
 endif
 ifdef NEEDS_NSL
-	LIBS += -lnsl
+	EXTLIBS += -lnsl
 	SIMPLE_LIB += -lnsl
 endif
 ifdef NO_D_TYPE_IN_DIRENT
@@ -446,7 +447,7 @@ ifdef MOZILLA_SHA1
 	LIB_OBJS += mozilla-sha1/sha1.o
 else
 	SHA1_HEADER = <openssl/sha.h>
-	LIBS += $(LIB_4_CRYPTO)
+	EXTLIBS += $(LIB_4_CRYPTO)
 endif
 endif
 endif
@@ -469,9 +470,13 @@ PERL_PATH_SQ = $(subst ','\'',$(PERL_PAT
 PYTHON_PATH_SQ = $(subst ','\'',$(PYTHON_PATH))
 GIT_PYTHON_DIR_SQ = $(subst ','\'',$(GIT_PYTHON_DIR))
 
+LIBS = $(GITLIBS) $(EXTLIBS)
+
 ALL_CFLAGS += -DSHA1_HEADER='$(SHA1_HEADER_SQ)' $(COMPAT_CFLAGS)
 LIB_OBJS += $(COMPAT_OBJS)
 export prefix TAR INSTALL DESTDIR SHELL_PATH template_dir
+
+
 ### Build rules
 
 all: $(ALL_PROGRAMS) $(BUILT_INS) git$X gitk
@@ -601,7 +606,7 @@ perl/Makefile:	perl/Git.pm perl/Makefile
 	(cd perl && $(PERL_PATH) Makefile.PL \
 		PREFIX="$(prefix)" \
 		DEFINE="$(ALL_CFLAGS) -DGIT_VERSION=\\\"$(GIT_VERSION)\\\"" \
-		LIBS="$(LIBS)")
+		LIBS="$(EXTLIBS)")
 
 doc:
 	$(MAKE) -C Documentation all

> --------------------------------
> 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.

What about just this?

Signed-off-by: Petr Baudis <pasky@suse.cz>

diff --git a/Makefile b/Makefile
index 91bef4e..55c07b6 100644
--- a/Makefile
+++ b/Makefile
@@ -731,7 +731,8 @@ clean:
 	rm -f $(GIT_TARNAME).tar.gz git-core_$(GIT_VERSION)-*.tar.gz
 	rm -f $(htmldocs).tar.gz $(manpages).tar.gz
 	$(MAKE) -C Documentation/ clean
-	[ ! -e perl/Makefile ] || $(MAKE) -C perl/ clean
+	# Try twice in case the Makefile has been rebuilt
+	[ ! -e perl/Makefile ] || $(MAKE) -C perl/ clean || $(MAKE) -C perl/ clean
 	$(MAKE) -C templates/ clean
 	$(MAKE) -C t/ clean
 	rm -f GIT-VERSION-FILE GIT-CFLAGS

-- 
				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 related

* [PATCH] Rename safe_strncpy() to strlcpy().
From: Peter Eriksen @ 2006-06-24 14:01 UTC (permalink / raw)
  To: git

This cleans up the use of safe_strncpy() even more.  Since it has the
same semantics as strlcpy() use this name instead.  Also move the
definition from inside path.c to its own file compat/strlcpy.c, and use
it conditionally at compile time, since some platforms already has
strlcpy().  It's included in the same way as compat/setenv.c.

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

I've introduced a NO_STRLCPY variable in the Makefile.  What do
you think about this?

I've made a qualified guess as to what systems have strlcpy() in
their standard library, but I am probably not completely right.

These are the ones I've said have it:
OpenBSD, FreeBSD, NetBSD

These I've said don't:
Linux, Darwin, SunOS, Cygwin, AIX, IRIX64

Regards,

Peter

 Makefile           |   14 ++++++++++++++
 builtin-log.c      |    2 +-
 builtin-tar-tree.c |    4 ++--
 cache.h            |    1 -
 compat/strlcpy.c   |   13 +++++++++++++
 config.c           |    6 +++---
 git-compat-util.h  |    5 +++++
 http-fetch.c       |    6 +++---
 http-push.c        |    6 +++---
 ident.c            |    4 ++--
 path.c             |   15 +--------------
 sha1_name.c        |    2 +-
 12 files changed, 48 insertions(+), 30 deletions(-)

diff --git a/Makefile b/Makefile
index e29e3fa..cde619c 100644
--- a/Makefile
+++ b/Makefile
@@ -26,6 +26,8 @@ # d_type in struct dirent (latest Cygwin
 #
 # Define NO_STRCASESTR if you don't have strcasestr.
 #
+# Define NO_STRLCPY if you don't have strlcpy.
+#
 # Define NO_SETENV if you don't have setenv in the C library.
 #
 # Define NO_SYMLINK_HEAD if you never want .git/HEAD to be a symbolic link.
@@ -240,9 +242,13 @@ # We choose to avoid "if .. else if .. e
 # because maintaining the nesting to match is a pain.  If
 # we had "elif" things would have been much nicer...
 
+ifeq ($(uname_S),Linux)
+	NO_STRLCPY = YesPlease
+endif
 ifeq ($(uname_S),Darwin)
 	NEEDS_SSL_WITH_CRYPTO = YesPlease
 	NEEDS_LIBICONV = YesPlease
+	NO_STRLCPY = YesPlease
 	## fink
 	ifeq ($(shell test -d /sw/lib && echo y),y)
 		ALL_CFLAGS += -I/sw/include
@@ -259,6 +265,7 @@ ifeq ($(uname_S),SunOS)
 	NEEDS_NSL = YesPlease
 	SHELL_PATH = /bin/bash
 	NO_STRCASESTR = YesPlease
+	NO_STRLCPY = YesPlease
 	ifeq ($(uname_R),5.8)
 		NEEDS_LIBICONV = YesPlease
 		NO_UNSETENV = YesPlease
@@ -276,6 +283,7 @@ ifeq ($(uname_O),Cygwin)
 	NO_D_TYPE_IN_DIRENT = YesPlease
 	NO_D_INO_IN_DIRENT = YesPlease
 	NO_STRCASESTR = YesPlease
+	NO_STRLCPY = YesPlease
 	NO_SYMLINK_HEAD = YesPlease
 	NEEDS_LIBICONV = YesPlease
 	# There are conflicting reports about this.
@@ -305,12 +313,14 @@ ifeq ($(uname_S),NetBSD)
 endif
 ifeq ($(uname_S),AIX)
 	NO_STRCASESTR=YesPlease
+	NO_STRLCPY = YesPlease
 	NEEDS_LIBICONV=YesPlease
 endif
 ifeq ($(uname_S),IRIX64)
 	NO_IPV6=YesPlease
 	NO_SETENV=YesPlease
 	NO_STRCASESTR=YesPlease
+	NO_STRLCPY = YesPlease
 	NO_SOCKADDR_STORAGE=YesPlease
 	SHELL_PATH=/usr/gnu/bin/bash
 	ALL_CFLAGS += -DPATH_MAX=1024
@@ -403,6 +413,10 @@ ifdef NO_STRCASESTR
 	COMPAT_CFLAGS += -DNO_STRCASESTR
 	COMPAT_OBJS += compat/strcasestr.o
 endif
+ifdef NO_STRLCPY
+	COMPAT_CFLAGS += -DNO_STRLCPY
+	COMPAT_OBJS += compat/strlcpy.o
+endif
 ifdef NO_SETENV
 	COMPAT_CFLAGS += -DNO_SETENV
 	COMPAT_OBJS += compat/setenv.o
diff --git a/builtin-log.c b/builtin-log.c
index 5a8a50b..44d2d13 100644
--- a/builtin-log.c
+++ b/builtin-log.c
@@ -115,7 +115,7 @@ static void reopen_stdout(struct commit 
 	int len = 0;
 
 	if (output_directory) {
-		safe_strncpy(filename, output_directory, 1010);
+		strlcpy(filename, output_directory, 1010);
 		len = strlen(filename);
 		if (filename[len - 1] != '/')
 			filename[len++] = '/';
diff --git a/builtin-tar-tree.c b/builtin-tar-tree.c
index 39a61b6..f2e48aa 100644
--- a/builtin-tar-tree.c
+++ b/builtin-tar-tree.c
@@ -233,8 +233,8 @@ static void write_entry(const unsigned c
 	/* XXX: should we provide more meaningful info here? */
 	sprintf(header.uid, "%07o", 0);
 	sprintf(header.gid, "%07o", 0);
-	safe_strncpy(header.uname, "git", sizeof(header.uname));
-	safe_strncpy(header.gname, "git", sizeof(header.gname));
+	strlcpy(header.uname, "git", sizeof(header.uname));
+	strlcpy(header.gname, "git", sizeof(header.gname));
 	sprintf(header.devmajor, "%07o", 0);
 	sprintf(header.devminor, "%07o", 0);
 
diff --git a/cache.h b/cache.h
index efeafea..14358a8 100644
--- a/cache.h
+++ b/cache.h
@@ -216,7 +216,6 @@ enum sharedrepo {
 int git_config_perm(const char *var, const char *value);
 int adjust_shared_perm(const char *path);
 int safe_create_leading_directories(char *path);
-size_t safe_strncpy(char *, const char *, size_t);
 char *enter_repo(char *path, int strict);
 
 /* Read and unpack a sha1 file into memory, write memory to a sha1 file */
diff --git a/compat/strlcpy.c b/compat/strlcpy.c
new file mode 100644
index 0000000..b66856a
--- /dev/null
+++ b/compat/strlcpy.c
@@ -0,0 +1,13 @@
+#include <string.h>
+
+size_t gitstrlcpy(char *dest, const char *src, size_t size)
+{
+	size_t ret = strlen(src);
+
+	if (size) {
+		size_t len = (ret >= size) ? size - 1 : ret;
+		memcpy(dest, src, len);
+		dest[len] = '\0';
+	}
+	return ret;
+}
diff --git a/config.c b/config.c
index 3e077d4..ec44827 100644
--- a/config.c
+++ b/config.c
@@ -280,17 +280,17 @@ int git_default_config(const char *var, 
 	}
 
 	if (!strcmp(var, "user.name")) {
-		safe_strncpy(git_default_name, value, sizeof(git_default_name));
+		strlcpy(git_default_name, value, sizeof(git_default_name));
 		return 0;
 	}
 
 	if (!strcmp(var, "user.email")) {
-		safe_strncpy(git_default_email, value, sizeof(git_default_email));
+		strlcpy(git_default_email, value, sizeof(git_default_email));
 		return 0;
 	}
 
 	if (!strcmp(var, "i18n.commitencoding")) {
-		safe_strncpy(git_commit_encoding, value, sizeof(git_commit_encoding));
+		strlcpy(git_commit_encoding, value, sizeof(git_commit_encoding));
 		return 0;
 	}
 
diff --git a/git-compat-util.h b/git-compat-util.h
index b3d4cf5..93f5580 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h
@@ -79,6 +79,11 @@ #define strcasestr gitstrcasestr
 extern char *gitstrcasestr(const char *haystack, const char *needle);
 #endif
 
+#ifdef NO_STRLCPY
+#define strlcpy gitstrlcpy
+extern size_t gitstrlcpy(char *, const char *, size_t);
+#endif
+
 static inline void *xmalloc(size_t size)
 {
 	void *ret = malloc(size);
diff --git a/http-fetch.c b/http-fetch.c
index 2b63d89..44eba5f 100644
--- a/http-fetch.c
+++ b/http-fetch.c
@@ -584,8 +584,8 @@ static void process_alternates_response(
 			// skip 'objects' at end
 			if (okay) {
 				target = xmalloc(serverlen + posn - i - 6);
-				safe_strncpy(target, base, serverlen);
-				safe_strncpy(target + serverlen, data + i, posn - i - 6);
+				strlcpy(target, base, serverlen);
+				strlcpy(target + serverlen, data + i, posn - i - 6);
 				if (get_verbosely)
 					fprintf(stderr,
 						"Also look at %s\n", target);
@@ -727,7 +727,7 @@ xml_cdata(void *userData, const XML_Char
 	if (ctx->cdata)
 		free(ctx->cdata);
 	ctx->cdata = xmalloc(len + 1);
-	safe_strncpy(ctx->cdata, s, len + 1);
+	strlcpy(ctx->cdata, s, len + 1);
 }
 
 static int remote_ls(struct alt_base *repo, const char *path, int flags,
diff --git a/http-push.c b/http-push.c
index 8d472f0..3c89a17 100644
--- a/http-push.c
+++ b/http-push.c
@@ -1271,7 +1271,7 @@ xml_cdata(void *userData, const XML_Char
 	if (ctx->cdata)
 		free(ctx->cdata);
 	ctx->cdata = xmalloc(len + 1);
-	safe_strncpy(ctx->cdata, s, len + 1);
+	strlcpy(ctx->cdata, s, len + 1);
 }
 
 static struct remote_lock *lock_remote(char *path, long timeout)
@@ -1473,7 +1473,7 @@ static void process_ls_object(struct rem
 		return;
 	path += 8;
 	obj_hex = xmalloc(strlen(path));
-	safe_strncpy(obj_hex, path, 3);
+	strlcpy(obj_hex, path, 3);
 	strcpy(obj_hex + 2, path + 3);
 	one_remote_object(obj_hex);
 	free(obj_hex);
@@ -2172,7 +2172,7 @@ static void fetch_symref(char *path, cha
 	/* If it's a symref, set the refname; otherwise try for a sha1 */
 	if (!strncmp((char *)buffer.buffer, "ref: ", 5)) {
 		*symref = xmalloc(buffer.posn - 5);
-		safe_strncpy(*symref, (char *)buffer.buffer + 5, buffer.posn - 5);
+		strlcpy(*symref, (char *)buffer.buffer + 5, buffer.posn - 5);
 	} else {
 		get_sha1_hex(buffer.buffer, sha1);
 	}
diff --git a/ident.c b/ident.c
index 7b44cbd..efec97f 100644
--- a/ident.c
+++ b/ident.c
@@ -71,9 +71,9 @@ int setup_ident(void)
 		len = strlen(git_default_email);
 		git_default_email[len++] = '.';
 		if (he && (domainname = strchr(he->h_name, '.')))
-			safe_strncpy(git_default_email + len, domainname + 1, sizeof(git_default_email) - len);
+			strlcpy(git_default_email + len, domainname + 1, sizeof(git_default_email) - len);
 		else
-			safe_strncpy(git_default_email + len, "(none)", sizeof(git_default_email) - len);
+			strlcpy(git_default_email + len, "(none)", sizeof(git_default_email) - len);
 	}
 	/* And set the default date */
 	datestamp(git_default_date, sizeof(git_default_date));
diff --git a/path.c b/path.c
index 36972fd..db8905f 100644
--- a/path.c
+++ b/path.c
@@ -77,25 +77,12 @@ int git_mkstemp(char *path, size_t len, 
 		pch += n;
 	}
 
-	safe_strncpy(pch, template, len);
+	strlcpy(pch, template, len);
 
 	return mkstemp(path);
 }
 
 
-size_t safe_strncpy(char *dest, const char *src, size_t size)
-{
-	size_t ret = strlen(src);
-
-	if (size) {
-		size_t len = (ret >= size) ? size - 1 : ret;
-		memcpy(dest, src, len);
-		dest[len] = '\0';
-	}
-	return ret;
-}
-
-
 int validate_symref(const char *path)
 {
 	struct stat st;
diff --git a/sha1_name.c b/sha1_name.c
index cd85d1f..f2cbafa 100644
--- a/sha1_name.c
+++ b/sha1_name.c
@@ -262,7 +262,7 @@ static int get_sha1_basic(const char *st
 		if (str[am] == '@' && str[am+1] == '{' && str[len-1] == '}') {
 			int date_len = len - am - 3;
 			char *date_spec = xmalloc(date_len + 1);
-			safe_strncpy(date_spec, str + am + 2, date_len + 1);
+			strlcpy(date_spec, str + am + 2, date_len + 1);
 			at_time = approxidate(date_spec);
 			free(date_spec);
 			len = am;
-- 
1.4.1.rc1.g8637

^ permalink raw reply related

* [PATCH 1/2] Git.pm: Add config() method
From: Petr Baudis @ 2006-06-24 14:34 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git

This accessor will retrieve value(s) of the given configuration variable.

Signed-off-by: Petr Baudis <pasky@suse.cz>
---

 Documentation/git-repo-config.txt |    3 ++-
 perl/Git.pm                       |   37 ++++++++++++++++++++++++++++++++++++-
 repo-config.c                     |    2 +-
 3 files changed, 39 insertions(+), 3 deletions(-)

diff --git a/Documentation/git-repo-config.txt b/Documentation/git-repo-config.txt
index 803c0d5..cc72fa9 100644
--- a/Documentation/git-repo-config.txt
+++ b/Documentation/git-repo-config.txt
@@ -54,7 +54,8 @@ OPTIONS
 
 --get::
 	Get the value for a given key (optionally filtered by a regex
-	matching the value).
+	matching the value). Returns error code 1 if the key was not
+	found and error code 2 if multiple key values were found.
 
 --get-all::
 	Like get, but does not fail if the number of values for the key
diff --git a/perl/Git.pm b/perl/Git.pm
index 7bbb5be..2e1241b 100644
--- a/perl/Git.pm
+++ b/perl/Git.pm
@@ -472,7 +472,6 @@ and the directory must exist.
 
 sub wc_chdir {
 	my ($self, $subdir) = @_;
-
 	$self->wc_path()
 		or throw Error::Simple("bare repository");
 
@@ -485,6 +484,42 @@ sub wc_chdir {
 }
 
 
+=item config ( VARIABLE )
+
+Retrieve the configuration C<VARIABLE> in the same manner as C<repo-config>
+does. In scalar context requires the variable to be set only one time
+(exception is thrown otherwise), in array context returns allows the
+variable to be set multiple times and returns all the values.
+
+Must be called on a repository instance.
+
+This currently wraps command('repo-config') so it is not so fast.
+
+=cut
+
+sub config {
+	my ($self, $var) = @_;
+	$self->repo_path()
+		or throw Error::Simple("not a repository");
+
+	try {
+		if (wantarray) {
+			return $self->command('repo-config', '--get-all', $var);
+		} else {
+			return $self->command_oneline('repo-config', '--get', $var);
+		}
+	} catch Git::Error::Command with {
+		my $E = shift;
+		if ($E->value() == 1) {
+			# Key not found.
+			return undef;
+		} else {
+			throw $E;
+		}
+	};
+}
+
+
 =item hash_object ( FILENAME [, TYPE ] )
 
 =item hash_object ( FILEHANDLE [, TYPE ] )
diff --git a/repo-config.c b/repo-config.c
index ab8f1af..346fb14 100644
--- a/repo-config.c
+++ b/repo-config.c
@@ -121,7 +121,7 @@ static int get_value(const char* key_, c
 	if (do_all)
 		ret = !seen;
 	else
-		ret =  (seen == 1) ? 0 : 1;
+		ret = (seen == 1) ? 0 : seen > 1 ? 2 : 1;
 
 free_strings:
 	if (repo_config)

^ permalink raw reply related


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