Gitweb: Convert UTF-8 encoded file names

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.

---
 gitweb/gitweb.perl | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

index a9f57d6..6046977 100755
@@ -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

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.

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

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

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

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"

The generated pages show the correct file names.

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

Michael Wagner:

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/

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

Obviously, you are right, thanks.

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

---

index a9f57d6..f1414e1 100755
@@ -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>;

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

                   binmode $fd, ':utf8';

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

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

Jakub Narebski

[...]

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

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?

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

Thanks all for digging this to the root.

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

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.

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.

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

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

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

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

W dniu 2014-05-16 19:05, Junio C Hamano pisze:

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 <[email protected]>
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.

---
 t/t9502-gitweb-standalone-parse-output.sh |   34 +++++++++++++++++++++++++++++
 1 files changed, 34 insertions(+), 0 deletions(-)

index 86dfee2..37246a3 100755
@@ -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

W dniu 2014-05-15 21:28, Jakub Narębski pisze:

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.

---
 gitweb/gitweb.perl |    7 +++++++
 1 files changed, 7 insertions(+), 0 deletions(-)

index a9f57d6..77e1312 100755
@@ -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(-)

index 77e1312..e4a50e7 100755
@@ -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

[...]

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.

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.

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

index cb103c8..09a4670 100644
@@ -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.
 +

2.1.3

Thanks, I think that explanation is very clear.

This version overall looks good to me.

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

Rest assured, this is a gpg 2.1 issue :-)

Kind regards,
Peter
https://lekensteyn.nl

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.

"and/or", I think.

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

Sounds sensible.

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

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

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.

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

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.

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

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.

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.

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

-Peff

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