What's cooking in git.git (May 2010, #01; Sun, 2)

What's cooking in git.git (May 2010, #01; Sun, 2)

[Junio C Hamano]

Here are the topics that have been cooking.  Commits prefixed with '-' are
only in 'pu' while commits prefixed with '+' are in 'next'.  The ones
marked with '.' do not appear in any of the integration branches, but I am
still holding onto them.

The latest release 1.7.1 has been out for a week and I will soon rewind
'next' and switch 'maint' for 1.7.1.X series.  It appears that no grave
regression has been reported, which is a good sign.

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

* cb/maint-stash-orphaned-file (2010-04-18) 2 commits
  (merged to 'next' on 2010-04-22 at f9de16c)
 + stash tests: stash can lose data in a file removed from the index
 + stash: Don't overwrite files that have gone from the index

* jn/maint-bundle (2010-04-19) 2 commits
  (merged to 'next' on 2010-04-22 at b2668a6)
 + fix "bundle --stdin" segfault
 + t5704 (bundle): add tests for bundle --stdin

* pb/patch-id-plus (2010-04-19) 2 commits
  (merged to 'next' on 2010-04-22 at 5177f8b)
 + patch-id: Add support for mbox format
 + patch-id: extract parsing one diff out of generate_id_list

* rr/doc-submitting (2010-04-19) 1 commit
  (merged to 'next' on 2010-04-22 at b557474)
 + SubmittingPatches: Add new section about what to base work on

* st/remote-tags-no-tags (2010-04-20) 2 commits
 - remote add: add a --[no-]tags option
 - Honor "tagopt = --tags" configuration option

* jc/rev-list-ancestry-path (2010-04-20) 1 commit
 - revision: --ancestry-path

Just an illustration patch.  merge simplification logic used when
pathspecs are in effect interacts with this rather badly.

* cb/assume-unchanged-fix (2010-05-01) 2 commits
 - Documentation: git-add does not update files marked "assume unchanged"
 - do not overwrite files marked "assume unchanged"

* jn/fsck-ident (2010-04-24) 1 commit
 - fsck: check ident lines in commit objects

* jn/gitweb-caching-prep (2010-04-24) 5 commits
 - gitweb: Move generating page title to separate subroutine
 - gitweb: Add custom error handler using die_error
 - gitweb: Use nonlocal jump instead of 'exit' in die_error
 - gitweb: href(..., -path_info => 0|1)
 - Export more test-related variables when running external tests

* jn/gitweb-install (2010-05-01) 2 commits
 - gitweb: Create install target for gitweb in Makefile
 - gitweb: Improve installation instructions in gitweb/INSTALL

* jn/gitweb-our-squelch (2010-04-30) 1 commit
 - gitweb: Silence 'Variable VAR may be unavailable' warnings

* jn/maint-request-pull (2010-04-24) 2 commits
 - request-pull: protect against OPTIONS_KEEPDASHDASH from environment
 - tests for request-pull
 (this branch is used by jn/request-pull.)

* jn/request-pull (2010-04-24) 2 commits
 - adapt request-pull tests for new pull request format
 - Merge branch 'jn/maint-request-pull' into jn/request-pull
 (this branch uses jn/maint-request-pull.)

* jn/shortlog (2010-04-26) 4 commits
 . pretty: Respect --abbrev option
 . shortlog: Document and test --format option
 . t4201 (shortlog): Test output format with multiple authors
 . t4201 (shortlog): guard setup with test_expect_success

This seems to cause intermittent test failure for me especially when run
under "make -j4".

* wp/pretty-enhancement (2010-04-30) 5 commits
 - pretty: add aliases for pretty formats
 - pretty: add infrastructure to allow format aliases
 - pretty: make it easier to add new formats
 - pretty: make %H/%h/etc respect --abbrev[-commit]
 - pretty: add conditional %C?colorname placeholders

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

* jc/doc-submit-gmail (2010-04-07) 1 commit
  (merged to 'next' on 2010-04-10 at cd4e8cf)
 + SubmittingPatches: update GMail section

* jg/auto-initialize-notes-with-percent-n-in-format (2010-04-13) 1 commit
 + pretty: Initialize notes if %N is used

* jk/maint-diffstat-overflow (2010-04-17) 1 commit
 + diff: use large integers for diffstat calculations

* rr/remote-helper-doc (2010-04-07) 3 commits
 + Documentation/remote-helpers: Add invocation section
 + Documentation/urls: Rewrite to accomodate <transport>::<address>
 + Documentation/remote-helpers: Rewrite description

* wp/doc-filter-direction (2010-04-17) 1 commit
 + documentation: clarify direction of core.autocrlf

These were pushed out as part of 1.7.1

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

* js/rebase-origin-x (2010-02-05) 1 commit
 - [RFC w/o test and incomplete] rebase: add -x option to record original commit name

I retract my objection against the idea of -x; needs polishing before
moving forward.

* js/grep-open (2010-05-01) 3 commits
 - grep: do not ignore return value from chdir()
 - grep -O: allow optional argument specifying the pager (or editor)
 - grep: Add the option '--open-files-in-pager'
 (this branch is used by jp/hold-string-list-sanity.)

Probably needs to support --no-index mode as well.

* jp/hold-string-list-sanity (2010-04-06) 9 commits
 . string_list: Fix argument order for string_list_append
 . Merge branch 'sr/remote-helper-export' into HEAD
 . Merge branch 'js/grep-open' into HEAD
 . Merge branch 'sb/fmt-merge-msg' into HEAD
 . string_list: Fix argument order for string_list_lookup
 . string_list: Fix argument order for string_list_insert_at_index
 . string_list: Fix argument order for string_list_insert
 . string_list: Fix argument order for for_each_string_list
 . string_list: Fix argument order for print_string_list
 (this branch uses js/grep-open and sr/remote-helper-export.)

Building this on top of slushy codebase is not a very promising endeavor.
Good thing to do, but it came at a bad time.

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

* jn/t7006-fixup (2010-04-14) 1 commit
  (merged to 'next' on 2010-04-22 at 358ea26)
 + t7006: guard cleanup with test_expect_success

* js/maint-receive-pack-symref-alias (2010-04-19) 3 commits
  (merged to 'next' on 2010-04-22 at 5d1338f)
 + t5516-fetch-push.sh: style cleanup
 + receive-pack: detect aliased updates which can occur with symrefs
 + receive-pack: switch global variable 'commands' to a parameter

* sp/maint-dumb-http-pack-reidx (2010-04-19) 12 commits
  (merged to 'next' on 2010-04-22 at c94f19e)
 + http.c::new_http_pack_request: do away with the temp variable filename
 + http-fetch: Use temporary files for pack-*.idx until verified
 + http-fetch: Use index-pack rather than verify-pack to check packs
 + Allow parse_pack_index on temporary files
 + Extract verify_pack_index for reuse from verify_pack
 + Introduce close_pack_index to permit replacement
 + http.c: Remove unnecessary strdup of sha1_to_hex result
 + http.c: Don't store destination name in request structures
 + http.c: Drop useless != NULL test in finish_http_pack_request
 + http.c: Tiny refactoring of finish_http_pack_request
 + t5550-http-fetch: Use subshell for repository operations
 + http.c: Remove bad free of static block

* hg/id-munging (2010-04-06) 2 commits
 - convert: Keep foreign $Id$ on checkout.
 - convert: Safer handling of $Id$ contraction.

* jn/submodule-basic-test (2010-04-10) 3 commits
  (merged to 'next' on 2010-04-22 at 6d22343)
 + t7400: clarify submodule update tests
 + t7400: clarify 'submodule add' tests
 + t7400: split setup into multiple tests

* np/index-pack-memsave (2010-04-12) 3 commits
  (merged to 'next' on 2010-04-22 at 4135456)
 + index-pack: smarter memory usage when appending objects
 + index-pack: rationalize unpack_entry_data()
 + index-pack: smarter memory usage when resolving deltas

* jc/am-3-show-corrupted-patch (2010-04-09) 1 commit
  (merged to 'next' on 2010-04-22 at 49959c8)
 + am -3: recover the diagnostic messages for corrupt patches

* jc/maint-no-reflog-expire-unreach-for-head (2010-04-09) 3 commits
  (merged to 'next' on 2010-04-22 at bf9c927)
 + reflog --expire-unreachable: special case entries in "HEAD" reflog
 + more war on "sleep" in tests
 + Document gc.<pattern>.reflogexpire variables
 (this branch uses jc/maint-reflog-expire-unreachable.)

Rerolled; instead of ignoring the configuration like the previous round
did, this one prunes stale entries that are unreachable from any ref from
the reflog of HEAD.

* cw/maint-exec-defpath (2010-04-15) 2 commits
  (merged to 'next' on 2010-04-22 at e990e5b)
 + autoconf: Check if <paths.h> exists and set HAVE_PATHS_H
 + exec_cmd.c: replace hard-coded path list with one from <paths.h>

* tr/word-diff (2010-04-14) 1 commit
  (merged to 'next' on 2010-04-22 at a10062e)
 + diff: add --word-diff option that generalizes --color-words

There are gitk patches that go with this series.

* sp/maint-describe-tiebreak-with-tagger-date (2010-04-12) 5 commits
  (merged to 'next' on 2010-04-22 at f712ae9)
 + describe: Break annotated tag ties by tagger date
 + tag.c: Parse tagger date (if present)
 + tag.c: Refactor parse_tag_buffer to be saner to program
 + tag.h: Remove unused signature field
 + tag.c: Correct indentation

* bg/apply-blank-trailing-context (2010-04-08) 1 commit
  (merged to 'next' on 2010-04-14 at a393c0b)
 + apply: Allow blank *trailing* context lines to match beyond EOF

* bg/send-email-smtpdomain (2010-04-10) 4 commits
  (merged to 'next' on 2010-04-14 at 88fff99)
 + send-email: Cleanup smtp-domain and add config
 + Document send-email --smtp-domain
 + send-email: Don't use FQDNs without a '.'
 + send-email: Cleanup { style

* rc/ls-remote-default (2010-04-09) 1 commit
  (merged to 'next' on 2010-04-14 at d5081bd)
 + ls-remote: fall-back to default remotes when no remote specified

* rc/maint-curl-helper (2010-04-08) 3 commits
  (merged to 'next' on 2010-04-14 at 2eb9177)
 + remote-curl: ensure that URLs have a trailing slash
 + http: make end_url_with_slash() public
 + t5541-http-push: add test for URLs with trailing slash

* hg/maint-attr-fix (2010-04-06) 3 commits
  (merged to 'next' on 2010-04-14 at a27c7d7)
 + attr: Expand macros immediately when encountered.
 + attr: Allow multiple changes to an attribute on the same line.
 + attr: Fixed debug output for macro expansion.

* jc/status-show-ignored (2010-05-01) 6 commits
  (merged to 'next' on 2010-05-01 at 6845abd)
 + wt-status: fix 'fprintf' compilation warning
  (merged to 'next' on 2010-04-22 at c6abe03)
 + status: --ignored option shows ignored files
 + wt-status: rename and restructure status-print-untracked
 + wt-status: collect ignored files
 + wt-status: plug memory leak while collecting untracked files
 + wt-status: remove unused workdir_untracked member

I am negative about conflating the JSON output to the mix at this point.
Let's stabilize "status" with the existing --porcelain/-z option first.

* eb/unpretty-b-format (2010-03-24) 1 commit
  (merged to 'next' on 2010-04-07 at 7f5c112)
 + Add `%B' in format strings for raw commit body in `git log' and friends

* ab/commit-empty-message (2010-04-06) 1 commit
  (merged to 'next' on 2010-04-07 at 701e863)
 + Add option to git-commit to allow empty log messages

* jc/test-sleepless (2010-04-06) 1 commit
  (merged to 'next' on 2010-04-07 at 8ccf40a)
 + war on "sleep" in tests

* jc/maint-reflog-expire-unreachable (2010-04-07) 1 commit
  (merged to 'next' on 2010-04-22 at 2a46172)
 + reflog --expire-unreachable: avoid merge-base computation
 (this branch is used by jc/maint-no-reflog-expire-unreach-for-head.)

* np/malloc-threading (2010-04-08) 2 commits
  (merged to 'next' on 2010-04-10 at e1730fb)
 + Thread-safe xmalloc and xrealloc needs a recursive mutex
 + Make xmalloc and xrealloc thread-safe

The fix should eventually go to 'maint' and 'master'.  This queues the one
from J6t for Windows compatibility.

* js/async-thread (2010-03-09) 7 commits
  (merged to 'next' on 2010-04-22 at ee8874e)
 + Enable threaded async procedures whenever pthreads is available
  (merged to 'next' on 2010-03-20 at 9939243)
 + Dying in an async procedure should only exit the thread, not the process.
 + Reimplement async procedures using pthreads
 + Windows: more pthreads functions
 + Fix signature of fcntl() compatibility dummy
 + Make report() from usage.c public as vreportf() and use it.
 + Modernize t5530-upload-pack-error.

As the malloc-threading topic has been cooking for a while, the tip one
may not be so bad to unleash to 'next' users, especially because we are in
the pre-release feature freeze.

* sd/log-decorate (2010-04-08) 6 commits
  (merged to 'next' on 2010-04-08 at 68fee56)
 + log.decorate: only ignore it under "log --pretty=raw"
 + script with rev-list instead of log
  (merged to 'next' on 2010-04-06 at 992c9ad)
 + log --pretty/--oneline: ignore log.decorate
  (merged to 'next' on 2010-03-08 at 58a6fba)
 + log.decorate: usability fixes
 + Add `log.decorate' configuration variable.
 + git_config_maybe_bool()

* mh/status-optionally-refresh (2010-04-03) 3 commits
  (merged to 'next' on 2010-04-05 at 0e64aac)
 + t7508: add a test for "git status" in a read-only repository
 + git status: refresh the index if possible
 + t7508: add test for "git status" refreshing the index

* cw/ws-indent-with-tab (2010-04-03) 6 commits
  (merged to 'next' on 2010-04-05 at 5b5e579)
 + whitespace: tests for git-apply --whitespace=fix with tab-in-indent
 + whitespace: add tab-in-indent support for --whitespace=fix
 + whitespace: replumb ws_fix_copy to take a strbuf *dst instead of char *dst
 + whitespace: tests for git-diff --check with tab-in-indent error class
 + whitespace: add tab-in-indent error class
 + whitespace: we cannot "catch all errors known to git" anymore

* sr/remote-helper-export (2010-04-12) 9 commits
  (merged to 'next' on 2010-04-12 at cefbb2f)
 + t5800: testgit helper requires Python support
  (merged to 'next' on 2010-04-09 at 94f6e3d)
 + Makefile: Simplify handling of python scripts
  (merged to 'next' on 2010-04-07 at 5651307)
 + remote-helpers: add tests for testgit helper
 + remote-helpers: add testgit helper
 + remote-helpers: add support for an export command
 + remote-helpers: allow requesing the path to the .git directory
 + fast-import: always create marks_file directories
 + clone: also configure url for bare clones
 + clone: pass the remote name to remote_get
 (this branch is used by jp/hold-string-list-sanity.)

* cc/revert-strategy (2010-03-31) 5 commits
  (merged to 'next' on 2010-04-05 at 50909c1)
 + revert: add "--strategy" option to choose merge strategy
 + merge: make function try_merge_command non static
 + merge: refactor code that calls "git merge-STRATEGY"
 + revert: refactor merge recursive code into its own function
 + revert: use strbuf to refactor the code that writes the merge message

This is more about debugging and we are not in a great hurry.

* sc/http-late-auth (2010-04-01) 1 commit
  (merged to 'next' on 2010-04-02 at c991acf)
 + Prompt for a username when an HTTP request 401s

* jk/cached-textconv (2010-04-01) 5 commits
  (merged to 'next' on 2010-04-02 at a023e3c)
 + diff: avoid useless filespec population
 + diff: cache textconv output
 + textconv: refactor calls to run_textconv
 + introduce notes-cache interface
 + make commit_tree a library function

The small leak fixes in the beginning of the series has been merged to
'master' after a minor fixup.

* ld/discovery-limit-to-fs (2010-04-04) 6 commits
 - write-index: check and warn when worktree crosses a filesystem boundary
  (merged to 'next' on 2010-04-07 at 11ea09a)
 + Rename ONE_FILESYSTEM to DISCOVERY_ACROSS_FILESYSTEM
 + GIT_ONE_FILESYSTEM: flip the default to stop at filesystem boundaries
 + Add support for GIT_ONE_FILESYSTEM
 + truncate cwd string before printing error message
 + config.c: remove static keyword from git_env_bool()

The tip one is a bit iffy; the whole series changes behaviour in a corner
case, and is not a 1.7.1 material.

* pc/remove-warn (2010-03-26) 4 commits
  (merged to 'next' on 2010-04-02 at 52fc00d)
 + Remove a redundant errno test in a usage of remove_path
 + Introduce remove_or_warn function
 + Implement the rmdir_or_warn function
 + Generalise the unlink_or_warn function

I cherry-picked only obviously sensible bits.

* em/checkout-orphan (2010-03-21) 1 commit
  (merged to 'next' on 2010-03-28 at 86b6a4f)
 + git checkout: create unparented branch by --orphan

Perhaps needs a bit of documentation updates, describing the "going open
source" scenario.

* ar/config-from-command-line (2010-03-26) 2 commits
  (merged to 'next' on 2010-04-07 at e50fd3a)
 + Use strbufs instead of open-coded string manipulation
 + Allow passing of configuration parameters in the command line

Re: What's cooking in git.git (May 2010, #01; Sun, 2)

[Johannes Sixt]

Am 02.05.2010 18:12, schrieb Junio C Hamano:
> * np/malloc-threading (2010-04-08) 2 commits
>    (merged to 'next' on 2010-04-10 at e1730fb)
>   + Thread-safe xmalloc and xrealloc needs a recursive mutex
>   + Make xmalloc and xrealloc thread-safe
> 
> The fix should eventually go to 'maint' and 'master'.  This queues the one
> from J6t for Windows compatibility.
> 
> * js/async-thread (2010-03-09) 7 commits
>    (merged to 'next' on 2010-04-22 at ee8874e)
>   + Enable threaded async procedures whenever pthreads is available
>    (merged to 'next' on 2010-03-20 at 9939243)
>   + Dying in an async procedure should only exit the thread, not the process.
>   + Reimplement async procedures using pthreads
>   + Windows: more pthreads functions
>   + Fix signature of fcntl() compatibility dummy
>   + Make report() from usage.c public as vreportf() and use it.
>   + Modernize t5530-upload-pack-error.
> 
> As the malloc-threading topic has been cooking for a while, the tip one
> may not be so bad to unleash to 'next' users, especially because we are in
> the pre-release feature freeze.

Note that np/malloc-threading does not help the potential xmalloc race
condition in js/async-thread topic at all. :-( np/malloc-threading is only
about a race that is internal in pack-objects.

Now that you force me to think about the problem, I suggest something along
these lines: Disable try_to_free_routine when GIT_TRACE is set, and require
callers of set_try_to_free_routine to restore the previous routine.

diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c
index 214d7ef..0e81673 100644
--- a/builtin/pack-objects.c
+++ b/builtin/pack-objects.c
@@ -1529,6 +1529,8 @@ static void try_to_free_from_threads(size_t size)
 	read_unlock();
 }
 
+try_to_free_t old_try_to_free_routine;
+
 /*
  * The main thread waits on the condition that (at least) one of the workers
  * has stopped working (which is indicated in the .working member of
@@ -1563,12 +1565,12 @@ static void init_threaded_search(void)
 	pthread_mutex_init(&cache_mutex, NULL);
 	pthread_mutex_init(&progress_mutex, NULL);
 	pthread_cond_init(&progress_cond, NULL);
-	set_try_to_free_routine(try_to_free_from_threads);
+	old_try_to_free_routine = set_try_to_free_routine(try_to_free_from_threads);
 }
 
 static void cleanup_threaded_search(void)
 {
-	set_try_to_free_routine(NULL);
+	set_try_to_free_routine(old_try_to_free_routine);
 	pthread_cond_destroy(&progress_cond);
 	pthread_mutex_destroy(&read_mutex);
 	pthread_mutex_destroy(&cache_mutex);
diff --git a/git-compat-util.h b/git-compat-util.h
index 4a508c1..51d2a01 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h
@@ -357,7 +357,8 @@ static inline void *gitmempcpy(void *dest, const void *src, size_t n)
 
 extern void release_pack_memory(size_t, int);
 
-extern void set_try_to_free_routine(void (*routine)(size_t));
+typedef void (*try_to_free_t)(size_t);
+extern try_to_free_t set_try_to_free_routine(try_to_free_t);
 
 extern char *xstrdup(const char *str);
 extern void *xmalloc(size_t size);
diff --git a/trace.c b/trace.c
index 4229ae1..9816a8b 100644
--- a/trace.c
+++ b/trace.c
@@ -72,6 +72,7 @@ void trace_printf(const char *fmt, ...)
 	if (!fd)
 		return;
 
+	set_try_to_free_routine(NULL);
 	strbuf_init(&buf, 64);
 	va_start(ap, fmt);
 	len = vsnprintf(buf.buf, strbuf_avail(&buf), fmt, ap);
@@ -103,6 +104,7 @@ void trace_argv_printf(const char **argv, const char *fmt, ...)
 	if (!fd)
 		return;
 
+	set_try_to_free_routine(NULL);
 	strbuf_init(&buf, 64);
 	va_start(ap, fmt);
 	len = vsnprintf(buf.buf, strbuf_avail(&buf), fmt, ap);
diff --git a/wrapper.c b/wrapper.c
index 62edb57..02da613 100644
--- a/wrapper.c
+++ b/wrapper.c
@@ -10,9 +10,11 @@ static void try_to_free_builtin(size_t size)
 
 static void (*try_to_free_routine)(size_t size) = try_to_free_builtin;
 
-void set_try_to_free_routine(void (*routine)(size_t))
+try_to_free_t set_try_to_free_routine(try_to_free_t routine)
 {
-	try_to_free_routine = (routine) ? routine : try_to_free_builtin;
+	try_to_free_t old = try_to_free_routine;
+	try_to_free_routine = routine;
+	return old;
 }
 
 char *xstrdup(const char *str)
@@ -33,8 +35,10 @@ void *xmalloc(size_t size)
 	if (!ret && !size)
 		ret = malloc(1);
 	if (!ret) {
-		try_to_free_routine(size);
-		ret = malloc(size);
+		if (try_to_free_routine) {
+			try_to_free_routine(size);
+			ret = malloc(size);
+		}
 		if (!ret && !size)
 			ret = malloc(1);
 		if (!ret)

Re: What's cooking in git.git (May 2010, #01; Sun, 2)

[Jakub Narebski]

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

> * jn/gitweb-caching-prep (2010-04-24) 5 commits
>  - gitweb: Move generating page title to separate subroutine
>  - gitweb: Add custom error handler using die_error
>  - gitweb: Use nonlocal jump instead of 'exit' in die_error

I'll try to check whether it is true what I think that using nonlocal
jump instead of 'exit' improves performance of gitweb when run from
mod_perl.

>  - gitweb: href(..., -path_info => 0|1)
>  - Export more test-related variables when running external tests
> 
> * jn/gitweb-install (2010-05-01) 2 commits
>  - gitweb: Create install target for gitweb in Makefile
>  - gitweb: Improve installation instructions in gitweb/INSTALL

I think it would be good to have install target for gitweb in Makefile
before gitweb output caching series.

> * jn/gitweb-our-squelch (2010-04-30) 1 commit
>  - gitweb: Silence 'Variable VAR may be unavailable' warnings

What about syntax hightlighting series?

     gitweb: Refactor syntax highlighting support
     gitweb: Syntax highlighting support

-- 
Jakub Narebski
Poland
ShadeHawk on #git

Re: What's cooking in git.git (May 2010, #01; Sun, 2)

[Will Palmer]

On Sun, 2010-05-02 at 09:12 -0700, Junio C Hamano wrote:
> This seems to cause intermittent test failure for me especially when run
> under "make -j4".
> 
> * wp/pretty-enhancement (2010-04-30) 5 commits
>  - pretty: add aliases for pretty formats
>  - pretty: add infrastructure to allow format aliases
>  - pretty: make it easier to add new formats
>  - pretty: make %H/%h/etc respect --abbrev[-commit]
>  - pretty: add conditional %C?colorname placeholders

Does it still happen with the latest (3-commit) series? I suspect the
most-likely to cause test failures would be the --abbrev[-commit]
changes, though I suppose "intermittent" could point to a memory
problem, which is more likely to be from "make it easier to add new
formats" or "add infrastructure..."

Haven't been able to reproduce here (just running make -m4 in a
while-loop all night) Any specific tests failing?

-- 
-- Will

jn/shortlog (Re: What's cooking in git.git (May 2010, #01; Sun, 2))

[Jonathan Nieder]

Hi,

Will Palmer wrote:
> Junio C Hamano wrote:

>> * jn/shortlog (2010-04-26) 4 commits
>>  . pretty: Respect --abbrev option
>>  . shortlog: Document and test --format option
>>  . t4201 (shortlog): Test output format with multiple authors
>>  . t4201 (shortlog): guard setup with test_expect_success
>> 
>> This seems to cause intermittent test failure for me especially when run
>> under "make -j4".
>> 
>> * wp/pretty-enhancement (2010-04-30) 5 commits
[...]
> Does it still happen with the latest (3-commit) series?

I think it was the shortlog tests series[1].

> Haven't been able to reproduce here (just running make -m4 in a
> while-loop all night) Any specific tests failing?

In t4201-shortlog.sh:

--- expect.template     2010-05-04 00:17:41.609754167 +0000
+++ log.predictable     2010-05-04 00:17:41.613754428 +0000
@@ -1,10 +0,0 @@
-A U Thor (5):
-      SUBJECT
-      SUBJECT
-      SUBJECT
-      SUBJECT
-      SUBJECT
-
-Someone else (1):
-      SUBJECT
-

* FAIL 2: default output format

		git shortlog >log &&
		fuzz log >log.predictable &&
		test_cmp expect.template log.predictable

--- expect      2010-05-04 00:17:41.625754445 +0000
+++ log.predictable     2010-05-04 00:17:41.629754567 +0000
@@ -1,10 +0,0 @@
-A U Thor (5):
-      OBJECT_NAME
-      OBJECT_NAME
-      OBJECT_NAME
-      OBJECT_NAME
-      OBJECT_NAME
-
-Someone else (1):
-      OBJECT_NAME
-

* FAIL 3: pretty format

	sed s/SUBJECT/OBJID/ expect.template >expect &&
	git shortlog --format="%h" --abbrev=5 >log &&
	fuzz log >log.predictable &&
	test_cmp expect log.predictable

--- expect      2010-05-04 00:17:41.637754532 +0000
+++ log.predictable     2010-05-04 00:17:41.645754776 +0000
@@ -1,10 +0,0 @@
-A U Thor (5):
-      OBJID
-      OBJID
-      OBJID
-      OBJID
-      OBJID
-
-Someone else (1):
-      OBJID
-
* FAIL 4: --abbrev

		sed s/SUBJECT/OBJID/ expect.template >expect &&
		git shortlog --format="%h" --abbrev=5 >log &&
		fuzz log >log.predictable &&
		test_cmp expect log.predictable

--- expect      2010-05-04 00:17:41.657754653 +0000
+++ log.predictable     2010-05-04 00:17:41.661754495 +0000
@@ -1,10 +0,0 @@
-A U Thor (5):
-      two lines
-      two lines
-      two lines
-      two lines
-      two lines
-
-Someone else (1):
-      two lines
-

* FAIL 5: output from user-defined format is re-wrapped
        
		sed "s/SUBJECT/two lines/" expect.template >expect &&
		git shortlog --format="two%nlines" >log &&
		fuzz log >log.predictable &&
		test_cmp expect log.predictable

Jonathan

[1] http://thread.gmane.org/gmane.comp.version-control.git/145769/focus=145801

Re: jn/shortlog

[Jonathan Nieder]

Jonathan Nieder wrote:
> Will Palmer wrote:

>> Haven't been able to reproduce here (just running make -m4 in a
>> while-loop all night) Any specific tests failing?
>
> In t4201-shortlog.sh:
> 
> --- expect.template     2010-05-04 00:17:41.609754167 +0000
> +++ log.predictable     2010-05-04 00:17:41.613754428 +0000
> @@ -1,10 +0,0 @@
[etc]

So apparently shortlog does not default to HEAD when stdin is not a
tty.

Possible fixes:

 - document the current unfriendly behavior
 - default to HEAD when input is empty

Jonathan

-- 8< --
Subject: t4200 (rerere): guard setup with test_expect_success

Ensure that GIT_SKIP_TESTS='t4200.*' actually suppresses all of t4200.

Also quote some stray dollar signs and test the result from some
git config invocations.

Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
 t/t4200-rerere.sh |  189 ++++++++++++++++++++++++++++++-----------------------
 1 files changed, 106 insertions(+), 83 deletions(-)

diff --git a/t/t4200-rerere.sh b/t/t4200-rerere.sh
index 70856d0..d9b87c9 100755
--- a/t/t4200-rerere.sh
+++ b/t/t4200-rerere.sh
@@ -56,23 +56,27 @@ test_expect_success 'conflicting merge' '
 	git reset --hard &&
 	mkdir .git/rr-cache &&
 	git config --unset rerere.enabled &&
-	test_must_fail git merge first
+	test_must_fail git merge first &&
+	sha1=$(perl -pe "s/	.*//" .git/MERGE_RR) &&
+	rr=.git/rr-cache/$sha1
 '
 
-sha1=$(perl -pe 's/	.*//' .git/MERGE_RR)
-rr=.git/rr-cache/$sha1
-test_expect_success 'recorded preimage' "grep ^=======$ $rr/preimage"
+test_expect_success 'recorded preimage' '
+	grep "^=======\$" "$rr/preimage"
+'
 
 test_expect_success 'rerere.enabled works, too' '
 	rm -rf .git/rr-cache &&
 	git config rerere.enabled true &&
 	git reset --hard &&
 	test_must_fail git merge first &&
-	grep ^=======$ $rr/preimage
+	grep "^=======\$" "$rr/preimage"
 '
 
-test_expect_success 'no postimage or thisimage yet' \
-	"test ! -f $rr/postimage -a ! -f $rr/thisimage"
+test_expect_success 'no postimage or thisimage yet' '
+	! test -f "$rr/postimage" &&
+	! test -f "$rr/thisimage"
+'
 
 test_expect_success 'preimage has right number of lines' '
 
@@ -81,56 +85,60 @@ test_expect_success 'preimage has right number of lines' '
 
 '
 
-git show first:a1 > a1
-
-cat > expect << EOF
---- a/a1
-+++ b/a1
-@@ -1,4 +1,4 @@
--Some Title
-+Some title
- ==========
- Whether 'tis nobler in the mind to suffer
- The slings and arrows of outrageous fortune,
-@@ -8,21 +8,11 @@
- The heart-ache and the thousand natural shocks
- That flesh is heir to, 'tis a consummation
- Devoutly to be wish'd.
--<<<<<<<
--Some Title
--==========
--To die! To sleep;
--=======
- Some title
- ==========
- To die, to sleep;
-->>>>>>>
- To sleep: perchance to dream: ay, there's the rub;
- For in that sleep of death what dreams may come
- When we have shuffled off this mortal coil,
- Must give us pause: there's the respect
- That makes calamity of so long life;
--<<<<<<<
--=======
--* END *
-->>>>>>>
-EOF
-git rerere diff > out
-
-test_expect_success 'rerere diff' 'test_cmp expect out'
-
-cat > expect << EOF
-a1
-EOF
-
-git rerere status > out
-
-test_expect_success 'rerere status' 'test_cmp expect out'
+test_expect_success 'rerere diff' "
+	git show first:a1 >a1 &&
+
+	cat >expect <<-\EOF
+	--- a/a1
+	+++ b/a1
+	@@ -1,4 +1,4 @@
+	-Some Title
+	+Some title
+	 ==========
+	 Whether 'tis nobler in the mind to suffer
+	 The slings and arrows of outrageous fortune,
+	@@ -8,21 +8,11 @@
+	 The heart-ache and the thousand natural shocks
+	 That flesh is heir to, 'tis a consummation
+	 Devoutly to be wish'd.
+	-<<<<<<<
+	-Some Title
+	-==========
+	-To die! To sleep;
+	-=======
+	 Some title
+	 ==========
+	 To die, to sleep;
+	->>>>>>>
+	 To sleep: perchance to dream: ay, there's the rub;
+	 For in that sleep of death what dreams may come
+	 When we have shuffled off this mortal coil,
+	 Must give us pause: there's the respect
+	 That makes calamity of so long life;
+	-<<<<<<<
+	-=======
+	-* END *
+	->>>>>>>
+	EOF
+	git rerere diff >out &&
+	test_cmp expect out
+"
+
+test_expect_success 'rerere status' '
+	cat >expect <<-\EOF &&
+	a1
+	EOF
+
+	git rerere status >out &&
+	test_cmp expect out
+'
 
 test_expect_success 'commit succeeds' \
 	"git commit -q -a -m 'prefer first over second'"
 
-test_expect_success 'recorded postimage' "test -f $rr/postimage"
+test_expect_success 'recorded postimage' '
+	test -f "$rr/postimage"
+'
 
 test_expect_success 'another conflicting merge' '
 	git checkout -b third master &&
@@ -139,47 +147,62 @@ test_expect_success 'another conflicting merge' '
 	test_must_fail git pull . first
 '
 
-git show first:a1 | sed 's/To die: t/To die! T/' > expect
-test_expect_success 'rerere kicked in' "! grep ^=======$ a1"
+test_expect_success 'rerere kicked in' '
+	! grep "^=======\$" a1
+'
 
-test_expect_success 'rerere prefers first change' 'test_cmp a1 expect'
+test_expect_success 'rerere prefers first change' '
+	git show first:a1 | sed "s/To die: t/To die! T/" >expect &&
+	test_cmp a1 expect
+'
 
-rm $rr/postimage
-echo "$sha1	a1" | perl -pe 'y/\012/\000/' > .git/MERGE_RR
+test_expect_success 'cleanup' '
+	rm $rr/postimage &&
+	echo "$sha1	a1" |
+		perl -pe "y/\012/\000/" >.git/MERGE_RR
+'
 
 test_expect_success 'rerere clear' 'git rerere clear'
 
 test_expect_success 'clear removed the directory' "test ! -d $rr"
 
-mkdir $rr
-echo Hello > $rr/preimage
-echo World > $rr/postimage
-
-sha2=4000000000000000000000000000000000000000
-rr2=.git/rr-cache/$sha2
-mkdir $rr2
-echo Hello > $rr2/preimage
-
-almost_15_days_ago=$((60-15*86400))
-just_over_15_days_ago=$((-1-15*86400))
-almost_60_days_ago=$((60-60*86400))
-just_over_60_days_ago=$((-1-60*86400))
+test_expect_success 'gc setup' '
+	mkdir "$rr" &&
+	echo Hello >"$rr/preimage" &&
+	echo World >"$rr/postimage" &&
 
-test-chmtime =$almost_60_days_ago $rr/preimage
-test-chmtime =$almost_15_days_ago $rr2/preimage
+	sha2=4000000000000000000000000000000000000000 &&
+	rr2=.git/rr-cache/$sha2 &&
+	mkdir "$rr2" &&
+	echo Hello >"$rr2/preimage" &&
 
-test_expect_success 'garbage collection (part1)' 'git rerere gc'
+	almost_15_days_ago=$((60-15*86400)) &&
+	just_over_15_days_ago=$((-1-15*86400)) &&
+	almost_60_days_ago=$((60-60*86400)) &&
+	just_over_60_days_ago=$((-1-60*86400))
+'
 
-test_expect_success 'young records still live' \
-	"test -f $rr/preimage && test -f $rr2/preimage"
+test_expect_success 'garbage collection (part1)' '
+	test-chmtime "=$almost_60_days_ago" "$rr/preimage" &&
+	test-chmtime "=$almost_15_days_ago" "$rr2/preimage" &&
+	git rerere gc
+'
 
-test-chmtime =$just_over_60_days_ago $rr/preimage
-test-chmtime =$just_over_15_days_ago $rr2/preimage
+test_expect_success 'young records still live' '
+	test -f "$rr/preimage" &&
+	test -f "$rr2/preimage"
+'
 
-test_expect_success 'garbage collection (part2)' 'git rerere gc'
+test_expect_success 'garbage collection (part2)' '
+	test-chmtime "=$just_over_60_days_ago" "$rr/preimage" &&
+	test-chmtime "=$just_over_15_days_ago" "$rr2/preimage" &&
+	git rerere gc
+'
 
-test_expect_success 'old records rest in peace' \
-	"test ! -f $rr/preimage && test ! -f $rr2/preimage"
+test_expect_success 'old records rest in peace' '
+	! test -f "$rr/preimage" &&
+	! test -f "$rr2/preimage"
+'
 
 test_expect_success 'file2 added differently in two branches' '
 	git reset --hard &&
@@ -214,7 +237,7 @@ test_expect_success 'resolution was recorded properly' '
 '
 
 test_expect_success 'rerere.autoupdate' '
-	git config rerere.autoupdate true
+	git config rerere.autoupdate true &&
 	git reset --hard &&
 	git checkout version2 &&
 	test_must_fail git merge fifth &&
@@ -230,7 +253,7 @@ test_expect_success 'merge --rerere-autoupdate' '
 '
 
 test_expect_success 'merge --no-rerere-autoupdate' '
-	git config rerere.autoupdate true
+	git config rerere.autoupdate true &&
 	git reset --hard &&
 	git checkout version2 &&
 	test_must_fail git merge --no-rerere-autoupdate fifth &&
-- 
1.7.1.5.g8d03f

[PATCH v2 0/5] jn/shortlog

[Jonathan Nieder]

Jonathan Nieder wrote:

> apparently shortlog does not default to HEAD when stdin is not a
> tty.
> 
> Possible fixes:
> 
>  - document the current unfriendly behavior
>  - default to HEAD when input is empty

Here’s the uncontroversial half.

Patch 1 is new: it explains why scripts that mean HEAD need to explicitly
say so.  The remaining patches are just like before, except tests pass a
HEAD arg now.  Sorry about the breakage, and thanks for catching it.

Jonathan Nieder (4):
  Documentation/shortlog: scripted users should not rely on implicit
    HEAD
  t4201 (shortlog): guard setup with test_expect_success
  t4201 (shortlog): Test output format with multiple authors
  shortlog: Document and test --format option

Will Palmer (1):
  pretty: Respect --abbrev option

 Documentation/git-shortlog.txt |   15 +++++-
 builtin/shortlog.c             |    3 +-
 pretty.c                       |    7 ++-
 shortlog.h                     |    1 +
 t/t4201-shortlog.sh            |  116 +++++++++++++++++++++++++++++++--------
 t/t6006-rev-list-format.sh     |   19 +++++++
 6 files changed, 132 insertions(+), 29 deletions(-)

[PATCH 1/5] Documentation/shortlog: scripted users should not rely on implicit HEAD

[Jonathan Nieder]

When passed no revision arguments, ‘git shortlog’ reads a log from
stdin if and only if stdin is not a tty.  So scripts that need to
function identically when standard input is a terminal (as when run
interactively) and not (as when run through a cron job) should either
supply a log themselves or specify the desired revisions explicitly.

Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
 Documentation/git-shortlog.txt |    7 ++++++-
 1 files changed, 6 insertions(+), 1 deletions(-)

diff --git a/Documentation/git-shortlog.txt b/Documentation/git-shortlog.txt
index dfd4d0c..f11ea90 100644
--- a/Documentation/git-shortlog.txt
+++ b/Documentation/git-shortlog.txt
@@ -9,7 +9,7 @@ SYNOPSIS
 --------
 [verse]
 git log --pretty=short | 'git shortlog' [-h] [-n] [-s] [-e] [-w]
-'git shortlog' [-n|--numbered] [-s|--summary] [-e|--email] [-w[<width>[,<indent1>[,<indent2>]]]] [<committish>...]
+'git shortlog' [-n|--numbered] [-s|--summary] [-e|--email] [-w[<width>[,<indent1>[,<indent2>]]]] <commit>...
 
 DESCRIPTION
 -----------
@@ -19,6 +19,11 @@ the first line of the commit message will be shown.
 
 Additionally, "[PATCH]" will be stripped from the commit description.
 
+If no revisions are passed on the command line and either standard input
+is not a terminal or there is no current branch, 'git shortlog' will
+output a summary of the log read from standard input, without
+reference to the current repository.
+
 OPTIONS
 -------
 
-- 
1.6.5

[PATCH 2/5] t4201 (shortlog): guard setup with test_expect_success

[Jonathan Nieder]

Follow the current prevailing style.  This also has the benefit of
capturing any stray output and noticing if any of the setup commands
start failing.

Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
As before.

 t/t4201-shortlog.sh |   68 +++++++++++++++++++++++++++++---------------------
 1 files changed, 39 insertions(+), 29 deletions(-)

diff --git a/t/t4201-shortlog.sh b/t/t4201-shortlog.sh
index a01e55b..438a826 100755
--- a/t/t4201-shortlog.sh
+++ b/t/t4201-shortlog.sh
@@ -8,30 +8,38 @@ test_description='git shortlog
 
 . ./test-lib.sh
 
-echo 1 > a1
-git add a1
-tree=$(git write-tree)
-commit=$( (echo "Test"; echo) | git commit-tree $tree )
-git update-ref HEAD $commit
-
-echo 2 > a1
-git commit --quiet -m "This is a very, very long first line for the commit message to see if it is wrapped correctly" a1
-
-# test if the wrapping is still valid when replacing all i's by treble clefs.
-echo 3 > a1
-git commit --quiet -m "$(echo "This is a very, very long first line for the commit message to see if it is wrapped correctly" | sed "s/i/1234/g" | tr 1234 '\360\235\204\236')" a1
-
-# now fsck up the utf8
-git config i18n.commitencoding non-utf-8
-echo 4 > a1
-git commit --quiet -m "$(echo "This is a very, very long first line for the commit message to see if it is wrapped correctly" | sed "s/i/1234/g" | tr 1234 '\370\235\204\236')" a1
-
-echo 5 > a1
-git commit --quiet -m "a								12	34	56	78" a1
-
-git shortlog -w HEAD > out
+test_expect_success 'setup' '
+	echo 1 >a1 &&
+	git add a1 &&
+	tree=$(git write-tree) &&
+	commit=$(printf "%s\n" "Test" "" | git commit-tree "$tree") &&
+	git update-ref HEAD "$commit" &&
+
+	echo 2 >a1 &&
+	git commit --quiet -m "This is a very, very long first line for the commit message to see if it is wrapped correctly" a1 &&
+
+	# test if the wrapping is still valid
+	# when replacing all is by treble clefs.
+	echo 3 >a1 &&
+	git commit --quiet -m "$(
+		echo "This is a very, very long first line for the commit message to see if it is wrapped correctly" |
+		sed "s/i/1234/g" |
+		tr 1234 "\360\235\204\236")" a1 &&
+
+	# now fsck up the utf8
+	git config i18n.commitencoding non-utf-8 &&
+	echo 4 >a1 &&
+	git commit --quiet -m "$(
+		echo "This is a very, very long first line for the commit message to see if it is wrapped correctly" |
+		sed "s/i/1234/g" |
+		tr 1234 "\370\235\204\236")" a1 &&
+
+	echo 5 >a1 &&
+	git commit --quiet -m "a								12	34	56	78" a1
+'
 
-cat > expect << EOF
+test_expect_success 'shortlog wrapping' '
+	cat >expect <<\EOF &&
 A U Thor (5):
       Test
       This is a very, very long first line for the commit message to see if
@@ -44,13 +52,15 @@ A U Thor (5):
          56	78
 
 EOF
+	git shortlog -w HEAD >out &&
+	test_cmp expect out
+'
 
-test_expect_success 'shortlog wrapping' 'test_cmp expect out'
-
-git log HEAD > log
-GIT_DIR=non-existing git shortlog -w < log > out
-
-test_expect_success 'shortlog from non-git directory' 'test_cmp expect out'
+test_expect_success 'shortlog from non-git directory' '
+	git log HEAD >log &&
+	GIT_DIR=non-existing git shortlog -w <log >out &&
+	test_cmp expect out
+'
 
 iconvfromutf8toiso88591() {
 	printf "%s" "$*" | iconv -f UTF-8 -t ISO8859-1
-- 
1.6.5

[PATCH 3/5] t4201 (shortlog): Test output format with multiple authors

[Jonathan Nieder]

Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
As before.

 t/t4201-shortlog.sh |    7 +++++++
 1 files changed, 7 insertions(+), 0 deletions(-)

diff --git a/t/t4201-shortlog.sh b/t/t4201-shortlog.sh
index 438a826..6bfd0c0 100755
--- a/t/t4201-shortlog.sh
+++ b/t/t4201-shortlog.sh
@@ -36,6 +36,10 @@ test_expect_success 'setup' '
 
 	echo 5 >a1 &&
 	git commit --quiet -m "a								12	34	56	78" a1
+
+	echo 6 >a1 &&
+	git commit --quiet -m "Commit by someone else" \
+		--author="Someone else <not!me>" a1
 '
 
 test_expect_success 'shortlog wrapping' '
@@ -51,6 +55,9 @@ A U Thor (5):
       a								12	34
          56	78
 
+Someone else (1):
+      Commit by someone else
+
 EOF
 	git shortlog -w HEAD >out &&
 	test_cmp expect out
-- 
1.6.5

[PATCH 4/5] shortlog: Document and test --format option

[Jonathan Nieder]

Do not document the --pretty synonym, since it takes too long to
explain the name to people.

Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
 - Pass HEAD to shortlog
 - Use {asterisk} helper for manual

 Documentation/git-shortlog.txt |    8 ++++++
 t/t4201-shortlog.sh            |   53 +++++++++++++++++++++++++++++++++++++++-
 2 files changed, 60 insertions(+), 1 deletions(-)

diff --git a/Documentation/git-shortlog.txt b/Documentation/git-shortlog.txt
index f11ea90..a820fcb 100644
--- a/Documentation/git-shortlog.txt
+++ b/Documentation/git-shortlog.txt
@@ -44,6 +44,14 @@ OPTIONS
 --email::
 	Show the email address of each author.
 
+--format[='<format>']::
+	Instead of the commit subject, use some other information to
+	describe each commit.  '<format>' can be any string accepted
+	by the `--format` option of 'git log', such as '{asterisk} [%h] %s'.
+	(See the "PRETTY FORMATS" section of linkgit:git-log[1].)
+
+	Each pretty-printed commit will be rewrapped before it is shown.
+
 -w[<width>[,<indent1>[,<indent2>]]]::
 	Linewrap the output by wrapping each line at `width`.  The first
 	line of each entry is indented by `indent1` spaces, and the second
diff --git a/t/t4201-shortlog.sh b/t/t4201-shortlog.sh
index 6bfd0c0..13fdeb2 100755
--- a/t/t4201-shortlog.sh
+++ b/t/t4201-shortlog.sh
@@ -39,7 +39,58 @@ test_expect_success 'setup' '
 
 	echo 6 >a1 &&
 	git commit --quiet -m "Commit by someone else" \
-		--author="Someone else <not!me>" a1
+		--author="Someone else <not!me>" a1 &&
+
+	cat >expect.template <<-\EOF
+	A U Thor (5):
+	      SUBJECT
+	      SUBJECT
+	      SUBJECT
+	      SUBJECT
+	      SUBJECT
+
+	Someone else (1):
+	      SUBJECT
+
+	EOF
+'
+
+fuzz() {
+	file=$1 &&
+	sed "
+			s/$_x40/OBJECT_NAME/g
+			s/$_x05/OBJID/g
+			s/^ \{6\}[CTa].*/      SUBJECT/g
+			s/^ \{8\}[^ ].*/        CONTINUATION/g
+		" <"$file" >"$file.fuzzy" &&
+	sed "/CONTINUATION/ d" <"$file.fuzzy"
+}
+
+test_expect_success 'default output format' '
+	git shortlog HEAD >log &&
+	fuzz log >log.predictable &&
+	test_cmp expect.template log.predictable
+'
+
+test_expect_success 'pretty format' '
+	sed s/SUBJECT/OBJECT_NAME/ expect.template >expect &&
+	git shortlog --format="%H" HEAD >log &&
+	fuzz log >log.predictable &&
+	test_cmp expect log.predictable
+'
+
+test_expect_failure '--abbrev' '
+	sed s/SUBJECT/OBJID/ expect.template >expect &&
+	git shortlog --format="%h" --abbrev=5 HEAD >log &&
+	fuzz log >log.predictable &&
+	test_cmp expect log.predictable
+'
+
+test_expect_success 'output from user-defined format is re-wrapped' '
+	sed "s/SUBJECT/two lines/" expect.template >expect &&
+	git shortlog --format="two%nlines" HEAD >log &&
+	fuzz log >log.predictable &&
+	test_cmp expect log.predictable
 '
 
 test_expect_success 'shortlog wrapping' '
-- 
1.6.5

[PATCH 5/5] pretty: Respect --abbrev option

[Jonathan Nieder]

Prior to this, the output of git log -1 --format=%h was always 7
characters long, without regard to whether --abbrev had been passed.

Signed-off-by: Will Palmer <wmpalmer@gmail.com>
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
incorporating some tests by Will Palmer:
http://thread.gmane.org/gmane.comp.version-control.git/146054/focus=146057

 builtin/shortlog.c         |    3 ++-
 pretty.c                   |    7 ++++---
 shortlog.h                 |    1 +
 t/t4201-shortlog.sh        |    2 +-
 t/t6006-rev-list-format.sh |   31 +++++++++++++++++++++++++++++++
 5 files changed, 39 insertions(+), 5 deletions(-)

diff --git a/builtin/shortlog.c b/builtin/shortlog.c
index 06320f5..5089502 100644
--- a/builtin/shortlog.c
+++ b/builtin/shortlog.c
@@ -162,7 +162,7 @@ void shortlog_add_commit(struct shortlog *log, struct commit *commit)
 		    sha1_to_hex(commit->object.sha1));
 	if (log->user_format) {
 		struct pretty_print_context ctx = {0};
-		ctx.abbrev = DEFAULT_ABBREV;
+		ctx.abbrev = log->abbrev;
 		ctx.subject = "";
 		ctx.after_subject = "";
 		ctx.date_mode = DATE_NORMAL;
@@ -290,6 +290,7 @@ parse_done:
 	}
 
 	log.user_format = rev.commit_format == CMIT_FMT_USERFORMAT;
+	log.abbrev = rev.abbrev;
 
 	/* assume HEAD if from a tty */
 	if (!nongit && !rev.pending.nr && isatty(0))
diff --git a/pretty.c b/pretty.c
index 7cb3a2a..1430616 100644
--- a/pretty.c
+++ b/pretty.c
@@ -716,7 +716,7 @@ static size_t format_commit_one(struct strbuf *sb, const char *placeholder,
 		if (add_again(sb, &c->abbrev_commit_hash))
 			return 1;
 		strbuf_addstr(sb, find_unique_abbrev(commit->object.sha1,
-		                                     DEFAULT_ABBREV));
+		                                     c->pretty_ctx->abbrev));
 		c->abbrev_commit_hash.len = sb->len - c->abbrev_commit_hash.off;
 		return 1;
 	case 'T':		/* tree hash */
@@ -726,7 +726,7 @@ static size_t format_commit_one(struct strbuf *sb, const char *placeholder,
 		if (add_again(sb, &c->abbrev_tree_hash))
 			return 1;
 		strbuf_addstr(sb, find_unique_abbrev(commit->tree->object.sha1,
-		                                     DEFAULT_ABBREV));
+		                                     c->pretty_ctx->abbrev));
 		c->abbrev_tree_hash.len = sb->len - c->abbrev_tree_hash.off;
 		return 1;
 	case 'P':		/* parent hashes */
@@ -743,7 +743,8 @@ static size_t format_commit_one(struct strbuf *sb, const char *placeholder,
 			if (p != commit->parents)
 				strbuf_addch(sb, ' ');
 			strbuf_addstr(sb, find_unique_abbrev(
-					p->item->object.sha1, DEFAULT_ABBREV));
+					p->item->object.sha1,
+					c->pretty_ctx->abbrev));
 		}
 		c->abbrev_parent_hashes.len = sb->len -
 		                              c->abbrev_parent_hashes.off;
diff --git a/shortlog.h b/shortlog.h
index bc02cc2..de4f86f 100644
--- a/shortlog.h
+++ b/shortlog.h
@@ -12,6 +12,7 @@ struct shortlog {
 	int in1;
 	int in2;
 	int user_format;
+	int abbrev;
 
 	char *common_repo_prefix;
 	int email;
diff --git a/t/t4201-shortlog.sh b/t/t4201-shortlog.sh
index 899ddbe..c49ca98 100755
--- a/t/t4201-shortlog.sh
+++ b/t/t4201-shortlog.sh
@@ -79,7 +79,7 @@ test_expect_success 'pretty format' '
 	test_cmp expect log.predictable
 '
 
-test_expect_failure '--abbrev' '
+test_expect_success '--abbrev' '
 	sed s/SUBJECT/OBJID/ expect.template >expect &&
 	git shortlog --format="%h" --abbrev=5 >log &&
 	fuzz log >log.predictable &&
diff --git a/t/t6006-rev-list-format.sh b/t/t6006-rev-list-format.sh
index a49b7c5..42a77ff 100755
--- a/t/t6006-rev-list-format.sh
+++ b/t/t6006-rev-list-format.sh
@@ -191,6 +191,31 @@ test_expect_success 'add LF before non-empty (2)' '
 	grep "^$" actual
 '
 
+test_expect_success '--abbrev' '
+	echo SHORT SHORT SHORT >expect2 &&
+	echo LONG LONG LONG >expect3 &&
+	git log -1 --format="%h %h %h" HEAD >actual1 &&
+	git log -1 --abbrev=5 --format="%h %h %h" HEAD >actual2 &&
+	git log -1 --abbrev=5 --format="%H %H %H" HEAD >actual3 &&
+	sed -e "s/$_x40/LONG/g" -e "s/$_x05/SHORT/g" <actual2 >fuzzy2 &&
+	sed -e "s/$_x40/LONG/g" -e "s/$_x05/SHORT/g" <actual3 >fuzzy3 &&
+	test_cmp expect2 fuzzy2 &&
+	test_cmp expect3 fuzzy3 &&
+	! test_cmp actual1 actual2
+'
+
+test_expect_success '%H is not affected by --abbrev-commit' '
+	git log -1 --format=%H --abbrev-commit --abbrev=20 HEAD >actual &&
+	len=$(wc -c <actual) &&
+	test $len = 40
+'
+
+test_expect_success '%h is not affected by --abbrev-commit' '
+	git log -1 --format=%h --abbrev-commit --abbrev=20 HEAD >actual &&
+	len=$(wc -c <actual) &&
+	test $len = 20
+'
+
 test_expect_success '"%h %gD: %gs" is same as git-reflog' '
 	git reflog >expect &&
 	git log -g --format="%h %gD: %gs" >actual &&
@@ -203,6 +228,12 @@ test_expect_success '"%h %gD: %gs" is same as git-reflog (with date)' '
 	test_cmp expect actual
 '
 
+test_expect_success '"%h %gD: %gs" is same as git-reflog (with --abbrev)' '
+	git reflog --abbrev=13 --date=raw >expect &&
+	git log -g --abbrev=13 --format="%h %gD: %gs" --date=raw >actual &&
+	test_cmp expect actual
+'
+
 test_expect_success '%gd shortens ref name' '
 	echo "master@{0}" >expect.gd-short &&
 	git log -g -1 --format=%gd refs/heads/master >actual.gd-short &&
-- 
1.7.1

Re: [PATCH 5/5] pretty: Respect --abbrev option

[Jonathan Nieder]

Jonathan Nieder wrote:

> Subject: [PATCH 5/5] pretty: Respect --abbrev option
[...]
> Signed-off-by: Will Palmer <wmpalmer@gmail.com>
> Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>

This patch was by Will Palmer <wmpalmer@gmail.com> (except two
of the tests).  Since I don’t use ‘git send-email’, I keep
forgetting to include the From: pseudo-header.

Sorry for the trouble,
Jonathan

diff --git a/pretty.c b/pretty.c
index 1430616..e5dea91 100644
--- a/pretty.c
+++ b/pretty.c
@@ -908,6 +908,7 @@ static void pp_header(enum cmit_fmt fmt,
 		      const char *encoding,
 		      const struct commit *commit,
 		      const char **msg_p,
+		      const char **author_p,
 		      struct strbuf *sb)
 {
 	int parents_shown = 0;
@@ -954,6 +955,8 @@ static void pp_header(enum cmit_fmt fmt,
 		 * FULLER shows both authors and dates.
 		 */
 		if (!memcmp(line, "author ", 7)) {
+			if (author_p)
+				*author_p = line + 7;
 			strbuf_grow(sb, linelen + 80);
 			pp_user_info("Author", fmt, sb, line + 7, dmode, encoding);
 		}
@@ -1004,6 +1007,22 @@ void pp_title_line(enum cmit_fmt fmt,
 	strbuf_release(&title);
 }
 
+static void pp_in_body_author(struct strbuf *sb,
+			      const char *author_line)
+{
+	const char *author_end, *sender;
+
+	author_end = strchr(author_line, '>') + 1;
+	sender = git_author_info(IDENT_NO_DATE);
+	if (!strncmp(author_line, sender, author_end - author_line))
+		return;
+
+	strbuf_addstr(sb, "From: ");
+	strbuf_add(sb, author_line, author_end - author_line);
+	strbuf_addch(sb, '\n');
+	strbuf_addch(sb, '\n');
+}
+
 void pp_remainder(enum cmit_fmt fmt,
 		  const char **msg_p,
 		  struct strbuf *sb,
@@ -1057,6 +1076,7 @@ void pretty_print_commit(enum cmit_fmt fmt, const struct commit *commit,
 	unsigned long beginning_of_body;
 	int indent = 4;
 	const char *msg = commit->buffer;
+	const char *author = NULL;
 	char *reencoded;
 	const char *encoding;
 	int need_8bit_cte = context->need_8bit_cte;
@@ -1082,15 +1102,13 @@ void pretty_print_commit(enum cmit_fmt fmt, const struct commit *commit,
 		int i, ch, in_body;
 
 		for (in_body = i = 0; (ch = msg[i]); i++) {
-			if (!in_body) {
-				/* author could be non 7-bit ASCII but
-				 * the log may be so; skip over the
-				 * header part first.
-				 */
-				if (ch == '\n' && msg[i+1] == '\n')
-					in_body = 1;
-			}
-			else if (non_ascii(ch)) {
+			/*
+			 * If the author is non 7-bit ASCII but
+			 * the log is 7bit, we still need a
+			 * Content-type field, in case an in-body
+			 * From: line is required.
+			 */
+			if (non_ascii(ch)) {
 				need_8bit_cte = 1;
 				break;
 			}
@@ -1098,7 +1116,7 @@ void pretty_print_commit(enum cmit_fmt fmt, const struct commit *commit,
 	}
 
 	pp_header(fmt, context->abbrev, context->date_mode, encoding,
-		  commit, &msg, sb);
+		  commit, &msg, &author, sb);
 	if (fmt != CMIT_FMT_ONELINE && !context->subject) {
 		strbuf_addch(sb, '\n');
 	}
@@ -1111,6 +1129,9 @@ void pretty_print_commit(enum cmit_fmt fmt, const struct commit *commit,
 		pp_title_line(fmt, &msg, sb, context->subject,
 			      context->after_subject, encoding, need_8bit_cte);
 
+	if (fmt == CMIT_FMT_EMAIL)
+		pp_in_body_author(sb, author);
+
 	beginning_of_body = sb->len;
 	if (fmt != CMIT_FMT_ONELINE)
 		pp_remainder(fmt, &msg, sb, indent);
-- 

[PATCH 1/2] Have set_try_to_free_routine return the previous routine

[Johannes Sixt]

This effectively requires from the callers of set_try_to_free_routine to
treat the try-to-free-routines as a stack.

We will need this for the next patch where the only current caller cannot
depend on that the previously set routine was the default routine.

Signed-off-by: Johannes Sixt <j6t@kdbg.org>
---
On Sonntag, 2. Mai 2010, Johannes Sixt wrote:
> Am 02.05.2010 18:12, schrieb Junio C Hamano:
> > * np/malloc-threading (2010-04-08) 2 commits
> >
> > * js/async-thread (2010-03-09) 7 commits
>
> Note that np/malloc-threading does not help the potential xmalloc race
> condition in js/async-thread topic at all. :-( np/malloc-threading is only
> about a race that is internal in pack-objects.
>
> Now that you force me to think about the problem, I suggest something along
> these lines: Disable try_to_free_routine when GIT_TRACE is set, and require
> callers of set_try_to_free_routine to restore the previous routine.

Here is a 2-patch series that implements this idea. I wrote it on top of
np/malloc-threading, because it implements the infrastructure that is
necessary for this.

 builtin-pack-objects.c |    6 ++++--
 git-compat-util.h      |    3 ++-
 wrapper.c              |    6 ++++--
 3 files changed, 10 insertions(+), 5 deletions(-)

diff --git a/builtin-pack-objects.c b/builtin-pack-objects.c
index 26fc7cd..5279cd9 100644
--- a/builtin-pack-objects.c
+++ b/builtin-pack-objects.c
@@ -1556,6 +1556,8 @@ static void try_to_free_from_threads(size_t size)
 	read_unlock();
 }
 
+try_to_free_t old_try_to_free_routine;
+
 /*
  * The main thread waits on the condition that (at least) one of the workers
  * has stopped working (which is indicated in the .working member of
@@ -1590,12 +1592,12 @@ static void init_threaded_search(void)
 	pthread_mutex_init(&cache_mutex, NULL);
 	pthread_mutex_init(&progress_mutex, NULL);
 	pthread_cond_init(&progress_cond, NULL);
-	set_try_to_free_routine(try_to_free_from_threads);
+	old_try_to_free_routine = set_try_to_free_routine(try_to_free_from_threads);
 }
 
 static void cleanup_threaded_search(void)
 {
-	set_try_to_free_routine(NULL);
+	set_try_to_free_routine(old_try_to_free_routine);
 	pthread_cond_destroy(&progress_cond);
 	pthread_mutex_destroy(&read_mutex);
 	pthread_mutex_destroy(&cache_mutex);
diff --git a/git-compat-util.h b/git-compat-util.h
index 1c171db..828aada 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h
@@ -346,7 +346,8 @@ static inline char *gitstrchrnul(const char *s, int c)
 
 extern void release_pack_memory(size_t, int);
 
-extern void set_try_to_free_routine(void (*routine)(size_t));
+typedef void (*try_to_free_t)(size_t);
+extern try_to_free_t set_try_to_free_routine(try_to_free_t);
 
 extern char *xstrdup(const char *str);
 extern void *xmalloc(size_t size);
diff --git a/wrapper.c b/wrapper.c
index 62edb57..8aa9df9 100644
--- a/wrapper.c
+++ b/wrapper.c
@@ -10,9 +10,11 @@ static void try_to_free_builtin(size_t size)
 
 static void (*try_to_free_routine)(size_t size) = try_to_free_builtin;
 
-void set_try_to_free_routine(void (*routine)(size_t))
+try_to_free_t set_try_to_free_routine(try_to_free_t routine)
 {
-	try_to_free_routine = (routine) ? routine : try_to_free_builtin;
+	try_to_free_t old = try_to_free_routine;
+	try_to_free_routine = routine;
+	return old;
 }
 
 char *xstrdup(const char *str)
-- 
1.7.1.64.ga1799.dirty

[PATCH 2/2] Do not call release_pack_memory in malloc wrappers when GIT_TRACE is used

[Johannes Sixt]

This avoids a potential race condition when async procedures are
implemented as threads where release_pack_memory() can be called from
different threads without locking under memory pressure.

Signed-off-by: Johannes Sixt <j6t@kdbg.org>
---
 It is worthwhile to have this in np/malloc-threading (and in maint or
 master) even if js/async-thread is not in master, yet, because on
 Windows we already have async procedures implemented in threads.

 trace.c |    6 ++++++
 1 files changed, 6 insertions(+), 0 deletions(-)

diff --git a/trace.c b/trace.c
index 4229ae1..1e560cb 100644
--- a/trace.c
+++ b/trace.c
@@ -25,6 +25,10 @@
 #include "cache.h"
 #include "quote.h"
 
+void do_nothing(size_t unused)
+{
+}
+
 /* Get a trace file descriptor from GIT_TRACE env variable. */
 static int get_trace_fd(int *need_close)
 {
@@ -72,6 +76,7 @@ void trace_printf(const char *fmt, ...)
 	if (!fd)
 		return;
 
+	set_try_to_free_routine(do_nothing);	/* is never reset */
 	strbuf_init(&buf, 64);
 	va_start(ap, fmt);
 	len = vsnprintf(buf.buf, strbuf_avail(&buf), fmt, ap);
@@ -103,6 +108,7 @@ void trace_argv_printf(const char **argv, const char *fmt, ...)
 	if (!fd)
 		return;
 
+	set_try_to_free_routine(do_nothing);	/* is never reset */
 	strbuf_init(&buf, 64);
 	va_start(ap, fmt);
 	len = vsnprintf(buf.buf, strbuf_avail(&buf), fmt, ap);
-- 
1.7.1.64.ga1799.dirty

Re: What's cooking in git.git (May 2010, #01; Sun, 2)

[Clemens Buchacher]

On Sun, May 02, 2010 at 09:12:12AM -0700, Junio C Hamano wrote:

> * cb/assume-unchanged-fix (2010-05-01) 2 commits
>  - Documentation: git-add does not update files marked "assume unchanged"

Please remove the second hunk from that commit, where I add "The assume
unchanged bit is unset for all paths." That is clearly wrong.

Thank you.

Clemens