[PATCH/RFC] Gitweb: Convert UTF-8 encoded file names

[PATCH/RFC] Gitweb: Convert UTF-8 encoded file names

[Michael Wagner]

[-- Attachment #1: 28222-Gitweb-Convert-UTF-8-encoded-file-names.patch --]
[-- Type: text/plain, Size: 876 bytes --]

Perl has an internal encoding used to store text strings. Currently, trying to
view files with UTF-8 encoded names results in an error (either "404 - Cannot
find file" [blob_plain] or "XML Parsing Error" [blob]). Converting these UTF-8
encoded file names into Perl's internal format resolves these errors.

Signed-off-by: Michael Wagner <accounts@mwagner.org>
---
 gitweb/gitweb.perl | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index a9f57d6..6046977 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -1056,7 +1056,7 @@ sub evaluate_and_validate_params {
 		}
 	}
 
-	our $file_name = $input_params{'file_name'};
+	our $file_name = decode("utf-8", $input_params{'file_name'});
 	if (defined $file_name) {
 		if (!is_valid_pathname($file_name)) {
 			die_error(400, "Invalid file parameter");
-- 
1.9.0

Re: [PATCH/RFC] Gitweb: Convert UTF-8 encoded file names

[Junio C Hamano]

Michael Wagner <accounts@mwagner.org> writes:

> Perl has an internal encoding used to store text strings. Currently, trying to
> view files with UTF-8 encoded names results in an error (either "404 - Cannot
> find file" [blob_plain] or "XML Parsing Error" [blob]). Converting these UTF-8
> encoded file names into Perl's internal format resolves these errors.
>
> Signed-off-by: Michael Wagner <accounts@mwagner.org>
> ---

Cc'ing Jakub, who have been the area maintainer, for comments.

One thing I wonder is that, if there are some additional calls to
encode() necessary before we embed $file_name (which are now decoded
to the internal string form, not a byte-sequence that happens to be
in utf-8) in the generated pages, if we were to do this change.

>  gitweb/gitweb.perl | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
> index a9f57d6..6046977 100755
> --- a/gitweb/gitweb.perl
> +++ b/gitweb/gitweb.perl
> @@ -1056,7 +1056,7 @@ sub evaluate_and_validate_params {
>  		}
>  	}
>  
> -	our $file_name = $input_params{'file_name'};
> +	our $file_name = decode("utf-8", $input_params{'file_name'});
>  	if (defined $file_name) {
>  		if (!is_valid_pathname($file_name)) {
>  			die_error(400, "Invalid file parameter");

Re: [PATCH/RFC] Gitweb: Convert UTF-8 encoded file names

[Jakub Narębski]

On Wed, May 14, 2014 at 11:57 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Michael Wagner <accounts@mwagner.org> writes:
>
>> Perl has an internal encoding used to store text strings. Currently, trying to
>> view files with UTF-8 encoded names results in an error (either "404 - Cannot
>> find file" [blob_plain] or "XML Parsing Error" [blob]). Converting these UTF-8
>> encoded file names into Perl's internal format resolves these errors.

Could you give us an example?  What is important is whether filename
is passed via path_info or via query string.

Because in evaluate_uri() there is

     our $path_info = decode_utf8($ENV{"PATH_INFO"});

and in evaluate_query_params() there is

    $input_params{$name} = decode_utf8($cgi->param($symbol));

>> Signed-off-by: Michael Wagner <accounts@mwagner.org>
>> ---
>
> Cc'ing Jakub, who have been the area maintainer, for comments.
>
> One thing I wonder is that, if there are some additional calls to
> encode() necessary before we embed $file_name (which are now decoded
> to the internal string form, not a byte-sequence that happens to be
> in utf-8) in the generated pages, if we were to do this change.

There should be no problem with output encoding.  esc_path(), which
should be used for filenames, includes to_utf8, which in turn uses
decode($fallback_encoding, $str, Encode::FB_DEFAULT);

>>  gitweb/gitweb.perl | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
>> index a9f57d6..6046977 100755
>> --- a/gitweb/gitweb.perl
>> +++ b/gitweb/gitweb.perl
>> @@ -1056,7 +1056,7 @@ sub evaluate_and_validate_params {
>>               }
>>       }
>>
>> -     our $file_name = $input_params{'file_name'};
>> +     our $file_name = decode("utf-8", $input_params{'file_name'});
>>       if (defined $file_name) {
>>               if (!is_valid_pathname($file_name)) {
>>                       die_error(400, "Invalid file parameter");

Hmm... all %input_params should have been properly decoded
already, how it was missed?

Also, branchname (hash_base etc.), search query, filename in file_parent,
project name can be UTF-8 too, so it is at best partial fix.

-- 
Jakub Narębski

Re: [PATCH/RFC] Gitweb: Convert UTF-8 encoded file names

[Michael Wagner]

On Thu, May 15, 2014 at 12:25:45AM +0200, Jakub Narębski wrote:
> On Wed, May 14, 2014 at 11:57 PM, Junio C Hamano <gitster@pobox.com> wrote:
> > Michael Wagner <accounts@mwagner.org> writes:
> >
> >> Perl has an internal encoding used to store text strings. Currently, trying to
> >> view files with UTF-8 encoded names results in an error (either "404 - Cannot
> >> find file" [blob_plain] or "XML Parsing Error" [blob]). Converting these UTF-8
> >> encoded file names into Perl's internal format resolves these errors.
> 
> Could you give us an example?  What is important is whether filename
> is passed via path_info or via query string.
> 

There is a file named "Gütekriterien.txt" in my repository. Trying to
view this file as "blob_plain" produces an 404 error (displaying the
file name with an additional print statement):

$ REQUEST_METHOD=GET QUERY_STRING='p=notes.git;a=blob_plain;f=work/G%C3%83%C2%BCtekriterien.txt;hb=HEAD' ./gitweb.cgi

work/Gütekriterien.txt
Status: 404 Not Found

Decoding the UTF-8 encoded file name (again with an additional print
statement):

$ REQUEST_METHOD=GET QUERY_STRING='p=notes.git;a=blob_plain;f=work/G%C3%83%C2%BCtekriterien.txt;hb=HEAD' ./gitweb.cgi

work/Gütekriterien.txt
Content-disposition: inline; filename="work/Gütekriterien.txt"

> Because in evaluate_uri() there is
> 
>      our $path_info = decode_utf8($ENV{"PATH_INFO"});
> 
> and in evaluate_query_params() there is
> 
>     $input_params{$name} = decode_utf8($cgi->param($symbol));
> 
> >> Signed-off-by: Michael Wagner <accounts@mwagner.org>
> >> ---
> >
> > Cc'ing Jakub, who have been the area maintainer, for comments.
> >
> > One thing I wonder is that, if there are some additional calls to
> > encode() necessary before we embed $file_name (which are now decoded
> > to the internal string form, not a byte-sequence that happens to be
> > in utf-8) in the generated pages, if we were to do this change.

The generated pages show the correct file names. 

> 
> There should be no problem with output encoding.  esc_path(), which
> should be used for filenames, includes to_utf8, which in turn uses
> decode($fallback_encoding, $str, Encode::FB_DEFAULT);
> 
> >>  gitweb/gitweb.perl | 2 +-
> >>  1 file changed, 1 insertion(+), 1 deletion(-)
> >>
> >> diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
> >> index a9f57d6..6046977 100755
> >> --- a/gitweb/gitweb.perl
> >> +++ b/gitweb/gitweb.perl
> >> @@ -1056,7 +1056,7 @@ sub evaluate_and_validate_params {
> >>               }
> >>       }
> >>
> >> -     our $file_name = $input_params{'file_name'};
> >> +     our $file_name = decode("utf-8", $input_params{'file_name'});
> >>       if (defined $file_name) {
> >>               if (!is_valid_pathname($file_name)) {
> >>                       die_error(400, "Invalid file parameter");
> 
> Hmm... all %input_params should have been properly decoded
> already, how it was missed?
> 
> Also, branchname (hash_base etc.), search query, filename in file_parent,
> project name can be UTF-8 too, so it is at best partial fix.
> 
> -- 
> Jakub Narębski

Re: [PATCH/RFC] Gitweb: Convert UTF-8 encoded file names

[Peter Krefting]

Michael Wagner:

> Decoding the UTF-8 encoded file name (again with an additional print
> statement):
>
> $ REQUEST_METHOD=GET QUERY_STRING='p=notes.git;a=blob_plain;f=work/G%C3%83%C2%BCtekriterien.txt;hb=HEAD' ./gitweb.cgi
>
> work/Gütekriterien.txt
> Content-disposition: inline; filename="work/Gütekriterien.txt"

You should fix the code path that created that URI, though, as it is 
not what you expected.

%C3%83 decodes to U+00C3 Latin Capital Letter A With Tilde
%C2%BC decodes to U+00BC Vulgar Graction One Quarter

The proper UTF-8 encoding for ü (U+00FC) is, as you can probably guess from 
looking at which two characters the sequence above yielded, C3 BC, 
which in a URI is represented as %C3%BC.

Your QUERY_STRING should thus be

   p=notes.git;a=blob_plain;f=work/G%C3%BCtekriterien.txt;hb=HEAD

which probably works as expected.

What is happening is that whatever is generating the URI us 
UTF-8-encoding the string twice (i.e., it generates a string with the 
proper C3 BC in it, and then interprets it as iso-8859-1 data and runs 
that through a UTF-8 encoder again, yielding the C3 83 C2 BC sequence 
you see above).

-- 
\\// Peter - http://www.softwolves.pp.se/

Re: [PATCH/RFC] Gitweb: Convert UTF-8 encoded file names

[Junio C Hamano]

Peter Krefting <peter@softwolves.pp.se> writes:

> What is happening is that whatever is generating the URI us
> UTF-8-encoding the string twice (i.e., it generates a string with the
> proper C3 BC in it, and then interprets it as iso-8859-1 data and runs
> that through a UTF-8 encoder again, yielding the C3 83 C2 BC sequence
> you see above).

Thanks for a quick response.  If the input was unnecessarily encoded
one extra time, it is no wonder it needed one unnecessary extra
decoding.

Re: [PATCH/RFC] Gitweb: Convert UTF-8 encoded file names

[Michael Wagner]

On Thu, May 15, 2014 at 10:04:24AM +0100, Peter Krefting wrote:
> Michael Wagner:
> 
> >Decoding the UTF-8 encoded file name (again with an additional print
> >statement):
> >
> >$ REQUEST_METHOD=GET QUERY_STRING='p=notes.git;a=blob_plain;f=work/G%C3%83%C2%BCtekriterien.txt;hb=HEAD' ./gitweb.cgi
> >
> >work/Gütekriterien.txt
> >Content-disposition: inline; filename="work/Gütekriterien.txt"
> 
> You should fix the code path that created that URI, though, as it is not
> what you expected.
> 
> %C3%83 decodes to U+00C3 Latin Capital Letter A With Tilde
> %C2%BC decodes to U+00BC Vulgar Graction One Quarter
> 
> The proper UTF-8 encoding for ü (U+00FC) is, as you can probably guess from
> looking at which two characters the sequence above yielded, C3 BC, which in
> a URI is represented as %C3%BC.
> 
> Your QUERY_STRING should thus be
> 
>   p=notes.git;a=blob_plain;f=work/G%C3%BCtekriterien.txt;hb=HEAD
> 
> which probably works as expected.

Obviously, you are right, thanks.

> 
> What is happening is that whatever is generating the URI us UTF-8-encoding
> the string twice (i.e., it generates a string with the proper C3 BC in it,
> and then interprets it as iso-8859-1 data and runs that through a UTF-8
> encoder again, yielding the C3 83 C2 BC sequence you see above).
> 

The subroutine "git tree" generates the tree view. It stores the output
of "git ls-tree -z ..." in an array named "@entries". Printing the content
of this array yields the following result:

00644 blob 6419cd06a9461c38d4f94d9705d97eaaa887156a     520 Gütekriterien.txt

This leads to the "doubled" encoding. Declaring the encoding in the call
to open yields the following result:

100644 blob 6419cd06a9461c38d4f94d9705d97eaaa887156a     520 Gütekriterien.txt

---

diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index a9f57d6..f1414e1 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -7138,7 +7138,7 @@ sub git_tree {
        my @entries = ();
        {
                local $/ = "\0";
-               open my $fd, "-|", git_cmd(), "ls-tree", '-z',
+               open my $fd, "-|encoding(UTF-8)", git_cmd(), "ls-tree", '-z',
                        ($show_sizes ? '-l' : ()), @extra_options, $hash
                        or die_error(500, "Open git-ls-tree failed");
                @entries = map { chomp; $_ } <$fd>;

> -- 
> \\// Peter - http://www.softwolves.pp.se/

Re: [PATCH/RFC] Gitweb: Convert UTF-8 encoded file names

[Jakub Narębski]

On Thu, May 15, 2014 at 8:48 PM, Michael Wagner <accounts@mwagner.org> wrote:
> On Thu, May 15, 2014 at 10:04:24AM +0100, Peter Krefting wrote:
>> Michael Wagner:
>>
>>>Decoding the UTF-8 encoded file name (again with an additional print
>>>statement):
>>>
>>>$ REQUEST_METHOD=GET QUERY_STRING='p=notes.git;a=blob_plain;f=work/G%C3%83%C2%BCtekriterien.txt;hb=HEAD' ./gitweb.cgi
>>>
>>>work/Gütekriterien.txt
>>>Content-disposition: inline; filename="work/Gütekriterien.txt"
>>
>> You should fix the code path that created that URI, though, as it is not
>> what you expected.
>>
>> %C3%83 decodes to U+00C3 Latin Capital Letter A With Tilde
>> %C2%BC decodes to U+00BC Vulgar Graction One Quarter
>>
>> The proper UTF-8 encoding for ü (U+00FC) is, as you can probably guess from
>> looking at which two characters the sequence above yielded, C3 BC, which in
>> a URI is represented as %C3%BC.
>>
>> Your QUERY_STRING should thus be
>>
>>   p=notes.git;a=blob_plain;f=work/G%C3%BCtekriterien.txt;hb=HEAD
>>
>> which probably works as expected.
>>
>> What is happening is that whatever is generating the URI us UTF-8-encoding
>> the string twice (i.e., it generates a string with the proper C3 BC in it,
>> and then interprets it as iso-8859-1 data and runs that through a UTF-8
>> encoder again, yielding the C3 83 C2 BC sequence you see above).
>
> The subroutine "git tree" generates the tree view. It stores the output
> of "git ls-tree -z ..." in an array named "@entries". Printing the content
> of this array yields the following result:
>
> 00644 blob 6419cd06a9461c38d4f94d9705d97eaaa887156a     520 Gütekriterien.txt
>
> This leads to the "doubled" encoding. Declaring the encoding in the call
> to open yields the following result:
>
> 100644 blob 6419cd06a9461c38d4f94d9705d97eaaa887156a     520 Gütekriterien.txt

Good catch.

Writing test for this would not be easy, and require some HTML
parser (WWW::Mechanize, Web::Scraper, HTML::Query, pQuery,
... or low level HTML::TreeBuilder, or other low level parser).

> ---
>
> diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
> index a9f57d6..f1414e1 100755
> --- a/gitweb/gitweb.perl
> +++ b/gitweb/gitweb.perl
> @@ -7138,7 +7138,7 @@ sub git_tree {
>         my @entries = ();
>         {
>                 local $/ = "\0";
> -               open my $fd, "-|", git_cmd(), "ls-tree", '-z',
> +               open my $fd, "-|encoding(UTF-8)", git_cmd(), "ls-tree", '-z',
>                         ($show_sizes ? '-l' : ()), @extra_options, $hash
>                         or die_error(500, "Open git-ls-tree failed");

Or put

                   binmode $fd, ':utf8';

like in the rest of the code.

>                 @entries = map { chomp; $_ } <$fd>;
>

Even better solution would be to use

    use open IN => ':encoding(utf-8)';

at the beginning of gitweb.perl, once and for all.

Unfortunately the output equivalent requires creating Perl
module for gitweb, to be able to use

    use open OUT => ':encoding(utf-8-with-fallback)';

-- 
Jakub Narebski

Re: [PATCH/RFC] Gitweb: Convert UTF-8 encoded file names

[Jakub Narębski]

On Thu, May 15, 2014 at 9:28 PM, Jakub Narębski <jnareb@gmail.com> wrote:
> On Thu, May 15, 2014 at 8:48 PM, Michael Wagner <accounts@mwagner.org> wrote:
[...]
>> The subroutine "git tree" generates the tree view. It stores the output
>> of "git ls-tree -z ..." in an array named "@entries". Printing the content
>> of this array yields the following result:
>>
>> 00644 blob 6419cd06a9461c38d4f94d9705d97eaaa887156a     520 Gütekriterien.txt
>>
>> This leads to the "doubled" encoding. Declaring the encoding in the call
>> to open yields the following result:
>>
>> 100644 blob 6419cd06a9461c38d4f94d9705d97eaaa887156a     520 Gütekriterien.txt

>> diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
>> index a9f57d6..f1414e1 100755
>> --- a/gitweb/gitweb.perl
>> +++ b/gitweb/gitweb.perl
>> @@ -7138,7 +7138,7 @@ sub git_tree {
>>         my @entries = ();
>>         {
>>                 local $/ = "\0";
>> -               open my $fd, "-|", git_cmd(), "ls-tree", '-z',
>> +               open my $fd, "-|encoding(UTF-8)", git_cmd(), "ls-tree", '-z',
>>                         ($show_sizes ? '-l' : ()), @extra_options, $hash
>>                         or die_error(500, "Open git-ls-tree failed");
>
> Or put
>
>                    binmode $fd, ':utf8';
>
> like in the rest of the code.
>
>>                 @entries = map { chomp; $_ } <$fd>;

Though to be exact there isn't any mechanism that ensures that
filenames in tree objects use utf-8 encoding, so perhaps a safer
solution would be to use

   to_utf8($file_name)

(which respects $fallback_encoding) in appropriate places.

-- 
Jakub Narębski

Re: [PATCH/RFC] Gitweb: Convert UTF-8 encoded file names

[Junio C Hamano]

Jakub Narębski <jnareb@gmail.com> writes:

> Writing test for this would not be easy, and require some HTML
> parser (WWW::Mechanize, Web::Scraper, HTML::Query, pQuery,
> ... or low level HTML::TreeBuilder, or other low level parser).

Hmph.  Is it more than just looking for a specific run of %xx we
would expect to see in the output of the tree view for a repository
in which there is one tree with non-ASCII name?

>> diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
>> index a9f57d6..f1414e1 100755
>> --- a/gitweb/gitweb.perl
>> +++ b/gitweb/gitweb.perl
>> @@ -7138,7 +7138,7 @@ sub git_tree {
>>         my @entries = ();
>>         {
>>                 local $/ = "\0";
>> -               open my $fd, "-|", git_cmd(), "ls-tree", '-z',
>> +               open my $fd, "-|encoding(UTF-8)", git_cmd(), "ls-tree", '-z',
>>                         ($show_sizes ? '-l' : ()), @extra_options, $hash
>>                         or die_error(500, "Open git-ls-tree failed");
>
> Or put
>
>                    binmode $fd, ':utf8';
>
> like in the rest of the code.

I expect a patch to do so and can forget about this thread myself,
then, OK?

Thanks all for digging this to the root.

Re: [PATCH/RFC] Gitweb: Convert UTF-8 encoded file names

[Jakub Narębski]

On Thu, May 15, 2014 at 9:38 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Jakub Narębski <jnareb@gmail.com> writes:
>
>> Writing test for this would not be easy, and require some HTML
>> parser (WWW::Mechanize, Web::Scraper, HTML::Query, pQuery,
>> ... or low level HTML::TreeBuilder, or other low level parser).
>
> Hmph.  Is it more than just looking for a specific run of %xx we
> would expect to see in the output of the tree view for a repository
> in which there is one tree with non-ASCII name?

There is if we want to check (in non-fragile way) that said
specific run is in 'href' *attribute* of 'a' element (link target).

-- 
Jakub Narebski

Re: [PATCH/RFC] Gitweb: Convert UTF-8 encoded file names

[Junio C Hamano]

Jakub Narębski <jnareb@gmail.com> writes:

> On Thu, May 15, 2014 at 9:38 PM, Junio C Hamano <gitster@pobox.com> wrote:
>> Jakub Narębski <jnareb@gmail.com> writes:
>>
>>> Writing test for this would not be easy, and require some HTML
>>> parser (WWW::Mechanize, Web::Scraper, HTML::Query, pQuery,
>>> ... or low level HTML::TreeBuilder, or other low level parser).
>>
>> Hmph.  Is it more than just looking for a specific run of %xx we
>> would expect to see in the output of the tree view for a repository
>> in which there is one tree with non-ASCII name?
>
> There is if we want to check (in non-fragile way) that said
> specific run is in 'href' *attribute* of 'a' element (link target).

Correct, but is "where does it appear" the question we are
primarily interested in, wrt this breakage and its fix?

If gitweb output has some volatile parts that do not depend on the
contents of the Git test repository (e.g. showing contents of
/etc/motd, date/time of when the test was run, or the full pathname
leading to the trash directory), then preparing a tree whose name is
äéìõû and making sure that the properly encoded version of äéìõû
appears anywhere in the output may not be sufficient to validate
that we got the encoding right, as that string may appear in the
parts that are totally unrelated to the contents being shown and not
under our control.  But is that really the case?

Also we may introduce a bug and misspell the attr name and produce
an anchor element with hpef attribute with the properly encoded URL
in it, and your "parse HTML properly" approach would catch it, but
is that the kind of breakage under discussion?  You hinted at new
tests for UTF-8 encoding in the other message in the thread earlier,
and I've been assuming that we were talking about the encoding test,
not a test to catch s/href/hpef/ kind of breakage.

Re: [PATCH/RFC] Gitweb: Convert UTF-8 encoded file names

[Jakub Narębski]

On Fri, May 16, 2014 at 3:26 AM, Junio C Hamano <gitster@pobox.com> wrote:
> Jakub Narębski <jnareb@gmail.com> writes:
>> On Thu, May 15, 2014 at 9:38 PM, Junio C Hamano <gitster@pobox.com> wrote:
>>> Jakub Narębski <jnareb@gmail.com> writes:
>>>
>>>> Writing test for this would not be easy, and require some HTML
>>>> parser (WWW::Mechanize, Web::Scraper, HTML::Query, pQuery,
>>>> ... or low level HTML::TreeBuilder, or other low level parser).
>>>
>>> Hmph.  Is it more than just looking for a specific run of %xx we
>>> would expect to see in the output of the tree view for a repository
>>> in which there is one tree with non-ASCII name?
>>
>> There is if we want to check (in non-fragile way) that said
>> specific run is in 'href' *attribute* of 'a' element (link target).
>
> Correct, but is "where does it appear" the question we are
> primarily interested in, wrt this breakage and its fix?

That of course depends on how we want to test gitweb output.
The simplest solution, comparing with known output with perhaps
fragile / variable elements masked out could be done quickly...
but changes in output (even if they don't change functionality,
or don't change visible output) require regenerating test cases
(expected output) to test against - which might be source of
errors in test suite.

Another simple solution, grepping for expected strings, also
easy to create, has the disadvantage of being only positive
test - you cannot [easily] test that there are no *wrong* output,
only that right string exists somewhere.

> If gitweb output has some volatile parts that do not depend on the
> contents of the Git test repository (e.g. showing contents of
> /etc/motd, date/time of when the test was run, or the full pathname
> leading to the trash directory), then preparing a tree whose name is
> äéìõû and making sure that the properly encoded version of äéìõû
> appears anywhere in the output may not be sufficient to validate
> that we got the encoding right, as that string may appear in the
> parts that are totally unrelated to the contents being shown and not
> under our control.  But is that really the case?

Well, I guess that any test is better than no test (though OTOH
Heartbleed and "goto fail" bugs shows the importance of negative
tests).

> Also we may introduce a bug and misspell the attr name and produce
> an anchor element with hpef attribute with the properly encoded URL
> in it, and your "parse HTML properly" approach would catch it, but
> is that the kind of breakage under discussion?  You hinted at new
> tests for UTF-8 encoding in the other message in the thread earlier,
> and I've been assuming that we were talking about the encoding test,
> not a test to catch s/href/hpef/ kind of breakage.

One of tests possible with HTML parser (e.g. WWW::Mechanize::CGI)
is to check that all [internal] links leads to 200-OK pages, which
accidentally would also be a test against this breakage.

-- 
Jakub Narebski

Re: [PATCH/RFC] Gitweb: Convert UTF-8 encoded file names

[Junio C Hamano]

Jakub Narębski <jnareb@gmail.com> writes:

>> Correct, but is "where does it appear" the question we are
>> primarily interested in, wrt this breakage and its fix?
>
> That of course depends on how we want to test gitweb output.
> The simplest solution, comparing with known output with perhaps
> fragile / variable elements masked out could be done quickly...
> but changes in output (even if they don't change functionality,
> or don't change visible output) require regenerating test cases
> (expected output) to test against - which might be source of
> errors in test suite.

I agree with your "to test it fully, we need extra dependencies",
but my point is that it does not have to be a full "HTML-validating,
picking the expected attribute via XPATH matching" kind of test if
what we want is only to add a new test to protect this particular
fix from future breakages.

For example, I think it is sufficient to grep for 'href="...%xx%xx"'
in the output after preparing a sample tree with one entry to show.
The expected substring either exists (in which case we got it
right), or it doesn't (in which case we are showing garbage).  Of
course that depends on the assumption that its output is not too
heavily contaminated with volatile parts outside our control, as I
already mentioned in the message you are responding to.

But it all depends on "if" we wanted to add a new test ;-)

Re: [PATCH/RFC] Gitweb: Convert UTF-8 encoded file names

[Jakub Narębski]

W dniu 2014-05-16 19:05, Junio C Hamano pisze:
> Jakub Narębski <jnareb@gmail.com> writes:
> 
>>> Correct, but is "where does it appear" the question we are
>>> primarily interested in, wrt this breakage and its fix?
>>
>> That of course depends on how we want to test gitweb output.
>> The simplest solution, comparing with known output with perhaps
>> fragile / variable elements masked out could be done quickly...
>> but changes in output (even if they don't change functionality,
>> or don't change visible output) require regenerating test cases
>> (expected output) to test against - which might be source of
>> errors in test suite.
> 
> I agree with your "to test it fully, we need extra dependencies",
> but my point is that it does not have to be a full "HTML-validating,
> picking the expected attribute via XPATH matching" kind of test if
> what we want is only to add a new test to protect this particular
> fix from future breakages.
> 
> For example, I think it is sufficient to grep for 'href="...%xx%xx"'
> in the output after preparing a sample tree with one entry to show.
> The expected substring either exists (in which case we got it
> right), or it doesn't (in which case we are showing garbage).  Of
> course that depends on the assumption that its output is not too
> heavily contaminated with volatile parts outside our control, as I
> already mentioned in the message you are responding to.
> 
> But it all depends on "if" we wanted to add a new test ;-)

I tried to add such simple test to t9502, but instead of tests
failing with current version, the test setup fails but succeeds
(i.e. test library says that it failed, but manual examination
shows that everything is O.K.).

-- >8 --
From: Jakub Narebski <jnareb@gmail.com>
Subject: [PATCH/RFC] gitweb test: Test proper encoding of non US-ASCII filenames in output (WIP)

This t9502 test is intended to test for proper encoding of non
US-ASCII filenames (i.e. UTF-8 filenames) in generated links (which
need some form of URI encoding) and in generated HTML (which needs
HTML encoding / escaping).

For now it tests only 'tree' view (though incidentally it also tests
UTF-8 in commit subject), as this was the action where reportedly
there was bug in link encoding: $t{'name'} coming from the
"git ls-tree -z ..." command via @ntries array was not marked as
UTF-8, making Perl assume that it is in internal Perl format
i.e. iso-8859-1 encoding and URI-escaping it as if it was in
iso-8859-1 encoding (e.g. "Gütekriterien.txt" in UTF-8 is
"Gütekriterien.txt" if treated as iso-8859-1, and it then
encodes to "G%C3%83%C2%BCtekriterien.txt" instead of correct
"G%C3%BCtekriterien.txt").

UNFORTUNATELY test does not fail as it should, even though the issue
was not fixed... OTOH it fails in setup though it is successful.

Reported-by: Michael Wagner <accounts@mwagner.org>
Signed-off-by: Jakub Narębski <jnareb@gmail.com>
---
 t/t9502-gitweb-standalone-parse-output.sh |   34 +++++++++++++++++++++++++++++
 1 files changed, 34 insertions(+), 0 deletions(-)

diff --git a/t/t9502-gitweb-standalone-parse-output.sh b/t/t9502-gitweb-standalone-parse-output.sh
index 86dfee2..37246a3 100755
--- a/t/t9502-gitweb-standalone-parse-output.sh
+++ b/t/t9502-gitweb-standalone-parse-output.sh
@@ -201,4 +201,38 @@ test_expect_success 'xss checks' '
 	xss "a=rss&p=foo.git&f=$TAG"
 '
 
+link_check () {
+	grep -F   "%3C__%C2%A3%C3%A5%C3%AB%C3%AE%C3%B1%C3%B2%C3%BB%C3%BD%C2%B6" \
+		gitweb.body &&
+	! grep -F "%3C__%A3%E5%EB%EE%F1%F2%FB%FD%B6" \
+		gitweb.body
+}
+
+test_expect_success 'prepare UTF-8 output tests' '
+	FILENAME="<__£åëîñòûý¶  +;?&__>" &&
+	test_commit "Adding $FILENAME" "$FILENAME" "$FILENAME contents"
+'
+
+test_expect_success 'check URI-escaped UTF-8 filename in query-params link' '
+	cat >>gitweb_config.perl <<-\EOF &&
+	$feature{"pathinfo"}{"default"} = [0];
+	EOF
+	gitweb_run "p=.git;a=tree" &&
+	link_check
+'
+
+test_expect_success 'check URI-escaped UTF-8 filename in path_info link' '
+	cat >>gitweb_config.perl <<-\EOF &&
+	$feature{"pathinfo"}{"default"} = [1];
+	EOF
+	gitweb_run "" "/.git/tree" &&
+	link_check
+'
+
+test_expect_success 'check HTML-escaped UTF-8 filename in body' '
+	gitweb_run "p=.git;a=tree" &&
+	grep -F "&lt;__£åëîñòûý¶  +;?&amp;__&gt;" gitweb.body &&
+	! grep -F  "<__£åëîñòûý¶  +;?&__>" gitweb.body
+'
+
 test_done
-- 
1.7.1


 

Re: [PATCH/RFC] Gitweb: Convert UTF-8 encoded file names

[Junio C Hamano]

(sorry if you receive a dup; pobox.com seems to be constipated right now)

Jakub Narębski <jnareb@gmail.com> writes:

>> Correct, but is "where does it appear" the question we are
>> primarily interested in, wrt this breakage and its fix?
>
> That of course depends on how we want to test gitweb output.
> The simplest solution, comparing with known output with perhaps
> fragile / variable elements masked out could be done quickly...
> but changes in output (even if they don't change functionality,
> or don't change visible output) require regenerating test cases
> (expected output) to test against - which might be source of
> errors in test suite.

I agree with your "to test it fully, we need extra dependencies",
but my point is that it does not have to be a full "HTML-validating,
picking the expected attribute via XPATH matching" kind of test if
what we want is only to add a new test to protect this particular
fix from future breakages.

For example, I think it is sufficient to grep for 'href="...%xx%xx"'
in the output after preparing a sample tree with one entry to show.
The expected substring either exists (in which case we got it
right), or it doesn't (in which case we are showing garbage).  Of
course that depends on the assumption that its output is not too
heavily contaminated with volatile parts outside our control, as I
already mentioned in the message you are responding to.

But it all depends on "if" we wanted to add a new test ;-)

[PATCH] gitweb: Harden UTF-8 handling in generated links

[Jakub Narębski]

W dniu 2014-05-15 21:28, Jakub Narębski pisze:
> On Thu, May 15, 2014 at 8:48 PM, Michael Wagner <accounts@mwagner.org> wrote:
>> On Thu, May 15, 2014 at 10:04:24AM +0100, Peter Krefting wrote:
>>> Michael Wagner:

>> diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
>> index a9f57d6..f1414e1 100755
>> --- a/gitweb/gitweb.perl
>> +++ b/gitweb/gitweb.perl
>> @@ -7138,7 +7138,7 @@ sub git_tree {
>>          my @entries = ();
>>          {
>>                  local $/ = "\0";
>> -               open my $fd, "-|", git_cmd(), "ls-tree", '-z',
>> +               open my $fd, "-|encoding(UTF-8)", git_cmd(), "ls-tree", '-z',
>>                          ($show_sizes ? '-l' : ()), @extra_options, $hash
>>                          or die_error(500, "Open git-ls-tree failed");
> 
> Or put
> 
>                     binmode $fd, ':utf8';
> 
> like in the rest of the code.
> 
>>                  @entries = map { chomp; $_ } <$fd>;
>>
> 
> Even better solution would be to use
> 
>      use open IN => ':encoding(utf-8)';
> 
> at the beginning of gitweb.perl, once and for all.

Or harden esc_param / esc_path_info the same way esc_html
is hardened against missing ':utf8' flag.

-- >8 -- 
Subject: [PATCH] gitweb: Harden UTF-8 handling in generated links

esc_html() ensures that its input is properly UTF-8 encoded and marked
as UTF-8 with to_utf8().  Make esc_param() (used for query parameters
in generated URLs), esc_path_info() (for escaping path_info
components) and esc_url() use it too.

This hardens gitweb against errors in UTF-8 handling; because
to_utf8() is idempotent it won't change correct output.

Reported-by: Michael Wagner <accounts@mwagner.org>
Signed-off-by: Jakub Narębski <jnareb@gmail.com>
---
 gitweb/gitweb.perl |    7 +++++++
 1 files changed, 7 insertions(+), 0 deletions(-)

diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index a9f57d6..77e1312 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -1548,8 +1548,11 @@ sub to_utf8 {
 sub esc_param {
 	my $str = shift;
 	return undef unless defined $str;
+
+	$str = to_utf8($str);
 	$str =~ s/([^A-Za-z0-9\-_.~()\/:@ ]+)/CGI::escape($1)/eg;
 	$str =~ s/ /\+/g;
+
 	return $str;
 }
 
@@ -1558,6 +1561,7 @@ sub esc_path_info {
 	my $str = shift;
 	return undef unless defined $str;
 
+	$str = to_utf8($str);
 	# path_info doesn't treat '+' as space (specially), but '?' must be escaped
 	$str =~ s/([^A-Za-z0-9\-_.~();\/;:@&= +]+)/CGI::escape($1)/eg;
 
@@ -1568,8 +1572,11 @@ sub esc_path_info {
 sub esc_url {
 	my $str = shift;
 	return undef unless defined $str;
+
+	$str = to_utf8($str);
 	$str =~ s/([^A-Za-z0-9\-_.~();\/;?:@&= ]+)/CGI::escape($1)/eg;
 	$str =~ s/ /\+/g;
+
 	return $str;
 }
 
-- 
1.7.1

Re: [PATCH] gitweb: Harden UTF-8 handling in generated links

[Michael Wagner]

On Tue, May 27, 2014 at 04:22:42PM +0200, Jakub Narębski wrote:
> W dniu 2014-05-15 21:28, Jakub Narębski pisze:
> > On Thu, May 15, 2014 at 8:48 PM, Michael Wagner <accounts@mwagner.org> wrote:
> >> On Thu, May 15, 2014 at 10:04:24AM +0100, Peter Krefting wrote:
> >>> Michael Wagner:
> 
> >> diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
> >> index a9f57d6..f1414e1 100755
> >> --- a/gitweb/gitweb.perl
> >> +++ b/gitweb/gitweb.perl
> >> @@ -7138,7 +7138,7 @@ sub git_tree {
> >>          my @entries = ();
> >>          {
> >>                  local $/ = "\0";
> >> -               open my $fd, "-|", git_cmd(), "ls-tree", '-z',
> >> +               open my $fd, "-|encoding(UTF-8)", git_cmd(), "ls-tree", '-z',
> >>                          ($show_sizes ? '-l' : ()), @extra_options, $hash
> >>                          or die_error(500, "Open git-ls-tree failed");
> > 
> > Or put
> > 
> >                     binmode $fd, ':utf8';
> > 
> > like in the rest of the code.
> > 
> >>                  @entries = map { chomp; $_ } <$fd>;
> >>
> > 
> > Even better solution would be to use
> > 
> >      use open IN => ':encoding(utf-8)';
> > 
> > at the beginning of gitweb.perl, once and for all.
> 
> Or harden esc_param / esc_path_info the same way esc_html
> is hardened against missing ':utf8' flag.
> 
> -- >8 -- 
> Subject: [PATCH] gitweb: Harden UTF-8 handling in generated links
> 
> esc_html() ensures that its input is properly UTF-8 encoded and marked
> as UTF-8 with to_utf8().  Make esc_param() (used for query parameters
> in generated URLs), esc_path_info() (for escaping path_info
> components) and esc_url() use it too.
> 
> This hardens gitweb against errors in UTF-8 handling; because
> to_utf8() is idempotent it won't change correct output.
> 
> Reported-by: Michael Wagner <accounts@mwagner.org>
> Signed-off-by: Jakub Narębski <jnareb@gmail.com>
> ---
>  gitweb/gitweb.perl |    7 +++++++
>  1 files changed, 7 insertions(+), 0 deletions(-)
> 
> diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
> index a9f57d6..77e1312 100755
> --- a/gitweb/gitweb.perl
> +++ b/gitweb/gitweb.perl
> @@ -1548,8 +1548,11 @@ sub to_utf8 {
>  sub esc_param {
>  	my $str = shift;
>  	return undef unless defined $str;
> +
> +	$str = to_utf8($str);
>  	$str =~ s/([^A-Za-z0-9\-_.~()\/:@ ]+)/CGI::escape($1)/eg;
>  	$str =~ s/ /\+/g;
> +
>  	return $str;
>  }
>  
> @@ -1558,6 +1561,7 @@ sub esc_path_info {
>  	my $str = shift;
>  	return undef unless defined $str;
>  
> +	$str = to_utf8($str);
>  	# path_info doesn't treat '+' as space (specially), but '?' must be escaped
>  	$str =~ s/([^A-Za-z0-9\-_.~();\/;:@&= +]+)/CGI::escape($1)/eg;
>  
> @@ -1568,8 +1572,11 @@ sub esc_path_info {
>  sub esc_url {
>  	my $str = shift;
>  	return undef unless defined $str;
> +
> +	$str = to_utf8($str);
>  	$str =~ s/([^A-Za-z0-9\-_.~();\/;?:@&= ]+)/CGI::escape($1)/eg;
>  	$str =~ s/ /\+/g;
> +
>  	return $str;
>  }
>  
> -- 
> 1.7.1
> 
> 

While trying to view a "blob_plain" of "Gütekritierien.txt", a 404 error
occured. "git_get_hash_by_path" tries to resolve the hash with the wrong
filename (git ls-tree -z HEAD -- Gütekriterien.txt) and fails.

The filename needs the correct encoding. Something like this is probably
needed for all filenames and should be done at a prior stage:
---
 gitweb/gitweb.perl |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index 77e1312..e4a50e7 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -4725,7 +4725,7 @@ sub git_print_tree_entry {
                }
                print " | " .
                        $cgi->a({-href => href(action=>"blob_plain", hash_base=>$hash_base,
-                                              file_name=>"$basedir$t->{'name'}")},
+                                              file_name=>"$basedir" . to_utf8($t->{'name'}))}, 
                                "raw");
                print "</td>\n";

-- 
1.7.1

Re: [PATCH] gitweb: Harden UTF-8 handling in generated links

[Jakub Narębski]

Michael Wagner wrote:
> On Tue, May 27, 2014 at 04:22:42PM +0200, Jakub Narębski wrote:

>> Subject: [PATCH] gitweb: Harden UTF-8 handling in generated links
>>
>> esc_html() ensures that its input is properly UTF-8 encoded and marked
>> as UTF-8 with to_utf8().  Make esc_param() (used for query parameters
>> in generated URLs), esc_path_info() (for escaping path_info
>> components) and esc_url() use it too.
>>
>> This hardens gitweb against errors in UTF-8 handling; because
>> to_utf8() is idempotent it won't change correct output.
[...]
>>   sub esc_param {
>>   	my $str = shift;
>>   	return undef unless defined $str;
>> +
>> +	$str = to_utf8($str);
>>   	$str =~ s/([^A-Za-z0-9\-_.~()\/:@ ]+)/CGI::escape($1)/eg;
>>   	$str =~ s/ /\+/g;
>> +
>>   	return $str;
>>   }   
 
> While trying to view a "blob_plain" of "Gütekritierien.txt", a 404 error
> occured. "git_get_hash_by_path" tries to resolve the hash with the wrong
> filename (git ls-tree -z HEAD -- Gütekriterien.txt) and fails.
> 
> The filename needs the correct encoding. Something like this is probably
> needed for all filenames and should be done at a prior stage:

True.

First, I wonder why the tests I did for this situation didn't
show any errors even before the "harden href()" patch. What
is different in your config that you see those errors?

> ---
>   gitweb/gitweb.perl |    2 +-
>   1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
> index 77e1312..e4a50e7 100755
> --- a/gitweb/gitweb.perl
> +++ b/gitweb/gitweb.perl
> @@ -4725,7 +4725,7 @@ sub git_print_tree_entry {
>                  }
>                  print " | " .
>                          $cgi->a({-href => href(action=>"blob_plain", hash_base=>$hash_base,
> -                                              file_name=>"$basedir$t->{'name'}")},
> +                                              file_name=>"$basedir" . to_utf8($t->{'name'}))},

Second, my "harder href()" patch does not work for this because
concatenation of non-UFT8 with UTF8 string screws up Perl
knowledge what is and isn't UTF8.  So to_utf8() after concat
doesn't help.


>                                  "raw");
>                  print "</td>\n";
> 

Re: [PATCH] gitweb: Harden UTF-8 handling in generated links

[Michael Wagner]

On Wed, Jun 04, 2014 at 08:47:54PM +0200, Jakub Narębski wrote:
> Michael Wagner wrote:
> > On Tue, May 27, 2014 at 04:22:42PM +0200, Jakub Narębski wrote:
> 
> >> Subject: [PATCH] gitweb: Harden UTF-8 handling in generated links
> >>
> >> esc_html() ensures that its input is properly UTF-8 encoded and marked
> >> as UTF-8 with to_utf8().  Make esc_param() (used for query parameters
> >> in generated URLs), esc_path_info() (for escaping path_info
> >> components) and esc_url() use it too.
> >>
> >> This hardens gitweb against errors in UTF-8 handling; because
> >> to_utf8() is idempotent it won't change correct output.
> [...]
> >>   sub esc_param {
> >>   	my $str = shift;
> >>   	return undef unless defined $str;
> >> +
> >> +	$str = to_utf8($str);
> >>   	$str =~ s/([^A-Za-z0-9\-_.~()\/:@ ]+)/CGI::escape($1)/eg;
> >>   	$str =~ s/ /\+/g;
> >> +
> >>   	return $str;
> >>   }   
>  
> > While trying to view a "blob_plain" of "Gütekritierien.txt", a 404 error
> > occured. "git_get_hash_by_path" tries to resolve the hash with the wrong
> > filename (git ls-tree -z HEAD -- Gütekriterien.txt) and fails.
> > 
> > The filename needs the correct encoding. Something like this is probably
> > needed for all filenames and should be done at a prior stage:
> 
> True.
> 
> First, I wonder why the tests I did for this situation didn't
> show any errors even before the "harden href()" patch. What
> is different in your config that you see those errors?
> 

Nothing special. It is reproducible with git 1.9.3 (Fedora 20), git
instaweb (lighttpd) and LANG=de_DE.UTF-8.  
 

What's cooking in git.git (Mar 2015, #08; Mon, 23)

[Junio C Hamano]

Here are the topics that have been cooking.  Commits prefixed with
'-' are only in 'pu' (proposed updates) while commits prefixed with
'+' are in 'next'.

This cycle is turning out to be a "shoot for product excellence"
release.  About half of the commits that have been merged to the
'master' so far have also been merged to 'maint' and v2.3.4 has been
tagged.

You can find the changes described here in the integration branches
of the repositories listed at

    http://git-blame.blogspot.com/p/git-public-repositories.html

--------------------------------------------------
[Graduated to "master"]

* jk/push-config (2015-03-14) 4 commits
  (merged to 'next' on 2015-03-16 at 6452570)
 + push: allow --follow-tags to be set by config push.followTags
 + cmd_push: pass "flags" pointer to config callback
 + cmd_push: set "atomic" bit directly
 + git_push_config: drop cargo-culted wt_status pointer

 Restructure "git push" codepath to make it easier to add new
 configuration bits and then add push.followTags configuration that
 turns --follow-tags option on by default.


* jk/smart-http-hide-refs (2015-03-12) 2 commits
  (merged to 'next' on 2015-03-16 at 530df4c)
 + upload-pack: do not check NULL return of lookup_unknown_object
 + upload-pack: fix transfer.hiderefs over smart-http

 The transfer.hiderefs support did not quite work for smart-http
 transport.


* jk/tag-h-column-is-a-listing-option (2015-03-12) 1 commit
  (merged to 'next' on 2015-03-16 at 42b04c6)
 + tag: fix some mis-organized options in "-h" listing

 "git tag -h" used to show the "--column" and "--sort" options
 that are about listing in a wrong section.


* jk/test-annoyances (2015-03-12) 5 commits
  (merged to 'next' on 2015-03-16 at 845b091)
 + t5551: make EXPENSIVE test cheaper
 + t5541: move run_with_cmdline_limit to test-lib.sh
 + t: pass GIT_TRACE through Apache
 + t: redirect stderr GIT_TRACE to descriptor 4
 + t: translate SIGINT to an exit

 Test fixes.


* js/completion-ctags-pattern-substitution-fix (2015-03-14) 1 commit
  (merged to 'next' on 2015-03-17 at 4a68861)
 + contrib/completion: escape the forward slash in __git_match_ctag

 The code that reads from the ctags file in the completion script
 (in contrib/) did not spell ${param/pattern/string} substitution
 correctly, which happened to work with bash but not with zsh.


* nd/config-doc-camelCase (2015-03-13) 1 commit
  (merged to 'next' on 2015-03-16 at 0e3fedb)
 + *config.txt: stick to camelCase naming convention

 Documentation updates.

--------------------------------------------------
[New Topics]

* jc/diff-no-index-d-f (2015-03-22) 1 commit
 - align D/F handling of "diff --no-index" with that of normal Git

 The usual "git diff" when seeing a file turning into a directory
 showed a patchset to remove the file and create all files in the
 directory, but "git diff --no-index" simply refused to work.


* jk/run-command-capture (2015-03-22) 7 commits
  (merged to 'next' on 2015-03-23 at f6db88b)
 + run-command: forbid using run_command with piped output
 + trailer: use capture_command
 + submodule: use capture_command
 + wt-status: use capture_command
 + run-command: introduce capture_command helper
 + wt_status: fix signedness mismatch in strbuf_read call
 + wt-status: don't flush before running "submodule status"

 The run-command interface was easy to abuse and make a pipe for us
 to read from the process, wait for the process to finish and then
 attempt to read its output, which is a pattern that lead to a
 deadlock.  Fix such uses by introducing a helper to do this
 correctly (i.e. we need to read first and then wait the process to
 finish) and also add code to prevent such abuse in the run-command
 helper.

 Will merge to 'master'.


* tg/fix-check-order-with-split-index (2015-03-20) 1 commit
  (merged to 'next' on 2015-03-23 at c361f8e)
 + read-cache: fix reading of split index

 The split-index mode introduced at v2.3.0-rc0~41 was broken in the
 codepath to protect us against a broken reimplementation of Git
 that writes an invalid index with duplicated index entries, etc.

 Will merge to 'master'.


* ts/man-pdf (2015-03-20) 1 commit
 - Documentation: make 'make pdf' format manpages to PDF as well

 For offline consumption of manual pages, a target to produce one
 pdf document per manual page was added to Documentation/Makefile.

 Its usefulness is unknown, given that this does not produce a
 single document with all the manual pages in it, which would be not
 much better than keeping a bunch of HTML manual pages we already
 produce and use them instead for offline consumption.


* nd/diff-i-t-a (2015-03-23) 1 commit
 - diff-lib.c: adjust position of i-t-a entries in diff


* sb/leaks (2015-03-23) 9 commits
  (merged to 'next' on 2015-03-23 at 5397daf)
 + read-cache: fix memleak
 + add_to_index(): free unused cache-entry
 + commit.c: fix a memory leak
 + http-push: remove unneeded cleanup
 + merge-recursive: fix memleaks
 + merge-blobs.c: fix a memleak
 + builtin/apply.c: fix a memleak
 + update-index: fix a memleak
 + read-cache: free cache entry in add_to_index in case of early return

 Will merge to 'master'.

--------------------------------------------------
[Stalled]

* mh/fdopen-with-retry (2015-03-06) 6 commits
 - buffer_fdinit(): use fdopen_with_retry()
 - update_info_file(): use fdopen_with_retry()
 - copy_to_log(): use fdopen_with_retry()
 - fdopen_lock_file(): use fdopen_with_retry()
 - SQUASH??? $gmane/264889
 - xfdopen(): if first attempt fails, free memory and try again

 Various parts of the code where they call fdopen() can fail when
 they run out of memory; attempt to proceed by retrying the
 operation after freeing some resource.

 Waiting for further comments.


* nd/untracked-cache (2015-03-12) 24 commits
 - git-status.txt: advertisement for untracked cache
 - untracked cache: guard and disable on system changes
 - mingw32: add uname()
 - t7063: tests for untracked cache
 - update-index: test the system before enabling untracked cache
 - update-index: manually enable or disable untracked cache
 - status: enable untracked cache
 - untracked-cache: temporarily disable with $GIT_DISABLE_UNTRACKED_CACHE
 - untracked cache: mark index dirty if untracked cache is updated
 - untracked cache: print stats with $GIT_TRACE_UNTRACKED_STATS
 - untracked cache: avoid racy timestamps
 - read-cache.c: split racy stat test to a separate function
 - untracked cache: invalidate at index addition or removal
 - untracked cache: load from UNTR index extension
 - untracked cache: save to an index extension
 - ewah: add convenient wrapper ewah_serialize_strbuf()
 - untracked cache: don't open non-existent .gitignore
 - untracked cache: mark what dirs should be recursed/saved
 - untracked cache: record/validate dir mtime and reuse cached output
 - untracked cache: make a wrapper around {open,read,close}dir()
 - untracked cache: invalidate dirs recursively if .gitignore changes
 - untracked cache: initial untracked cache validation
 - untracked cache: record .gitignore information and dir hierarchy
 - dir.c: optionally compute sha-1 of a .gitignore file

 Need extra sets of eyes to review this.


* ak/stash-store-create-help (2015-01-13) 1 commit
 - stash: show "create" and "store" subcommands in usage-help

 Will discard.


* bp/diff-relative-config (2015-01-07) 2 commits
 - diff: teach diff.relative to give default to --relative=<value>
 - diff: teach --no-relative to override earlier --relative

 No comments?  No interests?


* jc/diff-b-m (2015-02-23) 5 commits
 . WIPWIP
 . WIP: diff-b-m
 - diffcore-rename: allow easier debugging
 - diffcore-rename.c: add locate_rename_src()
 - diffcore-break: allow debugging

 "git diff -B -M" produced incorrect patch when the postimage of a
 completely rewritten file is similar to the preimage of a removed
 file; such a resulting file must not be expressed as a rename from
 other place.

 The fix in this patch is broken, unfortunately.


* pw/remote-set-url-fetch (2014-11-26) 1 commit
 - remote: add --fetch and --both options to set-url

 Expecting a reroll.


* je/quiltimport-no-fuzz (2014-10-21) 2 commits
 - git-quiltimport: flip the default not to allow fuzz
 - git-quiltimport.sh: allow declining fuzz with --exact option

 "quiltimport" drove "git apply" always with -C1 option to reduce
 context of the patch in order to give more chance to somewhat stale
 patches to apply.  Add an "--exact" option to disable, and also
 "-C$n" option to customize this behaviour.  The top patch
 optionally flips the default to "--exact".

 Tired of waiting for an Ack
 Will discard.


* tr/remerge-diff (2014-11-10) 9 commits
 - t4213: avoid "|" in sed regexp
 - log --remerge-diff: show what the conflict resolution changed
 - name-hash: allow dir hashing even when !ignore_case
 - merge-recursive: allow storing conflict hunks in index
 - merge_diff_mode: fold all merge diff variants into an enum
 - combine-diff: do not pass revs->dense_combined_merges redundantly
 - merge-recursive: -Xindex-only to leave worktree unchanged
 - merge-recursive: internal flag to avoid touching the worktree
 - merge-recursive: remove dead conditional in update_stages()

 "log -p" output learns a new way to let users inspect a merge
 commit by showing the differences between the automerged result
 with conflicts the person who recorded the merge would have seen
 and the final conflict resolution that was recorded in the merge.

 Waiting for a reroll ($gmane/256591).


* hv/submodule-config (2014-11-11) 4 commits
 - do not die on error of parsing fetchrecursesubmodules option
 - use new config API for worktree configurations of submodules
 - extract functions for submodule config set and lookup
 - implement submodule config cache for lookup of submodule names

 Kicked back to 'pu' per request ($gmane/255610).


* ab/add-interactive-show-diff-func-name (2014-05-12) 2 commits
 - SQUASH??? git-add--interactive: Preserve diff heading when splitting hunks
 - git-add--interactive: Preserve diff heading when splitting hunks

 Waiting for a reroll.


* jn/gitweb-utf8-in-links (2014-05-27) 1 commit
 - gitweb: Harden UTF-8 handling in generated links

 $gmane/250758?


* ss/userdiff-update-csharp-java (2014-06-02) 2 commits
 - userdiff: support Java try keyword
 - userdiff: support C# async methods and correct C# keywords

 Reviews sent; waiting for a response.


* bg/rebase-off-of-previous-branch (2014-04-16) 1 commit
 - git-rebase: print name of rev when using shorthand

 Teach "git rebase -" to report the concrete name of the branch
 (i.e. the previous one).

 But it stops short and does not do the same for "git rebase @{-1}".
 Expecting a reroll.


* rb/merge-prepare-commit-msg-hook (2014-01-10) 4 commits
 - merge: drop unused arg from abort_commit method signature
 - merge: make prepare_to_commit responsible for write_merge_state
 - t7505: ensure cleanup after hook blocks merge
 - t7505: add missing &&

 Expose more merge states (e.g. $GIT_DIR/MERGE_MODE) to hooks that
 run during "git merge".  The log message stresses too much on one
 hook, prepare-commit-msg, but it would equally apply to other hooks
 like post-merge, I think.

 Waiting for a reroll.


* jc/graph-post-root-gap (2013-12-30) 3 commits
 - WIP: document what we want at the end
 - graph: remove unused code a bit
 - graph: stuff the current commit into graph->columns[]

 This was primarily a RFH ($gmane/239580).


* tg/perf-lib-test-perf-cleanup (2013-09-19) 2 commits
 - perf-lib: add test_perf_cleanup target
 - perf-lib: split starting the test from the execution

 Add test_perf_cleanup shell function to the perf suite, that allows
 the script writers to define a test with a clean-up action.

 Will hold.


* jc/show-branch (2014-03-24) 5 commits
 - show-branch: use commit slab to represent bitflags of arbitrary width
 - show-branch.c: remove "all_mask"
 - show-branch.c: abstract out "flags" operation
 - show-branch.c: lift all_mask/all_revs to a global static
 - show-branch.c: update comment style

 Waiting for the final step to lift the hard-limit before sending it out.

--------------------------------------------------
[Cooking]

* jk/cleanup-failed-clone (2015-03-19) 2 commits
  (merged to 'next' on 2015-03-23 at 1f26d93)
 + clone: drop period from end of die_errno message
 + clone: initialize atexit cleanup handler earlier

 An failure early in the "git clone" that started creating the
 working tree and repository could have resulted in some directories
 and files left without getting cleaned up.

 Will merge to 'master'.


* jk/fetch-pack (2015-03-19) 4 commits
  (merged to 'next' on 2015-03-23 at 4357f3d)
 + fetch-pack: remove dead assignment to ref->new_sha1
 + fetch_refs_via_pack: free extra copy of refs
 + filter_ref: make a copy of extra "sought" entries
 + filter_ref: avoid overwriting ref->old_sha1 with garbage

 "git fetch" that fetches a commit using the allow-tip-sha1-in-want
 extension could have failed to fetch all the requested refs.

 Will merge to 'master'.


* jk/prune-with-corrupt-refs (2015-03-20) 5 commits
  (merged to 'next' on 2015-03-23 at 68af8a9)
 + refs.c: drop curate_packed_refs
 + repack: turn on "ref paranoia" when doing a destructive repack
 + prune: turn on ref_paranoia flag
 + refs: introduce a "ref paranoia" flag
 + t5312: test object deletion code paths in a corrupted repository

 "git prune" used to largely ignore broken refs when deciding which
 objects are still being used, which could spread an existing small
 damage and make it a larger one.

 Will merge to 'master'.


* jk/simplify-csum-file-sha1fd-check (2015-03-19) 1 commit
  (merged to 'next' on 2015-03-20 at 6f6d1c2)
 + sha1fd_check: die when we cannot open the file

 Code simplification.

 Will merge to 'master'.


* mh/numparse (2015-03-19) 14 commits
 - diff_opt_parse(): use convert_i() when handling --abbrev=<num>
 - diff_opt_parse(): use convert_i() when handling "-l<num>"
 - opt_arg(): simplify pointer handling
 - opt_arg(): report errors parsing option values
 - opt_arg(): use convert_i() in implementation
 - opt_arg(): val is always non-NULL
 - builtin_diff(): detect errors when parsing --unified argument
 - handle_revision_opt(): use convert_ui() when handling "--abbrev="
 - strtoul_ui(), strtol_i(): remove functions
 - handle_revision_opt(): use convert_i() when handling "-<digit>"
 - handle_revision_opt(): use skip_prefix() in many places
 - write_subdirectory(): use convert_ui() for parsing mode
 - cacheinfo_callback(): use convert_ui() when handling "--cacheinfo"
 - numparse: new module for parsing integral numbers

 Many codepaths use unchecked use of strtol() and friends (or even
 worse, atoi()).  Introduce a set of wrappers that try to be more
 careful.


* tf/gitweb-project-listing (2015-03-19) 5 commits
 - gitweb: make category headings into links when they are directories
 - gitweb: optionally set project category from its pathname
 - gitweb: add a link under the search box to clear a project filter
 - gitweb: if the PATH_INFO is incomplete, use it as a project_filter
 - gitweb: fix typo in man page

 Update gitweb to make it more pleasant to deal with a hierarchical
 forest of repositories.

 Any comments from those who use or have their own code in Gitweb?


* ws/grep-quiet-no-pager (2015-03-19) 1 commit
  (merged to 'next' on 2015-03-20 at b3f5fe6)
 + grep: fix "--quiet" overwriting current output

 Even though "git grep --quiet" is run merely to ask for the exit
 status, we spawned the pager regardless.  Stop doing that.

 Will merge to 'master'.


* kd/rev-list-bisect-first-parent (2015-03-19) 1 commit
  (merged to 'next' on 2015-03-20 at 5477bf5)
 + rev-list: refuse --first-parent combined with --bisect

 "git rev-list --bisect --first-parent" does not work (yet) and can
 even cause SEGV; forbid it.  "git log --bisect --first-parent"
 would not be useful until "git bisect --first-parent" materializes,
 so it is also forbidden for now.

 Will merge to 'master'.


* jk/test-chain-lint (2015-03-22) 28 commits
 - t6039: fix broken && chain
 - t9158, t9161: fix broken &&-chain in git-svn tests
 - t9104: fix test for following larger parents
 - t4104: drop hand-rolled error reporting
 - t0005: fix broken &&-chains
 - t7004: fix embedded single-quotes
 - t0050: appease --chain-lint
 - t9001: use test_when_finished
 - t4117: use modern test_* helpers
 - t6034: use modern test_* helpers
 - t1301: use modern test_* helpers
 - t0020: use modern test_* helpers
 - t6030: use modern test_* helpers
 - t9502: fix &&-chain breakage
 - t7201: fix &&-chain breakage
 - t3600: fix &&-chain breakage for setup commands
 - t: avoid using ":" for comments
 - t: wrap complicated expect_code users in a block
 - t: use test_expect_code instead of hand-rolled comparison
 - t: use test_might_fail for diff and grep
 - t: fix &&-chaining issues around setup which might fail
 - t: use test_must_fail instead of hand-rolled blocks
 - t: use verbose instead of hand-rolled errors
 - t: assume test_cmp produces verbose output
 - t: fix trivial &&-chain breakage
 - t: fix moderate &&-chain breakage
 - t: fix severe &&-chain breakage
 - t/test-lib: introduce --chain-lint option

 People often forget to chain the commands in their test together
 with &&, leaving a failure from an earlier command in the test go
 unnoticed.  The new GIT_TEST_CHAIN_LINT mechanism allows you to
 catch such a mistake more easily.

 What I queued here has fix to the issue J6t found in 15/25 squashed
 in, and also has 26/25 and 27/25 follow-up fixes from Michael, plus
 28/25 follow-up from Torsten.  If everybody involved is happy with
 it, we can just proceed with this copy, otherwise I'll let Peff
 reroll.  I am happy either way.


* tg/test-index-v4 (2015-03-20) 1 commit
  (merged to 'next' on 2015-03-23 at 6b2eb0a)
 + t1700: make test pass with index-v4

 A test fix.

 Will merge to 'master'.


* jc/a-lone-dash-stands-for-previous-branch (2015-03-16) 1 commit
 - "-" and "@{-1}" on various programs

 Lose special case code to make a lone dash "-" mean the previous
 branch aka "@{-1}" from a handful subcommands, and instead support
 the notation throughout the system by reimplementing it at the
 revisions layer.

 Needs tests, documentation updates, etc.  Also does only a half-way
 job dealing with range notation, which needs to be fixed before the
 series goes anywhere.


* jc/submitting-patches-mention-send-email (2015-03-15) 1 commit
  (merged to 'next' on 2015-03-23 at a9581fd)
 + SubmittingPatches: encourage users to use format-patch and send-email

 Recommend format-patch and send-email for those who want to submit
 patches to this project.

 Will merge to 'master'.


* as/userdiff-sh (2015-03-13) 1 commit
 - userdiff: funcname and word patterns for sh

 Add a built-in "userdiff" patterns to word-split and identify
 notable lines in shell scripts to help presentation of diff and
 grep output.

 Comments???


* bc/object-id (2015-03-13) 10 commits
 - apply: convert threeway_stage to object_id
 - patch-id: convert to use struct object_id
 - commit: convert parts to struct object_id
 - diff: convert struct combine_diff_path to object_id
 - bulk-checkin.c: convert to use struct object_id
 - zip: use GIT_SHA1_HEXSZ for trailers
 - archive.c: convert to use struct object_id
 - bisect.c: convert leaf functions to use struct object_id
 - define utility functions for object IDs
 - define a structure for object IDs

 Identify parts of the code that knows that we use SHA-1 hash to
 name our objects too much, and use (1) symbolic constants instead
 of hardcoded 20 as byte count and/or (2) use struct object_id
 instead of unsigned char [20] for object names.

 Will merge to and cook in 'next'.


* ct/prompt-untracked-fix (2015-03-15) 1 commit
  (merged to 'next' on 2015-03-20 at 0d57eaf)
 + git prompt: use toplevel to find untracked files

 The prompt script (in contrib/) did not show the untracked sign
 when working in a subdirectory without any untracked files.

 Will merge to 'master'.


* dj/log-graph-with-no-walk (2015-03-19) 1 commit
  (merged to 'next' on 2015-03-20 at cb636c0)
 + revision: forbid combining --graph and --no-walk

 "git log --graph --no-walk A B..." is a otcnflicting request that
 asks nonsense; no-walk tells us show discrete points in the
 history, while graph asks to draw connections between these
 discrete points. Forbid the combination.

 Will merge to 'master'.


* nd/slim-index-pack-memory-usage (2015-02-27) 2 commits
 - index-pack: kill union delta_base to save memory
 - index-pack: reduce object_entry size to save memory

 Memory usage of "git index-pack" has been trimmed by tens of
 per-cent.

 Waiting for further comments.


* nd/list-files (2015-02-09) 21 commits
 - t3080: tests for git-list-files
 - list-files: -M aka diff-cached
 - list-files -F: show submodules with the new indicator '&'
 - list-files: add -F/--classify
 - list-files: show directories as well as files
 - list-files: do not show duplicate cached entries
 - list-files: sort output and remove duplicates
 - list-files: add -t back
 - list-files: add -1 short for --no-column
 - list-files: add -R/--recursive short for --max-depth=-1
 - list-files: -u does not imply showing stages
 - list-files: make alias 'ls' default to 'list-files'
 - list-files: a user friendly version of ls-files and more
 - ls-files: support --max-depth
 - ls-files: add --column
 - ls-files: add --color to highlight file names
 - ls-files: buffer full item in strbuf before printing
 - ls_colors.c: highlight submodules like directories
 - ls_colors.c: add a function to color a file name
 - ls_colors.c: parse color.ls.* from config file
 - ls_colors.c: add $LS_COLORS parsing code

 A new "git list-files" Porcelain command, "ls-files" with bells and
 whistles.

 Reroll to base on wt-status work ($gmane/265142) has seen some
 positive discussions. Waiting for a further polished reroll
 ($gmane/265534).


* js/fsck-opt (2015-01-21) 19 commits
 - fsck: support ignoring objects in `git fsck` via fsck.skiplist
 - fsck: git receive-pack: support excluding objects from fsck'ing
 - fsck: introduce `git fsck --quick`
 - fsck: support demoting errors to warnings
 - fsck: document the new receive.fsck.* options
 - fsck: allow upgrading fsck warnings to errors
 - fsck: optionally ignore specific fsck issues completely
 - fsck: disallow demoting grave fsck errors to warnings
 - fsck: add a simple test for receive.fsck.*
 - fsck: make fsck_tag() warn-friendly
 - fsck: handle multiple authors in commits specially
 - fsck: make fsck_commit() warn-friendly
 - fsck: make fsck_ident() warn-friendly
 - fsck: report the ID of the error/warning
 - fsck: allow demoting errors to warnings via receive.fsck.warn = <key>
 - fsck: offer a function to demote fsck errors to warnings
 - fsck: provide a function to parse fsck message IDs
 - fsck: introduce identifiers for fsck messages
 - fsck: introduce fsck options

 "fsck.warnings = <list of error tokens>" I suggested turned out to
 be an unpopular choice (sorry Dscho).

 Expecting a reroll.


* nd/multiple-work-trees (2015-03-20) 39 commits
  (merged to 'next' on 2015-03-20 at cc98ed0)
 + t2026: fix broken &&-chain
  (merged to 'next' on 2015-02-18 at b51f696)
 + t2026 needs procondition SANITY
 + git-checkout.txt: a note about multiple checkout support for submodules
 + checkout: add --ignore-other-wortrees
 + checkout: pass whole struct to parse_branchname_arg instead of individual flags
 + git-common-dir: make "modules/" per-working-directory directory
 + checkout: do not fail if target is an empty directory
 + t2025: add a test to make sure grafts is working from a linked checkout
 + checkout: don't require a work tree when checking out into a new one
 + git_path(): keep "info/sparse-checkout" per work-tree
 + count-objects: report unused files in $GIT_DIR/worktrees/...
 + gc: support prune --worktrees
 + gc: factor out gc.pruneexpire parsing code
 + gc: style change -- no SP before closing parenthesis
 + checkout: clean up half-prepared directories in --to mode
 + checkout: reject if the branch is already checked out elsewhere
 + prune: strategies for linked checkouts
 + checkout: support checking out into a new working directory
 + use new wrapper write_file() for simple file writing
 + wrapper.c: wrapper to open a file, fprintf then close
 + setup.c: support multi-checkout repo setup
 + setup.c: detect $GIT_COMMON_DIR check_repository_format_gently()
 + setup.c: convert check_repository_format_gently to use strbuf
 + setup.c: detect $GIT_COMMON_DIR in is_git_directory()
 + setup.c: convert is_git_directory() to use strbuf
 + git-stash: avoid hardcoding $GIT_DIR/logs/....
 + *.sh: avoid hardcoding $GIT_DIR/hooks/...
 + git-sh-setup.sh: use rev-parse --git-path to get $GIT_DIR/objects
 + $GIT_COMMON_DIR: a new environment variable
 + commit: use SEQ_DIR instead of hardcoding "sequencer"
 + fast-import: use git_path() for accessing .git dir instead of get_git_dir()
 + reflog: avoid constructing .lock path with git_path
 + *.sh: respect $GIT_INDEX_FILE
 + git_path(): be aware of file relocation in $GIT_DIR
 + path.c: group git_path(), git_pathdup() and strbuf_git_path() together
 + path.c: rename vsnpath() to do_git_path()
 + git_snpath(): retire and replace with strbuf_git_path()
 + path.c: make get_pathname() call sites return const char *
 + path.c: make get_pathname() return strbuf instead of static buffer

 A replacement for contrib/workdir/git-new-workdir that does not
 rely on symbolic links and make sharing of objects and refs safer
 by making the borrowee and borrowers aware of each other.

 Will cook in 'next'.

[PATCH v4] remote: add --fetch and --both options to set-url

[Peter Wu]

git remote set-url knew about the '--push' option to update just the
pushurl, but it does not have a similar option for "update fetch URL and
leave whatever was in place for the push URL".

This patch adds support for a '--fetch' option which implements that use
case in a backwards compatible way: if no --both, --push or --fetch
options are given, then the push URL is modified too if it was not set
before. This is the case since the push URL is implicitly based on the
fetch URL.

A '--both' option is added to make the command independent of previous
pushurl settings. For the --add and --delete set operations, it will
always set the push and/ or the fetch URLs. For the primary mode of
operation (without --add or --delete), it will drop pushurl as the
implicit push URL is the (fetch) URL.

While '--both' could be implemented as '--fetch --push', it might also
be mistaken for "--push overrides --fetch". An option such as
"--only={fetch|push|both}" was also considered, but it is longer than
the current options, makes --push redundant and brings the confusing
option "--only=both". Options such as '--direction=...' is too long and
'--dir=' is ambiguous ("directory"?). Thus, for brevity three new
options were introduced.

The documentation has also been updated and a missing '--push' option
is added to the 'git remote -h' command.

Tests are also added to verify the documented behavior.

Signed-off-by: Peter Wu <peter@lekensteyn.nl>
---
Hi,

This is the fourth revision of the patch that allows for just setting the fetch
URL. Eric wondered why '--fetch --push' is not used to describe the state
'--both', so I added this to the commit message.

The t5505-remote.sh test still passes after this change (I was unable to run the
full test suite as it broke due to an unrelated gpg issue).

Kind regards,
Peter

 v2: fixed test case
 v3: added --both option, changed --fetch --push behavior, added more tests for
     --add/--delete cases, refactored to reduce duplication (not special-casing
     add_mode without oldurl, just skip initially setting oldurl).
 v4: incorporated documentation suggestion from Eric Sunshine;
     Tried to make the code easier to follow by replacing
     (modify_type == MODIFY_TYPE_FETCH) by
     (modify_type & MODIFY_TYPE_FETCH && modify_type != MODIFY_TYPE_HISTORIC)
     and added comments explaining why this is special (observed by Jeff King);
     Fixed dangling else issue reported by Torsten Bögershausen;
     Added considerations for other options to the commit message;
     Rebased on v2.2.0-65-g9abc44b.
---

 Documentation/git-remote.txt |  16 +++--
 builtin/remote.c             | 146 ++++++++++++++++++++++++++++---------------
 t/t5505-remote.sh            | 125 ++++++++++++++++++++++++++++++++++++
 3 files changed, 234 insertions(+), 53 deletions(-)

diff --git a/Documentation/git-remote.txt b/Documentation/git-remote.txt
index cb103c8..09a4670 100644
--- a/Documentation/git-remote.txt
+++ b/Documentation/git-remote.txt
@@ -15,9 +15,9 @@ SYNOPSIS
 'git remote remove' <name>
 'git remote set-head' <name> (-a | --auto | -d | --delete | <branch>)
 'git remote set-branches' [--add] <name> <branch>...
-'git remote set-url' [--push] <name> <newurl> [<oldurl>]
-'git remote set-url --add' [--push] <name> <newurl>
-'git remote set-url --delete' [--push] <name> <url>
+'git remote set-url' [--both | --fetch | --push] <name> <newurl> [<oldurl>]
+'git remote set-url' [--both | --fetch | --push] '--add' <name> <newurl>
+'git remote set-url' [--both | --fetch | --push] '--delete' <name> <url>
 'git remote' [-v | --verbose] 'show' [-n] <name>...
 'git remote prune' [-n | --dry-run] <name>...
 'git remote' [-v | --verbose] 'update' [-p | --prune] [(<group> | <remote>)...]
@@ -134,7 +134,15 @@ Changes URL remote points to. Sets first URL remote points to matching
 regex <oldurl> (first URL if no <oldurl> is given) to <newurl>. If
 <oldurl> doesn't match any URL, error occurs and nothing is changed.
 +
-With '--push', push URLs are manipulated instead of fetch URLs.
+With '--both', both the fetch and push URLs are manipulated.
++
+With '--fetch', only fetch URLs are manipulated.
++
+With '--push', only push URLs are manipulated.
++
+For historical reasons, if neither --fetch nor --push is specified then the
+fetch URL is changed, as well as the push URL if this was not already set. This
+behavior may change in the future.
 +
 With '--add', instead of changing some URL, new URL is added.
 +
diff --git a/builtin/remote.c b/builtin/remote.c
index 7f28f92..1fa2fd7 100644
--- a/builtin/remote.c
+++ b/builtin/remote.c
@@ -18,9 +18,9 @@ static const char * const builtin_remote_usage[] = {
 	N_("git remote prune [-n | --dry-run] <name>"),
 	N_("git remote [-v | --verbose] update [-p | --prune] [(<group> | <remote>)...]"),
 	N_("git remote set-branches [--add] <name> <branch>..."),
-	N_("git remote set-url [--push] <name> <newurl> [<oldurl>]"),
-	N_("git remote set-url --add <name> <newurl>"),
-	N_("git remote set-url --delete <name> <url>"),
+	N_("git remote set-url [--both | --fetch | --push] <name> <newurl> [<oldurl>]"),
+	N_("git remote set-url [--both | --fetch | --push] --add <name> <newurl>"),
+	N_("git remote set-url [--both | --fetch | --push] --delete <name> <url>"),
 	NULL
 };
 
@@ -66,9 +66,9 @@ static const char * const builtin_remote_update_usage[] = {
 };
 
 static const char * const builtin_remote_seturl_usage[] = {
-	N_("git remote set-url [--push] <name> <newurl> [<oldurl>]"),
-	N_("git remote set-url --add <name> <newurl>"),
-	N_("git remote set-url --delete <name> <url>"),
+	N_("git remote set-url [--both | --fetch | --push] <name> <newurl> [<oldurl>]"),
+	N_("git remote set-url [--both | --fetch | --push] --add <name> <newurl>"),
+	N_("git remote set-url [--both | --fetch | --push] --delete <name> <url>"),
 	NULL
 };
 
@@ -1503,21 +1503,39 @@ static int set_branches(int argc, const char **argv)
 	return set_remote_branches(argv[0], argv + 1, add_mode);
 }
 
+#define MODIFY_TYPE_FETCH       (1 << 0)
+#define MODIFY_TYPE_PUSH        (1 << 1)
+#define MODIFY_TYPE_BOTH        (MODIFY_TYPE_FETCH | MODIFY_TYPE_PUSH)
+/* The historic behavior behaves like --fetch, but does not touch the push URL
+ * configuration (and thereby may appear to change the push URL too if this was
+ * not set before).
+ */
+#define MODIFY_TYPE_HISTORIC    (MODIFY_TYPE_FETCH | (1 << 2))
+
 static int set_url(int argc, const char **argv)
 {
-	int i, push_mode = 0, add_mode = 0, delete_mode = 0;
+	int i, add_mode = 0, delete_mode = 0;
+	int modify_type = MODIFY_TYPE_HISTORIC;
 	int matches = 0, negative_matches = 0;
 	const char *remotename = NULL;
 	const char *newurl = NULL;
 	const char *oldurl = NULL;
 	struct remote *remote;
 	regex_t old_regex;
-	const char **urlset;
-	int urlset_nr;
-	struct strbuf name_buf = STRBUF_INIT;
+	struct strbuf name_buf_fetch = STRBUF_INIT;
+	struct strbuf name_buf_push = STRBUF_INIT;
 	struct option options[] = {
-		OPT_BOOL('\0', "push", &push_mode,
-			 N_("manipulate push URLs")),
+		OPT_GROUP(""),
+		OPT_SET_INT('\0', "fetch", &modify_type,
+			N_("manipulate just fetch URLs"),
+			MODIFY_TYPE_FETCH),
+		OPT_SET_INT('\0', "push", &modify_type,
+			N_("manipulate just push URLs"),
+			MODIFY_TYPE_PUSH),
+		OPT_SET_INT('\0', "both", &modify_type,
+			N_("manipulate both push and fetch URLs"),
+			MODIFY_TYPE_BOTH),
+		OPT_GROUP(""),
 		OPT_BOOL('\0', "add", &add_mode,
 			 N_("add URL")),
 		OPT_BOOL('\0', "delete", &delete_mode,
@@ -1535,7 +1553,8 @@ static int set_url(int argc, const char **argv)
 
 	remotename = argv[1];
 	newurl = argv[2];
-	if (argc > 3)
+	/* The old URL is only meaningful for the primary non-set operation. */
+	if (argc > 3 && !add_mode && !delete_mode)
 		oldurl = argv[3];
 
 	if (delete_mode)
@@ -1545,47 +1564,76 @@ static int set_url(int argc, const char **argv)
 		die(_("No such remote '%s'"), remotename);
 	remote = remote_get(remotename);
 
-	if (push_mode) {
-		strbuf_addf(&name_buf, "remote.%s.pushurl", remotename);
-		urlset = remote->pushurl;
-		urlset_nr = remote->pushurl_nr;
-	} else {
-		strbuf_addf(&name_buf, "remote.%s.url", remotename);
-		urlset = remote->url;
-		urlset_nr = remote->url_nr;
-	}
+	strbuf_addf(&name_buf_fetch, "remote.%s.url", remotename);
+	strbuf_addf(&name_buf_push, "remote.%s.pushurl", remotename);
 
-	/* Special cases that add new entry. */
-	if ((!oldurl && !delete_mode) || add_mode) {
-		if (add_mode)
-			git_config_set_multivar(name_buf.buf, newurl,
-				"^$", 0);
-		else
-			git_config_set(name_buf.buf, newurl);
-		strbuf_release(&name_buf);
-		return 0;
-	}
+	if (oldurl && !add_mode) {
+		/* Old URL specified, or deletion. Demand that one matches. */
+		if (regcomp(&old_regex, oldurl, REG_EXTENDED))
+			die(_("Invalid old URL pattern: %s"), oldurl);
 
-	/* Old URL specified. Demand that one matches. */
-	if (regcomp(&old_regex, oldurl, REG_EXTENDED))
-		die(_("Invalid old URL pattern: %s"), oldurl);
+		if (modify_type & MODIFY_TYPE_FETCH)
+			for (i = 0; i < remote->url_nr; i++) {
+				if (!regexec(&old_regex, remote->url[i], 0, NULL, 0))
+					matches++;
+				else
+					negative_matches++;
+			}
+		if (delete_mode && !negative_matches && modify_type & MODIFY_TYPE_FETCH)
+			die(_("Will not delete all non-push URLs"));
+		if (modify_type & MODIFY_TYPE_PUSH)
+			for (i = 0; i < remote->pushurl_nr; i++) {
+				if (!regexec(&old_regex, remote->pushurl[i], 0, NULL, 0))
+					matches++;
+				else
+					negative_matches++;
+			}
+		if (!delete_mode && !matches)
+			die(_("No such URL found: %s"), oldurl);
 
-	for (i = 0; i < urlset_nr; i++)
-		if (!regexec(&old_regex, urlset[i], 0, NULL, 0))
-			matches++;
-		else
-			negative_matches++;
-	if (!delete_mode && !matches)
-		die(_("No such URL found: %s"), oldurl);
-	if (delete_mode && !negative_matches && !push_mode)
-		die(_("Will not delete all non-push URLs"));
+		regfree(&old_regex);
+	}
 
-	regfree(&old_regex);
+	/* Make the implicit push URL explicit if the fetch URL is modified,
+	 * except when in the historic mode (then the pushurl configuration is
+	 * not changed). */
+	if (modify_type & MODIFY_TYPE_FETCH &&
+			modify_type != MODIFY_TYPE_HISTORIC &&
+			remote->pushurl_nr == 0 && remote->url_nr > 0)
+		for (i = 0; i < remote->url_nr; i++)
+			git_config_set_multivar(name_buf_push.buf,
+				remote->url[i], "^$", 0);
+
+	/* Set the new entry value (not a --add or --delete operation). */
+	if (!add_mode && !delete_mode && !oldurl) {
+		if (modify_type & MODIFY_TYPE_FETCH)
+			git_config_set(name_buf_fetch.buf, newurl);
+		/* URLs will be the same, so remove pushurl. */
+		if (modify_type == MODIFY_TYPE_BOTH)
+			git_config_set(name_buf_push.buf, NULL);
+		else if (modify_type == MODIFY_TYPE_PUSH)
+			git_config_set(name_buf_push.buf, newurl);
+
+		goto cleanup_ok;
+	}
 
-	if (!delete_mode)
-		git_config_set_multivar(name_buf.buf, newurl, oldurl, 0);
-	else
-		git_config_set_multivar(name_buf.buf, NULL, oldurl, 1);
+	/* Set operations (--add, --delete) or change request (oldurl given). */
+	if (delete_mode) {
+		if (modify_type & MODIFY_TYPE_FETCH)
+			git_config_set_multivar(name_buf_fetch.buf, NULL, oldurl, 1);
+		if (modify_type & MODIFY_TYPE_PUSH)
+			git_config_set_multivar(name_buf_push.buf, NULL, oldurl, 1);
+	} else {
+		if (add_mode) /* Do not replace oldurl, but add a new one. */
+			oldurl = "^$";
+		if (modify_type & MODIFY_TYPE_FETCH)
+			git_config_set_multivar(name_buf_fetch.buf, newurl, oldurl, 0);
+		if (modify_type & MODIFY_TYPE_PUSH)
+			git_config_set_multivar(name_buf_push.buf, newurl, oldurl, 0);
+	}
+cleanup_ok:
+	strbuf_release(&name_buf_fetch);
+	strbuf_release(&name_buf_push);
 	return 0;
 }
 
diff --git a/t/t5505-remote.sh b/t/t5505-remote.sh
index ac79dd9..390281a 100755
--- a/t/t5505-remote.sh
+++ b/t/t5505-remote.sh
@@ -961,6 +961,59 @@ test_expect_success 'remote set-url --push zot' '
 	cmp expect actual
 '
 
+test_expect_success 'remote set-url --fetch zox' '
+	git remote set-url --fetch someremote zox &&
+	echo zox >expect &&
+	echo "YYY" >>expect &&
+	echo zot >>expect &&
+	git config --get-all remote.someremote.url >actual &&
+	echo "YYY" >>actual &&
+	git config --get-all remote.someremote.pushurl >>actual &&
+	cmp expect actual
+'
+
+test_expect_success 'remote set-url --both foo' '
+	git remote set-url --both someremote foo &&
+	echo "YYY" >expect &&
+	echo foo >>expect &&
+	test_must_fail git config --get-all remote.someremote.pushurl >actual &&
+	echo "YYY" >>actual &&
+	git config --get-all remote.someremote.url >>actual &&
+	cmp expect actual
+'
+
+test_expect_success 'remote set-url --delete --push foo' '
+	git remote set-url --delete --push someremote foo &&
+	echo "YYY" >expect &&
+	echo foo >>expect &&
+	test_must_fail git config --get-all remote.someremote.pushurl >actual &&
+	echo "YYY" >actual &&
+	git config --get-all remote.someremote.url >>actual &&
+	cmp expect actual
+'
+
+test_expect_success 'remote set-url --push zot' '
+	git remote set-url --push someremote zot &&
+	echo zot >expect &&
+	echo "YYY" >>expect &&
+	echo foo >>expect &&
+	git config --get-all remote.someremote.pushurl >actual &&
+	echo "YYY" >>actual &&
+	git config --get-all remote.someremote.url >>actual &&
+	cmp expect actual
+'
+
+test_expect_success 'remote set-url --fetch baz foo' '
+	git remote set-url --fetch someremote baz foo &&
+	echo zot >expect &&
+	echo "YYY" >>expect &&
+	echo baz >>expect &&
+	git config --get-all remote.someremote.pushurl >actual &&
+	echo "YYY" >>actual &&
+	git config --get-all remote.someremote.url >>actual &&
+	cmp expect actual
+'
+
 test_expect_success 'remote set-url --push qux zot' '
 	git remote set-url --push someremote qux zot &&
 	echo qux >expect &&
@@ -1091,6 +1144,78 @@ test_expect_success 'remote set-url --delete baz' '
 	cmp expect actual
 '
 
+test_expect_success 'remote set-url --fetch --add bar' '
+	git remote set-url --fetch --add someremote bar &&
+	echo ccc >expect &&
+	echo "YYY" >>expect &&
+	echo ccc >>expect &&
+	echo bar >>expect &&
+	git config --get-all remote.someremote.pushurl >actual &&
+	echo "YYY" >>actual &&
+	git config --get-all remote.someremote.url >>actual &&
+	cmp expect actual
+'
+
+test_expect_success 'remote set-url --both --add foo' '
+	git remote set-url --both --add someremote foo &&
+	echo ccc >expect &&
+	echo foo >>expect &&
+	echo "YYY" >>expect &&
+	echo ccc >>expect &&
+	echo bar >>expect &&
+	echo foo >>expect &&
+	git config --get-all remote.someremote.pushurl >actual &&
+	echo "YYY" >>actual &&
+	git config --get-all remote.someremote.url >>actual &&
+	cmp expect actual
+'
+
+test_expect_success 'remote set-url --both --delete ccc' '
+	git remote set-url --both --delete someremote ccc &&
+	echo foo >expect &&
+	echo "YYY" >>expect &&
+	echo bar >>expect &&
+	echo foo >>expect &&
+	git config --get-all remote.someremote.pushurl >actual &&
+	echo "YYY" >>actual &&
+	git config --get-all remote.someremote.url >>actual &&
+	cmp expect actual
+'
+
+test_expect_success 'remote set-url --fetch --delete bar' '
+	git remote set-url --fetch --delete someremote bar &&
+	echo foo >expect &&
+	echo "YYY" >>expect &&
+	echo foo >>expect &&
+	git config --get-all remote.someremote.pushurl >actual &&
+	echo "YYY" >>actual &&
+	git config --get-all remote.someremote.url >>actual &&
+	cmp expect actual
+'
+
+test_expect_success 'remote set-url --push --add baz' '
+	git remote set-url --push --add someremote baz &&
+	echo foo >expect &&
+	echo baz >>expect &&
+	echo "YYY" >>expect &&
+	echo foo >>expect &&
+	git config --get-all remote.someremote.pushurl >actual &&
+	echo "YYY" >>actual &&
+	git config --get-all remote.someremote.url >>actual &&
+	cmp expect actual
+'
+
+test_expect_success 'remote set-url --push --delete foo' '
+	git remote set-url --push --delete someremote foo &&
+	echo baz >expect &&
+	echo "YYY" >>expect &&
+	echo foo >>expect &&
+	git config --get-all remote.someremote.pushurl >actual &&
+	echo "YYY" >>actual &&
+	git config --get-all remote.someremote.url >>actual &&
+	cmp expect actual
+'
+
 test_expect_success 'extra args: setup' '
 	# add a dummy origin so that this does not trigger failure
 	git remote add origin .
-- 
2.1.3

Re: [PATCH v4] remote: add --fetch and --both options to set-url

[Jeff King]

On Wed, Dec 17, 2014 at 03:18:56PM +0100, Peter Wu wrote:

> This is the fourth revision of the patch that allows for just setting the fetch
> URL. Eric wondered why '--fetch --push' is not used to describe the state
> '--both', so I added this to the commit message.

Thanks, I think that explanation is very clear.

This version overall looks good to me.

> The t5505-remote.sh test still passes after this change (I was unable to run the
> full test suite as it broke due to an unrelated gpg issue).

It is it because you have gpg 2.1, and the patches to handle that are
not yet merged to master? Or is it because you are using the new patches
and they are breaking things for your older gpg version?

If the former, that's fine. If the latter, it would be nice to see a
report of the breakage. :)

-Peff

Re: [PATCH v4] remote: add --fetch and --both options to set-url

[Peter Wu]

On Wednesday 17 December 2014 09:32:13 Jeff King wrote:
> On Wed, Dec 17, 2014 at 03:18:56PM +0100, Peter Wu wrote:
> 
> > This is the fourth revision of the patch that allows for just setting the fetch
> > URL. Eric wondered why '--fetch --push' is not used to describe the state
> > '--both', so I added this to the commit message.
> 
> Thanks, I think that explanation is very clear.
> 
> This version overall looks good to me.
> 
> > The t5505-remote.sh test still passes after this change (I was unable to run the
> > full test suite as it broke due to an unrelated gpg issue).
> 
> It is it because you have gpg 2.1, and the patches to handle that are
> not yet merged to master? Or is it because you are using the new patches
> and they are breaking things for your older gpg version?
> 
> If the former, that's fine. If the latter, it would be nice to see a
> report of the breakage. :)

Rest assured, this is a gpg 2.1 issue :-)
-- 
Kind regards,
Peter
https://lekensteyn.nl

Re: [PATCH v4] remote: add --fetch and --both options to set-url

[Junio C Hamano]

Peter Wu <peter@lekensteyn.nl> writes:

> git remote set-url knew about the '--push' option to update just the
> pushurl, but it does not have a similar option for "update fetch URL and
> leave whatever was in place for the push URL".
>
> This patch adds support for a '--fetch' option which implements that use
> case in a backwards compatible way: if no --both, --push or --fetch
> options are given, then the push URL is modified too if it was not set
> before. This is the case since the push URL is implicitly based on the
> fetch URL.

OK.  In other words, for those without asymmetric configuration
without a need to define pushURL, this default should be the most
convenient, as it does not have to fiddle with two variables.

> A '--both' option is added to make the command independent of previous
> pushurl settings. For the --add and --delete set operations, it will
> always set the push and/ or the fetch URLs. For the primary mode of

"and/or", I think.

> operation (without --add or --delete), it will drop pushurl as the
> implicit push URL is the (fetch) URL.

I think this description is clear enough without "(if exists)" at
the end.

> While '--both' could be implemented as '--fetch --push', it might also
> be mistaken for "--push overrides --fetch". An option such as
> "--only={fetch|push|both}" was also considered, but it is longer than
> the current options, makes --push redundant and brings the confusing
> option "--only=both". Options such as '--direction=...' is too long and
> '--dir=' is ambiguous ("directory"?). Thus, for brevity three new
> options were introduced.

Sounds sensible.

> @@ -134,7 +134,15 @@ Changes URL remote points to. Sets first URL remote points to matching
>  regex <oldurl> (first URL if no <oldurl> is given) to <newurl>. If
>  <oldurl> doesn't match any URL, error occurs and nothing is changed.
>  +
> -With '--push', push URLs are manipulated instead of fetch URLs.
> +With '--both', both the fetch and push URLs are manipulated.
> ++
> +With '--fetch', only fetch URLs are manipulated.
> ++
> +With '--push', only push URLs are manipulated.

I am afraid that this part is confusing when read in the wider
context of the document.

The first sentence of this section (outside the patch) is "Changes
URL remote points to"; I expect that the most people would think
that it is talking about "remote.*.url" configuration, and the
wording for "--push" in the original is also clear that it touches
'remote.*.pushurl' instead.

In the updated text, you use words "fetch URL" and "push URL" to
mean "regardless of how these are represented by the configuration
system, the URL to be used for fetching/pushing".  In other words,
in the updated vocabulary, 'remote.*.pushurl' is *NOT* called "push
URL" (and 'remote.*.url' is not 'fetch URL').

That may be easier for new people who aren't familiar with the
configuration system (read: I am saying that it may be a good idea
in the longer term), but the phrasing does not make it clear that
they are not referring remote.*.{url,pushURL} variables.

Rephrasing these to "URLs used for fetching" vs "URLs used for
pushing" may make things a bit less confusion-prone.

In any case, we would also need to update Documentation/config.txt
which has these:

    remote.<name>.url::
            The URL of a remote repository.  See linkgit:git-fetch[1] or
            linkgit:git-push[1].

    remote.<name>.pushurl::
            The push URL of a remote repository.  See linkgit:git-push[1].

It may be sufficient to rephrase them like so:

    remote.<name>.url::
	The URL of a remote repository, used for fetching from it
        and pushing into it unless a separate remote.<name>.pushURL
        is defined.  See linkgit:git-fetch[1] and linkgit:git-push[1]

    remote.<name>.pushurl::
	The URL of a remote repository, used for pushing into it
        (if undefined, remote.<name>.url is used to push into it).
	See linkgit:git-push[1].

perhaps?

> +For historical reasons, if neither --fetch nor --push is specified then the
> +fetch URL is changed, as well as the push URL if this was not already set. This
> +behavior may change in the future.

This paragraph is unwarranted.  As you explained in the second
paragraph in the log message, the traditional behaviour was a good
default for majority of people and I do not think we saw any
demonstrated need to deprecate it.

> +#define MODIFY_TYPE_FETCH       (1 << 0)
> +#define MODIFY_TYPE_PUSH        (1 << 1)
> +#define MODIFY_TYPE_BOTH        (MODIFY_TYPE_FETCH | MODIFY_TYPE_PUSH)
> +/* The historic behavior behaves like --fetch, but does not touch the push URL
> + * configuration (and thereby may appear to change the push URL too if this was
> + * not set before).
> + */
> +#define MODIFY_TYPE_HISTORIC    (MODIFY_TYPE_FETCH | (1 << 2))

	/*
         * Our multi-line comment begins with a slash-asterisk
         * without anything else on the remainder of the line
         * and ends with an asterisk-slash, possibly indented
         * and nothing else on it.
         */

Perhaps s/HISTORIC/TRADITIONAL/ or s/HISTORIC/LITERAL/ (the latter
is for "literally update 'URL' configuration for the named remote,
don't play with 'do we have pushURL?' games").

> +	/* Make the implicit push URL explicit if the fetch URL is modified,
> +	 * except when in the historic mode (then the pushurl configuration is
> +	 * not changed). */

Likewise.

Thanks.

Re: What's cooking in git.git (Mar 2015, #08; Mon, 23)

[Junio C Hamano]

> * pw/remote-set-url-fetch (2014-11-26) 1 commit
>  - remote: add --fetch and --both options to set-url

This has not seen any activity for a few months since $gmane/261483; 
is anybody still interested in resurrecting it?

Re: What's cooking in git.git (Mar 2015, #08; Mon, 23)

[Jeff King]

On Tue, Mar 24, 2015 at 03:21:24PM -0700, Junio C Hamano wrote:

> > * pw/remote-set-url-fetch (2014-11-26) 1 commit
> >  - remote: add --fetch and --both options to set-url
> 
> This has not seen any activity for a few months since $gmane/261483; 
> is anybody still interested in resurrecting it?

I actually thought this had been merged. I was happy enough with the v4
you linked above, though it looks like you had some minor-ish comments.
I do not personally have a big interest in it and wasn't planning
anything else on it. But if Peter is still interested, I think it is
close to ready, and I'd be happy to do another round of review.

-Peff

Re: What's cooking in git.git (Mar 2015, #08; Mon, 23)

[Junio C Hamano]

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

> * jk/test-chain-lint (2015-03-22) 28 commits
>  - t6039: fix broken && chain
>  - t9158, t9161: fix broken &&-chain in git-svn tests
>  - t9104: fix test for following larger parents
>  - t4104: drop hand-rolled error reporting
>  - t0005: fix broken &&-chains
>  - t7004: fix embedded single-quotes
>  - t0050: appease --chain-lint
>  - t9001: use test_when_finished
>  - t4117: use modern test_* helpers
>  - t6034: use modern test_* helpers
>  - t1301: use modern test_* helpers
>  - t0020: use modern test_* helpers
>  - t6030: use modern test_* helpers
>  - t9502: fix &&-chain breakage
>  - t7201: fix &&-chain breakage
>  - t3600: fix &&-chain breakage for setup commands
>  - t: avoid using ":" for comments
>  - t: wrap complicated expect_code users in a block
>  - t: use test_expect_code instead of hand-rolled comparison
>  - t: use test_might_fail for diff and grep
>  - t: fix &&-chaining issues around setup which might fail
>  - t: use test_must_fail instead of hand-rolled blocks
>  - t: use verbose instead of hand-rolled errors
>  - t: assume test_cmp produces verbose output
>  - t: fix trivial &&-chain breakage
>  - t: fix moderate &&-chain breakage
>  - t: fix severe &&-chain breakage
>  - t/test-lib: introduce --chain-lint option
>
>  People often forget to chain the commands in their test together
>  with &&, leaving a failure from an earlier command in the test go
>  unnoticed.  The new GIT_TEST_CHAIN_LINT mechanism allows you to
>  catch such a mistake more easily.
>
>  What I queued here has fix to the issue J6t found in 15/25 squashed
>  in, and also has 26/25 and 27/25 follow-up fixes from Michael, plus
>  28/25 follow-up from Torsten.  If everybody involved is happy with
>  it, we can just proceed with this copy, otherwise I'll let Peff
>  reroll.  I am happy either way.

I'll merge this to 'next' soonish, unless I hear otherwise.  I
double checked 15/25 (i.e. $feature{"forks"}{"default"} = [1];)
so I think we are in good shape.

Thanks.

Re: What's cooking in git.git (Mar 2015, #08; Mon, 23)

[Jeff King]

On Tue, Mar 24, 2015 at 01:02:35PM -0700, Junio C Hamano wrote:

> > * jk/test-chain-lint (2015-03-22) 28 commits
> [...]
> >  What I queued here has fix to the issue J6t found in 15/25 squashed
> >  in, and also has 26/25 and 27/25 follow-up fixes from Michael, plus
> >  28/25 follow-up from Torsten.  If everybody involved is happy with
> >  it, we can just proceed with this copy, otherwise I'll let Peff
> >  reroll.  I am happy either way.
> 
> I'll merge this to 'next' soonish, unless I hear otherwise.  I
> double checked 15/25 (i.e. $feature{"forks"}{"default"} = [1];)
> so I think we are in good shape.

Thanks, yeah, I think what you have queued is good.

-Peff

Re: What's cooking in git.git (Mar 2015, #08; Mon, 23)

[Junio C Hamano]

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

> [Stalled]
>
> * mh/fdopen-with-retry (2015-03-06) 6 commits
>  - buffer_fdinit(): use fdopen_with_retry()
>  - update_info_file(): use fdopen_with_retry()
>  - copy_to_log(): use fdopen_with_retry()
>  - fdopen_lock_file(): use fdopen_with_retry()
>  - SQUASH??? $gmane/264889
>  - xfdopen(): if first attempt fails, free memory and try again
>
>  Various parts of the code where they call fdopen() can fail when
>  they run out of memory; attempt to proceed by retrying the
>  operation after freeing some resource.
>
>  Waiting for further comments.

Sorry, but I lost track.  Is this one still viable, or have we
decided that it is not worth doing it?

Re: What's cooking in git.git (Mar 2015, #08; Mon, 23)

[Junio C Hamano]

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

> Here are the topics that have been cooking.  Commits prefixed with
> '-' are only in 'pu' (proposed updates) while commits prefixed with
> '+' are in 'next'.
>
> This cycle is turning out to be a "shoot for product excellence"
> release.  About half of the commits that have been merged to the
> 'master' so far have also been merged to 'maint' and v2.3.4 has been
> tagged.

I've merged a few more topics to 'next' and hopefully will push the
result out in a few hours.

Some topics that are in 'next' but not in 'master' are high-impact
ones that I'd feel more comfortable if we cooked them there for a
bit more, and am planning to merge them (if there are no issues
discovered in the meantime, of course) after this cycle finishes,
but for others, I'm hoping to merge them to 'master' before we tag
the -rc0 preview release.

Please give them a good beating and report any regressions you find.

Thanks.

Re: What's cooking in git.git (Mar 2015, #08; Mon, 23)

[Junio C Hamano]

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

> * jn/gitweb-utf8-in-links (2014-05-27) 1 commit
>  - gitweb: Harden UTF-8 handling in generated links

This has been lingering in my 'pu' branch without seeing any updates
since $gmane/250758; is anybody still interested in resurrecting it
and moving it forward?

Thanks.

Re: What's cooking in git.git (Mar 2015, #08; Mon, 23)

[Jakub Narębski]

On Tue, Mar 24, 2015 at 11:26 PM, Junio C Hamano <gitster@pobox.com> wrote:
>
> Junio C Hamano <gitster@pobox.com> writes:
>
> > * jn/gitweb-utf8-in-links (2014-05-27) 1 commit
> >  - gitweb: Harden UTF-8 handling in generated links
>
> This has been lingering in my 'pu' branch without seeing any updates
> since $gmane/250758; is anybody still interested in resurrecting it
> and moving it forward?

I can try to pick it up, but I am no longer sure that it is a good idea
to solve the problem.

In particular git does not require that paths (i.e. filesystem) use utf-8
encoding; it could use latin1 / iso-8859-1 and non-utf8 octet.
-- 
Jakub Narębski

Re: What's cooking in git.git (Mar 2015, #08; Mon, 23)

[Junio C Hamano]

Jakub Narębski <jnareb@gmail.com> writes:

> On Tue, Mar 24, 2015 at 11:26 PM, Junio C Hamano <gitster@pobox.com> wrote:
>>
>> Junio C Hamano <gitster@pobox.com> writes:
>>
>> > * jn/gitweb-utf8-in-links (2014-05-27) 1 commit
>> >  - gitweb: Harden UTF-8 handling in generated links
>>
>> This has been lingering in my 'pu' branch without seeing any updates
>> since $gmane/250758; is anybody still interested in resurrecting it
>> and moving it forward?
>
> I can try to pick it up, but I am no longer sure that it is a good idea
> to solve the problem.

After re-reading the discussion thread, I had the same impression.
Let's drop the patch for now.  It can be re-raised as/if needed.

Thanks.

Re: [PATCH/RFC] Gitweb: Convert UTF-8 encoded file names

[Jakub Narębski]

On Thu, May 15, 2014 at 7:08 AM, Michael Wagner <accounts@mwagner.org> wrote:
> On Thu, May 15, 2014 at 12:25:45AM +0200, Jakub Narębski wrote:
>> On Wed, May 14, 2014 at 11:57 PM, Junio C Hamano <gitster@pobox.com> wrote:
>>> Michael Wagner <accounts@mwagner.org> writes:
>>>
>>>> Perl has an internal encoding used to store text strings. Currently, trying to
>>>> view files with UTF-8 encoded names results in an error (either "404 - Cannot
>>>> find file" [blob_plain] or "XML Parsing Error" [blob]). Converting these UTF-8
>>>> encoded file names into Perl's internal format resolves these errors.
>>
>> Could you give us an example?  What is important is whether filename
>> is passed via path_info or via query string.
>>
>
> There is a file named "Gütekriterien.txt" in my repository. Trying to
> view this file as "blob_plain" produces an 404 error (displaying the
> file name with an additional print statement):
>
> $ REQUEST_METHOD=GET QUERY_STRING='p=notes.git;a=blob_plain;f=work/G%C3%83%C2%BCtekriterien.txt;hb=HEAD' ./gitweb.cgi
>
> work/Gütekriterien.txt
> Status: 404 Not Found

You have URI encoding of "ü" wrong! "ü" encodes as %C3%BC, not
as %C3%83%C2%BC (4 bytes?)

  http://www.url-encode-decode.com/

You tested with wrong input.

BTW. there probably should be test for UTF-8 encoding, similar to
the one for XSS in t9502-gitweb-standalone-parse-output
-- 
Jakub Narębski