Git development
 help / color / mirror / Atom feed
* Re: `git-send-email' doesn't specify `Content-Type'
From: Brian Swetland @ 2007-11-11  8:39 UTC (permalink / raw)
  To: Jeff King
  Cc: Björn Steinbrink, Johannes Schindelin, Ludovic Courtès,
	git
In-Reply-To: <20071111083224.GA30299@sigill.intra.peff.net>


This issue with the encoding of the author got me thinking...

What happens if the metadata has utf8 content and the patch itself has 
some *other* non-ascii encoding (some iso-latin variant perhaps).

Is there any way to deal with that situation sanely other than indicate
that it's 8bit content and not specify an encoding?  Is that what
happens currently?

Brian

^ permalink raw reply

* Re: [PATCH 0/3] Adding colors to git-add--interactive
From: Dan Zwell @ 2007-11-11  8:39 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: Jeff King, Shawn O. Pearce, Wincent Colaiuta, Git Mailing List,
	Jonathan del Strother, Johannes Schindelin, Frank Lichtenheld
In-Reply-To: <7vzlxlgpgt.fsf@gitster.siamese.dyndns.org>

Junio C Hamano wrote:
> This hunk makes the "show diff" subcommand honor user's external
> diff viewer if specified, which is a good change.  But it does
> not belong to the "colored add -i" series.
> 
> I mildly suspect that this change might have been my fault, but
> I think it should be treated in an independent patch anyway.
> 

Yep, that change was part of Junio's hunk coloring patch that he sent in 
reply to my last set of patches. When I revise this, I'll drop that change.

Dan

^ permalink raw reply

* Re: `git-send-email' doesn't specify `Content-Type'
From: Jeff King @ 2007-11-11  8:41 UTC (permalink / raw)
  To: Brian Swetland
  Cc: Björn Steinbrink, Johannes Schindelin, Ludovic Courtès,
	git
In-Reply-To: <20071111083915.GA18021@bulgaria>

On Sun, Nov 11, 2007 at 12:39:15AM -0800, Brian Swetland wrote:

> This issue with the encoding of the author got me thinking...
> 
> What happens if the metadata has utf8 content and the patch itself has 
> some *other* non-ascii encoding (some iso-latin variant perhaps).
> 
> Is there any way to deal with that situation sanely other than indicate
> that it's 8bit content and not specify an encoding?  Is that what
> happens currently?

The body has to be in one encoding, so at the time that you know both
encodings, you have to pick one and convert the data from the discarded
encoding into the used encoding.

-Peff

^ permalink raw reply

* Re: `git-send-email' doesn't specify `Content-Type'
From: Brian Swetland @ 2007-11-11  8:45 UTC (permalink / raw)
  To: Jeff King
  Cc: Björn Steinbrink, Johannes Schindelin, Ludovic Courtès,
	git
In-Reply-To: <20071111084117.GC30299@sigill.intra.peff.net>

[Jeff King <peff@peff.net>]
> On Sun, Nov 11, 2007 at 12:39:15AM -0800, Brian Swetland wrote:
> 
> > This issue with the encoding of the author got me thinking...
> > 
> > What happens if the metadata has utf8 content and the patch itself has 
> > some *other* non-ascii encoding (some iso-latin variant perhaps).
> > 
> > Is there any way to deal with that situation sanely other than indicate
> > that it's 8bit content and not specify an encoding?  Is that what
> > happens currently?
> 
> The body has to be in one encoding, so at the time that you know both
> encodings, you have to pick one and convert the data from the discarded
> encoding into the used encoding.

That seems potentially bad in that the transport (mailed patches) could
be altering the contents of the patch.  Or is this process reversed when 
the patch is finally applied?

Brian

^ permalink raw reply

* Re: `git-send-email' doesn't specify `Content-Type'
From: Jeff King @ 2007-11-11  8:51 UTC (permalink / raw)
  To: Brian Swetland
  Cc: Björn Steinbrink, Johannes Schindelin, Ludovic Courtès,
	git
In-Reply-To: <20071111084515.GB18021@bulgaria>

On Sun, Nov 11, 2007 at 12:45:15AM -0800, Brian Swetland wrote:

> > > What happens if the metadata has utf8 content and the patch itself has 
> > > some *other* non-ascii encoding (some iso-latin variant perhaps).
[...]
> > The body has to be in one encoding, so at the time that you know both
> > encodings, you have to pick one and convert the data from the discarded
> > encoding into the used encoding.
> 
> That seems potentially bad in that the transport (mailed patches) could
> be altering the contents of the patch.  Or is this process reversed when 
> the patch is finally applied?

My answer was for "how do you stick two things with different encoding
in the same mail" (which applies to the name + commit message
situation). However, we don't actually _have_ an encoding for the patch
data. We just assume that it matches the metadata.

-Peff

^ permalink raw reply

* Re: `git-send-email' doesn't specify `Content-Type'
From: Jeff King @ 2007-11-11  8:56 UTC (permalink / raw)
  To: Björn Steinbrink
  Cc: Brian Swetland, Johannes Schindelin, Ludovic Courtès, git
In-Reply-To: <20071111083224.GA30299@sigill.intra.peff.net>

On Sun, Nov 11, 2007 at 03:32:24AM -0500, Jeff King wrote:

> came out of git-format-patch with default-ish settings). So the easy,
> hackish way is probably to just add the MIME-Version and 'Content-type:
> text/plain; charset=utf-8' headers if we unquoted the author field.

Here is the quick and dirty patch. It is totally untested (as in, I
didn't even run git-send-email once), but maybe it can get somebody
started (I left some comments about how to make it less quick and
dirty).  My head is going to explode if I read any more of the ad-hoc
header parsing in git-send-email.perl.

-Peff

---
diff --git a/git-send-email.perl b/git-send-email.perl
index f9bd2e5..4a071f2 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -514,11 +514,13 @@ $time = time - scalar $#files;
 
 sub unquote_rfc2047 {
 	local ($_) = @_;
-	if (s/=\?utf-8\?q\?(.*)\?=/$1/g) {
+	my $encoding;
+	if (s/=\?([^?])+\?q\?(.*)\?=/$2/g) {
+		$encoding = $1;
 		s/_/ /g;
 		s/=([0-9A-F]{2})/chr(hex($1))/eg;
 	}
-	return "$_";
+	return wantarray ? ($_, $encoding) : $_;
 }
 
 # use the simplest quoting being able to handle the recipient
@@ -667,6 +669,9 @@ foreach my $t (@files) {
 	open(F,"<",$t) or die "can't open file $t";
 
 	my $author = undef;
+	my $author_encoding;
+	my $has_content_type;
+	my $body_encoding;
 	@cc = @initial_cc;
 	@xh = ();
 	my $input_format = undef;
@@ -692,12 +697,20 @@ foreach my $t (@files) {
 						next if ($suppress_from);
 					}
 					elsif ($1 eq 'From') {
-						$author = unquote_rfc2047($2);
+						($author, $author_encoding)
+						  = unquote_rfc2047($2);
 					}
 					printf("(mbox) Adding cc: %s from line '%s'\n",
 						$2, $_) unless $quiet;
 					push @cc, $2;
 				}
+				elsif (/^Content-type:/i) {
+					$has_content_type = 1;
+					if (/charset="?[^ "]+/) {
+						$body_encoding = $1;
+					}
+					push @xh, $_;
+				}
 				elsif (!/^Date:\s/ && /^[-A-Za-z]+:\s+\S/) {
 					push @xh, $_;
 				}
@@ -756,6 +769,21 @@ foreach my $t (@files) {
 
 	if (defined $author) {
 		$message = "From: $author\n\n$message";
+		if (defined $author_encoding) {
+			if ($has_content_type) {
+				if ($body_encoding eq $author_encoding) {
+					# ok, we already have the right encoding
+				}
+				else {
+					# uh oh, we should re-encode
+				}
+			}
+			else {
+				push @xh,
+				  'MIME-Version: 1.0',
+				  "Content-Type: text/plain; charset=$author_encoding";
+			}
+		}
 	}
 
 	send_message();

^ permalink raw reply related

* Re: Dose git-fetch need --reference option like git-clone?
From: Junio C Hamano @ 2007-11-11  9:19 UTC (permalink / raw)
  To: Mike Hommey; +Cc: Yin Ping, git
In-Reply-To: <20071111083840.GA17231@glandium.org>

Mike Hommey <mh@glandium.org> writes:

> It would probably be reasonable to have this on git-remote. Anyways, you
> can easily do it yourself by editing .git/objects/info/alternates and
> adding /path/to/remoteACloned in it. You can happily git fetch after
> that.

Not really.

There are two parts in the --reference dance git-clone plays:

 - After the reference clone is done, obviously because you are
   avoiding to actually transfer the data, you need to set up
   alternates to permanently borrow from the reference
   repository.

 - Because the fetch/clone procedure considers an object that is
   locally accessible (either it locally exists, or exists in an
   alternate object store) "complete" (meaning, there is no need
   to fetch the objects that are reachable from it) ONLY when it
   is reachable from one of the refs in the repository, the
   above alone would not prevent fetch/clone from actually
   transferring the objects.  For this reason, git-clone copies
   the refs that exist in the reference repository as temporary
   refs.  This ensures that the objects that the clone borrows
   from the reference repository are considered "complete"
   during the clone process.

The original design of git differenciated the object store (that
is, .git/objects/) and a repository (.git).  A repository has an
object store associated with it, but the design allowed an
object store to be shared among multiple repositories by
symlinking.  This is why alternates point at objects/ directory
of the reference repository, not one level above.

Today, no git tool creates such a "shared object store" layout,
so if the original design were "an object store belongs only to
one repository, and the ../refs directory relative to it always
is the only set of refs that defines the completeness of the
objects within it.  No sharing of object store across
repositories using symlink is allowed", we could redefine the
completeness rule to also follow the alternates and doing so
would eliminate the need for the latter "temporary ref" trick
git-clone plays.  But until we officially declare that a set of
old repositories that uses the "shared object store" layout are
not supported (and give a reasonably migration path), we
unfortunately cannot do so.

^ permalink raw reply

* Re: Subject: [PATCH 2/3] Let git-add--interactive read colors from .gitconfig
From: Junio C Hamano @ 2007-11-11  9:53 UTC (permalink / raw)
  To: Dan Zwell
  Cc: Jeff King, Shawn O. Pearce, Wincent Colaiuta, Git Mailing List,
	Jonathan del Strother, Johannes Schindelin, Frank Lichtenheld
In-Reply-To: <20071110202351.7b4544aa@paradox.zwell.net>

Dan Zwell <dzwell@zwell.net> writes:

> @@ -8,12 +9,18 @@ if ($color_config=~/true|always/ || -t STDOUT && $color_config=~/auto/) {
>  	eval { require Term::ANSIColor; };
>  	if (!$@) {
>  		$use_color = 1;
> +		# Set interactive colors:
> +
> +		# Grab the 3 main colors in git color string format, with sane
> +		# (visible) defaults:
> +		my $repo = Git->repository();
> +		$prompt_color = Git::color_to_ansi_code(
> +			Git::config($repo, "color.interactive.prompt") || "bold blue");
> +		$header_color = Git::color_to_ansi_code(
> +			Git::config($repo, "color.interactive.header") || "bold");
> +		$help_color = Git::color_to_ansi_code(
> +			Git::config($repo, "color.interactive.help") || "red bold");
> +		$normal_color = Git::color_to_ansi_code("normal");

Makes me wonder if you are better off with two new helper
functions defined in Git.pm, as in:

	$prompt_color = $repo->config_color("interactive.prompt") || "bold blue")
	$normal_color = Git::color_to_ansi_code("normal");

> +sub color_to_ansi_code {
> +	my ($git_string) = @_;
> +	my @ansi_words;
> +	my %attrib_mappings = (
> +		"bold"    => "bold",
> +		"ul"      => "underline",
> +		"blink"   => "blink",
> +		# not supported:
> +		#"dim"     => "",
> +		"reverse" => "reverse"
> +	);

I do not like a hash variable name that says it is a "mapping".
It being a hash is enough indication that it is a mapping.

You are better off naming such a mapping as if it is a function
that takes one parameter (in this case, git name) and returns a
single value (perl name).  So I'd probably say:

	my %perl_attrib = ( ... );

(or "git_attrib_to_perl").  A use site of such a hash would look
like this:

	push @perl_words, $perl_attrib{'ul'};
        push @perl_names, $git_attrib_to_perl{'blink'};

Maybe it is just me, but aren't these easier to read?

> +	my ($fg_done, $word);
> +
> +	foreach $word (split /\s+/, $git_string) {
> +		if ($word =~ /normal/) {

	$word eq 'normal' ?

> +			$fg_done = "true";
> +		}
> +		elsif ($word =~ /black|red|green|yellow/ ||
> +			   $word =~ /blue|magenta|cyan|white/) {

	exists $color_name{$word}

with

	my %color_name = map { $_ => 1 } qw(black red ... white);

at the beginning?

> +			# is a color.
> +			if ($fg_done) {
> +				# this is the background
> +				push @ansi_words, "on_" . $word;
> +			}
> +			else {
> +				# this is foreground
> +				$fg_done = "true";
> +				push @ansi_words, $word;
> +			}
> +		}
> +		else {
> +			# this is an attribute, not a color.
> +			if ($attrib_mappings{$word}) {

	exists $git_attrib_to_perl{$word}

?

^ permalink raw reply

* Re: [PATCH 3/3] Added diff hunk coloring to git-add--interactive
From: Junio C Hamano @ 2007-11-11 10:00 UTC (permalink / raw)
  To: Dan Zwell
  Cc: Jeff King, Shawn O. Pearce, Wincent Colaiuta, Git Mailing List,
	Jonathan del Strother, Johannes Schindelin, Frank Lichtenheld
In-Reply-To: <20071110180344.05a81497@paradox.zwell.net>

Dan Zwell <dzwell@zwell.net> writes:

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

It would be better to do the "if (!$diff_use_color)" part
upfront before entering the loop, wouldn't it?

	sub colored_diff_hunk {
        	my ($text) = @_;
		if (!$diff_use_color) {
	                return @$text;
		}

                my @ret;
                for (@$text) {
                	...
		}
	}

^ permalink raw reply

* Re: [PATCH 3/3] --format=pretty: avoid calculating expensive expansions twice
From: Junio C Hamano @ 2007-11-11 10:29 UTC (permalink / raw)
  To: René Scharfe
  Cc: Jeff King, Paul Mackerras, Git Mailing List, Pierre Habouzit,
	Johannes Schindelin
In-Reply-To: <47359382.1010600@lsrfire.ath.cx>

René Scharfe <rene.scharfe@lsrfire.ath.cx> writes:

> +static int add_again(struct strbuf *sb, struct chunk *chunk)
> +{
> +	if (chunk->len) {
> +		strbuf_adddup(sb, chunk->off, chunk->len);
> +		return 1;
> +	}
> +
> +	/*
> +	 * We haven't seen this chunk before.  Our caller is surely
> +	 * going to add it the hard way now.  Remember the most likely
> +	 * start of the to-be-added chunk: the current end of the
> +	 * struct strbuf.
> +	 */
> +	chunk->off = sb->len;
> +	return 0;
> +}
> +
>  static void parse_commit_header(struct format_commit_context *context)
>  {
>  	const char *msg = context->commit->buffer;
> @@ -447,15 +469,21 @@ static void format_commit_item(struct strbuf *sb, const char *placeholder,
>  		strbuf_addstr(sb, sha1_to_hex(commit->object.sha1));
>  		return;
>  	case 'h':		/* abbreviated commit hash */
> +		if (add_again(sb, &c->abbrev_commit_hash))
> +			return;
>  		strbuf_addstr(sb, find_unique_abbrev(commit->object.sha1,
>  		                                     DEFAULT_ABBREV));
> +		c->abbrev_commit_hash.len = sb->len - c->abbrev_commit_hash.off;
>  		return;

Brilliant.  Doubly brilliant is the adddup abstraction that does
not suffer from underlying strbuf being reallocated.

Me likee..

^ permalink raw reply

* Re: Subject: [PATCH 2/3] Let git-add--interactive read colors from .gitconfig
From: Junio C Hamano @ 2007-11-11 10:34 UTC (permalink / raw)
  To: Dan Zwell
  Cc: Jeff King, Shawn O. Pearce, Wincent Colaiuta, Git Mailing List,
	Jonathan del Strother, Johannes Schindelin, Frank Lichtenheld
In-Reply-To: <7vve89f6qy.fsf@gitster.siamese.dyndns.org>

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

> Makes me wonder if you are better off with two new helper
> functions defined in Git.pm, as in:
>
> 	$prompt_color = $repo->config_color("interactive.prompt") || "bold blue")
> 	$normal_color = Git::color_to_ansi_code("normal");

Sorry, but please disregard.  "bold blue" part was also
parameter to the string-to-ansi-color-escape function, so the
above does not make much sense.

^ permalink raw reply

* Re: gitk/git-gui misfeature: saving the geometry can the window out of reach
From: Shawn O. Pearce @ 2007-11-11 10:44 UTC (permalink / raw)
  To: Benoit Sigoure; +Cc: Git Mailing List
In-Reply-To: <8700373A-4878-4EBD-BA27-D4F31BE44907@lrde.epita.fr>

Benoit Sigoure <tsuna@lrde.epita.fr> wrote:
> On Nov 11, 2007, at 6:11 AM, Shawn O. Pearce wrote:
> >Benoit Sigoure <tsuna@lrde.epita.fr> wrote:
> >>gitk (and I think git-gui too) save their "geometry" (which includes
> >>X/Y position) in ~/.gitk.  So far so good.  The problem is that I
> >>often use 2 screens at work (one is the screen of my laptop, the
> >>other one is above) and I happen to put my gitk/git-gui windows on
> >>the 2nd screen.  When I go back home, I don't have a second screen
> >>and gitk/git-gui open their windows out of reach.
> >
> >Actually git-gui saves the geometry to .git/config so I'm not sure
> >why the sed line above would correct git-gui's display issues.  But I
> >have also noticed this problem on my Mac OS X laptop when running
> >again after leaving either git-gui or gitk on the external display.
> 
> it's been a long time since I last used Tcl/Tk but if you give me a  
> hint as to where to look / what should be done, I can give it a try.

Look in git-gui.sh for "-- Load geometry"; in my current master
it is on line 2625.  This is the block where we have taken the
geometry data back in from .git/config and are trying to update
the UI to match what it says.  Unfortunately we set a coordinate
that is off the desktop using a standard X geometry string in the
"wm geometry . [lindex $gm 0]" call...

-- 
Shawn.

^ permalink raw reply

* Re: [PATCH] Simplify strchrnul() compat code
From: Junio C Hamano @ 2007-11-11 10:44 UTC (permalink / raw)
  To: Andreas Ericsson
  Cc: René Scharfe, Pierre Habouzit, Git Mailing List,
	Johannes Schindelin, Jakub Narebski
In-Reply-To: <4735BA79.5020102@op5.se>

Andreas Ericsson <ae@op5.se> writes:

> René Scharfe wrote:
>>  -#ifdef NO_STRCHRNUL
>> +#if !defined(__GLIBC__) && !__GLIBC_PREREQ(2, 1)
>
> This will break things for users of glibc-2.1.1 (the first release still
> available from ftp://sources.redhat.com/pub/glibc/old-releases that
> includes the strchrnul() function), since __GLIBC_PREREQ() was invented
> after strchrnul() was introduced.
>
> Replacing __GLIBC__ with __GLIBC_PREREQ (as in the original patch) will
> solve it nicely. Users of glibc-2.1.1 will be the odd minority where
> strchrnul() is available in their libc but not used.

Do you mean this on top of René's patch?  Although I do not
think I saw "the original patch" that did it this way, I think
it makes sense.

diff --git a/git-compat-util.h b/git-compat-util.h
index 11e6df6..dd96f7a 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h
@@ -183,7 +183,7 @@ void *gitmemmem(const void *haystack, size_t haystacklen,
                 const void *needle, size_t needlelen);
 #endif
 
-#if !defined(__GLIBC__) && !__GLIBC_PREREQ(2, 1)
+#if !defined(__GLIBC_PREREQ) && !__GLIBC_PREREQ(2, 1)
 #define strchrnul gitstrchrnul
 static inline char *gitstrchrnul(const char *s, int c)
 {

^ permalink raw reply related

* Re: [PATCH 3/3] git-svn log: handle unreachable revisions like "svn log"
From: Benoit Sigoure @ 2007-11-11 10:51 UTC (permalink / raw)
  To: David D Kilzer; +Cc: git, gitster
In-Reply-To: <1194761435-7286-4-git-send-email-ddkilzer@kilzer.net>

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

Hi David,
thanks for the patches, the series looks good to me, I added some  
comments below, for this patch.

On Nov 11, 2007, at 7:10 AM, David D Kilzer wrote:

> When unreachable revisions are given to "svn log", it displays all  
> commit
> logs in the given range that exist in the current tree.  (If no commit
> logs are found in the current tree, it simply prints a single  
> commit log
> separator.)  This patch makes "git-svn log" behave the same way.
>
> Ten tests added to t/t9116-git-svn-log.sh.
>
> Signed-off-by: David D Kilzer <ddkilzer@kilzer.net>
> ---
>  git-svn.perl           |   58 +++++++++++++++++++++++++++ 
> +--------------
>  t/t9116-git-svn-log.sh |   66 +++++++++++++++++++++++++++++++++++++ 
> +++++++++++
>  2 files changed, 105 insertions(+), 19 deletions(-)
>
> diff --git a/git-svn.perl b/git-svn.perl
> index 39585d8..c584715 100755
> --- a/git-svn.perl
> +++ b/git-svn.perl
> @@ -2257,9 +2257,10 @@ sub rev_db_get {
>  }
>
>  sub find_rev_before {
> -	my ($self, $rev, $eq_ok) = @_;
> +	my ($self, $rev, $eq_ok, $min_rev) = @_;

Could you please document this function?  I guess that you had to  
figure out what each argument was for, so please save the time of the  
contributors that will read this code after you :)

>  	--$rev unless $eq_ok;
> -	while ($rev > 0) {
> +	$min_rev ||= 1;
> +	while ($rev >= $min_rev) {
>  		if (my $c = $self->rev_db_get($rev)) {
>  			return ($rev, $c);
>  		}
> @@ -2268,6 +2269,19 @@ sub find_rev_before {
>  	return (undef, undef);
>  }
>
> +sub find_rev_after {
> +	my ($self, $rev, $eq_ok, $max_rev) = @_;
> +	++$rev unless $eq_ok;
> +	$max_rev ||= $self->rev_db_max();
> +	while ($rev <= $max_rev) {
> +		if (my $c = $self->rev_db_get($rev)) {
> +			return ($rev, $c);
> +		}
> +		++$rev;
> +	}
> +	return (undef, undef);
> +}
> +

Too much code duplication.  It should be possible to write a sub  
find_rev_ (or _find_rev, don't know what's the naming convention for  
internal details) that takes a 5th argument, an anonymous sub that  
does the comparison.  So that basically, find_rev_before will be  
something along these (untested) lines:

sub find_rev_before {
	my ($self, $rev, $eq_ok, $min_rev) = @_;
	return find_rev_($self, $rev, $eq_ok, $min_rev, sub { my ($a, $b) =  
@_; return $a >= $b; });
}

>  sub _new {
>  	my ($class, $repo_id, $ref_id, $path) = @_;
>  	unless (defined $repo_id && length $repo_id) {
> @@ -3587,19 +3601,19 @@ sub git_svn_log_cmd {
>  			push @cmd, $c;
>  		}
>  	} elsif (defined $r_max) {
> -		my ($c_min, $c_max);
> -		$c_max = $gs->rev_db_get($r_max);
> -		$c_min = $gs->rev_db_get($r_min);
> -		if (defined $c_min && defined $c_max) {
> -			if ($r_max > $r_min) {
> -				push @cmd, "--boundary", "$c_min..$c_max";
> -			} else {
> -				push @cmd, "--boundary", "$c_max..$c_min";
> -			}
> -		} elsif ($r_max > $r_min) {
> -			push @cmd, $c_max;
> +		if ($r_max < $r_min) {
> +			($r_min, $r_max) = ($r_max, $r_min);
> +		}
> +		my (undef, $c_max) = $gs->find_rev_before($r_max, 1, $r_min);
> +		my (undef, $c_min) = $gs->find_rev_after($r_min, 1, $r_max);
> +		# If there are no commits in the range, both $c_max and $c_min
> +		# will be undefined.  If there is at least 1 commit in the
> +		# range, both will be defined.
> +		return () if !defined $c_min;

Fair enough but I'd strengthen the test by writing something like:
		return () if not defined $c_min || not defined $c_max;
unless you can prove that `rev_db_get' can never return `undef' or  
something like that.

> +		if ($c_min eq $c_max) {
> +			push @cmd, '--max-count=1', $c_min;
>  		} else {
> -			push @cmd, $c_min;
> +			push @cmd, '--boundary', "$c_min..$c_max";
>  		}
>  	}
>  	return (@cmd, @files);
> @@ -3705,9 +3719,13 @@ sub show_commit_changed_paths {
>  	print "Changed paths:\n", @{$c->{changed}};
>  }
>
> +sub commit_log_separator {
> +    return ('-' x 72) . "\n";
> +}
> +

This is basically a constant, I think that declaring it with a  
prototype helps Perl to optimize it:
sub commit_log_separator() {

>  sub show_commit_normal {
>  	my ($c) = @_;
> -	print '-' x72, "\nr$c->{r} | ";
> +	print commit_log_separator(), "r$c->{r} | ";
>  	print "$c->{c} | " if $show_commit;
>  	print "$c->{a} | ", strftime("%Y-%m-%d %H:%M:%S %z (%a, %d %b %Y)",
>  				 localtime($c->{t_utc})), ' | ';
> diff --git a/t/t9116-git-svn-log.sh b/t/t9116-git-svn-log.sh
> index 5000892..56dd8fe 100755
> --- a/t/t9116-git-svn-log.sh
> +++ b/t/t9116-git-svn-log.sh
> @@ -59,4 +65,64 @@ test_expect_success 'test descending revision  
> range' "
[...]
> +
> +echo  
> ---------------------------------------------------------------------- 
> -- > expected-separator

This will choke on shells with buggy/fragile `echo'.  I think it'd be  
safer to use printf here.


Cheers,

-- 
Benoit Sigoure aka Tsuna
EPITA Research and Development Laboratory



[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 186 bytes --]

^ permalink raw reply

* Re: Deprecate git-fetch-pack?
From: Johannes Schindelin @ 2007-11-11 11:05 UTC (permalink / raw)
  To: Ask Bjørn Hansen; +Cc: Junio C Hamano, Daniel Barkalow, git
In-Reply-To: <74415967-7F49-426C-8BF5-1A0210C337AB@develooper.com>

Hi,

On Sat, 10 Nov 2007, Ask Bj?rn Hansen wrote:

> For new users the superfluous programs are a burden making it harder to 
> learn how everything works.

This should be a non-issue.  We really should start deprecating 
"git-<command>" in favour of "git <command>" for real.

New users should not even be told that this is correct usage.

My reason?  We have plumbing, and we will always have plumbing, as 
commands.  A regular git user does not _want_ to see that.  Without said 
deprecation she _will_, however.

Ciao,
Dscho

^ permalink raw reply

* Re: git packs
From: Derek Fawcus @ 2007-11-11 11:09 UTC (permalink / raw)
  To: bob; +Cc: git
In-Reply-To: <134659C4-BA10-4B9E-9C64-2754A90D93F8@mac.com>

On Sat, Nov 10, 2007 at 01:01:46PM -0500, bob wrote:
> It is fairly disappointing as far as indicating the problem.  Here is  
> the entire report since it was so short.

> Error Formulating Crash Report:
> *** -[NSCFDictionary setObject:forKey:]: attempt to insert nil value  
> (key: VMUSignaturePath)

huh?  The above looks like an ObjC invocation.  Last I checked,  git was C.
So why is that in the frame?

DF

^ permalink raw reply

* Re: git-gui messes up the diff view on non ASCII characters
From: Johannes Schindelin @ 2007-11-11 11:20 UTC (permalink / raw)
  To: Shawn O. Pearce; +Cc: Michele Ballabio, git, Peter Baumann
In-Reply-To: <20071111055959.GW14735@spearce.org>

[-- Attachment #1: Type: TEXT/PLAIN, Size: 1765 bytes --]

Hi,

On Sun, 11 Nov 2007, Shawn O. Pearce wrote:

> Michele Ballabio <barra_cuda@katamail.com> wrote:
> > On Friday 09 November 2007, Peter Baumann wrote:
> > > I'm managing some UTF-8 encoded LaTeX files in git, which include some
> > > non ASCII characters like the german ä,ö and ü. If I view the diff with
> > > git-diff on an UTF8 enabled terminal, all looks nice. So does the diff
> > > view in gitk after I commited my changes. Only git-gui shows some
> > > "strange" characters, so I assume it is an encoding problem.
> > > 
> > > I have to admit that I'm totally unaware how this should work, but at
> > > least I think my configuration is correct here, because otherwise git-diff
> > > or gitk would show the same behaviour. Is there anything which could be
> > > done to make git-gui happy, too?
> > 
> > It's a known issue, and already on Shawn's ToDo list. I have to add that
> > viewing untracked UTF8 files in git-gui works just fine. Weird.
> 
> Cute.  That's because in the untracked case we open the file and
> let the platform's chosen encoding be used to convert it into the
> text viewer.  In the tracked diff case we force the encoding to
> be in binary.
> 
> Now gitk works because it assumes the diff is in the same character
> encoding as the commit message itself.  Since commit messages are
> typically in UTF-8 (as that is the Git default encoding) then a
> UTF-8 encoded file shows correctly in gitk.
> 
> What's the right behavior here?  Just assume the platform encoding
> is correct for the file we are showing and show it?  Assume the
> commit encoding configured in i18n.commitencoding is the correct
> one for the file content?  Something else?

My twopence: assume utf-8, but make it a _git-gui_ config variable.

Ciao,
Dscho

^ permalink raw reply

* Re: Dose git-fetch need --reference option like git-clone?
From: Yin Ping @ 2007-11-11 11:38 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7vsl3dgovv.fsf@gitster.siamese.dyndns.org>

On Nov 11, 2007 4:36 PM, Junio C Hamano <gitster@pobox.com> wrote:
>
> No, there is no such thing.
>
> I think what you are talking about is a reasonable thing to
> want.  It would have been easier to hack in back when git-fetch
> was a script, but now you would need to work a bit harder in the
> C code.  On the other hand, however, I suspect the resulting
> code would be cleaner without having to actually create and
> delete temporary refs in refs/reference-tmp/ namespace but fake
> them only in-core, with a proper transport API enhancements.
>
> But if you only want a quick-and-dirty workaround, you can
> manually do refs/reference-tmp and objects/info/alternates dance
> that is done by git-clone before running a git-fetch from such
> "nearby" remote.

Now my workaround is
$ git remote add remoteA path/to/remoteACloned
$ git fetch remoteA
Then update .git/config to let remote.remoteA.url=git://remoteAUrl
$ git fetch





-- 
Ping Yin

^ permalink raw reply

* [PATCH v3] Make GIT_INDEX_FILE apply to git-commit
From: Rémi Vanicat @ 2007-11-11 12:28 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git

Currently, when committing, git-commit ignore the value of
GIT_INDEX_FILE, and always use $GIT_DIR/index. This patch
fix it.

Signed-off-by: Rémi Vanicat <vanicat@debian.org>
---
 git-commit.sh     |    2 +-
 t/t7500-commit.sh |   13 +++++++++++++
 2 files changed, 14 insertions(+), 1 deletions(-)

diff --git a/git-commit.sh b/git-commit.sh
index fcb8443..6490045 100755
--- a/git-commit.sh
+++ b/git-commit.sh
@@ -26,7 +26,7 @@ refuse_partial () {
 }
 
 TMP_INDEX=
-THIS_INDEX="$GIT_DIR/index"
+THIS_INDEX="${GIT_INDEX_FILE:-$GIT_DIR/index}"
 NEXT_INDEX="$GIT_DIR/next-index$$"
 rm -f "$NEXT_INDEX"
 save_index () {
diff --git a/t/t7500-commit.sh b/t/t7500-commit.sh
index abbf54b..3e5abef 100755
--- a/t/t7500-commit.sh
+++ b/t/t7500-commit.sh
@@ -93,4 +93,17 @@ test_expect_success 'commit message from file should override template' '
        commit_msg_is "standard input msg"
 '
 
+test_expect_success 'using GIT_INDEX_FILE' '
+
+       echo "some new content" >file &&
+       GIT_INDEX_FILE=.git/another_index git add file &&
+       GIT_INDEX_FILE=.git/another_index \
+               git commit -m "commit using another index" &&
+       git reset HEAD &&
+       git diff HEAD -- file >current &&
+       touch empty-file &&
+       diff empty-file current
+
+'
+
 test_done
-- 
1.5.3.5

^ permalink raw reply related

* Re: [PATCH] Simplify strchrnul() compat code
From: Andreas Ericsson @ 2007-11-11 12:35 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: René Scharfe, Pierre Habouzit, Git Mailing List,
	Johannes Schindelin, Jakub Narebski
In-Reply-To: <7v6409f4eh.fsf@gitster.siamese.dyndns.org>

Junio C Hamano wrote:
> Andreas Ericsson <ae@op5.se> writes:
> 
>> René Scharfe wrote:
>>>  -#ifdef NO_STRCHRNUL
>>> +#if !defined(__GLIBC__) && !__GLIBC_PREREQ(2, 1)
>> This will break things for users of glibc-2.1.1 (the first release still
>> available from ftp://sources.redhat.com/pub/glibc/old-releases that
>> includes the strchrnul() function), since __GLIBC_PREREQ() was invented
>> after strchrnul() was introduced.
>>
>> Replacing __GLIBC__ with __GLIBC_PREREQ (as in the original patch) will
>> solve it nicely. Users of glibc-2.1.1 will be the odd minority where
>> strchrnul() is available in their libc but not used.
> 
> Do you mean this on top of René's patch?

Yes.

>  Although I do not
> think I saw "the original patch" that did it this way,

My memory seems to be failing. I never committed my proposal to my repo,
and the one I sent out was the __GLIBC__ || !__GLIBC_PREREQ thing. My
apologies.

-- 
Andreas Ericsson                   andreas.ericsson@op5.se
OP5 AB                             www.op5.se
Tel: +46 8-230225                  Fax: +46 8-230231

^ permalink raw reply

* git diff and file deleted from the cache
From: Remi Vanicat @ 2007-11-11 12:43 UTC (permalink / raw)
  To: git

The manual of git-diff tell me that :
  git-diff [--options] <commit> [--] [<path>...]
      This form is to view the changes you have in your working tree
      relative to the named <commit>. You can use HEAD to compare it with
      the latest commit, or a branch name to compare with the tip of a
      different branch.

So the following seem strange :

$ echo foo > bar
$ git commit -m "committing bar" -a
Created commit 074893b: committing bar
 1 files changed, 1 insertions(+), 0 deletions(-)
 create mode 100644 bar
$ git rm --cached bar
rm 'bar'
$ git diff HEAD --
diff --git a/bar b/bar
deleted file mode 100644
index 257cc56..0000000
--- a/bar
+++ /dev/null
@@ -1 +0,0 @@
-foo

I excepted the diff to be empty, as HEAD and the working directory are
synchronized, I've only modified the cache.

-- 
Rémi Vanicat

^ permalink raw reply related

* Re: git packs
From: bob @ 2007-11-11 12:54 UTC (permalink / raw)
  To: Derek Fawcus; +Cc: git
In-Reply-To: <20071111110942.A4013@edi-view2.cisco.com>

Well, there are actually two problems here.  The first is that
crashreporter crashed trying to create the crash report of
the git failure.  The second was the error in git itself.
Hope that helps.

I am submitting an Apple bugreport on the first and
Nicolas Pitre is probably correct in his fix.  When I
was looking at index-pack.c everything was using 32
bit unsigned and signed number where off_t was
64-bit which is what a stat() would return.  My problem
is that I was not familiar enough with git internals to
figure out the solution.  My review was the first time
that I ever looked at git source.

Thanks, Nicolas

On Nov 11, 2007, at 6:09 AM, Derek Fawcus wrote:

> On Sat, Nov 10, 2007 at 01:01:46PM -0500, bob wrote:
>> It is fairly disappointing as far as indicating the problem.  Here is
>> the entire report since it was so short.
>
>> Error Formulating Crash Report:
>> *** -[NSCFDictionary setObject:forKey:]: attempt to insert nil value
>> (key: VMUSignaturePath)
>
> huh?  The above looks like an ObjC invocation.  Last I checked,   
> git was C.
> So why is that in the frame?
>
> DF

^ permalink raw reply

* (unknown)
From: Michael Dressel @ 2007-11-11 13:08 UTC (permalink / raw)
  To: git


>Michael Dressel wrote:
>Ok nice. Another thing is that git-push will push all the tracking 
>branches in refs/remotes/origin. 

I learned that I only have to edit the .git/config file to avoid that 
git-push pushes everything. 

I modified the remotes names and added push lines explicitly.

Is that the recommended way?

In my example (git-branch -a -v):
* exp                 aa854c6 shtest 4
  master              34924b9 mastertest 1
  origin/exp          aa854c6 shtest 4
  origin/master       b68e7a9 brt master 1

I used the following .git/config:
[core]
        repositoryformatversion = 0
        filemode = true
        bare = false
        logallrefupdates = true
[remote "origin1"]
        url = /home/repo/src
        fetch = +refs/heads/master:refs/remotes/origin/master
        push = +refs/heads/master:refs/heads/master
[remote "origin2"]
        url = /home/repo/src
        fetch = +refs/heads/exp:refs/remotes/origin/exp
        push = +refs/heads/exp:refs/heads/exp
[branch "master"]
        remote = origin1
        merge = refs/heads/master
[branch "exp"]
        remote = origin2
        merge = refs/heads/exp

Cheers,
Michael

^ permalink raw reply

* Re: cogito remote branch
From: Michael Dressel @ 2007-11-11 13:11 UTC (permalink / raw)
  To: git



>Michael Dressel wrote:
>Ok nice. Another thing is that git-push will push all the tracking 
>branches in refs/remotes/origin. 

I learned that I only have to edit the .git/config file to avoid that 
git-push pushes everything. 

I modified the remotes names and added push lines explicitly.

Is that the recommended way?

In my example (git-branch -a -v):
* exp                 aa854c6 shtest 4
  master              34924b9 mastertest 1
  origin/exp          aa854c6 shtest 4
  origin/master       b68e7a9 brt master 1

I used the following .git/config:
[core]
        repositoryformatversion = 0
        filemode = true
        bare = false
        logallrefupdates = true
[remote "origin1"]
        url = /home/repo/src
        fetch = +refs/heads/master:refs/remotes/origin/master
        push = +refs/heads/master:refs/heads/master
[remote "origin2"]
        url = /home/repo/src
        fetch = +refs/heads/exp:refs/remotes/origin/exp
        push = +refs/heads/exp:refs/heads/exp
[branch "master"]
        remote = origin1
        merge = refs/heads/master
[branch "exp"]
        remote = origin2
        merge = refs/heads/exp

Cheers,
Michael

^ permalink raw reply

* [PATCH 5/6] push: use same rules as git-rev-parse to resolve refspecs
From: Steffen Prohaska @ 2007-11-11 14:01 UTC (permalink / raw)
  To: git; +Cc: Steffen Prohaska
In-Reply-To: <11947897083265-git-send-email-prohaska@zib.de>

This commit changes the rules for resolving refspecs to match the
rules for resolving refs in rev-parse. git-rev-parse uses clear rules
to resolve a short ref to its full name, which are well documented.
The rules for resolving refspecs documented in git-send-pack were
less strict and harder to understand. This commit replaces them by
the rules of git-rev-parse.

The unified rules are easier to understand and better resolve ambiguous
cases. You can now push from a repository containing several branches
ending on the same short name.

Note, this may break existing setups. For example "master" will no longer
resolve to "origin/master".

Signed-off-by: Steffen Prohaska <prohaska@zib.de>
---
 Documentation/git-send-pack.txt |    4 +++-
 remote.c                        |    5 +----
 t/t5516-fetch-push.sh           |   12 +++++++++++-
 3 files changed, 15 insertions(+), 6 deletions(-)

diff --git a/Documentation/git-send-pack.txt b/Documentation/git-send-pack.txt
index 2fa01d4..a2d9cb6 100644
--- a/Documentation/git-send-pack.txt
+++ b/Documentation/git-send-pack.txt
@@ -85,7 +85,9 @@ Each pattern pair consists of the source side (before the colon)
 and the destination side (after the colon).  The ref to be
 pushed is determined by finding a match that matches the source
 side, and where it is pushed is determined by using the
-destination side.
+destination side. The rules used to match a ref are the same
+rules used by gitlink:git-rev-parse[1] to resolve a symbolic ref
+name.
 
  - It is an error if <src> does not match exactly one of the
    local refs.
diff --git a/remote.c b/remote.c
index bec2ba1..28d8eb7 100644
--- a/remote.c
+++ b/remote.c
@@ -519,10 +519,7 @@ static int count_refspec_match(const char *pattern,
 		char *name = refs->name;
 		int namelen = strlen(name);
 
-		if (namelen < patlen ||
-		    memcmp(name + namelen - patlen, pattern, patlen))
-			continue;
-		if (namelen != patlen && name[namelen - patlen - 1] != '/')
+		if (!ref_abbrev_matches_full_with_rules(pattern, name, ref_rev_parse_rules))
 			continue;
 
 		/* A match is "weak" if it is with refs outside
diff --git a/t/t5516-fetch-push.sh b/t/t5516-fetch-push.sh
index b0ff488..fd5f284 100755
--- a/t/t5516-fetch-push.sh
+++ b/t/t5516-fetch-push.sh
@@ -145,11 +145,21 @@ test_expect_success 'push with no ambiguity (1)' '
 test_expect_success 'push with no ambiguity (2)' '
 
 	mk_test remotes/origin/master &&
-	git push testrepo master:master &&
+	git push testrepo master:origin/master &&
 	check_push_result $the_commit remotes/origin/master
 
 '
 
+test_expect_success 'push with colon-less refspec, no ambiguity' '
+
+	mk_test heads/master heads/t/master &&
+	git branch -f t/master master &&
+	git push testrepo master &&
+	check_push_result $the_commit heads/master &&
+	check_push_result $the_first_commit heads/t/master
+
+'
+
 test_expect_success 'push with weak ambiguity (1)' '
 
 	mk_test heads/master remotes/origin/master &&
-- 
1.5.3.5.578.g886d

^ 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