Git development
 help / color / mirror / Atom feed
* Re: Git and GCC
From: Daniel Berlin @ 2007-12-06  3:47 UTC (permalink / raw)
  To: David Miller; +Cc: ismail, gcc, git
In-Reply-To: <20071205.185203.262588544.davem@davemloft.net>

On 12/5/07, David Miller <davem@davemloft.net> wrote:
> From: "Daniel Berlin" <dberlin@dberlin.org>
> Date: Wed, 5 Dec 2007 21:41:19 -0500
>
> > It is true I gave up quickly, but this is mainly because i don't like
> > to fight with my tools.
> > I am quite fine with a distributed workflow, I now use 8 or so gcc
> > branches in mercurial (auto synced from svn) and merge a lot between
> > them. I wanted to see if git would sanely let me manage the commits
> > back to svn.  After fighting with it, i gave up and just wrote a
> > python extension to hg that lets me commit non-svn changesets back to
> > svn directly from hg.
>
> I find it ironic that you were even willing to write tools to
> facilitate your hg based gcc workflow.
Why?

> That really shows what your
> thinking is on this matter, in that you're willing to put effort
> towards making hg work better for you but you're not willing to expend
> that level of effort to see if git can do so as well.
See, now you claim to know my thinking.
I went back to hg because the GIT's space usage wasn't even in the
ballpark, i couldn't get git-svn rebase to update the revs after the
initial import (even though i had properly used a rewriteRoot).

The size is clearly not just svn data, it's in the git pack itself.

I spent a long time working on SVN to reduce it's space usage (repo
side and cleaning up the client side and giving a path to svn devs to
reduce it further), as well as ui issues, and I really don't feel like
having to do the same for GIT.

I'm tired of having to spend a large amount of effort to get my tools
to work.  If the community wants to find and fix the problem, i've
already said repeatedly i'll happily give over my repo, data,
whatever.  You are correct i am not going to spend even more effort
when i can be productive with something else much quicker.  The devil
i know (committing to svn) is better than the devil i don't (diving
into git source code and finding/fixing what is causing this space
blowup).
The python extension took me a few hours (< 4).
In git, i spent these hours waiting for git-gc to finish.

> This is what really eats me from the inside about your dissatisfaction
> with git.  Your analysis seems to be a self-fullfilling prophecy, and
> that's totally unfair to both hg and git.
Oh?
You seem to be taking this awfully personally.
I came into this completely open minded. Really, I did (i'm sure
you'll claim otherwise).
GIT people told me it would work great and i'd have a really small git
repo and be able to commit back to svn.
I tried it.
It didn't work out.
It doesn't seem to be usable for whatever reason.
I'm happy to give details, data, whatever.

I made the engineering decision that my effort would be better spent
doing something I knew i could do quickly (make hg commit back to svn
for my purposes) then trying to improve larger issues in GIT (UI and
space usage).  That took me a few hours, and I was happy again.

I would have been incredibly happy to have git just have come up with
a 400 meg gcc repository, and to be happily committing away from
git-svn to gcc's repository  ...
But it didn't happen.
So far, you have yet to actually do anything but incorrectly tell me
what I am thinking.

I'll probably try again in 6 months, and maybe it will be better.

^ permalink raw reply

* Re: * [BUG] "git clean" does not pay attention to its parameters
From: Jeff King @ 2007-12-06  3:28 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Nanako Shiraishi, Shawn Bohrer, git
In-Reply-To: <7veje1zibm.fsf@gitster.siamese.dyndns.org>

On Tue, Dec 04, 2007 at 11:55:41PM -0800, Junio C Hamano wrote:

> [PATCH] git-clean: Honor pathspec.
> 
> git-clean "*.rej" should attempt to look at only paths that match
> pattern "*.rej", but rewrite to C broke it.

And here is a test that fails without your patch (probably the commit
message should say "fixed in XX" once the commit id is known, or it
should be squashed in with your patch).

-- >8 --
t7300: add test for clean with wildcard pathspec

Signed-off-by: Jeff King <peff@peff.net>
---
 t/t7300-clean.sh |   14 ++++++++++++++
 1 files changed, 14 insertions(+), 0 deletions(-)

diff --git a/t/t7300-clean.sh b/t/t7300-clean.sh
index f013c17..dfd1188 100755
--- a/t/t7300-clean.sh
+++ b/t/t7300-clean.sh
@@ -126,6 +126,20 @@ test_expect_success 'git-clean symbolic link' '
 
 '
 
+test_expect_success 'git-clean with wildcard' '
+
+	touch a.clean b.clean other.c &&
+	git-clean "*.clean" &&
+	test -f Makefile &&
+	test -f README &&
+	test -f src/part1.c &&
+	test -f src/part2.c &&
+	test ! -f a.clean &&
+	test ! -f b.clean &&
+	test -f other.c
+
+'
+
 test_expect_success 'git-clean -n' '
 
 	mkdir -p build docs &&
-- 
1.5.3.7.2099.gd6d7-dirty

^ permalink raw reply related

* Re: Git and GCC
From: David Miller @ 2007-12-06  2:52 UTC (permalink / raw)
  To: dberlin; +Cc: ismail, gcc, git
In-Reply-To: <4aca3dc20712051841o71ab773ft6dd0714ebc355dd5@mail.gmail.com>

From: "Daniel Berlin" <dberlin@dberlin.org>
Date: Wed, 5 Dec 2007 21:41:19 -0500

> It is true I gave up quickly, but this is mainly because i don't like
> to fight with my tools.
> I am quite fine with a distributed workflow, I now use 8 or so gcc
> branches in mercurial (auto synced from svn) and merge a lot between
> them. I wanted to see if git would sanely let me manage the commits
> back to svn.  After fighting with it, i gave up and just wrote a
> python extension to hg that lets me commit non-svn changesets back to
> svn directly from hg.

I find it ironic that you were even willing to write tools to
facilitate your hg based gcc workflow.  That really shows what your
thinking is on this matter, in that you're willing to put effort
towards making hg work better for you but you're not willing to expend
that level of effort to see if git can do so as well.

This is what really eats me from the inside about your dissatisfaction
with git.  Your analysis seems to be a self-fullfilling prophecy, and
that's totally unfair to both hg and git.

^ permalink raw reply

* Re: Git and GCC
From: Daniel Berlin @ 2007-12-06  2:41 UTC (permalink / raw)
  To: David Miller; +Cc: ismail, gcc, git
In-Reply-To: <20071205.182815.249974508.davem@davemloft.net>

On 12/5/07, David Miller <davem@davemloft.net> wrote:
> From: "Daniel Berlin" <dberlin@dberlin.org>
> Date: Wed, 5 Dec 2007 14:08:41 -0500
>
> > So I tried a full history conversion using git-svn of the gcc
> > repository (IE every trunk revision from 1-HEAD as of yesterday)
> > The git-svn import was done using repacks every 1000 revisions.
> > After it finished, I used git-gc --aggressive --prune.  Two hours
> > later, it finished.
> > The final size after this is 1.5 gig for all of the history of gcc for
> > just trunk.
> >
> > dberlin@home:/compilerstuff/gitgcc/gccrepo/.git/objects/pack$ ls -trl
> > total 1568899
> > -r--r--r-- 1 dberlin dberlin 1585972834 2007-12-05 14:01
> > pack-cd328fcf0bd673d8f2f72c42fbe67da64cbcd218.pack
> > -r--r--r-- 1 dberlin dberlin   19008488 2007-12-05 14:01
> > pack-cd328fcf0bd673d8f2f72c42fbe67da64cbcd218.idx
> >
> > This is 3x bigger than hg *and* hg doesn't require me to waste my life
> > repacking every so often.
> > The hg operations run roughly as fast as the git ones
> >
> > I'm sure there are magic options, magic command lines, etc, i could
> > use to make it smaller.
> >
> > I'm sure if i spent the next few weeks fucking around with git, it may
> > even be usable!
> >
> > But given that git is harder to use, requires manual repacking to get
> > any kind of sane space usage, and is 3x bigger anyway, i don't see any
> > advantage to continuing to experiment with git and gcc.
>
> I would really appreciate it if you would share experiences
> like this with the GIT community, who have been now CC:'d.
>
> That's the only way this situation is going to improve.
>
> When you don't CC: the people who can fix the problem, I can only
> speculate that perhaps at least subconsciously you don't care if
> the situation improves or not.
>
I didn't cc the git community for three reasons

1. It's not the nicest message in the world, and thus, more likely to
get bad responses than constructive ones.

2. Based on the level of usability, I simply assume it is too young
for regular developers to use.  At least, I hope this is the case.

3. People i know have had bad experiences talking usability issues
with the git community in the past.  I am not likely to fare any
better, so I would rather have someone who is involved with both our
community and theirs, raise these issues, rather than a complete
newcomer.

But hey, whatever floats your boat :)

It is true I gave up quickly, but this is mainly because i don't like
to fight with my tools.
I am quite fine with a distributed workflow, I now use 8 or so gcc
branches in mercurial (auto synced from svn) and merge a lot between
them. I wanted to see if git would sanely let me manage the commits
back to svn.  After fighting with it, i gave up and just wrote a
python extension to hg that lets me commit non-svn changesets back to
svn directly from hg.

--Dan

^ permalink raw reply

* Re: Git and GCC
From: David Miller @ 2007-12-06  2:28 UTC (permalink / raw)
  To: dberlin; +Cc: ismail, gcc, git
In-Reply-To: <4aca3dc20712051108s216d3331t8061ef45b9aa324a@mail.gmail.com>

From: "Daniel Berlin" <dberlin@dberlin.org>
Date: Wed, 5 Dec 2007 14:08:41 -0500

> So I tried a full history conversion using git-svn of the gcc
> repository (IE every trunk revision from 1-HEAD as of yesterday)
> The git-svn import was done using repacks every 1000 revisions.
> After it finished, I used git-gc --aggressive --prune.  Two hours
> later, it finished.
> The final size after this is 1.5 gig for all of the history of gcc for
> just trunk.
> 
> dberlin@home:/compilerstuff/gitgcc/gccrepo/.git/objects/pack$ ls -trl
> total 1568899
> -r--r--r-- 1 dberlin dberlin 1585972834 2007-12-05 14:01
> pack-cd328fcf0bd673d8f2f72c42fbe67da64cbcd218.pack
> -r--r--r-- 1 dberlin dberlin   19008488 2007-12-05 14:01
> pack-cd328fcf0bd673d8f2f72c42fbe67da64cbcd218.idx
> 
> This is 3x bigger than hg *and* hg doesn't require me to waste my life
> repacking every so often.
> The hg operations run roughly as fast as the git ones
> 
> I'm sure there are magic options, magic command lines, etc, i could
> use to make it smaller.
> 
> I'm sure if i spent the next few weeks fucking around with git, it may
> even be usable!
> 
> But given that git is harder to use, requires manual repacking to get
> any kind of sane space usage, and is 3x bigger anyway, i don't see any
> advantage to continuing to experiment with git and gcc.

I would really appreciate it if you would share experiences
like this with the GIT community, who have been now CC:'d.

That's the only way this situation is going to improve.

When you don't CC: the people who can fix the problem, I can only
speculate that perhaps at least subconsciously you don't care if
the situation improves or not.

The OpenSolaris folks behaved similarly, and that really ticked me
off.

^ permalink raw reply

* [PATCH 3/3] Color support for "git-add -i"
From: Junio C Hamano @ 2007-12-06  2:05 UTC (permalink / raw)
  To: git
In-Reply-To: <1196906706-11170-2-git-send-email-gitster@pobox.com>

This is mostly lifted from earlier series by Dan Zwell, but updated to
use "git config --get-color" and "git config --get-colorbool" to make it
simpler and more consistent with commands written in C.

A new configuration color.interactive variable is like color.diff and
color.status, and controls if "git-add -i" uses color.

A set of configuration variables, color.interactive.<slot>, are used to
define what color is used for the prompt, header, and help text.

For perl scripts, Git.pm provides $repo->get_color() method, which takes
the slot name and the default color, and returns the terminal escape
sequence to color the output text.  $repo->get_colorbool() method can be
used to check if color is set to be used for a given operation.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 Documentation/config.txt  |   12 +++++
 git-add--interactive.perl |  119 +++++++++++++++++++++++++++++++++++++--------
 perl/Git.pm               |   35 +++++++++++++
 3 files changed, 146 insertions(+), 20 deletions(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index 0e45ec5..736fcd7 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -391,6 +391,18 @@ color.diff.<slot>::
 	whitespace).  The values of these variables may be specified as
 	in color.branch.<slot>.
 
+color.interactive::
+	When set to `always`, always use colors in `git add --interactive`.
+	When false (or `never`), never.  When set to `true` or `auto`, use
+	colors only when the output is to the terminal. Defaults to false.
+
+color.interactive.<slot>::
+	Use customized color for `git add --interactive`
+	output. `<slot>` may be `prompt`, `header`, or `help`, for
+	three distinct types of normal output from interactive
+	programs.  The values of these variables may be specified as
+	in color.branch.<slot>.
+
 color.pager::
 	A boolean to enable/disable colored output when the pager is in
 	use (default is true).
diff --git a/git-add--interactive.perl b/git-add--interactive.perl
index 335c2c6..1019a72 100755
--- a/git-add--interactive.perl
+++ b/git-add--interactive.perl
@@ -1,6 +1,55 @@
 #!/usr/bin/perl -w
 
 use strict;
+use Git;
+
+# Prompt colors:
+my ($prompt_color, $header_color, $help_color, $normal_color);
+# Diff colors:
+my ($new_color, $old_color, $fraginfo_color, $metainfo_color, $whitespace_color);
+
+my ($use_color, $diff_use_color);
+my $repo = Git->repository();
+
+$use_color = $repo->get_colorbool('color.interactive');
+
+if ($use_color) {
+	# Set interactive colors:
+
+	# Grab the 3 main colors in git color string format, with sane
+	# (visible) defaults:
+	$prompt_color = $repo->get_color("color.interactive.prompt", "bold blue");
+	$header_color = $repo->get_color("color.interactive.header", "bold");
+	$help_color = $repo->get_color("color.interactive.help", "red bold");
+	$normal_color = $repo->get_color("", "reset");
+
+	# Do we also set diff colors?
+	$diff_use_color = $repo->get_colorbool('color.diff');
+	if ($diff_use_color) {
+		$new_color = $repo->get_color("color.diff.new", "green");
+		$old_color = $repo->get_color("color.diff.old", "red");
+		$fraginfo_color = $repo->get_color("color.diff.frag", "cyan");
+		$metainfo_color = $repo->get_color("color.diff.meta", "bold");
+		$whitespace_color = $repo->get_color("color.diff.whitespace", "normal red");
+	}
+}
+
+sub colored {
+	my $color = shift;
+	my $string = join("", @_);
+
+	if ($use_color) {
+		# Put a color code at the beginning of each line, a reset at the end
+		# color after newlines that are not at the end of the string
+		$string =~ s/(\n+)(.)/$1$color$2/g;
+		# reset before newlines
+		$string =~ s/(\n+)/$normal_color$1/g;
+		# codes at beginning and end (if necessary):
+		$string =~ s/^/$color/;
+		$string =~ s/$/$normal_color/ unless $string =~ /\n$/;
+	}
+	return $string;
+}
 
 # command line options
 my $patch_mode;
@@ -246,10 +295,20 @@ sub is_valid_prefix {
 sub highlight_prefix {
 	my $prefix = shift;
 	my $remainder = shift;
-	return $remainder unless defined $prefix;
-	return is_valid_prefix($prefix) ?
-	    "[$prefix]$remainder" :
-	    "$prefix$remainder";
+
+	if (!defined $prefix) {
+		return $remainder;
+	}
+
+	if (!is_valid_prefix($prefix)) {
+		return "$prefix$remainder";
+	}
+
+	if (!$use_color) {
+		return "[$prefix]$remainder";
+	}
+
+	return "$prompt_color$prefix$normal_color$remainder";
 }
 
 sub list_and_choose {
@@ -266,7 +325,7 @@ sub list_and_choose {
 			if (!$opts->{LIST_FLAT}) {
 				print "     ";
 			}
-			print "$opts->{HEADER}\n";
+			print colored $header_color, "$opts->{HEADER}\n";
 		}
 		for ($i = 0; $i < @stuff; $i++) {
 			my $chosen = $chosen[$i] ? '*' : ' ';
@@ -304,7 +363,7 @@ sub list_and_choose {
 
 		return if ($opts->{LIST_ONLY});
 
-		print $opts->{PROMPT};
+		print colored $prompt_color, $opts->{PROMPT};
 		if ($opts->{SINGLETON}) {
 			print "> ";
 		}
@@ -371,7 +430,7 @@ sub list_and_choose {
 }
 
 sub singleton_prompt_help_cmd {
-	print <<\EOF ;
+	print colored $help_color, <<\EOF ;
 Prompt help:
 1          - select a numbered item
 foo        - select item based on unique prefix
@@ -380,7 +439,7 @@ EOF
 }
 
 sub prompt_help_cmd {
-	print <<\EOF ;
+	print colored $help_color, <<\EOF ;
 Prompt help:
 1          - select a single item
 3-5        - select a range of items
@@ -477,6 +536,31 @@ sub parse_diff {
 	return @hunk;
 }
 
+sub colored_diff_hunk {
+	my ($text) = @_;
+	# return the text, so that it can be passed to print()
+	my @ret;
+	for (@$text) {
+		if (!$diff_use_color) {
+			push @ret, $_;
+			next;
+		}
+
+		if (/^\+/) {
+			push @ret, colored($new_color, $_);
+		} elsif (/^\-/) {
+			push @ret, colored($old_color, $_);
+		} elsif (/^\@/) {
+			push @ret, colored($fraginfo_color, $_);
+		} elsif (/^ /) {
+			push @ret, colored($normal_color, $_);
+		} else {
+			push @ret, colored($metainfo_color, $_);
+		}
+	}
+	return @ret;
+}
+
 sub hunk_splittable {
 	my ($text) = @_;
 
@@ -671,7 +755,7 @@ sub coalesce_overlapping_hunks {
 }
 
 sub help_patch_cmd {
-	print <<\EOF ;
+	print colored $help_color, <<\EOF ;
 y - stage this hunk
 n - do not stage this hunk
 a - stage this and all the remaining hunks in the file
@@ -710,9 +794,7 @@ sub patch_update_file {
 	my ($ix, $num);
 	my $path = shift;
 	my ($head, @hunk) = parse_diff($path);
-	for (@{$head->{TEXT}}) {
-		print;
-	}
+	print colored_diff_hunk($head->{TEXT});
 	$num = scalar @hunk;
 	$ix = 0;
 
@@ -754,10 +836,8 @@ sub patch_update_file {
 		if (hunk_splittable($hunk[$ix]{TEXT})) {
 			$other .= '/s';
 		}
-		for (@{$hunk[$ix]{TEXT}}) {
-			print;
-		}
-		print "Stage this hunk [y/n/a/d$other/?]? ";
+		print colored_diff_hunk($hunk[$ix]{TEXT});
+		print colored $prompt_color, "Stage this hunk [y/n/a/d$other/?]? ";
 		my $line = <STDIN>;
 		if ($line) {
 			if ($line =~ /^y/i) {
@@ -811,7 +891,7 @@ sub patch_update_file {
 			elsif ($other =~ /s/ && $line =~ /^s/) {
 				my @split = split_hunk($hunk[$ix]{TEXT});
 				if (1 < @split) {
-					print "Split into ",
+					print colored $header_color, "Split into ",
 					scalar(@split), " hunks.\n";
 				}
 				splice(@hunk, $ix, 1,
@@ -894,8 +974,7 @@ sub diff_cmd {
 				     HEADER => $status_head, },
 				   @mods);
 	return if (!@them);
-	system(qw(git diff-index -p --cached HEAD --),
-	       map { $_->{VALUE} } @them);
+	system(qw(git diff -p --cached HEAD --), map { $_->{VALUE} } @them);
 }
 
 sub quit_cmd {
@@ -904,7 +983,7 @@ sub quit_cmd {
 }
 
 sub help_cmd {
-	print <<\EOF ;
+	print colored $help_color, <<\EOF ;
 status        - show paths with changes
 update        - add working tree state to the staged set of changes
 revert        - revert staged set of changes back to the HEAD version
diff --git a/perl/Git.pm b/perl/Git.pm
index 7468460..a2812ea 100644
--- a/perl/Git.pm
+++ b/perl/Git.pm
@@ -581,6 +581,41 @@ sub config_int {
 	};
 }
 
+=item get_colorbool ( NAME )
+
+Finds if color should be used for NAMEd operation from the configuration,
+and returns boolean (true for "use color", false for "do not use color").
+
+=cut
+
+sub get_colorbool {
+	my ($self, $var) = @_;
+	my $stdout_to_tty = (-t STDOUT) ? "true" : "false";
+	my $use_color = $self->command_oneline('config', '--get-colorbool',
+					       $var, $stdout_to_tty);
+	return ($use_color eq 'true');
+}
+
+=item get_color ( SLOT, COLOR )
+
+Finds color for SLOT from the configuration, while defaulting to COLOR,
+and returns the ANSI color escape sequence:
+
+	print $repo->get_color("color.interactive.prompt", "underline blue white");
+	print "some text";
+	print $repo->get_color("", "normal");
+
+=cut
+
+sub get_color {
+	my ($self, $slot, $default) = @_;
+	my $color = $self->command_oneline('config', '--get-color', $slot, $default);
+	if (!defined $color) {
+		$color = "";
+	}
+	return $color;
+}
+
 =item ident ( TYPE | IDENTSTR )
 
 =item ident_person ( TYPE | IDENTSTR | IDENTARRAY )
-- 
1.5.3.7-2132-gbd1cf

^ permalink raw reply related

* [PATCH 2/3] git config --get-colorbool
From: Junio C Hamano @ 2007-12-06  2:05 UTC (permalink / raw)
  To: git
In-Reply-To: <1196906706-11170-1-git-send-email-gitster@pobox.com>

This adds an option to help scripts find out color settings from
the configuration file.

    git config --get-colorbool color.diff

inspects color.diff variable, and exits with status 0 (i.e. success) if
color is to be used.  It exits with status 1 otherwise.

If a script wants "true"/"false" answer to the standard output of the
command, it can pass an additional boolean parameter to its command
line, telling if its standard output is a terminal, like this:

    git config --get-colorbool color.diff true

When called like this, the command outputs "true" to its standard output
if color is to be used (i.e. "color.diff" says "always", "auto", or
"true"), and "false" otherwise.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 Documentation/git-config.txt |   10 ++++++++++
 builtin-branch.c             |    2 +-
 builtin-config.c             |   41 ++++++++++++++++++++++++++++++++++++++++-
 color.c                      |    6 ++++--
 color.h                      |    2 +-
 diff.c                       |    2 +-
 wt-status.c                  |    2 +-
 7 files changed, 58 insertions(+), 7 deletions(-)

diff --git a/Documentation/git-config.txt b/Documentation/git-config.txt
index 7640450..98509b4 100644
--- a/Documentation/git-config.txt
+++ b/Documentation/git-config.txt
@@ -21,6 +21,7 @@ SYNOPSIS
 'git-config' [<file-option>] --remove-section name
 'git-config' [<file-option>] [-z|--null] -l | --list
 'git-config' [<file-option>] --get-color name [default]
+'git-config' [<file-option>] --get-colorbool name [stdout-is-tty]
 
 DESCRIPTION
 -----------
@@ -135,6 +136,15 @@ See also <<FILES>>.
 	output without getting confused e.g. by values that
 	contain line breaks.
 
+--get-colorbool name [stdout-is-tty]::
+
+	Find the color setting for `name` (e.g. `color.diff`) and output
+	"true" or "false".  `stdout-is-tty` should be either "true" or
+	"false", and is taken into account when configuration says
+	"auto".  If `stdout-is-tty` is missing, then checks the standard
+	output of the command itself, and exits with status 0 if color
+	is to be used, or exits with status 1 otherwise.
+
 --get-color name default::
 
 	Find the color configured for `name` (e.g. `color.diff.new`) and
diff --git a/builtin-branch.c b/builtin-branch.c
index c64768b..089cae5 100644
--- a/builtin-branch.c
+++ b/builtin-branch.c
@@ -65,7 +65,7 @@ static int parse_branch_color_slot(const char *var, int ofs)
 static int git_branch_config(const char *var, const char *value)
 {
 	if (!strcmp(var, "color.branch")) {
-		branch_use_color = git_config_colorbool(var, value);
+		branch_use_color = git_config_colorbool(var, value, -1);
 		return 0;
 	}
 	if (!prefixcmp(var, "color.branch.")) {
diff --git a/builtin-config.c b/builtin-config.c
index 6175dc3..d10b03f 100644
--- a/builtin-config.c
+++ b/builtin-config.c
@@ -3,7 +3,7 @@
 #include "color.h"
 
 static const char git_config_set_usage[] =
-"git-config [ --global | --system | [ -f | --file ] config-file ] [ --bool | --int ] [ -z | --null ] [--get | --get-all | --get-regexp | --replace-all | --add | --unset | --unset-all] name [value [value_regex]] | --rename-section old_name new_name | --remove-section name | --list | --get-color var [default]";
+"git-config [ --global | --system | [ -f | --file ] config-file ] [ --bool | --int ] [ -z | --null ] [--get | --get-all | --get-regexp | --replace-all | --add | --unset | --unset-all] name [value [value_regex]] | --rename-section old_name new_name | --remove-section name | --list | --get-color var [default] | --get-colorbool name [stdout-is-tty]";
 
 static char *key;
 static regex_t *key_regexp;
@@ -208,6 +208,43 @@ static int get_color(int argc, const char **argv)
 	return 0;
 }
 
+static int stdout_is_tty;
+static int get_colorbool_found;
+static int git_get_colorbool_config(const char *var, const char *value)
+{
+	if (!strcmp(var, get_color_slot))
+		get_colorbool_found =
+			git_config_colorbool(var, value, stdout_is_tty);
+	return 0;
+}
+
+static int get_colorbool(int argc, const char **argv)
+{
+	/*
+	 * git config --get-colorbool <slot> [<stdout-is-tty>]
+	 *
+	 * returns "true" or "false" depending on how <slot>
+	 * is configured.
+	 */
+
+	if (argc == 2)
+		stdout_is_tty = git_config_bool("command line", argv[1]);
+	else if (argc == 1)
+		stdout_is_tty = isatty(1);
+	else
+		usage(git_config_set_usage);
+	get_colorbool_found = 0;
+	get_color_slot = argv[0];
+	git_config(git_get_colorbool_config);
+
+	if (argc == 1) {
+		return get_colorbool_found ? 0 : 1;
+	} else {
+		printf("%s\n", get_colorbool_found ? "true" : "false");
+		return 0;
+	}
+}
+
 int cmd_config(int argc, const char **argv, const char *prefix)
 {
 	int nongit = 0;
@@ -283,6 +320,8 @@ int cmd_config(int argc, const char **argv, const char *prefix)
 			return 0;
 		} else if (!strcmp(argv[1], "--get-color")) {
 			return get_color(argc-2, argv+2);
+		} else if (!strcmp(argv[1], "--get-colorbool")) {
+			return get_colorbool(argc-2, argv+2);
 		} else
 			break;
 		argc--;
diff --git a/color.c b/color.c
index 97cfbda..7bd424a 100644
--- a/color.c
+++ b/color.c
@@ -116,7 +116,7 @@ bad:
 	die("bad config value '%s' for variable '%s'", value, var);
 }
 
-int git_config_colorbool(const char *var, const char *value)
+int git_config_colorbool(const char *var, const char *value, int stdout_is_tty)
 {
 	if (value) {
 		if (!strcasecmp(value, "never"))
@@ -133,7 +133,9 @@ int git_config_colorbool(const char *var, const char *value)
 
 	/* any normal truth value defaults to 'auto' */
  auto_color:
-	if (isatty(1) || (pager_in_use && pager_use_color)) {
+	if (stdout_is_tty < 0)
+		stdout_is_tty = isatty(1);
+	if (stdout_is_tty || (pager_in_use && pager_use_color)) {
 		char *term = getenv("TERM");
 		if (term && strcmp(term, "dumb"))
 			return 1;
diff --git a/color.h b/color.h
index 6809800..ff63513 100644
--- a/color.h
+++ b/color.h
@@ -4,7 +4,7 @@
 /* "\033[1;38;5;2xx;48;5;2xxm\0" is 23 bytes */
 #define COLOR_MAXLEN 24
 
-int git_config_colorbool(const char *var, const char *value);
+int git_config_colorbool(const char *var, const char *value, int stdout_is_tty);
 void color_parse(const char *var, const char *value, char *dst);
 int color_fprintf(FILE *fp, const char *color, const char *fmt, ...);
 int color_fprintf_ln(FILE *fp, const char *color, const char *fmt, ...);
diff --git a/diff.c b/diff.c
index 6b54959..be6cf68 100644
--- a/diff.c
+++ b/diff.c
@@ -146,7 +146,7 @@ int git_diff_ui_config(const char *var, const char *value)
 		return 0;
 	}
 	if (!strcmp(var, "diff.color") || !strcmp(var, "color.diff")) {
-		diff_use_color_default = git_config_colorbool(var, value);
+		diff_use_color_default = git_config_colorbool(var, value, -1);
 		return 0;
 	}
 	if (!strcmp(var, "diff.renames")) {
diff --git a/wt-status.c b/wt-status.c
index d35386d..02dbb75 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -391,7 +391,7 @@ void wt_status_print(struct wt_status *s)
 int git_status_config(const char *k, const char *v)
 {
 	if (!strcmp(k, "status.color") || !strcmp(k, "color.status")) {
-		wt_status_use_color = git_config_colorbool(k, v);
+		wt_status_use_color = git_config_colorbool(k, v, -1);
 		return 0;
 	}
 	if (!prefixcmp(k, "status.color.") || !prefixcmp(k, "color.status.")) {
-- 
1.5.3.7-2132-gbd1cf

^ permalink raw reply related

* Re: * [BUG] "git clean" does not pay attention to its parameters
From: Junio C Hamano @ 2007-12-06  2:14 UTC (permalink / raw)
  To: Shawn Bohrer; +Cc: Nanako Shiraishi, git
In-Reply-To: <20071205152816.GA21347@mediacenter.austin.rr.com>

Shawn Bohrer <shawn.bohrer@gmail.com> writes:

> Before the rewrite in C git clean would refuse to remove a directory if
> you said:
>
>    git clean dir
>
> without using the -d parameter.  Per your suggestion this check causes
> git clean to remove the directory anyway since you explicitly asked it
> to.

Thanks for reminding me of that issue.

^ permalink raw reply

* [PATCH 0/3] Reroll colorized "git add -i"
From: Junio C Hamano @ 2007-12-06  2:05 UTC (permalink / raw)
  To: git
In-Reply-To: <1196906706-11170-3-git-send-email-gitster@pobox.com>

Another try of the colorized "git add -i" series.

[PATCH 1/3] Documentation: color.* = true means "auto"

This is a documentation fix that has already been applied to 'master'
(not pushed out yet as of this writing).

[PATCH 2/3] git config --get-colorbool

This allows scripts to figure out if they should use colors.

[PATCH 3/3] Color support for "git-add -i"

This is Dan's colorized git-add -i, but uses --get-color and
--get-colorbool options to git-config.

^ permalink raw reply

* [PATCH 1/3] Documentation: color.* = true means "auto"
From: Junio C Hamano @ 2007-12-06  2:05 UTC (permalink / raw)
  To: git
In-Reply-To: <475697BC.2090701@viscovery.net>

We forgot to document the earlier sanity-fix.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 Documentation/config.txt |   14 +++++++-------
 1 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index 72a33e9..0e45ec5 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -359,8 +359,8 @@ clean.requireForce::
 
 color.branch::
 	A boolean to enable/disable color in the output of
-	gitlink:git-branch[1]. May be set to `true` (or `always`),
-	`false` (or `never`) or `auto`, in which case colors are used
+	gitlink:git-branch[1]. May be set to `always`,
+	`false` (or `never`) or `auto` (or `true`), in which case colors are used
 	only when the output is to a terminal. Defaults to false.
 
 color.branch.<slot>::
@@ -378,9 +378,9 @@ second is the background.  The position of the attribute, if any,
 doesn't matter.
 
 color.diff::
-	When true (or `always`), always use colors in patch.
-	When false (or `never`), never.  When set to `auto`, use
-	colors only when the output is to the terminal.
+	When set to `always`, always use colors in patch.
+	When false (or `never`), never.  When set to `true` or `auto`, use
+	colors only when the output is to the terminal. Defaults to false.
 
 color.diff.<slot>::
 	Use customized color for diff colorization.  `<slot>` specifies
@@ -397,8 +397,8 @@ color.pager::
 
 color.status::
 	A boolean to enable/disable color in the output of
-	gitlink:git-status[1]. May be set to `true` (or `always`),
-	`false` (or `never`) or `auto`, in which case colors are used
+	gitlink:git-status[1]. May be set to `always`,
+	`false` (or `never`) or `auto` (or `true`), in which case colors are used
 	only when the output is to a terminal. Defaults to false.
 
 color.status.<slot>::
-- 
1.5.3.7-2132-gbd1cf

^ permalink raw reply related

* [PATCH] Revert "git-am: catch missing author date early."
From: Junio C Hamano @ 2007-12-06  1:20 UTC (permalink / raw)
  To: git; +Cc: Jens Axboe
In-Reply-To: <7vir3conab.fsf@gitster.siamese.dyndns.org>

This reverts commit 6e9e0327b7d7f384d8a223b4bc40330ef3e7fb61.  People
can prepare a text file with Subject: and From: headers and feed it to
"am" (pretending the file is a piece of e-mail), and have actually been
doing so.  Strict checking for Date: breaks this established workflow,
which wants to record the time of the commit as the author time.

Thanks go to Jens Axboe for injection of sanity.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 git-am.sh |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/git-am.sh b/git-am.sh
index 2e40708..76c1c84 100755
--- a/git-am.sh
+++ b/git-am.sh
@@ -307,9 +307,9 @@ do
 	GIT_AUTHOR_EMAIL="$(sed -n '/^Email/ s/Email: //p' "$dotest/info")"
 	GIT_AUTHOR_DATE="$(sed -n '/^Date/ s/Date: //p' "$dotest/info")"
 
-	if test -z "$GIT_AUTHOR_EMAIL" || test -z "$GIT_AUTHOR_DATE"
+	if test -z "$GIT_AUTHOR_EMAIL"
 	then
-		echo "Patch does not have valid authorship information."
+		echo "Patch does not have a valid e-mail address."
 		stop_here $this
 	fi
 
-- 
1.5.3.7-2132-gbd1cf

^ permalink raw reply related

* Re: [PATCH] Do check_repository_format() early
From: Junio C Hamano @ 2007-12-06  1:18 UTC (permalink / raw)
  To: Nguyễn Thái Ngọc Duy; +Cc: git, Johannes Schindelin
In-Reply-To: <20071205132514.GA5580@laptop>

Thanks, this looks very sensible to me.

^ permalink raw reply

* Re: fetch_refs_via_pack() discards status?
From: André Goddard Rosa @ 2007-12-06  0:38 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Daniel Barkalow, Shawn O. Pearce, Git Mailing List
In-Reply-To: <7vwsrsonqm.fsf@gitster.siamese.dyndns.org>

On Dec 5, 2007 7:06 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Daniel Barkalow <barkalow@iabervon.org> writes:
>
> > On Tue, 4 Dec 2007, Junio C Hamano wrote:
> >
> >> The code calls fetch_pack() to get the list of refs it fetched, and
> >> discards refs and always returns 0 to signal success.
> >>
> >> But builtin-fetch-pack.c::fetch_pack() has error cases.  The function
> >> returns NULL if error is detected (shallow-support side seems to choose
> >> to die but I suspect that is easily fixable to error out as well).
> >>
> >> Shouldn't fetch_refs_via_pack() propagate that error to the caller?
> >
> > I think that's right. I think I got as far as having the error status from
> > fetch_pack() actually returned correctly, and then failed to look at it.
> > I'd personally avoid testing a pointer to freed memory, but that's
> > obviously not actually wrong.
> >
> >       -Daniel
>
> Hmph, is that an Ack that the patchlet is actually a bugfix?
>

Hi, Mr. Junio!

     My 2 cents: I think he means that we should not test a freed pointer:

       free_refs(refs);
       free(dest);                     <===
-       return 0;
+       return (refs ? 0 : -1);     <===

Best regards,
-- 
[]s,
André Goddard

^ permalink raw reply

* [BUG/PATCH] git grep shows the same hit repeatedly for unmerged paths
From: Junio C Hamano @ 2007-12-06  0:13 UTC (permalink / raw)
  To: git

When the index is unmerged, e.g.

	$ git ls-files -u
        100644 faf413748eb6ccb15161a212156c5e348302b1b6 1	setup.c
        100644 145eca50f41d811c4c8fcb21ed2604e6b2971aba 2	setup.c
        100644 cb9558c49b6027bf225ba2a6154c4d2a52bcdbe2 3	setup.c

running "git grep" for work tree files repeats hits for each unmerged
stage.

	$ git grep -n -e setup_work_tree -- '*.[ch]'
        setup.c:209:void setup_work_tree(void)
        setup.c:209:void setup_work_tree(void)
        setup.c:209:void setup_work_tree(void)

This should fix it.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---

 builtin-grep.c |   12 ++++++------
 1 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/builtin-grep.c b/builtin-grep.c
index bbf747f..f1ff8dc 100644
--- a/builtin-grep.c
+++ b/builtin-grep.c
@@ -343,12 +343,12 @@ static int external_grep(struct grep_opt *opt, const char **paths, int cached)
 			memcpy(name + 2, ce->name, len + 1);
 		}
 		argv[argc++] = name;
-		if (argc < MAXARGS)
-			continue;
-		status = flush_grep(opt, argc, nr, argv, &kept);
-		if (0 < status)
-			hit = 1;
-		argc = nr + kept;
+		if (MAXARGS <= argc) {
+			status = flush_grep(opt, argc, nr, argv, &kept);
+			if (0 < status)
+				hit = 1;
+			argc = nr + kept;
+		}
 		if (ce_stage(ce)) {
 			do {
 				i++;

^ permalink raw reply related

* Re: Cosmetic git-am interactive bug
From: Jeff Garzik @ 2007-12-05 23:58 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Git Mailing List
In-Reply-To: <7v8x491v79.fsf@gitster.siamese.dyndns.org>

Junio C Hamano wrote:
> Jeff Garzik <jeff@garzik.org> writes:
> 
>> The use of the older one-line summary led me to believe that it had
>> not committed my changelog edits.  Looking at the result, however,
>> proved that the commit changelog was my new, corrected version.
> 
> I knew about this for quite some time but it was a very low priority for
> me.  This should fix it.
> 
> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Thanks!  :)

^ permalink raw reply

* Re: builtin command's prefix question
From: Junio C Hamano @ 2007-12-05 23:43 UTC (permalink / raw)
  To: Nguyen Thai Ngoc Duy; +Cc: Git Mailing List, Johannes Schindelin
In-Reply-To: <7vk5nsloa8.fsf@gitster.siamese.dyndns.org>

Junio C Hamano <gitster@pobox.com> writes:

> I am not convinced this is giving any natural user experience, nor an
> alternative:
>
>       $ cd $HOME
>       $ GIT_WORK_TREE=$HOME; export GIT_WORK_TREE
>       $ cd $HOME/gits/vimrc.git
>       $ edit $HOME/.vimrc
>       $ git commit .vimrc
>       $ cd $HOME/gits/pinerc.git
>       $ edit $HOME/.pinerc
>       $ git commit .pinerc
>
> While I still think the combination is simply crazy and does not make
> any sense, if enough users on the list agrees that it makes sense, I
> wouldn't mind setup() did (1) to (3) mentioned above.  The alternative
> is simply to declare GIT_WORK_TREE without GIT_DIR is a nonsense and
> either error error out or ignore GIT_WORK_TREE, which might be easier to
> explain to people.
>
> Opinions?

Side note.

By saying the above, I do not mean it is nonsense to try supporting a
work tree that is an overlay of disjoint set of work tree files from
multiple repositories/projects.  I do think it is a worthwhile goal to
support such a layout.

What I do not like is the way the ugly workaround does it, by
encouraging (rather, requiring) to issue git commands from a location
that is completely separate from the actual editing of the content
happens.

An independent issue of supporting such a overlayed work tree layout is
what to do with .gitignore files.  I think, especially with the recent
addition of --exclude-standard to ls-files and setup_standard_excludes()
in dir.c, we could have a per repository configuration that names the
per repository exclude files, so that .gitignore-vim and .gitignore-pine
can co-exist in $HOME, each excluding everything other than the
project's own files.

^ permalink raw reply

* Re: [PATCH/RFC (amend)] autoconf: Add test for OLD_ICONV (squelching compiler warning)
From: Junio C Hamano @ 2007-12-05 23:27 UTC (permalink / raw)
  To: Jakub Narebski
  Cc: git, Pascal Obry, Ramsay Jones, Arjen Laarhoven, Brian Gernhardt
In-Reply-To: <1196895948-25115-1-git-send-email-jnareb@gmail.com>

Jakub Narebski <jnareb@gmail.com> writes:

>> Which means the real-life compilation will get the warning on type
>> mismatch.  Wasn't OLD_ICONV about squelching that?
>
> Gah, I don't know why I though OLD_ICONV was about compile errors, and
> not about compile warnings. This version uses -Werror to check for
> warnings; I hope it doesn't give false positives...

But use of -Werror means you are married to gcc, doesn't it?

How important is it to detect OLD_ICONV anyway, I have to wonder?

> On Wed, 5 Dec 2007, Pascal Obry wrote:
> ...
>> Note also that you should remove all the hard-coded settings
>> in Makefile anyway.
>
> No, I should not. ./configure script is purely optional in git,

Correct.  Thanks.

^ permalink raw reply

* Re: builtin command's prefix question
From: Junio C Hamano @ 2007-12-05 23:22 UTC (permalink / raw)
  To: Nguyen Thai Ngoc Duy; +Cc: Git Mailing List, Johannes Schindelin
In-Reply-To: <7vlk88n648.fsf@gitster.siamese.dyndns.org>

Junio C Hamano <gitster@pobox.com> writes:

>  * I would say it is a misconfiguration if GIT_DIR is not set and
>    GIT_WORK_TREE is, as the sole purpose of GIT_WORK_TREE is so that you
>    can work from a subdirectory when you set GIT_DIR.  I may be missing
>    an obvious use case that this is useful, but I do not think of any.
>    Dscho may be able to correct me on this, as he fixed up the original
>    work tree series that was even messier quite a bit during the last
>    round.

I had a short discussion with Dscho on this.  One scenario that was
brought up was this.

You have a work tree of mixed contents that logically belong to
separate repository.  Think $HOME/.?*, and tracking .vimrc and
.pinerc as separate "projects".  You have $HOME/gits/vim.git and
$HOME/gits/pine.git bare-looking repositories.

The "kosher" way of doing this might be:

        $ cd $HOME
        $ GIT_WORK_TREE=$HOME; export GIT_WORK_TREE
        $ edit .vimrc
        $ GIT_DIR=gits/vim.git git commit .vimrc
        $ edit .pinerc
        $ GIT_DIR=gits/pine.git git commit .pinerc

However, if we define setup() to behave this way when GIT_DIR is not
defined and GIT_WORK_TREE is:

 (1) internally pretend as if GIT_DIR was specified to be the
     directory where the command was started from (iow, do getcwd()
     once upon startup);

 (2) chdir to GIT_WORK_TREE (which means "callers of setup() always
     run from the top of the work tree");

 (3) set prefix to NULL;

Then this workflow becomes possible:

	$ cd $HOME
        $ GIT_WORK_TREE=$HOME; export GIT_WORK_TREE
	$ edit .vimrc .pinerc
        $ cd $HOME/gits/vimrc.git && git commit .vimrc
        $ cd $HOME/gits/pinerc.git && git commit .pinerc

I am not convinced this is giving any natural user experience, nor an
alternative:

	$ cd $HOME
        $ GIT_WORK_TREE=$HOME; export GIT_WORK_TREE
        $ cd $HOME/gits/vimrc.git
	$ edit $HOME/.vimrc
        $ git commit .vimrc
        $ cd $HOME/gits/pinerc.git
	$ edit $HOME/.pinerc
        $ git commit .pinerc

While I still think the combination is simply crazy and does not make
any sense, if enough users on the list agrees that it makes sense, I
wouldn't mind setup() did (1) to (3) mentioned above.  The alternative
is simply to declare GIT_WORK_TREE without GIT_DIR is a nonsense and
either error error out or ignore GIT_WORK_TREE, which might be easier to
explain to people.

Opinions?

^ permalink raw reply

* [PATCH/RFC (amend)] autoconf: Add test for OLD_ICONV (squelching compiler warning)
From: Jakub Narebski @ 2007-12-05 23:05 UTC (permalink / raw)
  To: git, Junio C Hamano, Pascal Obry
  Cc: Ramsay Jones, Arjen Laarhoven, Brian Gernhardt, Jakub Narebski
In-Reply-To: <7vd4tkn5mk.fsf@gitster.siamese.dyndns.org>

On Wed, 5 Dec 2007, Junio C Hamano wrote:
> Jakub Narebski <jnareb@gmail.com> writes:
>> Junio C Hamano wrote:
>>> Jakub Narebski <jnareb@gmail.com> writes:
>>>
>>>> +AC_MSG_CHECKING([for old iconv()])
>>>> +AC_COMPILE_IFELSE(OLDICONVTEST_SRC,
>>>> +	[AC_MSG_RESULT([no])],
>>>> +	[AC_MSG_RESULT([yes])
>>>> +	OLD_ICONV=YesPlease])
>>>> +AC_SUBST(OLD_ICONV)
>>>>  
>>> 
>>> Which result does COMPILE_IFELSE give for non error warnings?
>>> Ok, or Bad?
>>
>>  - Macro: AC_COMPILE_IFELSE (INPUT, [ACTION-IF-FOUND],
>>           [ACTION-IF-NOT-FOUND])
>>      Run the compiler and compilation flags of the current language
>>      (*note Language Choice::) on the INPUT, run the shell commands
>>      ACTION-IF-TRUE on success, ACTION-IF-FALSE otherwise.  The INPUT
>>      can be made by `AC_LANG_PROGRAM' and friends.
>>
>> And if I have checked correctly code which causes only warnings
>> returns Ok (in this case print 'no' after 'checking for old iconv()... '
>> and do not set OLD_ICONV, which means it will be unset).
> 
> Which means the real-life compilation will get the warning on type
> mismatch.  Wasn't OLD_ICONV about squelching that?

Gah, I don't know why I though OLD_ICONV was about compile errors, and
not about compile warnings. This version uses -Werror to check for
warnings; I hope it doesn't give false positives...


On Wed, 5 Dec 2007, Pascal Obry wrote:
> Jakub Narebski a écrit :
>> ---
>> This patch needs checking if it correctly sets OLD_ICONV
>> when needed.  I have checked only that it is not set when
>> with new iconv() declaration.  Could people using Cygwin
>> (and other with OLD_ICONV: Darwin) test it?
> 
> Not working on Cygwin:
> 
>    $ autoconf
>    $ ./configure --prefix=/usr/local --build=i686-pc-cygwin
>    ...
>    configure: CHECKS for header files
>    checking for old iconv()... no
> 
> It should be yes above. And in config.mak.autogen we have:
> 
>    OLD_ICONV=

Check out current version of patch. It should work correctly now
(I thought OLD_ICONV was about compile errors, and it is about
squelching compile warnings). It should give now:

  $ make configure
  $ ./configure --prefix=/usr/local --build=i686-pc-cygwin

  configure: CHECKS for header files
  checking for old iconv()... yes
 
  $ cat config.mak.autogen
 
  OLD_ICONV=UnfortunatelyYes

> Note also that you should remove all the hard-coded settings
> in Makefile anyway.

No, I should not. ./configure script is purely optional in git,
and compiling should work with reasonable defaults even if you
don't have autoconf installed and/or you don't want to run
./configure script (because e.g. it is too slow).

-- >8 --
From: Jakub Narebski <jnareb@gmail.com>
Subject: [PATCH] autoconf: Add test for OLD_ICONV (squelching compiler warning)

Update configure.ac (and config.mak.in) to keep up with git
development by adding [compile] test whether your library has an old
iconv(), where the second (input buffer pointer) parameter is declared
with type (const char **) (OLD_ICONV).

Test-proposed-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Jakub Narebski <jnareb@gmail.com>
---
 config.mak.in |    1 +
 configure.ac  |   24 ++++++++++++++++++++++++
 2 files changed, 25 insertions(+), 0 deletions(-)

diff --git a/config.mak.in b/config.mak.in
index 11d256e..7d5df9b 100644
--- a/config.mak.in
+++ b/config.mak.in
@@ -41,4 +41,5 @@ NO_STRTOUMAX=@NO_STRTOUMAX@
 NO_SETENV=@NO_SETENV@
 NO_MKDTEMP=@NO_MKDTEMP@
 NO_ICONV=@NO_ICONV@
+OLD_ICONV=@OLD_ICONV@
 NO_DEFLATE_BOUND=@NO_DEFLATE_BOUND@
diff --git a/configure.ac b/configure.ac
index 5f8a15b..196ab3e 100644
--- a/configure.ac
+++ b/configure.ac
@@ -212,6 +212,30 @@ test -n "$NEEDS_SOCKET" && LIBS="$LIBS -lsocket"
 
 
 ## Checks for header files.
+AC_MSG_NOTICE([CHECKS for header files])
+#
+# Define OLD_ICONV if your library has an old iconv(), where the second
+# (input buffer pointer) parameter is declared with type (const char **).
+AC_DEFUN([OLDICONVTEST_SRC], [[
+#include <iconv.h>
+
+int main(void)
+{
+	iconv_t cd;
+	char *ibp, *obp;
+	size_t insz, outsz;
+	iconv(cd, &ibp, &insz, &obp, &outsz);
+}
+]])
+AC_MSG_CHECKING([for old iconv()])
+CFLAGS_ORIG=$CFLAGS
+CFLAGS="$CFLAGS -Werror"
+AC_COMPILE_IFELSE(OLDICONVTEST_SRC,
+	[AC_MSG_RESULT([no])],
+	[AC_MSG_RESULT([yes])
+	OLD_ICONV=UnfortunatelyYes])
+CFLAGS=$CFLAGS_ORIG
+AC_SUBST(OLD_ICONV)
 
 
 ## Checks for typedefs, structures, and compiler characteristics.
-- 
1.5.3.7

^ permalink raw reply related

* Re: [PATCH 3/6] Test "git remote show" and "git remote prune"
From: Johannes Schindelin @ 2007-12-05 22:38 UTC (permalink / raw)
  To: René Scharfe; +Cc: git, gitster
In-Reply-To: <47571F3E.1060903@lsrfire.ath.cx>

Hi,

On Wed, 5 Dec 2007, Ren? Scharfe wrote:

> Johannes Schindelin schrieb:
> > While at it, also fix a few instances where a cd was done outside of a 
> > subshell.
> > 
> > Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> > ---
> >  t/t5505-remote.sh |   34 ++++++++++++++++++++++++++++++++++
> >  1 files changed, 34 insertions(+), 0 deletions(-)
> 
> It seems to me the patch only adds tests, but doesn't fix existing ones. 
> And looking at t5505-remote.sh, every call of cd is already done inside 
> of a subshell, so there doesn't seem to be anything to fix either. :-?

Right.  This comment is from a long time ago, back when I had my own 
(incomplete) t5505-remote.sh.  It was funny to me that Junio chose exactly 
the same name... but his implementation was different ;-)

I agree that this comment is obsolete.

Thanks,
Dscho

^ permalink raw reply

* Re: [PATCH v4] Allow update hooks to update refs on their own.
From: Junio C Hamano @ 2007-12-05 22:29 UTC (permalink / raw)
  To: Steven Grimm; +Cc: Jeff King, git
In-Reply-To: <7vhciwn5rl.fsf@gitster.siamese.dyndns.org>

Junio C Hamano <gitster@pobox.com> writes:

> Ok, but the output from fetch is meant to be human readable and we do
> not promise parsability, so if we go this route (which I think you made

s/parsability/machine &/;

> a sensible argument for) we would need a hook on the pushing end to act
> on this (perhaps record the correspondence of pushed and rewritten sha1
> somewhere for the hook's own use).

s/on this/& information/;
s/own use/& in machine readable way/;

^ permalink raw reply

* Re: [PATCH/RFC] autoconf: Add test for OLD_ICONV
From: Junio C Hamano @ 2007-12-05 22:22 UTC (permalink / raw)
  To: Jakub Narebski; +Cc: git, Ramsay Jones, Arjen Laarhoven, Brian Gernhardt
In-Reply-To: <200712052226.53543.jnareb@gmail.com>

Jakub Narebski <jnareb@gmail.com> writes:

> Junio C Hamano wrote:
>> Jakub Narebski <jnareb@gmail.com> writes:
>>
>>> +AC_MSG_CHECKING([for old iconv()])
>>> +AC_COMPILE_IFELSE(OLDICONVTEST_SRC,
>>> +	[AC_MSG_RESULT([no])],
>>> +	[AC_MSG_RESULT([yes])
>>> +	OLD_ICONV=YesPlease])
>>> +AC_SUBST(OLD_ICONV)
>>>  
>> 
>> Which result does COMPILE_IFELSE give for non error warnings?  Ok, or
>> Bad?
>
>  - Macro: AC_COMPILE_IFELSE (INPUT, [ACTION-IF-FOUND],
>           [ACTION-IF-NOT-FOUND])
>      Run the compiler and compilation flags of the current language
>      (*note Language Choice::) on the INPUT, run the shell commands
>      ACTION-IF-TRUE on success, ACTION-IF-FALSE otherwise.  The INPUT
>      can be made by `AC_LANG_PROGRAM' and friends.
>
> And if I have checked correctly code which causes only warnings
> returns Ok (in this case print 'no' after 'checking for old iconv()... '
> and do not set OLD_ICONV, which means it will be unset).

Which means the real-life compilation will get the warning on type
mismatch.  Wasn't OLD_ICONV about squelching that?

^ permalink raw reply

* Re: [PATCH v4] Allow update hooks to update refs on their own.
From: Junio C Hamano @ 2007-12-05 22:19 UTC (permalink / raw)
  To: Steven Grimm; +Cc: Jeff King, git
In-Reply-To: <5920F34B-A94B-4C24-A95B-D35F35A4F0C0@midwinter.com>

Steven Grimm <koreth@midwinter.com> writes:

> On Dec 2, 2007, at 6:16 PM, Junio C Hamano wrote:
>>> ..., but an
>>> "ok, but btw I changed your commit" status from receive-pack seems
>>> like
>>> it would be useful, for two reasons:
>> Sensible argument.  I stand corrected.
>
> If we want that status in principle, I'd argue that sending down the
> updated commit SHA1 is actually the right way to indicate it, because
> it gives the client all the information it needs to make an
> intelligent choice about what to do next. If you don't transmit the
> modified SHA1, the client will have to do another fetch to find out
> what rewriting was done by the server, and if another push happened in
> the meantime, the client will have to basically guess about which
> commits correspond to the ones it pushed.

Ok, but the output from fetch is meant to be human readable and we do
not promise parsability, so if we go this route (which I think you made
a sensible argument for) we would need a hook on the pushing end to act
on this (perhaps record the correspondence of pushed and rewritten sha1
somewhere for the hook's own use).

^ permalink raw reply

* Re: [PATCH v4] Allow update hooks to update refs on their own.
From: Steven Grimm @ 2007-12-05 22:14 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Jeff King, git
In-Reply-To: <7vlk8csetl.fsf@gitster.siamese.dyndns.org>

On Dec 2, 2007, at 6:16 PM, Junio C Hamano wrote:
>> ..., but an
>> "ok, but btw I changed your commit" status from receive-pack seems  
>> like
>> it would be useful, for two reasons:
> Sensible argument.  I stand corrected.

If we want that status in principle, I'd argue that sending down the  
updated commit SHA1 is actually the right way to indicate it, because  
it gives the client all the information it needs to make an  
intelligent choice about what to do next. If you don't transmit the  
modified SHA1, the client will have to do another fetch to find out  
what rewriting was done by the server, and if another push happened in  
the meantime, the client will have to basically guess about which  
commits correspond to the ones it pushed.

I'm going to have to modify the "ok" line for this either way, and  
it's not like the extra 39 bytes (for sending a hex SHA1 instead of a  
one-character status indicator) is going to hurt in any but the  
narrowest corner cases.

But if people really object to that, I will add a simple flag to the  
"ok" line.

-Steve

^ permalink raw reply

* Re: builtin command's prefix question
From: Junio C Hamano @ 2007-12-05 22:12 UTC (permalink / raw)
  To: Nguyen Thai Ngoc Duy; +Cc: Git Mailing List
In-Reply-To: <fcaeb9bf0712050856t5d730779q82783fdb9876f41@mail.gmail.com>

"Nguyen Thai Ngoc Duy" <pclouds@gmail.com> writes:

> I have been looking at setup_git_directory_gently() lately. From my
> understanding, setup_git_directory* will return a string called
> "prefix" that is passed to builtin commands. What is the exact meaning
> of this prefix?

Some historical background is in order.  For a long time, we only worked
from the very top of the work tree and nowhere else.  The path you get
from the user on the command line was supposed to be relative to the top
of the work tree, the user was supposed to be at the top of the work
tree, no work from subdirectories.

This was limiting, so "setup" was introduced.  The commands originally
worked only from the top would chdir up to the top of the work tree if
it was run from a subdirectory.  This however needs adjustments to the
paths we get from the user from the command line (or stdin for commands
that take such).  If we claim we work from a subdirectory, we should
interpret path given by the user who is in a subdirectory as relative to
that subdirectory.  The way to do this adjustment is to prefix the
subdirectory path to the path given by the user.

So, if you start a command from Documentation/ subdirectory, like this:

	$ cd Documentation
	$ git ls-files howto

internally, ls-files would:

 * Notice it was run from Documentation/ subdirectory;

 * cd up to the top level;

 * prefix "Documentation/" to the pathspec given by the user
   (i.e. "howto"), to form "Documentation/howto";

 * Act as before, except that it strips "Documentation/" from its usual
   output, to retain the illusion of working from that subdirectory.

And prefix is "Documentation/" (note the trailing slash) in such a
case.  If you run from the top, it is NULL to signal that there is no
need to do any of these tricks.

> ... Correct me if I'm wrong. In early (read: no worktree)
> days, cwd was moved to working root directory and prefix contained
> relative path from working root directory to the original cwd. So it
> had a few implications:

>  1. A non-empty prefix indicates cwd has been moved

Correct (I do not know if we care, though)

>  2. If cwd is moved, it is moved to working root directory

Correct ("we always work from top of the tree internally" matters)

>  3. cwd+prefix should point to the current directory at the time the
> command was issued (let's call it "user cwd")

Correct.

> Things change a bit since the rise of worktree:
>  - If GIT_DIR is set and GIT_WORK_TREE is not, prefix is relative to
> the to-be-set-up worktree, but cwd is not changed, so point 3 is gone.
>  - If GIT_DIR is not set and GiT_WORK_TREE is,
>   - and it is found that user cwd is inside a gitdir (bare repo), cwd
> has been moved and prefix is empty, cwd+prefix no longer point to user
> cwd
>   - for other cases, cwd may not be worktree (the real worktree will
> be setup in setup_work_tree or setup_git_directory)

The intention is:

 * If GIT_DIR is set but not GIT_WORK_TREE (nor core.worktree in
   config), you are either inside project.git/ directory of bare
   repository or at the toplevel of worktree-full directory.  This has
   been the traditional behaviour before GIT_WORK_TREE and we shouldn't
   break the existing setups that assume this behaviour.  So in that
   sense, with this combination:

   - If the repository is bare, the value of the prefix should not
     matter; the command that wants to look at prefix by definition
     wants to run from a subdirectory but there is no notion of
     "the user directory being a subdirectory of the top of the work
     tree" in a bare repository;

   - If the repository is not bare, the user directory _MUST_ be at the
     top of the work tree, as that is what the traditional behaviour is.
     Anything else would break existing setups.

     IOW, if you use GIT_DIR and still want to run from a subdirectory
     of the worktree, you must have either GIT_WORK_TREE or
     core.worktree to tell where the top of the worktree is, and if you
     don't, then you must be at the top.

   So the right thing to do in this case is not going anywhere and using
   prefix=NULL.

 * I would say it is a misconfiguration if GIT_DIR is not set and
   GIT_WORK_TREE is, as the sole purpose of GIT_WORK_TREE is so that you
   can work from a subdirectory when you set GIT_DIR.  I may be missing
   an obvious use case that this is useful, but I do not think of any.
   Dscho may be able to correct me on this, as he fixed up the original
   work tree series that was even messier quite a bit during the last
   round.

^ 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