branch: split validate_new_branchname() into two

Checking if a proposed name is appropriate for a branch is strictly
a subset of checking if we want to allow creating or updating a
branch with such a name.  The mysterious sounding 'attr_only'
parameter to validate_new_branchname() is used to switch the
function between these two roles.

Instead, split the function into two, and adjust the callers.  A new
helper validate_branchname() only checks the name and reports if the
branch already exists.

This loses one NEEDSWORK from the branch API.

Signed-off-by: Junio C Hamano <gitster@pobox.com>

branch: streamline "attr_only" handling in validate_new_branchname()

The function takes a parameter "attr_only" (which is a name that is
hard to reason about, which will be corrected soon) and skips some
checks when it is set.  Reorganize the conditionals to make it more
obvious that some checks are never made when this parameter is set.

Signed-off-by: Junio C Hamano <gitster@pobox.com>

Git 2.15-rc1

Signed-off-by: Junio C Hamano <gitster@pobox.com>

Merge branch 'ls/filter-process-delayed'

Bugfixes to an already graduated series.

* ls/filter-process-delayed:
  write_entry: untangle symlink and regular-file cases
  write_entry: avoid reading blobs in CE_RETRY case
  write_entry: fix leak when retrying delayed filter
  entry.c: check if file exists after checkout
  entry.c: update cache entry only for existing files

Merge branch 'ds/avoid-overflow-in-midpoint-computation'

Code clean-up.

* ds/avoid-overflow-in-midpoint-computation:
  cleanup: fix possible overflow errors in binary search

Merge branch 'tb/complete-describe'

Docfix.

* tb/complete-describe:
  completion: add --broken and --dirty to describe

Merge branch 'sb/test-cmp-expect-actual'

Test tweak.

* sb/test-cmp-expect-actual:
  tests: fix diff order arguments in test_cmp

Merge branch 'jk/refs-df-conflict'

An ancient bug that made Git misbehave with creation/renaming of
refs has been fixed.

* jk/refs-df-conflict:
  refs_resolve_ref_unsafe: handle d/f conflicts for writes
  t3308: create a real ref directory/file conflict

Merge branch 'rs/rs-mailmap'

* rs/rs-mailmap:
  .mailmap: normalize name for René Scharfe

Merge branch 'rs/fsck-null-return-from-lookup'

Improve behaviour of "git fsck" upon finding a missing object.

* rs/fsck-null-return-from-lookup:
  fsck: handle NULL return of lookup_blob() and lookup_tree()

Merge branch 'jk/sha1-loose-object-info-fix'

Leakfix and futureproofing.

* jk/sha1-loose-object-info-fix:
  sha1_loose_object_info: handle errors from unpack_sha1_rest

Merge branch 'hn/string-list-doc'

Docfix.

* hn/string-list-doc:
  api-argv-array.txt: remove broken link to string-list API

Merge branch 'tb/show-trailers-in-ref-filter'

"git for-each-ref --format=..." learned a new format element,
%(trailers), to show only the commit log trailer part of the log
message.

* tb/show-trailers-in-ref-filter:
  ref-filter.c: parse trailers arguments with %(contents) atom
  ref-filter.c: use trailer_opts to format trailers
  t6300: refactor %(trailers) tests
  doc: use "`<literal>`"-style quoting for literal strings
  doc: 'trailers' is the preferred way to format trailers
  t4205: unfold across multiple lines

Merge branch 'jt/oidmap'

Introduce a new "oidmap" API and rewrite oidset to use it.

* jt/oidmap:
  oidmap: map with OID as key

Merge branch 'jr/hash-migration-plan-doc'

Lay out plans for weaning us off of SHA-1.

* jr/hash-migration-plan-doc:
  technical doc: add a design doc for hash function transition

write_entry: untangle symlink and regular-file cases

The write_entry() function switches on the mode of the entry
we're going to write out. The cases for S_IFLNK and S_IFREG
are lumped together. In earlier versions of the code, this
made some sense. They have a shared preamble (which reads
the blob content), a short type-specific body, and a shared
conclusion (which writes out the file contents; always for
S_IFREG and only sometimes for S_IFLNK).

But over time this has grown to make less sense. The preamble
now has conditional bits for each type, and the S_IFREG body
has grown a lot more complicated. It's hard to follow the
logic of which code is running for which mode.

Let's give each mode its own case arm. We will still share
the conclusion code, which means we now jump to it with a
goto. Ideally we'd pull that shared code into its own
function, but it touches so much internal state in the
write_entry() function that the end result is actually
harder to follow than the goto.

While we're here, we'll touch up a few bits of whitespace to
make the beginning and endings of the cases easier to read.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

write_entry: avoid reading blobs in CE_RETRY case

When retrying a delayed filter-process request, we don't
need to send the blob to the filter a second time. However,
we read it unconditionally into a buffer, only to later
throw away that buffer. We can make this more efficient by
skipping the read in the first place when it isn't
necessary.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

write_entry: fix leak when retrying delayed filter

When write_entry() retries a delayed filter request, we
don't need to send the blob content to the filter again, and
set the pointer to NULL. But doing so means we leak the
contents we read earlier from read_blob_entry(). Let's make
sure to free it before dropping the pointer.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

cleanup: fix possible overflow errors in binary search

A common mistake when writing binary search is to allow possible
integer overflow by using the simple average:

mid = (min + max) / 2;

Instead, use the overflow-safe version:

mid = min + (max - min) / 2;

This translation is safe since the operation occurs inside a loop
conditioned on "min < max". The included changes were found using
the following git grep:

git grep '/ *2;' '*.c'

Making this cleanup will prevent future review friction when a new
binary search is contructed based on existing code.

Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
Reviewed-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

Merge branch 'js/rebase-i-final'

* js/rebase-i-final:
  i18n: add a missing space in message

i18n: add a missing space in message

The message spans over 2 lines but the C conconcatenation does not add
the needed space between the two lines.

Signed-off-by: Jean-Noel Avila <jn.avila@free.fr>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

Prepare for -rc1

Signed-off-by: Junio C Hamano <gitster@pobox.com>

Merge branch 'tb/ref-filter-empty-modifier'

In the "--format=..." option of the "git for-each-ref" command (and
its friends, i.e. the listing mode of "git branch/tag"), "%(atom:)"
(e.g. "%(refname:)", "%(body:)" used to error out.  Instead, treat
them as if the colon and an empty string that follows it were not
there.

* tb/ref-filter-empty-modifier:
  ref-filter.c: pass empty-string as NULL to atom parsers

Merge branch 'ks/verify-filename-non-option-error-message-tweak'

Error message tweak.

* ks/verify-filename-non-option-error-message-tweak:
  setup: update error message to be more meaningful

Merge branch 'ks/branch-tweak-error-message-for-extra-args'

Error message tweak.

* ks/branch-tweak-error-message-for-extra-args:
  branch: change the error messages to be more meaningful

Merge branch 'jk/ui-color-always-to-auto'

Fix regression of "git add -p" for users with "color.ui = always"
in their configuration, by merging the topic below and adjusting it
for the 'master' front.

* jk/ui-color-always-to-auto:
  t7301: use test_terminal to check color
  t4015: use --color with --color-moved
  color: make "always" the same as "auto" in config
  provide --color option for all ref-filter users
  t3205: use --color instead of color.branch=always
  t3203: drop "always" color test
  t6006: drop "always" color config tests
  t7502: use diff.noprefix for --verbose test
  t7508: use test_terminal for color output
  t3701: use test-terminal to collect color output
  t4015: prefer --color to -c color.diff=always
  test-terminal: set TERM=vt100

Merge branch 'ma/builtin-unleak'

Many variables that points at a region of memory that will live
throughout the life of the program have been marked with UNLEAK
marker to help the leak checkers concentrate on real leaks..

* ma/builtin-unleak:
  builtin/: add UNLEAKs

Merge branch 'rb/compat-poll-fix'

Backports a moral equivalent of 2015 fix to the poll emulation from
the upstream gnulib to fix occasional breakages on HPE NonStop.

* rb/compat-poll-fix:
  poll.c: always set revents, even if to zero

Merge branch 'tg/memfixes'

Fixes for a handful memory access issues identified by valgrind.

* tg/memfixes:
  sub-process: use child_process.args instead of child_process.argv
  http-push: fix construction of hex value from path
  path.c: fix uninitialized memory access

Merge branch 'sb/branch-avoid-repeated-strbuf-release'

* sb/branch-avoid-repeated-strbuf-release:
  branch: reset instead of release a strbuf

Merge branch 'rs/qsort-s'

* rs/qsort-s:
  test-stringlist: avoid buffer underrun when sorting nothing

Merge branch 'jn/strbuf-doc-re-reuse'

* jn/strbuf-doc-re-reuse:
  strbuf doc: reuse after strbuf_release is fine

Merge branch 'tb/delimit-pretty-trailers-args-with-comma'

The feature that allows --pretty='%(trailers)' to take modifiers
like "fold" and "only" used to separate these modifiers with a
comma, i.e. "%(trailers:fold:only)", but we changed our mind and
use a comma, i.e. "%(trailers:fold,only)".  Fast track this change
before this new feature becomes part of any official release.

* tb/delimit-pretty-trailers-args-with-comma:
  pretty.c: delimit "%(trailers)" arguments with ","

completion: add --broken and --dirty to describe

When the flags for broken and dirty were implemented in
b0176ce6b5 (builtin/describe: introduce --broken flag, 2017-03-21)
and 9f67d2e827 (Teach "git describe" --dirty option, 2009-10-21)
the completion was not updated, although these flags are useful
completions. Add them.

Signed-off-by: Thomas Braun <thomas.braun@virtuell-zuhause.de>
Helped-by: Stefan Beller <sbeller@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

tests: fix diff order arguments in test_cmp

Fix the argument order for test_cmp. When given the expected
result first the diff shows the actual output with '+' and the
expectation with '-', which is the convention for our tests.

Signed-off-by: Stefan Beller <sbeller@google.com>
Reviewed-by: Jonathan Nieder <jrnieder@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

refs_resolve_ref_unsafe: handle d/f conflicts for writes

If our call to refs_read_raw_ref() fails, we check errno to
see if the ref is simply missing, or if we encountered a
more serious error. If it's just missing, then in "write"
mode (i.e., when RESOLVE_REFS_READING is not set), this is
perfectly fine.

However, checking for ENOENT isn't sufficient to catch all
missing-ref cases. In the filesystem backend, we may also
see EISDIR when we try to resolve "a" and "a/b" exists.
Likewise, we may see ENOTDIR if we try to resolve "a/b" and
"a" exists. In both of those cases, we know that our
resolved ref doesn't exist, but we return an error (rather
than reporting the refname and returning a null sha1).

This has been broken for a long time, but nobody really
noticed because the next step after resolving without the
READING flag is usually to lock the ref and write it. But in
both of those cases, the write will fail with the same
errno due to the directory/file conflict.

There are two cases where we can notice this, though:

  1. If we try to write "a" and there's a leftover directory
     already at "a", even though there is no ref "a/b". The
     actual write is smart enough to move the empty "a" out
     of the way.

     This is reasonably rare, if only because the writing
     code has to do an independent resolution before trying
     its write (because the actual update_ref() code handles
     this case fine). The notes-merge code does this, and
     before the fix in the prior commit t3308 erroneously
     expected this case to fail.

  2. When resolving symbolic refs, we typically do not use
     the READING flag because we want to resolve even
     symrefs that point to unborn refs. Even if those unborn
     refs could not actually be written because of d/f
     conflicts with existing refs.

     You can see this by asking "git symbolic-ref" to report
     the target of a symref pointing past a d/f conflict.

We can fix the problem by recognizing the other "missing"
errnos and treating them like ENOENT. This should be safe to
do even for callers who are then going to actually write the
ref, because the actual writing process will fail if the d/f
conflict is a real one (and t1404 checks these cases).

Arguably this should be the responsibility of the
files-backend to normalize all "missing ref" errors into
ENOENT (since something like EISDIR may not be meaningful at
all to a database backend). However other callers of
refs_read_raw_ref() may actually care about the distinction;
putting this into resolve_ref() is the minimal fix for now.

The new tests in t1401 use git-symbolic-ref, which is the
most direct way to check the resolution by itself.
Interestingly we actually had a test that setup this case
already, but we only used it to verify that the funny state
could be overwritten, not that it could be resolved.

We also add a new test in t3200, as "branch -m" was the
original motivation for looking into this. What happens is
this:

  0. HEAD is pointing to branch "a"

  1. The user asks to rename "a" to "a/b".

  2. We create "a/b" and delete "a".

  3. We then try to update any worktree HEADs that point to
     the renamed ref (including the main repo HEAD). To do
     that, we have to resolve each HEAD. But now our HEAD is
     pointing at "a", and we get EISDIR due to the loose
     "a/b". As a result, we think there is no HEAD, and we
     do not update it. It now points to the bogus "a".

Interestingly this case used to work, but only accidentally.
Before 31824d180d (branch: fix branch renaming not updating
HEADs correctly, 2017-08-24), we'd update any HEAD which we
couldn't resolve. That was wrong, but it papered over the
fact that we were incorrectly failing to resolve HEAD.

So while the bug demonstrated by the git-symbolic-ref is
quite old, the regression to "branch -m" is recent.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

t3308: create a real ref directory/file conflict

A test in t3308 wants to make sure that we don't
accidentally merge into "refs/notes/dir" when it exists as a
directory, so it does:

  mkdir .git/refs/notes/dir
  git -c core.notesRef=refs/notes/dir merge ...

and expects the second command to fail. But that
understimates the refs code, which is smart enough to remove
useless directories in the refs hierarchy. The test
succeeded only because of a bug which prevented resolving
refs/notes/dir for writing, even though an actual ref update
would succeed.

In preparation for fixing that bug, let's switch to creating
a real ref in refs/notes/dir, which is a more realistic
situation.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

api-argv-array.txt: remove broken link to string-list API

In 4f665f2cf3 (string-list.h: move documentation from Documentation/api/
into header, 2017-09-26) the string-list API documentation was moved to
string-list.h.  The argv-array API documentation may follow a similar
course in the future.  Until then, prevent the broken link from making
it to the end-user documentation.

Signed-off-by: Todd Zullinger <tmz@pobox.com>
Reviewed-by: Jonathan Nieder <jrnieder@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

entry.c: check if file exists after checkout

If we are checking out a file and somebody else racily deletes our file,
then we would write garbage to the cache entry. Fix that by checking
the result of the lstat() call on that file. Print an error to the user
if the file does not exist.

Reported-by: Jeff King <peff@peff.net>
Signed-off-by: Lars Schneider <larsxschneider@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

sha1_loose_object_info: handle errors from unpack_sha1_rest

When a caller of sha1_object_info_extended() sets the
"contentp" field in object_info, we call unpack_sha1_rest()
but do not check whether it signaled an error.

This causes two problems:

  1. We pass back NULL to the caller via the contentp field,
     but the function returns "0" for success. A caller
     might reasonably expect after a successful return that
     it can access contentp without a NULL check and
     segfault.

     As it happens, this is impossible to trigger in the
     current code. There is exactly one caller which uses
     contentp, read_object(). And the only thing it does
     after a successful call is to return the content
     pointer to its caller, using NULL as a sentinel for
     errors. So in effect it converts the success code from
     sha1_object_info_extended() back into an error!

     But this is still worth addressing avoid problems for
     future users of "contentp".

  2. Callers of unpack_sha1_rest() are expected to close the
     zlib stream themselves on error. Which means that we're
     leaking the stream.

The problem in (1) comes from from c84a1f3ed4 (sha1_file:
refactor read_object, 2017-06-21), which added the contentp
field.  Before that, we called unpack_sha1_rest() via
unpack_sha1_file(), which directly used the NULL to signal
an error.

But note that the leak in (2) is actually older than that.
The original unpack_sha1_file() directly returned the result
of unpack_sha1_rest() to its caller, when it should have
been closing the zlib stream itself on error.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

.mailmap: normalize name for René Scharfe

Reported-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Reported-by: Stefan Beller <sbeller@google.com>
Signed-off-by: Rene Scharfe <l.s.r@web.de>
Reviewed-by: Jonathan Nieder <jrnieder@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

fsck: handle NULL return of lookup_blob() and lookup_tree()

lookup_blob() and lookup_tree() can return NULL if they find an object
of an unexpected type.  Accessing the object member is undefined in that
case.  Cast the result to a struct object pointer instead; we can do
that because object is the first member of all object types.  This trick
is already used in other places in the code.

An error message is already shown by object_as_type(), which is called
by the lookup functions.  The walk callback functions are expected to
handle NULL object pointers passed to them, but put_object_name() needs
a valid object, so avoid calling it without one.

Suggested-by: SZEDER Gábor <szeder.dev@gmail.com>
Helped-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Rene Scharfe <l.s.r@web.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

entry.c: update cache entry only for existing files

In 2841e8f ("convert: add "status=delayed" to filter process protocol",
2017-06-30) we taught the filter process protocol to delay responses.

That means an external filter might answer in the first write_entry()
call on a file that requires filtering  "I got your request, but I
can't answer right now. Ask again later!". As Git got no answer, we do
not write anything to the filesystem. Consequently, the lstat() call in
the finish block of the function writes garbage to the cache entry.
The garbage is eventually overwritten when the filter answers with
the final file content in a subsequent write_entry() call.

Fix the brief time window of garbage in the cache entry by adding a
special finish block that does nothing for delayed responses. The cache
entry is written properly in a subsequent write_entry() call where
the filter responds with the final file content.

Reported-by: Jeff King <peff@peff.net>
Signed-off-by: Lars Schneider <larsxschneider@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

Git 2.15-rc0

Signed-off-by: Junio C Hamano <gitster@pobox.com>

Merge branch 'ar/request-pull-phrasofix'

Spell the name of our system as "Git" in the output from
request-pull script.

* ar/request-pull-phrasofix:
  request-pull: capitalise "Git" to make it a proper noun

Merge branch 'rs/run-command-use-alloc-array'

Code clean-up.

* rs/run-command-use-alloc-array:
  run-command: use ALLOC_ARRAY

Merge branch 'sb/git-clang-format'

Add comment to clarify that the style file is meant to be used with
clang-5 and the rules are still work in progress.

* sb/git-clang-format:
  clang-format: add a comment about the meaning/status of the

Merge branch 'rs/use-free-and-null'

Code clean-up.

* rs/use-free-and-null:
  repository: use FREE_AND_NULL

Merge branch 'rs/tag-null-pointer-arith-fix'

Code clean-up.

* rs/tag-null-pointer-arith-fix:
  tag: avoid NULL pointer arithmetic

Merge branch 'rs/cocci-de-paren-call-params'

Code clean-up.

* rs/cocci-de-paren-call-params:
  coccinelle: remove parentheses that become unnecessary

Merge branch 'rs/cleanup-strbuf-users'

Code clean-up.

* rs/cleanup-strbuf-users:
  graph: use strbuf_addchars() to add spaces
  use strbuf_addstr() for adding strings to strbufs
  path: use strbuf_add_real_path()

Merge branch 'rs/resolve-ref-optional-result'

Code clean-up.

* rs/resolve-ref-optional-result:
  refs: pass NULL to resolve_refdup() if hash is not needed
  refs: pass NULL to refs_resolve_refdup() if hash is not needed

Merge branch 'er/fast-import-dump-refs-on-checkpoint'

The checkpoint command "git fast-import" did not flush updates to
refs and marks unless at least one object was created since the
last checkpoint, which has been corrected, as these things can
happen without any new object getting created.

* er/fast-import-dump-refs-on-checkpoint:
  fast-import: checkpoint: dump branches/tags/marks even if object_count==0

ref-filter.c: pass empty-string as NULL to atom parsers

Peff points out that different atom parsers handle the empty
"sub-argument" list differently. An example of this is the format
"%(refname:)".

Since callers often use `string_list_split` (which splits the empty
string with any delimiter as a 1-ary string_list containing the empty
string), this makes handling empty sub-argument strings non-ergonomic.

Let's fix this by declaring that atom parser implementations must
not care about distinguishing between the empty string "%(refname:)"
and no sub-arguments "%(refname)".  Current code aborts, either with
"unrecognised arg" (e.g. "refname:") or "does not take args"
(e.g. "body:") as an error message.

Signed-off-by: Taylor Blau <me@ttaylorr.com>
Reviewed-by: Jeff King <peff@peff.net>
Reviewed-by: Jonathan Nieder <jrnieder@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

strbuf doc: reuse after strbuf_release is fine

strbuf_release leaves the strbuf in a valid, initialized state, so
there is no need to call strbuf_init after it.

Moreover, this is not likely to change in the future: strbuf_release
leaving the strbuf in a valid state has been easy to maintain and has
been very helpful for Git's robustness and simplicity (e.g.,
preventing use-after-free vulnerabilities).

Document the semantics so the next generation of Git developers can
become familiar with them without reading the implementation.  It is
still not advisable to call strbuf_release too often because it is
wasteful, so add a note pointing to strbuf_reset for that.

The same semantics apply to strbuf_detach.  Add a similar note to its
docstring to make that clear.

Improved-by: Jeff King <peff@peff.net>
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

branch: reset instead of release a strbuf

Our documentation advises to not re-use a strbuf, after strbuf_release
has been called on it. Use the proper reset instead.

Currently 'strbuf_release' releases and re-initializes the strbuf, so it
is safe, but slow. 'strbuf_reset' only resets the internal length variable,
such that this could also be accounted for as a micro-optimization.

Reviewed-by: Jonathan Nieder <jrnieder@gmail.com>
Signed-off-by: Stefan Beller <sbeller@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

sub-process: use child_process.args instead of child_process.argv

Currently the argv is only allocated on the stack, and then assigned to
process->argv.  When the start_subprocess function goes out of scope,
the local argv variable is eliminated from the stack, but the pointer is
still kept around in process->argv.

Much later when we try to access the same process->argv in
finish_command, this leads us to access a memory location that no longer
contains what we want.  As argv0 is only used for printing errors, this
is not easily noticed in normal git operations.  However when running
t0021-conversion.sh through valgrind, valgrind rightfully complains:

==21024== Invalid read of size 8
==21024==    at 0x2ACF64: finish_command (run-command.c:869)
==21024==    by 0x2D6B18: subprocess_exit_handler (sub-process.c:72)
==21024==    by 0x2AB41E: cleanup_children (run-command.c:45)
==21024==    by 0x2AB526: cleanup_children_on_exit (run-command.c:81)
==21024==    by 0x54AD487: __run_exit_handlers (in /usr/lib/libc-2.26.so)
==21024==    by 0x54AD4D9: exit (in /usr/lib/libc-2.26.so)
==21024==    by 0x11A9EF: handle_builtin (git.c:550)
==21024==    by 0x11ABCC: run_argv (git.c:602)
==21024==    by 0x11AD8E: cmd_main (git.c:679)
==21024==    by 0x1BF125: main (common-main.c:43)
==21024==  Address 0x1ffeffec00 is on thread 1's stack
==21024==  1504 bytes below stack pointer
==21024==

These days, the child_process structure has its own args array, and
the standard way to set up its argv[] is to use that one, instead of
assigning to process->argv to point at an array that is outside.
Use that facility automatically fixes this issue.

Reported-by: Thomas Gummerer <t.gummerer@gmail.com>
Signed-off-by: Johannes Sixt <j6t@kdbg.org>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

http-push: fix construction of hex value from path

The get_oid_hex_from_objpath takes care of creating a oid from a
pathname.  It does this by memcpy'ing the first two bytes of the path to
the "hex" string, then skipping the '/', and then copying the rest of the
path to the "hex" string.  Currently it fails to increase the pointer to
the hex string, so the second memcpy invocation just mashes over what
was copied in the first one, and leaves the last two bytes in the string
uninitialized.

This breaks valgrind in t5540, although the test passes without
valgrind:

==5490== Use of uninitialised value of size 8
==5490==    at 0x13C6B5: hexval (cache.h:1238)
==5490==    by 0x13C6DB: hex2chr (cache.h:1247)
==5490==    by 0x13C734: get_sha1_hex (hex.c:42)
==5490==    by 0x13C78E: get_oid_hex (hex.c:53)
==5490==    by 0x118BDA: get_oid_hex_from_objpath (http-push.c:1023)
==5490==    by 0x118C92: process_ls_object (http-push.c:1038)
==5490==    by 0x118E5B: handle_remote_ls_ctx (http-push.c:1077)
==5490==    by 0x118227: xml_end_tag (http-push.c:815)
==5490==    by 0x50C1448: ??? (in /usr/lib/libexpat.so.1.6.6)
==5490==    by 0x50C221B: ??? (in /usr/lib/libexpat.so.1.6.6)
==5490==    by 0x50BFBF2: ??? (in /usr/lib/libexpat.so.1.6.6)
==5490==    by 0x50C0B24: ??? (in /usr/lib/libexpat.so.1.6.6)
==5490==  Uninitialised value was created by a stack allocation
==5490==    at 0x118B63: get_oid_hex_from_objpath (http-push.c:1012)
==5490==

Fix this by correctly incrementing the pointer to the "hex" variable, so
the first two bytes are left untouched by the memcpy call, and the last
two bytes are correctly initialized.

Signed-off-by: Thomas Gummerer <t.gummerer@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

path.c: fix uninitialized memory access

In cleanup_path we're passing in a char array, run a memcmp on it, and
run through it without ever checking if something is in the array in the
first place.  This can lead us to access uninitialized memory, for
example in t5541-http-push-smart.sh test 7, when run under valgrind:

==4423== Conditional jump or move depends on uninitialised value(s)
==4423==    at 0x242FA9: cleanup_path (path.c:35)
==4423==    by 0x242FA9: mkpath (path.c:456)
==4423==    by 0x256CC7: refname_match (refs.c:364)
==4423==    by 0x26C181: count_refspec_match (remote.c:1015)
==4423==    by 0x26C181: match_explicit_lhs (remote.c:1126)
==4423==    by 0x26C181: check_push_refs (remote.c:1409)
==4423==    by 0x2ABB4D: transport_push (transport.c:870)
==4423==    by 0x186703: push_with_options (push.c:332)
==4423==    by 0x18746D: do_push (push.c:409)
==4423==    by 0x18746D: cmd_push (push.c:566)
==4423==    by 0x1183E0: run_builtin (git.c:352)
==4423==    by 0x11973E: handle_builtin (git.c:539)
==4423==    by 0x11973E: run_argv (git.c:593)
==4423==    by 0x11973E: main (git.c:698)
==4423==  Uninitialised value was created by a heap allocation
==4423==    at 0x4C2CD8F: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==4423==    by 0x4C2F195: realloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==4423==    by 0x2C196B: xrealloc (wrapper.c:137)
==4423==    by 0x29A30B: strbuf_grow (strbuf.c:66)
==4423==    by 0x29A30B: strbuf_vaddf (strbuf.c:277)
==4423==    by 0x242F9F: mkpath (path.c:454)
==4423==    by 0x256CC7: refname_match (refs.c:364)
==4423==    by 0x26C181: count_refspec_match (remote.c:1015)
==4423==    by 0x26C181: match_explicit_lhs (remote.c:1126)
==4423==    by 0x26C181: check_push_refs (remote.c:1409)
==4423==    by 0x2ABB4D: transport_push (transport.c:870)
==4423==    by 0x186703: push_with_options (push.c:332)
==4423==    by 0x18746D: do_push (push.c:409)
==4423==    by 0x18746D: cmd_push (push.c:566)
==4423==    by 0x1183E0: run_builtin (git.c:352)
==4423==    by 0x11973E: handle_builtin (git.c:539)
==4423==    by 0x11973E: run_argv (git.c:593)
==4423==    by 0x11973E: main (git.c:698)
==4423==

Avoid this by using skip_prefix(), which knows not to go beyond the
end of the string.

Reported-by: Thomas Gummerer <t.gummerer@gmail.com>
Signed-off-by: Jeff King <peff@peff.net>
Reviewed-by: Jonathan Nieder <jrnieder@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

test-stringlist: avoid buffer underrun when sorting nothing

Check if the strbuf containing data to sort is empty before attempting
to trim a trailing newline character.

Signed-off-by: Rene Scharfe <l.s.r@web.de>
Reviewed-by: Jonathan Nieder <jrnieder@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

setup: update error message to be more meaningful

The error message shown when a flag is found when expecting a
filename wasn't clear as it didn't communicate what was wrong
using the 'suitable' words in *all* cases.

        $ git ls-files
        README.md
        test-file

Correct case,

        $ git rev-parse README.md --flags
        README.md
        --flags
        fatal: bad flag '--flags' used after filename

Incorrect case,

        $ git grep "some random regex" -n
        fatal: bad flag '-n' used after filename

The above case is incorrect as "some random regex" isn't a filename
in this case.

Change the error message to be general and communicative. This results
in the following output,

        $ git rev-parse README.md --flags
        README.md
        --flags
        fatal: option '--flags' must come before non-option arguments

        $ git grep "some random regex" -n
        fatal: option '-n' must come before non-option arguments

Signed-off-by: Kaartic Sivaraam <kaarticsivaraam91196@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

branch: change the error messages to be more meaningful

The error messages shown when the branch command is misused
by supplying it wrong number of parameters wasn't meaningful.
That's because it used the the phrase "too many branches"
assuming all parameters to be "valid" branch names. It's not
always the case as exemplified below,

        $ git branch
          foo
        * master

        $ git branch -m foo foo old
        fatal: too many branches for a rename operation

Change the messages to be more general thus making no assumptions
about the "parameters".

Signed-off-by: Kaartic Sivaraam <kaarticsivaraam91196@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

Merge branch 'jk/ui-color-always-to-auto-maint' into jk/ui-color-always-to-auto

* jk/ui-color-always-to-auto-maint:
  color: make "always" the same as "auto" in config
  provide --color option for all ref-filter users
  t3205: use --color instead of color.branch=always
  t3203: drop "always" color test
  t6006: drop "always" color config tests
  t7502: use diff.noprefix for --verbose test
  t7508: use test_terminal for color output
  t3701: use test-terminal to collect color output
  t4015: prefer --color to -c color.diff=always
  test-terminal: set TERM=vt100

t7301: use test_terminal to check color

This test wants to confirm that "clean -i" shows color
output. Using test_terminal gives us a more realistic
environment than "color.ui=always", and prepares us for the
behavior of "always" changing in a future patch.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

t4015: use --color with --color-moved

The tests for --color-moved write their output to a file,
but doing so suppresses color output under "auto". Right now
this is solved by running the whole script under
"color.diff=always". In preparation for the behavior of
"always" changing, let's explicitly enable color.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

color: make "always" the same as "auto" in config

It can be handy to use `--color=always` (or it's synonym
`--color`) on the command-line to convince a command to
produce color even if it's stdout isn't going to the
terminal or a pager.

What's less clear is whether it makes sense to set config
variables like color.ui to `always`. For a one-shot like:

  git -c color.ui=always ...

it's potentially useful (especially if the command doesn't
directly support the `--color` option). But setting `always`
in your on-disk config is much muddier, as you may be
surprised when piped commands generate colors (and send them
to whatever is consuming the pipe downstream).

Some people have done this anyway, because:

  1. The documentation for color.ui makes it sound like
     using `always` is a good idea, when you almost
     certainly want `auto`.

  2. Traditionally not every command (and especially not
     plumbing) respected color.ui in the first place. So
     the confusion came up less frequently than it might
     have.

The situation changed in 136c8c8b8f (color: check color.ui
in git_default_config(), 2017-07-13), which negated point
(2): now scripts using only plumbing commands (like
add-interactive) are broken by this setting.

That commit was fixing real issues (e.g., by making
`color.ui=never` work, since `auto` is the default), so we
don't want to just revert it.  We could turn `always` into a
noop in plumbing commands, but that creates a hard-to-explain
inconsistency between the plumbing and other commands.

Instead, let's just turn `always` into `auto` for all config.
This does break the "one-shot" config shown above, but again,
we're probably better to have simple and consistent rules than
to try to special-case command-line config.

There is one place where `always` should retain its meaning:
on the command line, `--color=always` should continue to be
the same as `--color`, overriding any isatty checks. Since the
command-line parser also depends on git_config_colorbool(), we
can use the existence of the "var" string to deterine whether
we are serving the command-line or the config.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

provide --color option for all ref-filter users

When ref-filter learned about want_color() in 11b087adfd
(ref-filter: consult want_color() before emitting colors,
2017-07-13), it became useful to be able to turn colors off
and on for specific commands. For git-branch, you can do so
with --color/--no-color.

But for git-for-each-ref and git-tag, the other users of
ref-filter, you have no option except to tweak the
"color.ui" config setting. Let's give both of these commands
the usual color command-line options.

This is a bit more obvious as a method for overriding the
config. And it also prepares us for the behavior of "always"
changing (so that we are still left with a way of forcing
color when our output goes to a non-terminal).

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

t3205: use --color instead of color.branch=always

To test the color output, we must convince "git branch" to
write colors to a non-terminal. We do that now by setting
the color config to "always".  In preparation for the
behavior of "always" changing, let's switch to using the
"--color" command-line option, which is more direct.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

t3203: drop "always" color test

In preparation for the behavior of "always" changing to
match "auto", we can simply drop this test. We already check
other forms (like "--color") independently.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

t6006: drop "always" color config tests

We test the %C() format placeholders with a variety of
color-inducing options, including "--color" and
"-c color.ui=always". In preparation for the behavior of
"always" changing, we need to do something with those
"always" tests.

We can drop ones that expect "always" to turn on color even
to a file, as that will become a synonym for "auto", which
is already tested.

For the "--no-color" test, we need to make sure that color
would otherwise be shown. To do this, we can use
test_terminal, which enables colors in the default setup.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

t7502: use diff.noprefix for --verbose test

To check that "status -v" respects diff config, we set
"color.diff" and look at the output of "status". We could
equally well use any diff config. Since color output depends
on a lot of other factors (like whether stdout is a tty, and
how we interpret "always"), let's use a more mundane option.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

t7508: use test_terminal for color output

This script tests the output of status with various formats
when color is enabled. It uses the "always" setting so that
the output is valid even though we capture it in a file.
Using test_terminal gives us a more realistic environment,
and prepares us for the behavior of "always" changing.

Arguably we are testing less than before, since "auto" is
already the default, and we can no longer tell if the config
is actually doing anything.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

t3701: use test-terminal to collect color output

When testing whether "add -p" can generate colors, we set
color.ui to "always". This isn't a very good test, as in the
real-world a user typically has "auto" coupled with stdout
going to a terminal (and it's plausible that this could mask
a real bug in add--interactive if we depend on plumbing's
isatty check).

Let's switch to test_terminal, which gives us a more
realistic environment. This also prepare us for future
changes to the "always" color option.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

t4015: prefer --color to -c color.diff=always

t4015 contains many color-related tests which need to
override the "is stdout a tty" check. They do so by setting
the color.diff config, but we can accomplish the same with
the --color option. Besides being shorter to type, switching
will prepare us for upcoming changes to "always" when see it
in config.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

test-terminal: set TERM=vt100

The point of the test-terminal script is to simulate in the
test scripts an environment where output is going to a real
terminal.

But since test-lib.sh also sets TERM=dumb, the simulation
isn't very realistic. The color code will skip auto-coloring
for TERM=dumb, leading to us liberally sprinkling

  test_terminal env TERM=vt100 git ...

through the test suite to convince the tests to actually
generate colors. Let's set TERM for programs run under
test_terminal, which is one less thing for test-writers to
remember.

In most cases the callers can be simplified, but note there
is one interesting case in t4202. It uses test_terminal to
check the auto-enabling of --decorate, but the expected
output _doesn't_ contain colors (because TERM=dumb
suppresses them). Using TERM=vt100 is closer to what the
real world looks like; adjust the expected output to match.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

The twelfth batch for 2.15

Signed-off-by: Junio C Hamano <gitster@pobox.com>

Merge branch 'bw/git-clang-format'

Adjust clang-format penalty parameters.

* bw/git-clang-format:
  clang-format: adjust line break penalties

Merge branch 'ad/doc-markup-fix'

Docfix.

* ad/doc-markup-fix:
  doc: correct command formatting

Merge branch 'mh/mmap-packed-refs'

Operations that do not touch (majority of) packed refs have been
optimized by making accesses to packed-refs file lazy; we no longer
pre-parse everything, and an access to a single ref in the
packed-refs does not touch majority of irrelevant refs, either.

* mh/mmap-packed-refs: (21 commits)
  packed-backend.c: rename a bunch of things and update comments
  mmapped_ref_iterator: inline into `packed_ref_iterator`
  ref_cache: remove support for storing peeled values
  packed_ref_store: get rid of the `ref_cache` entirely
  ref_store: implement `refs_peel_ref()` generically
  packed_read_raw_ref(): read the reference from the mmapped buffer
  packed_ref_iterator_begin(): iterate using `mmapped_ref_iterator`
  read_packed_refs(): ensure that references are ordered when read
  packed_ref_cache: keep the `packed-refs` file mmapped if possible
  packed-backend.c: reorder some definitions
  mmapped_ref_iterator_advance(): no peeled value for broken refs
  mmapped_ref_iterator: add iterator over a packed-refs file
  packed_ref_cache: remember the file-wide peeling state
  read_packed_refs(): read references with minimal copying
  read_packed_refs(): make parsing of the header line more robust
  read_packed_refs(): only check for a header at the top of the file
  read_packed_refs(): use mmap to read the `packed-refs` file
  die_unterminated_line(), die_invalid_line(): new functions
  packed_ref_cache: add a backlink to the associated `packed_ref_store`
  prefix_ref_iterator: break when we leave the prefix
  ...

Merge branch 'mr/doc-negative-pathspec'

Doc updates.

* mr/doc-negative-pathspec:
  docs: improve discoverability of exclude pathspec

Merge branch 'sb/submodule-diff-header-fix'

Error message tweak.

* sb/submodule-diff-header-fix:
  submodule: correct error message for missing commits

Merge branch 'sb/diff-color-move'

The output from "git diff --summary" was broken in a recent topic
that has been merged to 'master' and lost a LF after reporting of
mode change.  This has been fixed.

* sb/diff-color-move:
  diff: correct newline in summary for renamed files

Merge branch 'sb/test-submodule-update-config'

* sb/test-submodule-update-config:
  t7406: submodule.<name>.update command must not be run from .gitmodules

Merge branch 'jk/validate-headref-fix'

Code clean-up.

* jk/validate-headref-fix:
  validate_headref: use get_oid_hex for detached HEADs
  validate_headref: use skip_prefix for symref parsing
  validate_headref: NUL-terminate HEAD buffer

Merge branch 'jk/read-in-full'

Code clean-up to prevent future mistakes by copying and pasting
code that checks the result of read_in_full() function.

* jk/read-in-full:
  worktree: check the result of read_in_full()
  worktree: use xsize_t to access file size
  distinguish error versus short read from read_in_full()
  avoid looking at errno for short read_in_full() returns
  prefer "!=" when checking read_in_full() result
  notes-merge: drop dead zero-write code
  files-backend: prefer "0" for write_in_full() error check

Merge branch 'jk/no-optional-locks'

Some commands (most notably "git status") makes an opportunistic
update when performing a read-only operation to help optimize later
operations in the same repository.  The new "--no-optional-locks"
option can be passed to Git to disable them.

* jk/no-optional-locks:
  git: add --no-optional-locks option

Merge branch 'hn/string-list-doc'

Doc reorg.

* hn/string-list-doc:
  string-list.h: move documentation from Documentation/api/ into header

Merge branch 'hn/path-ownership-comment'

Add comment to a few functions that use a short-lived buffer the
caller can peek and copy out of.

* hn/path-ownership-comment:
  read_gitfile_gently: clarify return value ownership.
  real_path: clarify return value ownership

Merge branch 'hn/submodule-comment'

* hn/submodule-comment:
  submodule.c: describe submodule_to_gitdir() in a new comment

Merge branch 'sd/branch-copy'

"git branch" learned "-c/-C" to create a new branch by copying an
existing one.

* sd/branch-copy:
  branch: fix "copy" to never touch HEAD
  branch: add a --copy (-c) option to go with --move (-m)
  branch: add test for -m renaming multiple config sections
  config: create a function to format section headers

Merge branch 'bc/rev-parse-parseopt-fix'

Recent versions of "git rev-parse --parseopt" did not parse the
option specification that does not have the optional flags (*=?!)
correctly, which has been corrected.

* bc/rev-parse-parseopt-fix:
  parse-options: only insert newline in help text if needed
  parse-options: write blank line to correct output stream
  t0040,t1502: Demonstrate parse_options bugs
  git-rebase: don't ignore unexpected command line arguments
  rev-parse parseopt: interpret any whitespace as start of help text
  rev-parse parseopt: do not search help text for flag chars
  t1502: demonstrate rev-parse --parseopt option mis-parsing

Merge branch 'js/rebase-i-final'

The final batch to "git rebase -i" updates to move more code from
the shell script to C.

* js/rebase-i-final:
  rebase -i: rearrange fixup/squash lines using the rebase--helper
  t3415: test fixup with wrapped oneline
  rebase -i: skip unnecessary picks using the rebase--helper
  rebase -i: check for missing commits in the rebase--helper
  t3404: relax rebase.missingCommitsCheck tests
  rebase -i: also expand/collapse the SHA-1s via the rebase--helper
  rebase -i: do not invent onelines when expanding/collapsing SHA-1s
  rebase -i: remove useless indentation
  rebase -i: generate the script via rebase--helper
  t3415: verify that an empty instructionFormat is handled as before

request-pull: capitalise "Git" to make it a proper noun

Of the many ways to spell the three-letter word, the variant "Git"
should be used when referring to a repository in a description; or, in
general, when it is used as a proper noun.

We thus change the pull-request template message so that it reads

   "...in the Git repository at:"

Besides, this brings us in line with the documentation, see
Documentation/howto/using-signed-tag-in-pull-request.txt

Signed-off-by: Ann T Ropea <bedhanger@gmx.de>
Acked-by: Jonathan Nieder <jrnieder@gmail.com>
Reviewed-by: Jonathan Nieder <jrnieder@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

run-command: use ALLOC_ARRAY

Use the macro ALLOC_ARRAY to allocate an array.  This is shorter and
easier, as it automatically infers the size of elements.

Patch generated with Coccinelle and contrib/coccinelle/array.cocci.

Signeg-off-by: Rene Scharfe <l.s.r@web.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

ref-filter.c: parse trailers arguments with %(contents) atom

The %(contents) atom takes a contents "field" as its argument. Since
"trailers" is one of those fields, extend contents_atom_parser to parse
"trailers"'s arguments when used through "%(contents)", like:

  %(contents:trailers:unfold,only)

A caveat: trailers_atom_parser expects NULL when no arguments are given
(see: `parse_ref_filter_atom`). This is because string_list_split (given
a maxsplit of -1) returns a 1-ary string_list* containing the given
string if the delimiter could not be found using `strchr`.

To simulate this behavior without teaching trailers_atom_parser to
accept strings with length zero, conditionally pass NULL to
trailers_atom_parser if the arguments portion of the argument to
%(contents) is empty.

Signed-off-by: Taylor Blau <me@ttaylorr.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

ref-filter.c: use trailer_opts to format trailers

Fill trailer_opts with "unfold" and "only" to match the sub-arguments
given to the "%(trailers)" atom. Then, let's use the filled trailer_opts
instance with 'format_trailers_from_commit' in order to format trailers
in the desired manner.

Signed-off-by: Taylor Blau <me@ttaylorr.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

t6300: refactor %(trailers) tests

We currently have one test for %(trailers) in `git-for-each-ref(1)`,
through "%(contents:trailers)". In preparation for more, let's add a few
things:

  - Move the commit creation step to its own test so that it can be
  re-used.

  - Add a non-trailer to the commit's trailers to test that non-trailers
  aren't shown using "%(trailers:only)".

  - Add a multi-line trailer to ensure that trailers are unfolded
  correctly using "%(trailers:unfold)".

Signed-off-by: Taylor Blau <me@ttaylorr.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

doc: use "`<literal>`"-style quoting for literal strings

"'<string>'"-style quoting is not appropriate when quoting literal
strings in the "Documentation/" subtree.

In preparation for adding additional information to this section of
git-for-each-ref(1)'s documentation, update them to use "`<literal>`"
instead.

Signed-off-by: Taylor Blau <me@ttaylorr.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

doc: 'trailers' is the preferred way to format trailers

The documentation makes reference to 'contents:trailers' as an example
to dig the trailers out of a commit. 'trailers' is an unmentioned
alternative, which is treated as an alias of 'contents:trailers'.

Since 'trailers' is easier to type, prefer that as the designated way to
dig out trailers information.

Signed-off-by: Taylor Blau <me@ttaylorr.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

t4205: unfold across multiple lines

Tests in t4205 test the following:

  git log --format='%(trailers:unfold)' ...

By ensuring the multi-line trailers are unfolded back onto the same
line. t4205 only includes tests for 2-line trailers, but `unfold()` will
fail for folded trailers on 3 or more lines.

In preparation for adding subsequent tests in t6300 that test similar
behavior in `git-for-each-ref(1)`, let's harden t4205 (and make it
consistent with the changes in t6300) by ensuring that 3 or more
line folded trailers are unfolded correctly.

Signed-off-by: Taylor Blau <me@ttaylorr.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>