Git development
 help / color / mirror / Atom feed
* Re: How to find and analyze bad merges?
From: Junio C Hamano @ 2012-02-02 20:09 UTC (permalink / raw)
  To: norbert.nemec; +Cc: git
In-Reply-To: <jgdjd1$5mn$1@dough.gmane.org>

"norbert.nemec" <norbert.nemec@native-instruments.de> writes:

>> Bisect?
>
> This is not the point: My colleague knew exactly which commit
> contained the bugfix. The trouble was finding out why this bugfix
> disappeared even though everything indicated that it was cleanly
> merged into the current branch.

Then again "Bisect?"

I wasn't and I am not suggesting to use Bisect to find the original fix. I
was suggesting to use Bisect to find the _merge_ you were looking for.

^ permalink raw reply

* [PATCH/RFC (version B)] gitweb: Allow UTF-8 encoded CGI query parameters and  path_info
From: Jakub Narebski @ 2012-02-02 20:10 UTC (permalink / raw)
  To: Michał Kiedrowicz; +Cc: git
In-Reply-To: <m37h05c8c1.fsf@localhost.localdomain>

Gitweb tries hard to properly process UTF-8 data, by marking output
from git commands and contents of files as UTF-8 with to_utf8()
subroutine.  This ensures that gitweb would print correctly UTF-8
e.g. in 'log' and 'commit' views.

Unfortunately it misses another source of potentially Unicode input,
namely query parameters.  The result is that one cannot search for a
string containing characters outside US-ASCII.  For example searching
for "Michał Kiedrowicz" (containing letter 'ł' - LATIN SMALL LETTER L
WITH STROKE, with Unicode codepoint U+0142, represented with 0xc5 0x82
bytes in UTF-8 and percent-encoded as %C5%81) result in the following
incorrect data in search field

	Michał Kiedrowicz

This is caused by CGI by default treating '0xc5 0x82' bytes as two
characters in Perl legacy encoding latin-1 (iso-8859-1), because 's'
query parameter is not processed explicitly as UTF-8 encoded string.

The solution used here follows "Using Unicode in a Perl CGI script"
article on http://www.lemoda.net/cgi/perl-unicode/index.html:

	use CGI;
	use Encode 'decode_utf8;
	my $value = params('input');
	$value = decode_utf8($value);

This is done when filling %input_params hash; this required to move
from explicit $cgi->param(<label>) to $input_params{<name>} in a few
places.

Alternate solution would be to simply use the '-utf8' pragma (via
"use CGI '-utf8';"), but according to CGI.pm documentation it may
cause problems with POST requests containing binary files... and
it doesn't work with old CGI.pm version 3.10 from Perl v5.8.6.

Noticed-by: Michał Kiedrowicz <michal.kiedrowicz@gmail.com>
Signed-off-by: Jakub Narębski <jnareb@gmail.com>
---
 gitweb/gitweb.perl |   12 ++++++------
 1 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index 9cf7e71..55b2c24 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -52,7 +52,7 @@ sub evaluate_uri {
 	# as base URL.
 	# Therefore, if we needed to strip PATH_INFO, then we know that we have
 	# to build the base URL ourselves:
-	our $path_info = $ENV{"PATH_INFO"};
+	our $path_info = decode_utf8($ENV{"PATH_INFO"});
 	if ($path_info) {
 		if ($my_url =~ s,\Q$path_info\E$,, &&
 		    $my_uri =~ s,\Q$path_info\E$,, &&
@@ -816,9 +816,9 @@ sub evaluate_query_params {
 
 	while (my ($name, $symbol) = each %cgi_param_mapping) {
 		if ($symbol eq 'opt') {
-			$input_params{$name} = [ $cgi->param($symbol) ];
+			$input_params{$name} = [ map { decode_utf8($_) } $cgi->param($symbol) ];
 		} else {
-			$input_params{$name} = $cgi->param($symbol);
+			$input_params{$name} = decode_utf8($cgi->param($symbol));
 		}
 	}
 }
@@ -2767,7 +2767,7 @@ sub git_populate_project_tagcloud {
 	}
 
 	my $cloud;
-	my $matched = $cgi->param('by_tag');
+	my $matched = $input_params{'ctag'};
 	if (eval { require HTML::TagCloud; 1; }) {
 		$cloud = HTML::TagCloud->new;
 		foreach my $ctag (sort keys %ctags_lc) {
@@ -5282,7 +5282,7 @@ sub git_project_list_body {
 
 	my $check_forks = gitweb_check_feature('forks');
 	my $show_ctags  = gitweb_check_feature('ctags');
-	my $tagfilter = $show_ctags ? $cgi->param('by_tag') : undef;
+	my $tagfilter = $show_ctags ? $input_params{'ctag'} : undef;
 	$check_forks = undef
 		if ($tagfilter || $searchtext);
 
@@ -6197,7 +6197,7 @@ sub git_tag {
 
 sub git_blame_common {
 	my $format = shift || 'porcelain';
-	if ($format eq 'porcelain' && $cgi->param('js')) {
+	if ($format eq 'porcelain' && $input_params{'javascript'}) {
 		$format = 'incremental';
 		$action = 'blame_incremental'; # for page title etc
 	}
-- 
1.7.6

^ permalink raw reply related

* Re: [PATCH/RFC (version A)] gitweb: use CGI with -utf8 to process Unicode query  parameters correctly
From: Jakub Narebski @ 2012-02-02 20:11 UTC (permalink / raw)
  To: Michał Kiedrowicz; +Cc: git
In-Reply-To: <201202022108.51353.jnareb@gmail.com>

> According to "Using Unicode in a Perl CGI script" article on
> http://www.lemoda.net/cgi/perl-unicode/index.html the simplest
> solution is to just import '-utf8' pragma for CGI module:
> 
> 	use CGI '-utf8';
> 	my $value = params('input');
[...]
> ---

Except it doesn't work for me...

>  gitweb/gitweb.perl |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
> index 9cf7e71..a7441ef 100755
> --- a/gitweb/gitweb.perl
> +++ b/gitweb/gitweb.perl
> @@ -10,7 +10,7 @@
>  use 5.008;
>  use strict;
>  use warnings;
> -use CGI qw(:standard :escapeHTML -nosticky);
> +use CGI qw(:standard :escapeHTML -nosticky -utf8);
>  use CGI::Util qw(unescape);
>  use CGI::Carp qw(fatalsToBrowser set_message);
>  use Encode;
> -- 
> 1.7.6
> 
> 

-- 
Jakub Narebski
Poland

^ permalink raw reply

* Re: [PATCH, RFC] Fix build problems related to profile-directed optimization
From: Ted Ts'o @ 2012-02-02 20:12 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, Andi Kleen, Clemens Buchacher
In-Reply-To: <7vvcnpuhpo.fsf@alter.siamese.dyndns.org>

On Thu, Feb 02, 2012 at 12:02:27PM -0800, Junio C Hamano wrote:
> 
> Thanks for a patch.  How does this compare with what was discussed in the
> other thread?
> 
>   http://thread.gmane.org/gmane.comp.version-control.git/188992/focus=189172

I wasn't aware of this other approach when I created this patch (I
must have missed the e-mail thread, sorry).

One of the reasons why I did it this way was for more flexibility.  I
wanted to be able to do:

$ make PROFILE_GEN=YesPlease PROFILE_DIR=/var/cache/FDO all
# make PROFILE_GEN=YesPlease PROFILE_DIR=/var/cache/FDO install

run a bunch of git commands on various git repositories to get
real-life usage...

Then do...

$ make PROFILE_USE=YesPlease PROFILE_DIR=/var/cache/FDO all
# make PROFILE_GEN=YesPlease PROFILE_DIR=/var/cache/FDO install

But for many people they would probably be satisfied with something
that builds git using a single magic recipe, even if they give up some
fractional performance improvement (keep in mind that the feedback
directed optimization seems to buy you only a single digit percentage
improvement according according to Andi's original experiment; I just
got interested in this more for amusement value than any thought that
it would save me serious amounts of time).

> I would wish a solution ideally would support
> 
> 	make PROFILE_BUILD=YesPlease
>         make PROFILE_BUILD=YesPlease install

At least in theory, it should be possible to have something which
supports both PROFILE_GEN/PROFILE_USE as well as a combined
PROFILE_BUILD.

The hard part is that PROFILE_BUILD requires a multi-pass process; you
need to build with one set of CFLAGS, then run the sample workload to
get the data for your feedback directed optimizations, and then re-run
the build with another set of CFLAGS.  I think what we could to check
for PROFILE_BUILD, and if it is set, do the first PROFILE_GEN / make
test commands as part of the top-level Makefile's all: rule, and then
do the normal build after that.

It's a little kludgy, but does that sound acceptable to you?

       	      	      	       	    	  - Ted

^ permalink raw reply

* Re: [PATCH, RFC] Fix build problems related to profile-directed optimization
From: Ted Ts'o @ 2012-02-02 20:14 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, Andi Kleen, Clemens Buchacher
In-Reply-To: <20120202201226.GA1032@thunk.org>

On Thu, Feb 02, 2012 at 03:12:26PM -0500, Ted Ts'o wrote:
> Then do...
> 
> $ make PROFILE_USE=YesPlease PROFILE_DIR=/var/cache/FDO all
> # make PROFILE_GEN=YesPlease PROFILE_DIR=/var/cache/FDO install

Err, that last line should have been:

# make PROFILE_USE=YesPlease PROFILE_DIR=/var/cache/FDO install

of course...

					- Ted

^ permalink raw reply

* Re: [msysGit] Breakage in master?
From: Torsten Bögershausen @ 2012-02-02 20:15 UTC (permalink / raw)
  To: Johannes Schindelin
  Cc: Erik Faye-Lund, Git Mailing List, msysGit,
	Ævar Arnfjörð Bjarmason
In-Reply-To: <alpine.DEB.1.00.1202021956370.1249@s15462909.onlinehome-server.info>

On 02.02.12 19:57, Johannes Schindelin wrote:
> Hi Erik,
> 
> On Thu, 2 Feb 2012, Erik Faye-Lund wrote:
> 
>> Something strange is going on in Junio's current 'master' branch
>> (f3fb075). "git show" has started to error out on Windows with a
>> complaint about our vsnprintf:
>> ---8<---
>>
>> $ git show
>> commit f3fb07509c2e0b21b12a598fcd0a19a92fc38a9d
>> Author: Junio C Hamano <gitster@pobox.com>
>> Date:   Tue Jan 31 22:31:35 2012 -0800
>>
>>     Update draft release notes to 1.7.10
>>
>>     Signed-off-by: Junio C Hamano <gitster@pobox.com>
>>
>> fatal: BUG: your vsnprintf is broken (returned -1)
>> ---8<---
>>
>> [...]
>>
>> I'm at a loss here. Does anyone have a hunch about what's going on?
> 
> It very much reminds me of 6ef404095bc1162031fc3cb43430b512e975bc6a...
> 
> Is it possible that NO_GETTEXT is either not set, or ignored?
> 
> Ciao,
> Dscho
Hi,
same problem here, tested on Win XP.

Good news (?):
Setting 
NO_GETTEXT=YesPlease
in the Makefile makes the problem go away for me.
/Torsten

^ permalink raw reply

* Re: [PATCH] t0300-credentials: Word around a solaris /bin/sh bug
From: Jonathan Nieder @ 2012-02-02 20:16 UTC (permalink / raw)
  To: Ben Walton; +Cc: git, gitster, Jeff King
In-Reply-To: <1328211135-25217-1-git-send-email-bwalton@artsci.utoronto.ca>

Ben Walton wrote:

> --- a/t/t0300-credentials.sh
> +++ b/t/t0300-credentials.sh
> @@ -8,10 +8,12 @@ test_expect_success 'setup helper scripts' '
>  	cat >dump <<-\EOF &&
>  	whoami=`echo $0 | sed s/.*git-credential-//`
>  	echo >&2 "$whoami: $*"
> +	OIFS=$IFS
>  	while IFS== read key value; do
>  		echo >&2 "$whoami: $key=$value"
>  		eval "$key=$value"
>  	done
> +	IFS=$OIFS

Oh, good catch.  Technically "read" is not a special builtin so POSIX shells
are not supposed to do this (and Jeff's patch definitely looks right), but in
any case temporary variable settings while running a builtin are close
enough to the assignment-during-special-builtin-or-function case to
make me shiver a little. ;-)

Would something like

	(
		IFS==
		while read key value
		do
			...
		done
	)

make sense?

^ permalink raw reply

* [PATCH v4 0/4] completion: couple of cleanups
From: Felipe Contreras @ 2012-02-02 20:30 UTC (permalink / raw)
  To: git
  Cc: Junio C Hamano, SZEDER Gábor, Jonathan Nieder, Thomas Rast,
	Felipe Contreras

And an improvement for zsh.

v4:

Same as v3, but with an improved commit message with input from Thomas Rast and sending directly to Junio.

v3:

Junio: I see you already picked most of them for 'pu', but I've made further changes based on the feedback:

 * completion: be nicer with zsh
       Improved the code-style

 * completion: simplify __gitcomp*
       Fix
       Improved commit message

Felipe Contreras (4):
  completion: work around zsh option propagation bug
  completion: simplify __git_remotes
  completion: remove unused code
  completion: simplify __gitcomp*

 contrib/completion/git-completion.bash |   66 +++++---------------------------
 1 files changed, 10 insertions(+), 56 deletions(-)

-- 
1.7.9

^ permalink raw reply

* Re: How best to handle multiple-authorship commits in GIT?
From: David Howells @ 2012-02-02 20:33 UTC (permalink / raw)
  To: Frans Klaver; +Cc: dhowells, git, valerie.aurora
In-Reply-To: <CAH6sp9P8ehXoC075dcK9ni5rJBV9iCZmLHTBr-UR+-jbD3c6Ww@mail.gmail.com>

Frans Klaver <fransklaver@gmail.com> wrote:

> I always thought of the author field as being an indication of who is
> ultimately responsible for its implementation (the one in the
> pestering spot).

Define 'ultimate responsibility for an implementation'.  I'm further developing
patches that Val has (at least partially) implemented.  By Val's admission some
of the patches she further developed beyond what Jan Blunck had implemented.
The chain may extend further.  To that end all three of us are authors of some
of the patches.

However, if you meant 'maintenance' rather than 'implementation', then, yes,
that would be me (for the moment at least).  But if that's the case, then
shouldn't it be 'Maintainer' and not 'Author'?  And, besides, that's what the
MAINTAINERS file is for.

> (1) may seem desirous, but doesn't (2) seem like a cleaner and more
> maintainable solution?

No.  I would say that properly supporting multiple authors in the commit object
is the cleaner solution.  It's not the *easier* solution, however, and would
require an upgrade to the version of GIT used to parse these commits.  That
I'll grant you.

> Gitweb will show the entire log message if people are interested in the
> exact change, right?

But if I say to Gitweb "show me the patches authored by Val" it will *not* turn
up these patches, and in that way will deny Val credit.  Yes, you can see that
Val altered that patch if you look at that patch directly - but you have to
know where to go and look, in which case you already know or suspect that Val
is credited with patches in that area.

So to make (2) work, Gitweb needs to search for the additional authoring fields
when asked to credit people with the patches they've worked on.

Similarly gitk and possibly other tools would need to do the same.

*That* would be fine by me, I suppose.  I don't think it's the correct way to
do it, but it might be the logical way since this wasn't build in from the
beginning - and the main thing would be to turn up the prior or joint
authorship to author-based searches.

David

^ permalink raw reply

* Re: [RFC PATCH] gitweb: use CGI with -utf8
From: Michał Kiedrowicz @ 2012-02-02 20:38 UTC (permalink / raw)
  To: Jakub Narebski; +Cc: git
In-Reply-To: <m37h05c8c1.fsf@localhost.localdomain>

Jakub Narebski <jnareb@gmail.com> wrote:

> Michał Kiedrowicz <michal.kiedrowicz@gmail.com> writes:
> 
> > I noticed that gitweb tries a lot to properly process UTF-8 data, for
> > example it prints my name correctly in log and commit information, but
> > it echos junk in the search field. It looks like:
> > 
> > 	Michał Kiedrowicz
> > 
> > I don't know CGI well and I never touched gitewb code, but I found this
> > on http://www.lemoda.net/cgi/perl-unicode/index.html:
> > 
> > 	use CGI '-utf8';
> > 	my $value = params ('input');
> > 
> > I tried it and that fixed my problem. I'm not sure about the
> > consequences, maybe someone more experienced in CGI might help?
> 
> I have reworded this to form a proper commit message (see
> Documentation/SubmittingPatches) and I'll resend this as a reply to
> this email.

Thanks, your message is much better.

> 
> > ---
> >  gitweb/gitweb.perl |    2 +-
> >  1 files changed, 1 insertions(+), 1 deletions(-)
> > 
> > diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
> > index abb5a79..74d45b1 100755
> > --- a/gitweb/gitweb.perl
> > +++ b/gitweb/gitweb.perl
> > @@ -10,7 +10,7 @@
> >  use 5.008;
> >  use strict;
> >  use warnings;
> > -use CGI qw(:standard :escapeHTML -nosticky);
> > +use CGI qw(:standard :escapeHTML -nosticky -utf8);
> >  use CGI::Util qw(unescape);
> >  use CGI::Carp qw(fatalsToBrowser set_message);
> >  use Encode;
> > -- 
> 
> Does this actually work for you? 

Yes. It correctly displays "ł" in the search form.

>  Because it doesn't work for me
> (perhaps I have too old CGI module: what CGI.pm and what Perl version
> do you use?).
> 

$ perl --version

This is perl 5, version 12, subversion 4 (v5.12.4) built for x86_64-linux
(with 12 registered patches, see perl -V for more detail)

$ eix -e CGI -c
[I] perl-core/CGI (3.510@01.02.2012): Simple Common Gateway Interface Class

> See other solution to this in other reply to this email.
> 

^ permalink raw reply

* Re: [PATCH/RFC (version A)] gitweb: use CGI with -utf8 to process Unicode query  parameters correctly
From: Michał Kiedrowicz @ 2012-02-02 20:43 UTC (permalink / raw)
  To: Jakub Narebski; +Cc: git
In-Reply-To: <201202022108.51353.jnareb@gmail.com>

Jakub Narebski <jnareb@gmail.com> wrote:

> Gitweb tries hard to properly process UTF-8 data, by marking output
> from git commands and contents of files as UTF-8 with to_utf8()
> subroutine.  This ensures that gitweb would print correctly UTF-8
> e.g. in 'log' and 'commit' views.
> 
> Unfortunately it misses another source of potentially Unicode input,
> namely query parameters.  The result is that one cannot search for a
> string containing characters outside US-ASCII.  For example searching
> for "Michał Kiedrowicz" (containing letter 'ł' - LATIN SMALL LETTER L
> WITH STROKE, with Unicode codepoint U+0142, represented with 0xc5 0x82
> bytes in UTF-8 and percent-encoded as %C5%81) result in the following
> incorrect data in search field
> 
> 	Michał Kiedrowicz
> 
> This is caused by CGI by default treating '0xc5 0x82' bytes as two
> characters in Perl legacy encoding latin-1 (iso-8859-1), because 's'
> query parameter is not processed explicitly as UTF-8 encoded string.
> 
> According to "Using Unicode in a Perl CGI script" article on
> http://www.lemoda.net/cgi/perl-unicode/index.html the simplest
> solution is to just import '-utf8' pragma for CGI module:
> 
> 	use CGI '-utf8';
> 	my $value = params('input');
> 
> According to CGI module documentation, the '-utf8' pragma may cause
> problems with POST requests containing binary files... but gitweb
> currently do not use POST requests at all, so this should be not a
> problem now.

This was exactly my feeling  when I sent this patch :).

> 
> Alternate solution would be to explicity decode query parameters when
> storing them in %input_params (and perhaps also path_info).
> 
> [jn: reworded / rewritten commit message]
> 
> Signed-off-by: Michał Kiedrowicz <michal.kiedrowicz@gmail.com>

Thanks, I forgot about that.

> Signed-off-by: Jakub Narębski <jnareb@gmail.com>
> ---
>  gitweb/gitweb.perl |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
> index 9cf7e71..a7441ef 100755
> --- a/gitweb/gitweb.perl
> +++ b/gitweb/gitweb.perl
> @@ -10,7 +10,7 @@
>  use 5.008;
>  use strict;
>  use warnings;
> -use CGI qw(:standard :escapeHTML -nosticky);
> +use CGI qw(:standard :escapeHTML -nosticky -utf8);
>  use CGI::Util qw(unescape);
>  use CGI::Carp qw(fatalsToBrowser set_message);
>  use Encode;

^ permalink raw reply

* Re: [PATCH] t0300-credentials: Word around a solaris /bin/sh bug
From: Matthieu Moy @ 2012-02-02 20:43 UTC (permalink / raw)
  To: Jonathan Nieder; +Cc: Ben Walton, git, gitster, Jeff King
In-Reply-To: <20120202201629.GA20200@burratino>

Jonathan Nieder <jrnieder@gmail.com> writes:

> Would something like
>
> 	(
> 		IFS==
> 		while read key value
> 		do
> 			...
> 		done
> 	)
>
> make sense?

I don't think so since the "..." contains

    eval "$key=$value"

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/

^ permalink raw reply

* Re: [msysGit] Breakage in master?
From: Johannes Sixt @ 2012-02-02 20:45 UTC (permalink / raw)
  To: kusmabite
  Cc: Git Mailing List, msysGit, Ævar Arnfjörð Bjarmason
In-Reply-To: <CABPQNSbWu0r_gKGvCHk567pUtQiyDOCO8vFfrzPMFW1eUaj1nw@mail.gmail.com>

Am 02.02.2012 13:14, schrieb Erik Faye-Lund:
> Something strange is going on in Junio's current 'master' branch
> (f3fb075). "git show" has started to error out on Windows with a
> complaint about our vsnprintf:
> ---8<---
> 
> $ git show
> commit f3fb07509c2e0b21b12a598fcd0a19a92fc38a9d
> Author: Junio C Hamano <gitster@pobox.com>
> Date:   Tue Jan 31 22:31:35 2012 -0800
> 
>     Update draft release notes to 1.7.10
> 
>     Signed-off-by: Junio C Hamano <gitster@pobox.com>
> 
> fatal: BUG: your vsnprintf is broken (returned -1)

FWIW, I run a version of master on Windows with almost no additional
patches that touch compat/ (the few changes I do have are unsuspicious).
But I do not observe this behavior. I build with NO_GETTEXT and -O0.

-- Hannes

^ permalink raw reply

* Re: [PATCH v3 1/4] completion: be nicer with zsh
From: Felipe Contreras @ 2012-02-02 20:45 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Jonathan Nieder, git, SZEDER Gábor
In-Reply-To: <7v8vklvxwh.fsf@alter.siamese.dyndns.org>

On Thu, Feb 2, 2012 at 9:27 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Jonathan Nieder <jrnieder@gmail.com> writes:
>> However, clearly I did not say it clearly enough. :)  I guess it's
>> better to take a cue from storytellers and show rather than tell.
>
> Very big thanks for this ;-)

Not a single comment regarding what I said? I don't see what's not
sensible about a commit message with this order:

 1) short description of the *purpose* of the patch
 2) Summary of the problem.
 3) Proposed solution
 4) Extra information, useful for future references, etc.

This ensures the most relevant information for most people is at the top.

-- 
Felipe Contreras

^ permalink raw reply

* Re: [PATCH/RFC (version B)] gitweb: Allow UTF-8 encoded CGI query parameters and  path_info
From: Michał Kiedrowicz @ 2012-02-02 20:46 UTC (permalink / raw)
  To: Jakub Narebski; +Cc: git
In-Reply-To: <201202022110.07127.jnareb@gmail.com>

Jakub Narebski <jnareb@gmail.com> wrote:

> Gitweb tries hard to properly process UTF-8 data, by marking output
> from git commands and contents of files as UTF-8 with to_utf8()
> subroutine.  This ensures that gitweb would print correctly UTF-8
> e.g. in 'log' and 'commit' views.
> 
> Unfortunately it misses another source of potentially Unicode input,
> namely query parameters.  The result is that one cannot search for a
> string containing characters outside US-ASCII.  For example searching
> for "Michał Kiedrowicz" (containing letter 'ł' - LATIN SMALL LETTER L
> WITH STROKE, with Unicode codepoint U+0142, represented with 0xc5 0x82
> bytes in UTF-8 and percent-encoded as %C5%81) result in the following
> incorrect data in search field
> 
> 	Michał Kiedrowicz
> 
> This is caused by CGI by default treating '0xc5 0x82' bytes as two
> characters in Perl legacy encoding latin-1 (iso-8859-1), because 's'
> query parameter is not processed explicitly as UTF-8 encoded string.
> 
> The solution used here follows "Using Unicode in a Perl CGI script"
> article on http://www.lemoda.net/cgi/perl-unicode/index.html:
> 
> 	use CGI;
> 	use Encode 'decode_utf8;
> 	my $value = params('input');
> 	$value = decode_utf8($value);
> 
> This is done when filling %input_params hash; this required to move
> from explicit $cgi->param(<label>) to $input_params{<name>} in a few
> places.

I'm sorry but this doesn't work for me. I would be happy to help if you
have some questions about it.

> 
> Alternate solution would be to simply use the '-utf8' pragma (via
> "use CGI '-utf8';"), but according to CGI.pm documentation it may
> cause problems with POST requests containing binary files... and
> it doesn't work with old CGI.pm version 3.10 from Perl v5.8.6.
> 
> Noticed-by: Michał Kiedrowicz <michal.kiedrowicz@gmail.com>
> Signed-off-by: Jakub Narębski <jnareb@gmail.com>
> ---
>  gitweb/gitweb.perl |   12 ++++++------
>  1 files changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
> index 9cf7e71..55b2c24 100755
> --- a/gitweb/gitweb.perl
> +++ b/gitweb/gitweb.perl
> @@ -52,7 +52,7 @@ sub evaluate_uri {
>  	# as base URL.
>  	# Therefore, if we needed to strip PATH_INFO, then we know that we have
>  	# to build the base URL ourselves:
> -	our $path_info = $ENV{"PATH_INFO"};
> +	our $path_info = decode_utf8($ENV{"PATH_INFO"});
>  	if ($path_info) {
>  		if ($my_url =~ s,\Q$path_info\E$,, &&
>  		    $my_uri =~ s,\Q$path_info\E$,, &&
> @@ -816,9 +816,9 @@ sub evaluate_query_params {
>  
>  	while (my ($name, $symbol) = each %cgi_param_mapping) {
>  		if ($symbol eq 'opt') {
> -			$input_params{$name} = [ $cgi->param($symbol) ];
> +			$input_params{$name} = [ map { decode_utf8($_) } $cgi->param($symbol) ];
>  		} else {
> -			$input_params{$name} = $cgi->param($symbol);
> +			$input_params{$name} = decode_utf8($cgi->param($symbol));
>  		}
>  	}
>  }
> @@ -2767,7 +2767,7 @@ sub git_populate_project_tagcloud {
>  	}
>  
>  	my $cloud;
> -	my $matched = $cgi->param('by_tag');
> +	my $matched = $input_params{'ctag'};
>  	if (eval { require HTML::TagCloud; 1; }) {
>  		$cloud = HTML::TagCloud->new;
>  		foreach my $ctag (sort keys %ctags_lc) {
> @@ -5282,7 +5282,7 @@ sub git_project_list_body {
>  
>  	my $check_forks = gitweb_check_feature('forks');
>  	my $show_ctags  = gitweb_check_feature('ctags');
> -	my $tagfilter = $show_ctags ? $cgi->param('by_tag') : undef;
> +	my $tagfilter = $show_ctags ? $input_params{'ctag'} : undef;
>  	$check_forks = undef
>  		if ($tagfilter || $searchtext);
>  
> @@ -6197,7 +6197,7 @@ sub git_tag {
>  
>  sub git_blame_common {
>  	my $format = shift || 'porcelain';
> -	if ($format eq 'porcelain' && $cgi->param('js')) {
> +	if ($format eq 'porcelain' && $input_params{'javascript'}) {
>  		$format = 'incremental';
>  		$action = 'blame_incremental'; # for page title etc
>  	}

^ permalink raw reply

* Re: [ANNOUNCE] Git 1.7.9
From: Jakub Narebski @ 2012-02-02 20:50 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, Linux Kernel
In-Reply-To: <7vipjwzvc2.fsf@alter.siamese.dyndns.org>

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

> The latest feature release Git 1.7.9 is now available at the usual
> places.
> 
> The release tarballs are found at:
> 
>     http://code.google.com/p/git-core/downloads/list
> 
> and their SHA-1 checksums are:
> 
> ed51ef5ef250daaa6e98515cf2641820cd268d4c  git-1.7.9.tar.gz
> c7b1fa20dc501beb2cb5091dd24dbfd2a0013a0c  git-htmldocs-1.7.9.tar.gz
> 1ca1fc430b2814f9e9cf82ec3bf7f2eaf5209b7a  git-manpages-1.7.9.tar.gz

When trying to rebuild RPM out of tarball with

  $ rpmbuild -tb git-1.7.9.tar.gz

I get the following error at the end of build phase:

 RPM build errors:
     Installed (but unpackaged) file(s) found:
    /usr/share/locale/is/LC_MESSAGES/git.mo

Shouldn't this file be put in git package, or in separate 
git-i18n-is / git-i18n-Icelandic RPM package?

-- 
Jakub Narebski

^ permalink raw reply

* Re: [PATCH/RFC (version B)] gitweb: Allow UTF-8 encoded CGI query parameters and  path_info
From: Jakub Narebski @ 2012-02-02 21:07 UTC (permalink / raw)
  To: Michał Kiedrowicz; +Cc: git
In-Reply-To: <20120202214646.1b84f23e@gmail.com>

On Thu, 2 Jan 2012, Michał Kiedrowicz wrote:
> Jakub Narebski <jnareb@gmail.com> wrote:
> 
> > Gitweb tries hard to properly process UTF-8 data, by marking output
> > from git commands and contents of files as UTF-8 with to_utf8()
> > subroutine.  This ensures that gitweb would print correctly UTF-8
> > e.g. in 'log' and 'commit' views.
> > 
> > Unfortunately it misses another source of potentially Unicode input,
> > namely query parameters.  The result is that one cannot search for a
> > string containing characters outside US-ASCII.  For example searching
> > for "Michał Kiedrowicz" (containing letter 'ł' - LATIN SMALL LETTER L
> > WITH STROKE, with Unicode codepoint U+0142, represented with 0xc5 0x82
> > bytes in UTF-8 and percent-encoded as %C5%81) result in the following
> > incorrect data in search field
> > 
> > 	Michał Kiedrowicz
> > 
> > This is caused by CGI by default treating '0xc5 0x82' bytes as two
> > characters in Perl legacy encoding latin-1 (iso-8859-1), because 's'
> > query parameter is not processed explicitly as UTF-8 encoded string.
> > 
> > The solution used here follows "Using Unicode in a Perl CGI script"
> > article on http://www.lemoda.net/cgi/perl-unicode/index.html:
> > 
> > 	use CGI;
> > 	use Encode 'decode_utf8;
> > 	my $value = params('input');
> > 	$value = decode_utf8($value);
> > 
> > This is done when filling %input_params hash; this required to move
> > from explicit $cgi->param(<label>) to $input_params{<name>} in a few
> > places.
> 
> I'm sorry but this doesn't work for me. I would be happy to help if you
> have some questions about it.

Strange.  http://www.lemoda.net/cgi/perl-unicode/index.html says that
those two approaches should be equivalent.  The -utf8 pragma version
doesn't work for me at all, while this one works in that if finds what
it is supposed to, but shows garbage in search form.

Will investigate.
 
> > Alternate solution would be to simply use the '-utf8' pragma (via
> > "use CGI '-utf8';"), but according to CGI.pm documentation it may
> > cause problems with POST requests containing binary files... and
> > it doesn't work with old CGI.pm version 3.10 from Perl v5.8.6.

[...]
> > @@ -816,9 +816,9 @@ sub evaluate_query_params {
> >  
> >  	while (my ($name, $symbol) = each %cgi_param_mapping) {
> >  		if ($symbol eq 'opt') {
> > -			$input_params{$name} = [ $cgi->param($symbol) ];
> > +			$input_params{$name} = [ map { decode_utf8($_) } $cgi->param($symbol) ];
> >  		} else {
> > -			$input_params{$name} = $cgi->param($symbol);
> > +			$input_params{$name} = decode_utf8($cgi->param($symbol));
> >  		}
> >  	}
> >  }
-- 
Jakub Narebski
Poland

^ permalink raw reply

* Re: [PATCH] t0300-credentials: Word around a solaris /bin/sh bug
From: Jonathan Nieder @ 2012-02-02 21:11 UTC (permalink / raw)
  To: Matthieu Moy; +Cc: Ben Walton, git, gitster, Jeff King
In-Reply-To: <vpq62fp3r15.fsf@bauges.imag.fr>

Matthieu Moy wrote:
> Jonathan Nieder <jrnieder@gmail.com> writes:

>> 	(
>> 		IFS==
>> 		while read key value
>> 		do
>> 			...
>> 		done
>> 	)
[...]
> I don't think so since the "..." contains
>
>     eval "$key=$value"

Oh, whoops.  Thanks for noticing.

Here's an updated patch, for amusement value.  No functional change
intended.  I don't think it's actually worth applying unless people
actively working on this file find the result easier to work with.

-- >8 --
Subject: t0300 (credentials): shell scripting style cleanups

As Ben noticed, the helper used by this test script assigns a
temporary value to IFS while calling the "read" builtin, which in
ancient shells causes the value to leak into the environment and
affect later code in the same script.  Explicitly save and restore IFS
to avoid rekindling old memories.

While at it, put the "do" associated to a "while" statement on its own
line to match the house style and define helper scripts in the test
data section above all test assertions so the "setup" test itself is
less cluttered and we can worry a little less about quoting issues.

Inspired-by: Ben Walton <bwalton@artsci.utoronto.ca>
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
 t/t0300-credentials.sh |   36 ++++++++++++++++++++----------------
 1 files changed, 20 insertions(+), 16 deletions(-)

diff --git a/t/t0300-credentials.sh b/t/t0300-credentials.sh
index edf65478..780d5dcb 100755
--- a/t/t0300-credentials.sh
+++ b/t/t0300-credentials.sh
@@ -4,33 +4,37 @@ test_description='basic credential helper tests'
 . ./test-lib.sh
 . "$TEST_DIRECTORY"/lib-credential.sh
 
-test_expect_success 'setup helper scripts' '
-	cat >dump <<-\EOF &&
+cat >dump <<-\EOF
 	whoami=`echo $0 | sed s/.*git-credential-//`
 	echo >&2 "$whoami: $*"
-	while IFS== read key value; do
+	save_IFS=$IFS
+	IFS==
+	while read key value
+	do
 		echo >&2 "$whoami: $key=$value"
 		eval "$key=$value"
 	done
-	EOF
+	IFS=$save_IFS
+EOF
 
-	cat >git-credential-useless <<-EOF &&
+cat >git-credential-useless <<-EOF
 	#!$SHELL_PATH
 	. ./dump
 	exit 0
-	EOF
+EOF
+
+cat >git-credential-verbatim <<-EOF
+	#!$SHELL_PATH
+	user=\$1; shift
+	pass=\$1; shift
+	. ./dump
+	test -z "\$user" || echo username=\$user
+	test -z "\$pass" || echo password=\$pass
+EOF
+
+test_expect_success setup '
 	chmod +x git-credential-useless &&
-
-	echo "#!$SHELL_PATH" >git-credential-verbatim &&
-	cat >>git-credential-verbatim <<-\EOF &&
-	user=$1; shift
-	pass=$1; shift
-	. ./dump
-	test -z "$user" || echo username=$user
-	test -z "$pass" || echo password=$pass
-	EOF
 	chmod +x git-credential-verbatim &&
-
 	PATH="$PWD:$PATH"
 '
 
-- 
1.7.9

^ permalink raw reply related

* Re: I18N.pm is incompatible with perl < 5.8.3
From: Tom G. Christensen @ 2012-02-02 21:23 UTC (permalink / raw)
  To: Ævar Arnfjörð Bjarmason; +Cc: git
In-Reply-To: <CACBZZX5YuN8vpmTiP_38Aa=c3KDqEHCKBX3DE2YKkeMCdR4GBg@mail.gmail.com>

Ævar Arnfjörð Bjarmason wrote:
> On Thu, Feb 2, 2012 at 14:11, Tom G. Christensen
> <tgc@statsbiblioteket.dk> wrote:
>> Hello,
>>
> 
> Thanks Tom, I'll submit a patch for that. Does this work, i.e. does
> 5.8.3 need *{import} = instead of *import = ?
> 
<snip>
I applied the patch and have run the test with perl 5.8.0 (el3) and 
5.8.5 (el4) and it passes the test on both versions.

I noticed an additional problem which is that Git/I18N.pm is not 
installed when NO_PERL_MAKEMAKER is used.
It looks like perl/Makefile did not get updated when Git/I18N.pm was 
added to perl/Makefile.PL.

In perl 5.8.0 ExtUtils::MakeMaker is too old (6.03) to be used with 
perl/Makefile.PL.

-tgc

^ permalink raw reply

* [RFC/PATCH 0/2] Commits with ancient timestamps
From: Junio C Hamano @ 2012-02-02 21:41 UTC (permalink / raw)
  To: git

Result of conversion of ancient history from other SCMs, and output from
other third-party tools, can record timestamps that predates inception of
Git. They can cause "git am", "git rebase" and "git commit --amend" to
misbehave, because the raw git timestamp e.g.

	author <a.u.thor@example.com> 1328214896 -0800

are read from the commit object and passed to parse_date() machinery,
which rejects raw git timestamps without sufficient number of digits to
avoid misinterpreting human-written timestamp in other formats, and
timestamps before 1975 do not have enough number of digits in them.

Here is a two-patch series that may improve the situation.

Note that this is not meant for direct application, unless somebody uses
this extensively to make sure there is no regression.


Junio C Hamano (2):
  parse_date(): allow ancient timestamps in in-header format
  parse_date(): '@' prefix forces git-timestamp interpretation

 date.c          |   30 ++++++++++++++++++++++++++++++
 git-sh-setup.sh |    2 +-
 2 files changed, 31 insertions(+), 1 deletions(-)

-- 
1.7.9.172.ge26ae

^ permalink raw reply

* [RFC/PATCH 1/2] parse_date(): allow ancient git-timestamp
From: Junio C Hamano @ 2012-02-02 21:41 UTC (permalink / raw)
  To: git
In-Reply-To: <1328218903-5681-1-git-send-email-gitster@pobox.com>

The date-time parser parses out a human-readble datestring piece by
piece, so that it could even parse a string in a rather strange
notation like 'noon november 11, 2005', but restricts itself from
parsing strings in "<seconds since epoch> <timezone>" format only
for reasonably new timestamps (like 1974 or newer) with 10 or more
digits. This is to prevent a string like "20100917" from getting
interpreted as seconds since epoch (we want to treat it as September
17, 2010 instead) while doing so.

The same codepath is used to read back the timestamp that we have
already recorded in the headers of commit and tag objects; because
of this, such a commit with timestamp "0 +0000" cannot be rebased or
amended very easily.

Teach parse_date() codepath to special case a string of the form
"<digits> +<4-digits>" to work this issue around, but require that
there is no other cruft around the string when parsing a timestamp
of this format for safety.

Note that this has a slight backward incompatibility implications.

If somebody writes "git commit --date='20100917 +0900'" and wants it
to mean a timestamp in September 2010 in Japan, this change will
break such a use case.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 date.c |   29 +++++++++++++++++++++++++++++
 1 files changed, 29 insertions(+), 0 deletions(-)

diff --git a/date.c b/date.c
index 896fbb4..c212946 100644
--- a/date.c
+++ b/date.c
@@ -585,6 +585,33 @@ static int date_string(unsigned long date, int offset, char *buf, int len)
 	return snprintf(buf, len, "%lu %c%02d%02d", date, sign, offset / 60, offset % 60);
 }
 
+/*
+ * Parse a string like "0 +0000" as ancient timestamp near epoch, but
+ * only when it appears not as part of any other string.
+ */
+static int match_object_header_date(const char *date, unsigned long *timestamp, int *offset)
+{
+	char *end;
+	unsigned long stamp;
+	int ofs;
+
+	if (*date < '0' || '9' <= *date)
+		return -1;
+	stamp = strtoul(date, &end, 10);
+	if (*end != ' ' || stamp == ULONG_MAX || (end[1] != '+' && end[1] != '-'))
+		return -1;
+	date = end + 2;
+	ofs = strtol(date, &end, 10);
+	if ((*end != '\0' && (*end != '\n')) || end != date + 4)
+		return -1;
+	ofs = (ofs / 100) * 60 + (ofs % 100);
+	if (date[-1] == '-')
+		ofs = -ofs;
+	*timestamp = stamp;
+	*offset = ofs;
+	return 0;
+}
+
 /* Gr. strptime is crap for this; it doesn't have a way to require RFC2822
    (i.e. English) day/month names, and it doesn't work correctly with %z. */
 int parse_date_basic(const char *date, unsigned long *timestamp, int *offset)
@@ -610,6 +637,8 @@ int parse_date_basic(const char *date, unsigned long *timestamp, int *offset)
 	*offset = -1;
 	tm_gmt = 0;
 
+	if (!match_object_header_date(date, timestamp, offset))
+		return 0; /* success */
 	for (;;) {
 		int match = 0;
 		unsigned char c = *date;
-- 
1.7.9.172.ge26ae

^ permalink raw reply related

* [RFC/PATCH 2/2] parse_date(): '@' prefix forces git-timestamp
From: Junio C Hamano @ 2012-02-02 21:41 UTC (permalink / raw)
  To: git
In-Reply-To: <1328218903-5681-1-git-send-email-gitster@pobox.com>

The only place that the issue this series addresses was observed
where we read "cat-file commit" output and put it in GIT_AUTHOR_DATE
in order to replay a commit with an ancient timestamp.

With the previous patch alone, "git commit --date='20100917 +0900'"
can be misinterpreted to mean an ancient timestamp, not September in
year 2010.  Guard this codepath by requring an extra '@' in front of
the raw git timestamp on the parsing side. This of course needs to
be compensated by updating get_author_ident_from_commit and the code
for "git commit --amend" to prepend '@' to the string read from the
existing commit in the GIT_AUTHOR_DATE environment variable.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 builtin/commit.c  |    6 ++++++
 date.c            |    3 ++-
 git-sh-setup.sh   |    2 +-
 t/t3400-rebase.sh |   23 +++++++++++++++++++++++
 4 files changed, 32 insertions(+), 2 deletions(-)

diff --git a/builtin/commit.c b/builtin/commit.c
index cbc9613..bcb0db2 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -534,6 +534,7 @@ static void determine_author_info(struct strbuf *author_ident)
 
 	if (author_message) {
 		const char *a, *lb, *rb, *eol;
+		size_t len;
 
 		a = strstr(author_message_buffer, "\nauthor ");
 		if (!a)
@@ -554,6 +555,11 @@ static void determine_author_info(struct strbuf *author_ident)
 					 (a + strlen("\nauthor "))));
 		email = xmemdupz(lb + strlen("<"), rb - (lb + strlen("<")));
 		date = xmemdupz(rb + strlen("> "), eol - (rb + strlen("> ")));
+		len = eol - (rb + strlen("> "));
+		date = xmalloc(len + 2);
+		*date = '@';
+		memcpy(date + 1, rb + strlen("> "), len);
+		date[len + 1] = '\0';
 	}
 
 	if (force_author) {
diff --git a/date.c b/date.c
index c212946..ca60767 100644
--- a/date.c
+++ b/date.c
@@ -637,7 +637,8 @@ int parse_date_basic(const char *date, unsigned long *timestamp, int *offset)
 	*offset = -1;
 	tm_gmt = 0;
 
-	if (!match_object_header_date(date, timestamp, offset))
+	if (*date == '@' &&
+	    !match_object_header_date(date + 1, timestamp, offset))
 		return 0; /* success */
 	for (;;) {
 		int match = 0;
diff --git a/git-sh-setup.sh b/git-sh-setup.sh
index 8e427da..015fe6e 100644
--- a/git-sh-setup.sh
+++ b/git-sh-setup.sh
@@ -200,7 +200,7 @@ get_author_ident_from_commit () {
 		s/.*/GIT_AUTHOR_EMAIL='\''&'\''/p
 
 		g
-		s/^author [^<]* <[^>]*> \(.*\)$/\1/
+		s/^author [^<]* <[^>]*> \(.*\)$/@\1/
 		s/.*/GIT_AUTHOR_DATE='\''&'\''/p
 
 		q
diff --git a/t/t3400-rebase.sh b/t/t3400-rebase.sh
index 6eaecec..e26e14d 100755
--- a/t/t3400-rebase.sh
+++ b/t/t3400-rebase.sh
@@ -218,4 +218,27 @@ test_expect_success 'rebase -m can copy notes' '
 	test "a note" = "$(git notes show HEAD)"
 '
 
+test_expect_success 'rebase commit with an ancient timestamp' '
+	git reset --hard &&
+
+	>old.one && git add old.one && test_tick &&
+	git commit --date="@12345 +0400" -m "Old one" &&
+	>old.two && git add old.two && test_tick &&
+	git commit --date="@23456 +0500" -m "Old two" &&
+	>old.three && git add old.three && test_tick &&
+	git commit --date="@34567 +0600" -m "Old three" &&
+
+	git cat-file commit HEAD^^ >actual &&
+	grep "author .* 12345 +0400$" actual &&
+	git cat-file commit HEAD^ >actual &&
+	grep "author .* 23456 +0500$" actual &&
+	git cat-file commit HEAD >actual &&
+	grep "author .* 34567 +0600$" actual &&
+
+	git rebase --onto HEAD^^ HEAD^ &&
+
+	git cat-file commit HEAD >actual &&
+	grep "author .* 34567 +0600$" actual
+'
+
 test_done
-- 
1.7.9.172.ge26ae

^ permalink raw reply related

* Re: Alternates corruption issue
From: Jeff King @ 2012-02-02 21:59 UTC (permalink / raw)
  To: Jonathan Nieder
  Cc: Junio C Hamano, Richard Purdie, GIT Mailing-list, Hart, Darren,
	Ashfield, Bruce
In-Reply-To: <20120131222258.GG13252@burratino>

On Tue, Jan 31, 2012 at 04:22:58PM -0600, Jonathan Nieder wrote:

> Anyway, thanks for explaining.  Hopefully I can get to this soon and
> factor out a common function for get_repo_path and enter_repo to call
> so playing with the ordering becomes a little less scary. ;-)

So here's what I think we should apply to fix the particular issue that
Richard mentioned at the start of this thread.

Besides tweaking the ordering, the main contribution is a set of tests
that actually check some of these ambiguous cases (especially checking
the fact that both code paths behave identically!). I didn't factor the
logic into a common function, but doing so should be a little safer on
top of these tests, if you're still interested.

-- >8 --
Subject: standardize and improve lookup rules for external local repos

When you specify a local repository on the command line of
clone, ls-remote, upload-pack, receive-pack, or upload-archive,
or in a request to git-daemon, we perform a little bit of
lookup magic, doing things like looking in working trees for
.git directories and appending ".git" for bare repos.

For clone, this magic happens in get_repo_path. For
everything else, it happens in enter_repo. In both cases,
there are some ambiguous or confusing cases that aren't
handled well, and there is one case that is not handled the
same by both methods.

This patch tries to provide (and test!) standard, sensible
lookup rules for both code paths. The intended changes are:

  1. When looking up "foo", we have always preferred
     a working tree "foo" (containing "foo/.git" over the
     bare "foo.git". But we did not prefer a bare "foo" over
     "foo.git". With this patch, we do so.

  2. We would select directories that existed but didn't
     actually look like git repositories. With this patch,
     we make sure a selected directory looks like a git
     repo. Not only is this more sensible in general, but it
     will help anybody who is negatively affected by change
     (1) negatively (e.g., if they had "foo.git" next to its
     separate work tree "foo", and expect to keep finding
     "foo.git" when they reference "foo").

  3. The enter_repo code path would, given "foo", look for
     "foo.git/.git" (i.e., do the ".git" append magic even
     for a repo with working tree). The clone code path did
     not; with this patch, they now behave the same.

In the unlikely case of a working tree overlaying a bare
repo (i.e., a ".git" directory _inside_ a bare repo), we
continue to treat it as a working tree (prefering the
"inner" .git over the bare repo). This is mainly because the
combination seems nonsensical, and I'd rather stick with
existing behavior on the off chance that somebody is relying
on it.

Signed-off-by: Jeff King <peff@peff.net>
---
I could go either way with the "prefer foo to foo/.git" thing we've been
discussing. I left it as-is here out of a sense of being conservative.
But I have no objection at all to you doing a patch on top that
justifies the change. You'll just need to tweak the "check" line of the
final test.

 builtin/clone.c           |    4 +-
 cache.h                   |    1 +
 path.c                    |    7 ++-
 setup.c                   |    2 +-
 t/t5900-repo-selection.sh |  100 +++++++++++++++++++++++++++++++++++++++++++++
 5 files changed, 109 insertions(+), 5 deletions(-)
 create mode 100755 t/t5900-repo-selection.sh

diff --git a/builtin/clone.c b/builtin/clone.c
index c62d4b5..637611a 100644
--- a/builtin/clone.c
+++ b/builtin/clone.c
@@ -107,7 +107,7 @@ static const char *argv_submodule[] = {
 
 static char *get_repo_path(const char *repo, int *is_bundle)
 {
-	static char *suffix[] = { "/.git", ".git", "" };
+	static char *suffix[] = { "/.git", "", ".git/.git", ".git" };
 	static char *bundle_suffix[] = { ".bundle", "" };
 	struct stat st;
 	int i;
@@ -117,7 +117,7 @@ static char *get_repo_path(const char *repo, int *is_bundle)
 		path = mkpath("%s%s", repo, suffix[i]);
 		if (stat(path, &st))
 			continue;
-		if (S_ISDIR(st.st_mode)) {
+		if (S_ISDIR(st.st_mode) && is_git_directory(path)) {
 			*is_bundle = 0;
 			return xstrdup(absolute_path(path));
 		} else if (S_ISREG(st.st_mode) && st.st_size > 8) {
diff --git a/cache.h b/cache.h
index 9bd8c2d..ae0396b 100644
--- a/cache.h
+++ b/cache.h
@@ -432,6 +432,7 @@ extern char *git_work_tree_cfg;
 extern int is_inside_work_tree(void);
 extern int have_git_dir(void);
 extern const char *get_git_dir(void);
+extern int is_git_directory(const char *path);
 extern char *get_object_directory(void);
 extern char *get_index_file(void);
 extern char *get_graft_file(void);
diff --git a/path.c b/path.c
index b6f71d1..6f2aa69 100644
--- a/path.c
+++ b/path.c
@@ -293,7 +293,7 @@ const char *enter_repo(const char *path, int strict)
 
 	if (!strict) {
 		static const char *suffix[] = {
-			".git/.git", "/.git", ".git", "", NULL,
+			"/.git", "", ".git/.git", ".git", NULL,
 		};
 		const char *gitfile;
 		int len = strlen(path);
@@ -324,8 +324,11 @@ const char *enter_repo(const char *path, int strict)
 			return NULL;
 		len = strlen(used_path);
 		for (i = 0; suffix[i]; i++) {
+			struct stat st;
 			strcpy(used_path + len, suffix[i]);
-			if (!access(used_path, F_OK)) {
+			if (!stat(used_path, &st) &&
+			    (S_ISREG(st.st_mode) ||
+			    (S_ISDIR(st.st_mode) && is_git_directory(used_path)))) {
 				strcat(validated_path, suffix[i]);
 				break;
 			}
diff --git a/setup.c b/setup.c
index 61c22e6..7a3618f 100644
--- a/setup.c
+++ b/setup.c
@@ -247,7 +247,7 @@ const char **get_pathspec(const char *prefix, const char **pathspec)
  *    a proper "ref:", or a regular file HEAD that has a properly
  *    formatted sha1 object name.
  */
-static int is_git_directory(const char *suspect)
+int is_git_directory(const char *suspect)
 {
 	char path[PATH_MAX];
 	size_t len = strlen(suspect);
diff --git a/t/t5900-repo-selection.sh b/t/t5900-repo-selection.sh
new file mode 100755
index 0000000..3d5b418
--- /dev/null
+++ b/t/t5900-repo-selection.sh
@@ -0,0 +1,100 @@
+#!/bin/sh
+
+test_description='selecting remote repo in ambiguous cases'
+. ./test-lib.sh
+
+reset() {
+	rm -rf foo foo.git fetch clone
+}
+
+make_tree() {
+	git init "$1" &&
+	(cd "$1" && test_commit "$1")
+}
+
+make_bare() {
+	git init --bare "$1" &&
+	(cd "$1" &&
+	 tree=`git hash-object -w -t tree /dev/null` &&
+	 commit=$(echo "$1" | git commit-tree $tree) &&
+	 git update-ref HEAD $commit
+	)
+}
+
+get() {
+	git init --bare fetch &&
+	(cd fetch && git fetch "../$1") &&
+	git clone "$1" clone
+}
+
+check() {
+	echo "$1" >expect &&
+	(cd fetch && git log -1 --format=%s FETCH_HEAD) >actual.fetch &&
+	(cd clone && git log -1 --format=%s HEAD) >actual.clone &&
+	test_cmp expect actual.fetch &&
+	test_cmp expect actual.clone
+}
+
+test_expect_success 'find .git dir in worktree' '
+	reset &&
+	make_tree foo &&
+	get foo &&
+	check foo
+'
+
+test_expect_success 'automagically add .git suffix' '
+	reset &&
+	make_bare foo.git &&
+	get foo &&
+	check foo.git
+'
+
+test_expect_success 'automagically add .git suffix to worktree' '
+	reset &&
+	make_tree foo.git &&
+	get foo &&
+	check foo.git
+'
+
+test_expect_success 'prefer worktree foo over bare foo.git' '
+	reset &&
+	make_tree foo &&
+	make_bare foo.git &&
+	get foo &&
+	check foo
+'
+
+test_expect_success 'prefer bare foo over bare foo.git' '
+	reset &&
+	make_bare foo &&
+	make_bare foo.git &&
+	get foo &&
+	check foo
+'
+
+test_expect_success 'disambiguate with full foo.git' '
+	reset &&
+	make_bare foo &&
+	make_bare foo.git &&
+	get foo.git &&
+	check foo.git
+'
+
+test_expect_success 'we are not fooled by non-git foo directory' '
+	reset &&
+	make_bare foo.git &&
+	mkdir foo &&
+	get foo &&
+	check foo.git
+'
+
+test_expect_success 'prefer inner .git over outer bare' '
+	reset &&
+	make_tree foo &&
+	make_bare foo.git &&
+	mv foo/.git foo.git &&
+	get foo.git &&
+	check foo
+'
+
+test_done
-- 
1.7.9.rc1.28.gf4be5

^ permalink raw reply related

* BUG 1.7.9: git-update-ref strange behavior with ref with trailing newline
From: Mark Jason Dominus @ 2012-02-02 22:09 UTC (permalink / raw)
  To: git


Here I use git symbolic-ref to update HEAD with a ref whose name
contains trailing newlines:

        $ git symbolic-ref -m "this message does not appear" HEAD 'refs/heads/master
        >
        >
        > '

The newlines are inserted into .git/HEAD, but are innocuous, because
other git commands ignore them.  The bug is that the -m option is
completely ignored:

        $ git reflog HEAD | grep 'message does not appear'
        $

If trailing newlines are considered acceptable, the -m option should
be honored.  If not, an error message should be printed and thecommand
should exit with a nonzero exit status.

I will prepare a patch if you will say which behavior would be
preferable.

Mark Jason Dominus 	  			                 mjd@plover.com

^ permalink raw reply

* Git-gui: crashes on OS X when entering combining ("dead") keys
From: Beat Bolli @ 2012-02-02 22:04 UTC (permalink / raw)
  To: git

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Hi

I've just had git-gui crash on me when I tried to enter the ~ (tilde)
character on my Mac mini under OS X 10.6.8:

2012-02-02 22:26:25.992 Wish[49140:60f] An uncaught exception was raised
2012-02-02 22:26:25.995 Wish[49140:60f] *** -[NSCFString
characterAtIndex:]: Range or index out of bounds
2012-02-02 22:26:25.996 Wish[49140:60f] *** Terminating app due to
uncaught exception 'NSRangeException', reason: '*** -[NSCFString
characterAtIndex:]: Range or index out of bounds'
*** Call stack at first throw:
(
        0   CoreFoundation                      0x00007fff8273d784
__exceptionPreprocess + 180
        1   libobjc.A.dylib                     0x00007fff87c10f03
objc_exception_throw + 45
        2   CoreFoundation                      0x00007fff8273d5a7
+[NSException raise:format:arguments:] + 103
        3   CoreFoundation                      0x00007fff8273d534
+[NSException raise:format:] + 148
        4   Foundation                          0x00007fff843ec6ad
- -[NSCFString characterAtIndex:] + 97
        5   Tk                                  0x00000001000bdbcf
Tk_SetCaretPos + 663
        6   Tk                                  0x00000001000c3d94
Tk_MacOSXSetupTkNotifier + 699
        7   Tcl                                 0x00000001001c629e
Tcl_DoOneEvent + 297
        8   Tk                                  0x000000010001c080
Tk_MainLoop + 24
        9   Tk                                  0x0000000100028ab4
Tk_MainEx + 1555
        10  Wish                                0x0000000100004545 0x0
+ 4294985029
        11  Wish                                0x00000001000044a4 0x0
+ 4294984868
)
terminate called after throwing an instance of 'NSException'
error: git-gui died of signal 6

[sorry for wrapping]

I use the Swiss German keyboard layout where the tilde is a dead key
on Fn+n (for entering the spanish ñ, I assume). Other dead keys like
¨, ´ and ` also cause git-gui to crash. It does not matter whether the
dead key is the last one in the commit message or not.

Interestingly, the commit text was saved to .git/COMMIT_MSG in spite
of the crash, so I didn't lose any of it.

I use the current git 1.7.9 installed via homebrew.

Thanks,
Beat Bolli

PS: Please CC me on replies, I'm not on the list. Thanks.
- -- 
pgp: 0x506A903A; 49D5 794A EA77 F907 764F  D89E 304B 93CF 506A 903A
gsm: 4.7.7.6.0.7.7.9.7.1.4.e164.arpa
icbm: 47.0452 N, 7.2715 E

"It takes love over gold, and mind over matter" -- Dire Straits
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.9 (Darwin)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iEYEARECAAYFAk8rCFwACgkQMEuTz1BqkDoMhwCfZr+/FAsl7LsVxxHmelirDh+w
RZsAoOzusVol5b8zZEGq9NLanHlfi4bt
=hFrL
-----END PGP SIGNATURE-----

^ 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