What's cooking in git.git (Apr 2012, #03; Fri, 6)

What's cooking in git.git (Apr 2012, #03; Fri, 6)

[Junio C Hamano]

What's cooking in git.git (Apr 2012, #03; Fri, 6)
--------------------------------------------------

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

After slipping for about a week, 1.7.10 final has been tagged.  Let's wait
for a few days to give time for the dust to settle and then open the new
development cycle sometime next week.

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

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

* lp/maint-diff-three-dash-with-graph (2012-03-20) 3 commits
 - t4202: add test for "log --graph --stat -p" separator lines
 - log --graph: fix break in graph lines
 - log --graph --stat: three-dash separator should come after graph lines

The combination of two options "log --graph --stat" was an obscure corner
case nobody cared about, and did not correctly show the ancestry graph
lines.

I've split the original patch into three pieces, one for fixes to two
different issues and a test.  Also the test is adjusted so that the series
can be back-merged to older codebase that did not have 7f81463 (Use
correct grammar in diffstat summary line, 2012-02-01) that first appeared
in v1.7.9.2

With a review from Zbigniew, I would expect that this would be rerolled
again.

* cn/apply-fix-ws-can-lengthen-lines (2012-03-11) 1 commit
 . apply: reallocate the postimage buffer when needed

Attempts to address an ancient bug that dates back to the addition
of an oddball "tab-in-indent" whitespace breakage class that wants
to have longer lines than the original when fixing things up.

Needs more work; results in double-frees.

* nd/columns (2012-03-13) 12 commits
 - column: support grouping entries
 - column: support "denser" mode
 - ls-files: support --column
 - tag: add --column
 - column: support piping stdout to external git-column process
 - status: add --column
 - branch: add --column
 - help: reuse print_columns() for help -a
 - column: add dense layout support
 - column: add columnar layout
 - Stop starting pager recursively
 - Add column layout skeleton and git-column

Rerolled again.  Modulo minor nits, looked nicer than the previous round.

* nd/threaded-index-pack (2012-03-11) 2 commits
 - index-pack: support multithreaded delta resolving
 - index-pack: split second pass obj handling into own function

Another reroll after a bugreport on pthread usage discovered by Ramsey,
but it seems the topic is cooking between Ramsay and Duy out of tree.
Waiting for resolution.

* jh/apply-free-patch (2012-03-28) 7 commits
 - apply.c: WIP ownership audit
 - apply: free unused fragments for submodule patch
 - apply: free patch->result
 - apply: release memory for fn_table
 - apply: free patch->{def,old,new}_name fields
 - apply: rename free_patch() to free_patch_list()
 - apply: do not leak patches and fragments

Valgrind reports quite a lot of discarded memory inside apply.  I started
auditing the memory ownership rules in the command, and am almost done.

Will defer til 1.7.10.

* ss/git-svn-prompt-sans-terminal (2012-01-04) 3 commits
 - fixup! 15eaaf4
 - git-svn, perl/Git.pm: extend Git::prompt helper for querying users
 - perl/Git.pm: "prompt" helper to honor GIT_ASKPASS and SSH_ASKPASS

The bottom one has been replaced with a rewrite based on comments
from Ævar. The second one needs more work, both in perl/Git.pm and
prompt.c, to give precedence to tty over SSH_ASKPASS when terminal
is available.

* jc/split-blob (2012-04-03) 7 commits
 - chunked-object: streaming checkout
 - chunked-object: fallback checkout codepaths
 - bulk-checkin: support chunked-object encoding
 - bulk-checkin: allow the same data to be multiply hashed
 - new representation types in the packstream
 - packfile: use varint functions
 - varint: make it available outside the context of pack
 (this branch is tangled with jc/index-v4.)

Not ready.

I finished the streaming checkout codepath, but as explained in
127b177 (bulk-checkin: support chunked-object encoding, 2011-11-30),
these are still early steps of a long and painful journey. At least
pack-objects and fsck need to learn the new encoding for the series
to be usable locally, and then index-pack/unpack-objects needs to
learn it to be used remotely.

Given that I heard a lot of noise that people want large files, and
that I was asked by somebody at GitTogether'11 privately for an
advice on how to pay developers (not me) to help adding necessary
support, I am somewhat dissapointed that the original patch series
that was sent almost two months ago still remains here without much
comments and updates from the developer community. I even made the
interface to the logic that decides where to split chunks easily
replaceable, and I deliberately made the logic in the original patch
extremely stupid to entice others, especially the "bup" fanboys, to
come up with a better logic, thinking that giving people an easy
target to shoot for, they may be encouraged to help out. The plan is
not working :-(.

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

* jc/index-v4 (2012-04-04) 10 commits
 - update-index: upgrade/downgrade on-disk index version
 - read-cache.c: write prefix-compressed names in the index
 - read-cache.c: read prefix-compressed names in index on-disk version v4
 - read-cache.c: move code to copy incore to ondisk cache to a helper function
 - read-cache.c: move code to copy ondisk to incore cache to a helper function
 - read-cache.c: report the header version we do not understand
 - read-cache.c: make create_from_disk() report number of bytes it consumed
 - read-cache.c: allow unaligned mapping of the index file
 - cache.h: hide on-disk index details
 - varint: make it available outside the context of pack
 (this branch is tangled with jc/split-blob.)

Trivially shrinks the on-disk size of the index file to save both I/O and
checksum overhead.  The "v4" format represented here may not be what we
eventually want to have, but the preparatory refactoring steps and the way
how backward compatibility is retained should give plenty to learn to GSoC
student-hopefuls.

* jk/add-p-skip-conflicts (2012-04-05) 1 commit
 - add--interactive: ignore unmerged entries in patch mode

Excludes conflicted paths from "add -p" processing, as it is not prepared
to handle them.

* bw/spawn-via-shell-path (2012-04-03) 1 commit
 - Use SHELL_PATH from build system in run_command.c:prepare_shell_cmd

"sh" on the user's PATH may be utterly broken on some systems;
consistently use SHELL_PATH even from inside run-command API.

* jc/commit-unedited-template (2012-04-03) 5 commits
 - Documentation/git-commit: rephrase the "initial-ness" of templates
 - git-commit.txt: clarify -t requires editing message
 - commit: rephrase the error when user did not touch templated log message
 - commit: do not trigger bogus "has templated message edited" check
 - t7501: test the right kind of breakage

When "git commit --template F" errors out because the user did not touch
the message, it claimed that it aborts due to "empty message", which was
utterly wrong.

* jc/push-upstream-sanity (2012-04-05) 1 commit
 - push: error out when the "upstream" semantics does not make sense

Fix broken 'push to upstream' implementation.  "git push $there" without
refspec, when the current branch is set to push to a remote different from
$there, used to push to $there using the upstream information to a remote
unreleated to $there.

* jk/http-backend-keep-committer-ident-env (2012-03-30) 1 commit
 - http-backend: respect existing GIT_COMMITTER_* variables

The smart-http backend used to always override GIT_COMMITTER_* variables
with REMOTE_USER and REMOTE_ADDR.

* mk/gitweb-diff-hl (2012-04-05) 8 commits
 - gitweb: Refinement highlightning in combined diffs
 - gitweb: Highlight interesting parts of diff
 - gitweb: Push formatting diff lines to print_diff_chunk()
 - gitweb: Use print_diff_chunk() for both side-by-side and inline diffs
 - gitweb: Extract print_sidebyside_diff_lines()
 - gitweb: Pass esc_html_hl_regions() options to esc_html()
 - gitweb: esc_html_hl_regions(): Don't create empty <span> elements
 - gitweb: Use descriptive names in esc_html_hl_regions()

Rerolled.

* it/fetch-pack-many-refs (2012-04-02) 5 commits
 - remote-curl: main test case for the OS command line overflow
 - fetch-pack: test cases for the new --stdin option
 - fixup? no longer need flex argv[]
 - remote-curl: send the refs to fetch-pack on stdin
 - fetch-pack: new --stdin option to read refs from stdin

Will squash the fix-up one and then requeue.

* jn/debian-customizes-default-editor (2012-03-31) 3 commits
 - fixup? do not hide the "usual" default from readers
 - var doc: advertise current DEFAULT_PAGER and DEFAULT_EDITOR settings
 - var doc: default editor and pager are configurable at build time

Haven't heard anything back for the fix-up suggestion, but otherwise
looked sane.

* rs/commit-list-sort-in-batch (2012-04-02) 3 commits
 - revision: insert unsorted, then sort in prepare_revision_walk()
 - commit: use mergesort() in commit_list_sort_by_date()
 - add mergesort() for linked lists

* hv/submodule-recurse-push (2012-03-30) 3 commits
 - push: teach --recurse-submodules the on-demand option
 - Refactor submodule push check to use string list instead of integer
 - Teach revision walking machinery to walk multiple times sequencially

* dg/subtree (2012-03-25) 112 commits
 - Add 'contrib/subtree/' from commit '2e63f75b8f49abe220ef55ec4e978e7a3b8dc351'
 - Add Subtree Test Makefile
 - Build Subtree
 - Use Project Config Files
 - Remove Unneeded Files
 - ...

A test merge of the 'subtree'.
Waiting for an updated pull request.

* jk/branch-quiet (2012-03-26) 2 commits
 - teach "git branch" a --quiet option
 - checkout: suppress tracking message with "-q"

Even with "-q"uiet option, "checkout" used to report setting up tracking.
Also "branch" learns "-q"uiet option to squelch informational message.

* jk/run-command-eacces (2012-04-05) 2 commits
 - run-command: treat inaccessible directories as ENOENT
 - compat/mingw.[ch]: Change return type of exec functions to int

When PATH contains an unreadable directory, alias expansion code did not
kick in, and failed with an error that said "git-subcmd" was not found.

* jb/am-include (2012-03-28) 1 commit
 - am: support --include option

* jc/am-report-3way (2012-03-28) 1 commit
 - am -3: list the paths that needed 3-way fallback

* rs/combine-diff-zero-context-at-the-beginning (2012-03-25) 1 commit
 - combine-diff: fix loop index underflow

Fixes an age old corner case bug in combine diff (only triggered with -U0
and the hunk at the beginning of the file needs to be shown).

* sl/autoconf (2012-03-26) 3 commits
 - configure: be more idiomatic
 - configure: avoid some code repetitions thanks to m4_{push,pop}def
 - configure: move definitions of private m4 macros before AC_INIT invocation

Updates our configure.ac to follow a better "autoconf" style.

* wk/gitweb-snapshot-use-if-modified-since (2012-03-30) 3 commits
 - gitweb: add If-Modified-Since handling to git_snapshot().
 - gitweb: refactor If-Modified-Since handling
 - gitweb: add `status` headers to git_feed() responses.

Makes 'snapshot' request to "gitweb" honor If-Modified-Since: header,
based on the commit date.

* jk/diff-no-rename-empty (2012-03-23) 4 commits
 - merge-recursive: don't detect renames of empty files
 - teach diffcore-rename to optionally ignore empty content
 - make is_empty_blob_sha1 available everywhere
 - drop casts from users EMPTY_TREE_SHA1_BIN

Forbids rename detection logic from matching two empty files as renames
during merge-recursive to prevent mismerges.

* th/difftool-diffall (2012-04-04) 8 commits
 - difftool: print list of valid tools with '--tool-help'
 - difftool: teach difftool to handle directory diffs
 - difftool: eliminate setup_environment function
 - difftool: stop appending '.exe' to git
 - difftool: remove explicit change of PATH
 - difftool: exit(0) when usage is printed
 - difftool: add '--no-gui' option
 - difftool: parse options using Getopt::Long

Rolls the two-directory-diff logic from diffall script (in contrib/) into
"git difftool" framework. 

* jc/maint-clean-nested-worktree-in-subdir (2012-03-15) 2 commits
  (merged to 'next' on 2012-03-20 at fb5485e)
 + clean: preserve nested git worktree in subdirectories
 + remove_dir_recursively(): Add flag for skipping removal of toplevel dir
 (this branch is tangled with jh/notes-merge-in-git-dir-worktree.)

"git clean -d -f" (not "-d -f -f") is supposed to protect nested working
trees of independent git repositories that exist in the current project
working tree from getting removed, but the protection applied only to such
working trees that are at the top-level of the current project by mistake.

Not urgent.

* ct/advise-push-default (2012-03-26) 2 commits
  (merged to 'next' on 2012-03-28 at 62764ae)
 + clean up struct ref's nonfastforward field
 + push: Provide situational hints for non-fast-forward errors

Breaks down the cases in which "git push" fails due to non-ff into three
categories, and gives separate advise messages.  This should be a good
change regardless of mm/push-default-switch-warning topic.

* nl/rebase-i-cheat-sheet (2012-03-20) 1 commit
  (merged to 'next' on 2012-03-20 at 3092a2b)
 + rebase -i: remind that the lines are top-to-bottom

Not urgent.

* da/difftool-test (2012-03-19) 1 commit
  (merged to 'next' on 2012-03-20 at 0ada7d4)
 + t7800: Test difftool passing arguments to diff

Makes sure "difftool" options can be given in any order.

* jh/notes-merge-in-git-dir-worktree (2012-03-15) 4 commits
  (merged to 'next' on 2012-03-20 at 0c1b1de)
 + notes-merge: Don't remove .git/NOTES_MERGE_WORKTREE; it may be the user's cwd
 + notes-merge: use opendir/readdir instead of using read_directory()
 + t3310: illustrate failure to "notes merge --commit" inside $GIT_DIR/
 + remove_dir_recursively(): Add flag for skipping removal of toplevel dir
 (this branch is tangled with jc/maint-clean-nested-worktree-in-subdir.)

Running "notes merge --commit" failed to perform correctly when run
from any directory inside $GIT_DIR/.  When "notes merge" stops with
conflicts, $GIT_DIR/NOTES_MERGE_WORKTREE is the place a user edits
to resolve it.

Not urgent.

* jn/diffstat-tests (2012-03-13) 7 commits
  (merged to 'next' on 2012-03-20 at 8791b2f)
 + diffstat summary line varies by locale: miscellany
 + test: use numstat instead of diffstat in binary-diff test
 + test: use --numstat instead of --stat in "git stash show" tests
 + test: test cherry-pick functionality and output separately
 + test: modernize funny-names test style
 + test: use numstat instead of diffstat in funny-names test
 + test: use test_i18ncmp when checking --stat output

Some tests checked the "diff --stat" output when they do not have to,
which unnecessarily made things harder to verify under GETTEXT_POISON.

Not urgent.

* tr/maint-word-diff-regex-sticky (2012-03-14) 3 commits
  (merged to 'next' on 2012-03-20 at b3f67cd)
 + diff: tweak a _copy_ of diff_options with word-diff
 + diff: refactor the word-diff setup from builtin_diff_cmd
 + t4034: diff.*.wordregex should not be "sticky" in --word-diff

The regexp configured with wordregex was incorrectly reused across
files.
Not urgent.

* zj/test-cred-helper-nicer-prove (2012-03-15) 2 commits
  (merged to 'next' on 2012-03-20 at b675ec1)
 + t0303: resurrect commit message as test documentation
 + t0303: immediately bail out w/o GIT_TEST_CREDENTIAL_HELPER

Minor improvement to t0303.
Not urgent.

* jc/commit-hook-authorship (2012-03-11) 3 commits
  (merged to 'next' on 2012-03-12 at 05ca7f8)
 + commit: pass author/committer info to hooks
 + t7503: does pre-commit-hook learn authorship?
 + ident.c: add split_ident_line() to parse formatted ident line
 (this branch is tangled with jc/run-hook-env-1.)

"git commit --author=$name" did not tell the name that was being
recorded in the resulting commit to hooks, even though it does do so
when the end user overrode the authorship via the "GIT_AUTHOR_NAME"
environment variable.  This is a simpler of the two approaches.

Will defer til 1.7.10.

* jc/run-hook-env-1 (2012-03-11) 3 commits
 - run_hook(): enhance the interface to pass arbitrary environment
 + t7503: does pre-commit-hook learn authorship?
 + ident.c: add split_ident_line() to parse formatted ident line
 (this branch is tangled with jc/commit-hook-authorship.)

Not urgent.

Updates run_hook() API to be much less specific to "commit".  It would
only be useful if people start doing more interesting things with hooks.

* jc/diff-algo-cleanup (2012-02-19) 2 commits
  (merged to 'next' on 2012-03-15 at cca0032)
 + xdiff: PATIENCE/HISTOGRAM are not independent option bits
 + xdiff: remove XDL_PATCH_* macros

Resurrects the preparatory clean-up patches from another topic that was
discarded, as this would give a saner foundation to build on diff.algo
configuration option series.

Not urgent.

* rs/unpack-trees-leakfix (2012-03-06) 1 commit
  (merged to 'next' on 2012-03-07 at 69a69cd)
 + unpack-trees: plug minor memory leak

Will defer til 1.7.10.

* mm/push-default-switch-warning (2012-03-09) 1 commit
  (merged to 'next' on 2012-03-28 at 074b16b)
 + push: start warning upcoming default change for push.default

Not urgent.

This resurrects an ancient patch I wrote during a discussion we had in the
1.6.3-1.6.4 era.  This should probably come after ct/advise-push-default
topic and at that point the advise messages need to be rephrased, taking
the future default change into account.

* jc/fmt-merge-msg-people (2012-03-13) 1 commit
 - fmt-merge-msg: show those involved in a merged series

The "fmt-merge-msg" command learns to list the primary contributors
involved in the side topic you are merging.

Will defer til 1.7.10.

* nl/http-proxy-more (2012-03-15) 5 commits
  (merged to 'next' on 2012-03-20 at c004001)
 + http: rename HTTP_REAUTH to HTTP_AUTH_RETRY
 + http: Avoid limit of retrying request only twice
 + http: handle proxy authentication failure (error 407)
 + http: handle proxy proactive authentication
 + http: try http_proxy env var when http.proxy config option is not set

The code to talk to http proxies learn to use the same credential
API used to talk to the final http destinations.

Will defer til 1.7.10.

* nd/stream-more (2012-03-07) 7 commits
  (merged to 'next' on 2012-03-07 at 7325922)
 + update-server-info: respect core.bigfilethreshold
 + fsck: use streaming API for writing lost-found blobs
 + show: use streaming API for showing blobs
 + parse_object: avoid putting whole blob in core
 + cat-file: use streaming API to print blobs
 + Add more large blob test cases
 + streaming: make streaming-write-entry to be more reusable

Use API to read blob data in smaller chunks in more places to
reduce the memory footprint.  In general, looked fairly good.

Will defer til 1.7.10.

--------------------------------------------------
[Discarded]

* tb/maint-remove-irrelevant-i18n-test (2012-03-06) 1 commit
  (merged to 'next' on 2012-03-07 at 23f2dd1)
 + t0204: remove a test that checks undefined behaviour

I tentatively parked this in 'next' but later reverted the merge.

* dg/test-from-elsewhere (2012-03-04) 2 commits
 . Support out-of-tree Valgrind tests
 . Allow overriding GIT_BUILD_DIR

No immediate need; dropped.

rs/commit-list-sort-in-batch

[Jeff King]

On Fri, Apr 06, 2012 at 02:02:56PM -0700, Junio C Hamano wrote:

> * rs/commit-list-sort-in-batch (2012-04-02) 3 commits
>  - revision: insert unsorted, then sort in prepare_revision_walk()
>  - commit: use mergesort() in commit_list_sort_by_date()
>  - add mergesort() for linked lists

Looks like you applied these patches right on v1.7.9. However, they need
Thomas's 5e8617f to compile, which comes later. There's no textual
conflict, but test-mergesort.c need strbuf_getwholeline_fd, which is
made public by 5e8617f.

However, I don't think there's any reason to prefer the fd version over
the stdio one (in fact, it is much less efficient). So this could be
squashed in to the first patch instead of rebasing:

diff --git a/test-mergesort.c b/test-mergesort.c
index 02441ab..1dd82fd 100644
--- a/test-mergesort.c
+++ b/test-mergesort.c
@@ -28,7 +28,7 @@ int main(int argc, const char **argv)
 	struct strbuf sb = STRBUF_INIT;
 
 	for (;;) {
-		if (strbuf_getwholeline_fd(&sb, 0, '\n'))
+		if (strbuf_getwholeline(&sb, stdin, '\n'))
 			break;
 		line = xmalloc(sizeof(struct line));
 		line->text = strbuf_detach(&sb, NULL);

-Peff

Re: rs/commit-list-sort-in-batch

[Jeff King]

On Sat, Apr 07, 2012 at 03:27:24AM -0400, Jeff King wrote:

> On Fri, Apr 06, 2012 at 02:02:56PM -0700, Junio C Hamano wrote:
> 
> > * rs/commit-list-sort-in-batch (2012-04-02) 3 commits
> >  - revision: insert unsorted, then sort in prepare_revision_walk()
> >  - commit: use mergesort() in commit_list_sort_by_date()
> >  - add mergesort() for linked lists
> 
> Looks like you applied these patches right on v1.7.9. However, they need
> Thomas's 5e8617f to compile, which comes later. There's no textual
> conflict, but test-mergesort.c need strbuf_getwholeline_fd, which is
> made public by 5e8617f.

By the way, building on top of René's patches[1], I was able to drop the
time of a particular fetch into a repository with many refs from:

    real    0m2.081s
    user    0m2.180s
    sys     0m0.712s

to:

    real    0m1.784s
    user    0m1.936s
    sys     0m0.652s

with this patch:

diff --git a/builtin/fetch-pack.c b/builtin/fetch-pack.c
index 7124c4b..5d80e65 100644
--- a/builtin/fetch-pack.c
+++ b/builtin/fetch-pack.c
@@ -503,7 +503,7 @@ static int mark_complete(const char *refname, const unsigned char *sha1, int fla
 		struct commit *commit = (struct commit *)o;
 		if (!(commit->object.flags & COMPLETE)) {
 			commit->object.flags |= COMPLETE;
-			commit_list_insert_by_date(commit, &complete);
+			commit_list_insert(commit, &complete);
 		}
 	}
 	return 0;
@@ -615,6 +615,7 @@ static int everything_local(struct ref **refs, int nr_match, char **match)
 	if (!args.depth) {
 		for_each_ref(mark_complete, NULL);
 		for_each_alternate_ref(mark_alternate_complete, NULL);
+		commit_list_sort_by_date(&complete);
 		if (cutoff)
 			mark_recent_complete_commits(cutoff);
 	}

I'm not sure if that vindicates the patches, or implies that these
things are lurking everywhere and that we should really move to a better
data structure so we don't have to fix them all manually. :)

-Peff

[1] Actually, it was René's patches merged into v1.7.10. My ccdc603
    (parse_object: try internal cache before reading object db,
    2012-01-05) makes such a huge difference it is not worth timing
    without it.

Re: rs/commit-list-sort-in-batch

[Junio C Hamano]

Jeff King <peff@peff.net> writes:

> I'm not sure if that vindicates the patches, or implies that these
> things are lurking everywhere and that we should really move to a better
> data structure so we don't have to fix them all manually. :)

;-).

I do not know offhand how widespread the assumption of "list"-ness of the
commit_list is in the codebase.  The last time I looked at it was when I
was wondering if we could give a custom allocator to "struct commit_list",
as we seemed to liberally malloc()/free() quite many of them, with the
hope to reduce memory footprint, but I didn't finish it, so...

But at least inside revision walking machinery, your earlier heap based
implementation to optimize for time based priority queue access pattern
looked a very sensible approach to me.