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

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

Merge branch 'jt/fast-export-copy-modify-fix' into maint

"git fast-export" with -M/-C option issued "copy" instruction on a
path that is simultaneously modified, which was incorrect.

* jt/fast-export-copy-modify-fix:
  fast-export: do not copy from modified file

Merge branch 'nd/worktree-kill-parse-ref' into maint

"git branch -M a b" while on a branch that is completely unrelated
to either branch a or branch b misbehaved when multiple worktree
was in use.  This has been fixed.

* nd/worktree-kill-parse-ref:
  branch: fix branch renaming not updating HEADs correctly

Prepare for 2.14.3

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

Merge branch 'jk/ref-filter-colors-fix' into maint

This is the "theoretically more correct" approach of simply
stepping back to the state before plumbing commands started paying
attention to "color.ui" configuration variable.

* jk/ref-filter-colors-fix:
  tag: respect color.ui config
  Revert "color: check color.ui in git_default_config()"
  Revert "t6006: drop "always" color config tests"
  Revert "color: make "always" the same as "auto" in config"
  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 'jc/doc-checkout' into maint

Doc update.

* jc/doc-checkout:
  checkout doc: clarify command line args for "checkout paths" mode

Merge branch 'tb/complete-describe' into maint

Docfix.

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

Merge branch 'rs/rs-mailmap' into maint

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

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

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' into maint

Leakfix and futureproofing.

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

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

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

Merge branch 'rs/qsort-s' into maint

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

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

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

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

Code clean-up.

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

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

Code clean-up.

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

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

Code clean-up.

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

Merge branch 'ad/doc-markup-fix' into maint

Docfix.

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

Merge branch 'mr/doc-negative-pathspec' into maint

Doc updates.

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

Merge branch 'jk/validate-headref-fix' into maint

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 'ks/doc-use-camelcase-for-config-name' into maint

Doc update.

* ks/doc-use-camelcase-for-config-name:
  doc: camelCase the config variables to improve readability

Merge branch 'jk/doc-read-tree-table-asciidoctor-fix' into maint

A docfix.

* jk/doc-read-tree-table-asciidoctor-fix:
  doc: put literal block delimiter around table

Merge branch 'hn/typofix' into maint

* hn/typofix:
  submodule.h: typofix

Merge branch 'ks/test-readme-phrasofix' into maint

Doc updates.

* ks/test-readme-phrasofix:
  t/README: fix typo and grammatically improve a sentence

Merge branch 'ez/doc-duplicated-words-fix' into maint

Typofix.

* ez/doc-duplicated-words-fix:
  doc: fix minor typos (extra/duplicated words)

Merge branch 'kd/doc-for-each-ref' into maint

Doc update.

* kd/doc-for-each-ref:
  doc/for-each-ref: explicitly specify option names
  doc/for-each-ref: consistently use '=' to between argument names and values

Merge branch 'cc/subprocess-handshake-missing-capabilities' into maint

Finishing touches to a topic already in 'master'.

* cc/subprocess-handshake-missing-capabilities:
  subprocess: loudly die when subprocess asks for an unsupported capability

Merge branch 'jk/system-path-cleanup' into maint

Code clean-up.

* jk/system-path-cleanup:
  git_extract_argv0_path: do nothing without RUNTIME_PREFIX
  system_path: move RUNTIME_PREFIX to a sub-function

Merge branch 'bb/doc-eol-dirty' into maint

Doc update.

* bb/doc-eol-dirty:
  Documentation: mention that `eol` can change the dirty status of paths

Merge branch 'mg/timestamp-t-fix' into maint

A mismerge fix.

* mg/timestamp-t-fix:
  name-rev: change ULONG_MAX to TIME_MAX

Merge branch 'ma/pkt-line-leakfix' into maint

A leakfix.

* ma/pkt-line-leakfix:
  pkt-line: re-'static'-ify buffer in packet_write_fmt_1()

Merge branch 'jk/config-lockfile-leak-fix' into maint

A leakfix.

* jk/config-lockfile-leak-fix:
  config: use a static lock_file struct

Merge branch 'dw/diff-highlight-makefile-fix' into maint

Build clean-up.

* dw/diff-highlight-makefile-fix:
  diff-highlight: add clean target to Makefile

Merge branch 'jk/drop-sha1-entry-pos' into maint

Code clean-up.

* jk/drop-sha1-entry-pos:
  sha1-lookup: remove sha1_entry_pos() from header file
  sha1_file: drop experimental GIT_USE_LOOKUP search

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

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 'rb/compat-poll-fix' into maint

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' into maint

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 'ar/request-pull-phrasofix' into maint

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 'jc/merge-x-theirs-docfix' into maint

The documentation for '-X<option>' for merges was misleadingly
written to suggest that "-s theirs" exists, which is not the case.

* jc/merge-x-theirs-docfix:
  merge-strategies: avoid implying that "-s theirs" exists

Merge branch 'rs/mailinfo-qp-decode-fix' into maint

"git mailinfo" was loose in decoding quoted printable and produced
garbage when the two letters after the equal sign are not
hexadecimal.  This has been fixed.

* rs/mailinfo-qp-decode-fix:
  mailinfo: don't decode invalid =XY quoted-printable sequences

Merge branch 'ik/userdiff-html-h-element-fix' into maint

The built-in pattern to detect the "function header" for HTML did
not match <H1>..<H6> elements without any attributes, which has
been fixed.

* ik/userdiff-html-h-element-fix:
  userdiff: fix HTML hunk header regexp

Merge branch 'jk/diff-blob' into maint

"git cat-file --textconv" started segfaulting recently, which
has been corrected.

* jk/diff-blob:
  cat-file: handle NULL object_context.path

Merge branch 'jk/describe-omit-some-refs' into maint

"git describe --match" learned to take multiple patterns in v2.13
series, but the feature ignored the patterns after the first one
and did not work at all.  This has been fixed.

* jk/describe-omit-some-refs:
  describe: fix matching to actually match all patterns

Merge branch 'mh/for-each-string-list-item-empty-fix' into maint

Code cmp.std.c nitpick.

* mh/for-each-string-list-item-empty-fix:
  for_each_string_list_item: avoid undefined behavior for empty list

Merge branch 'tb/test-lint-echo-e' into maint

The test linter has been taught that we do not like "echo -e".

* tb/test-lint-echo-e:
  test-lint: echo -e (or -E) is not portable

Merge branch 'aw/gc-lockfile-fscanf-fix' into maint

"git gc" tries to avoid running two instances at the same time by
reading and writing pid/host from and to a lock file; it used to
use an incorrect fscanf() format when reading, which has been
corrected.

* aw/gc-lockfile-fscanf-fix:
  gc: call fscanf() with %<len>s, not %<len>c, when reading hostname

Merge branch 'tg/refs-allowed-flags' into maint

API error-proofing which happens to also squelch warnings from GCC.

* tg/refs-allowed-flags:
  refs: strip out not allowed flags from ref_transaction_update

Merge branch 'rs/archive-excluded-directory' into maint

"git archive", especially when used with pathspec, stored an empty
directory in its output, even though Git itself never does so.
This has been fixed.

* rs/archive-excluded-directory:
  archive: don't add empty directories to archives

Merge branch 'rk/commit-tree-make-F-verbatim' into maint

Unlike "git commit-tree < file", "git commit-tree -F file" did not
pass the contents of the file verbatim and instead completed an
incomplete line at the end, if exists.  The latter has been updated
to match the behaviour of the former.

* rk/commit-tree-make-F-verbatim:
  commit-tree: do not complete line in -F input

Merge branch 'mh/packed-ref-store-prep' into maint

Fix regression to "gitk --bisect" by a recent update.

* mh/packed-ref-store-prep:
  rev-parse: don't trim bisect refnames

Merge branch 'mm/send-email-cc-cruft' into maint

In addition to "cc: <a@dd.re.ss> # cruft", "cc: a@dd.re.ss # cruft"
was taught to "git send-email" as a valid way to tell it that it
needs to also send a carbon copy to <a@dd.re.ss> in the trailer
section.

* mm/send-email-cc-cruft:
  send-email: don't use Mail::Address, even if available
  send-email: fix garbage removal after address

Merge branch 'rs/strbuf-getwholeline-fix' into maint

A helper function to read a single whole line into strbuf
mistakenly triggered OOM error at EOF under certain conditions,
which has been fixed.

* rs/strbuf-getwholeline-fix:
  strbuf: clear errno before calling getdelim(3)

tag: respect color.ui config

Since 11b087adfd (ref-filter: consult want_color() before
emitting colors, 2017-07-13), we expect that setting
"color.ui" to "always" will enable color tag formats even
without a tty.  As that commit was built on top of
136c8c8b8f (color: check color.ui in git_default_config(),
2017-07-13) from the same series, we didn't need to touch
tag's config parsing at all.

However, since we reverted 136c8c8b8f, we now need to
explicitly call git_color_default_config() to make this
work.

Let's do so, and also restore the test dropped in 0c88bf5050
(provide --color option for all ref-filter users,
2017-10-03). That commit swapped out our "color.ui=always"
test for "--color" in preparation for "always" going away.
But since it is here to stay, we should test both cases.

Note that for-each-ref also lost its color.ui support as
part of reverting 136c8c8b8f. But as a plumbing command, it
should _not_ respect the color.ui config. Since it also
gained a --color option in 0c88bf5050, that's the correct
way to ask it for color. We'll continue to test that, and
confirm that "color.ui" is not respected.

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

Revert "color: check color.ui in git_default_config()"

This reverts commit 136c8c8b8fa39f1315713248473dececf20f8fe7.

That commit was trying to address a bug caused by 4c7f1819b3
(make color.ui default to 'auto', 2013-06-10), in which
plumbing like diff-tree defaulted to "auto" color, but did
not respect a "color.ui" directive to disable it.

But it also meant that we started respecting "color.ui" set
to "always". This was a known problem, but 4c7f1819b3 argued
that nobody ought to be doing that. However, that turned out
to be wrong, and we got a number of bug reports related to
"add -p" regressing in v2.14.2.

Let's revert 136c8c8b8, fixing the regression to "add -p".
This leaves the problem from 4c7f1819b3 unfixed, but:

  1. It's a pretty obscure problem in the first place. I
     only noticed it while working on the color code, and we
     haven't got a single bug report or complaint about it.

  2. We can make a more moderate fix on top by respecting
     "never" but not "always" for plumbing commands. This
     is just the minimal fix to go back to the working state
     we had before v2.14.2.

Note that this isn't a pure revert. We now have a test in
t3701 which shows off the "add -p" regression. This can be
flipped to success.

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

Revert "t6006: drop "always" color config tests"

This reverts commit c5bdfe677cfab5b2e87771c35565d44d3198efda.

That commit was done primarily to prepare for the weakening
of "always" in 6be4595edb (color: make "always" the same as
"auto" in config, 2017-10-03). But since we've now reverted
6be4595edb, there's no need for us to remove "-c
color.ui=always" from the tests. And in fact it's a good
idea to restore these tests, to make sure that "always"
continues to work.

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

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

This reverts commit 6be4595edb8e5b616c6e8b9fbc78b0f831fa2a87.

That commit weakened the "always" setting of color config so
that it acted as "auto". This was meant to solve regressions
in v2.14.2 in which setting "color.ui=always" in the on-disk
config broke scripts like add--interactive, because the
plumbing diff commands began to generate color output.

This was due to 136c8c8b8f (color: check color.ui in
git_default_config(), 2017-07-13), which was in turn trying
to fix issues caused by 4c7f1819b3 (make color.ui default to
'auto', 2013-06-10). But in weakening "always", we created
even more problems, as people expect to be able to use "git
-c color.ui=always" to force color (especially because some
commands don't have their own --color flag). We can fix that
by special-casing the command-line "-c", but now things are
getting pretty confusing.

Instead of piling hacks upon hacks, let's start peeling off
the hacks. The first step is dropping the weakening of
"always", which this revert does.

Note that we could actually revert the whole series merged
in by da15b78e52642bd45fd5513ab0000fdf2e58a6f4. Most of that
series consists of preparations to the tests to handle the
weakening of "-c color.ui=always". But it's worth keeping
for a few reasons:

  - there are some other preparatory cleanups, like
    e433749d86 (test-terminal: set TERM=vt100, 2017-10-03)

  - it adds "--color" options more consistently in
    0c88bf5050 (provide --color option for all ref-filter
    users, 2017-10-03)

  - some of the cases dropping "-c" end up being more robust
    and realistic tests, as in 01c94e9001 (t7508: use
    test_terminal for color output, 2017-10-03)

  - the preferred tool for overriding config is "--color",
    and we should be modeling that consistently

We can individually revert the few commits necessary to
restore some useful tests (which will be done on top of this
patch).

Note that this isn't a pure revert; we'll keep the test
added in t3701, but mark it as failure for now.

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

Merge branch 'jk/ui-color-always-to-auto-maint' (early part) into jk/ref-filter-colors-fix-maint

* 'jk/ui-color-always-to-auto-maint' (early part):
  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

checkout doc: clarify command line args for "checkout paths" mode

There are "git checkout [-p][<tree-ish>][--][<paths>...]" in the
SYNOPSIS section, and "git checkout [-p][<tree-ish>][--]<paths>..."
as the header for the section that explains the "check out paths
from index/tree-ish" mode.  It is unclear if we require at least one
path, or it is entirely optional.

Actually, both are wrong.  Without the "-p(atch)" option, you must
have <pathspec> (otherwise, with a commit that is a <tree-ish>, you
would be checking out that commit to build a new history on top of
it).  With it, it is already clear that you are checking out paths,
it is optional.  In other words, you cannot omit both.

The source of the confusion is that -p(atch) is described as if it
is just another "optional" part and its description is lumped
together with the non patch mode, even though the actual end user
experience is vastly different.

Let's split the entry into two, and describe the regular mode and
the patch mode separately.  This allows us to make it clear that the
regular mode MUST be given at least one pathspec, that the patch
mode can be invoked with either '-p' or '--patch' but one of these
must be given, and that the pathspec is entirely optional in the
patch mode.

Also, revamp the explanation of "checkout paths" by removing
extraneous description at the beginning, that says "checking out
paths is not checking out a branch".  Explaining what it is for and
when the user wants to use it upfront is the most direct way to help
the readers.

Noticed-by: Robert P J Day
Helped-by: Jonathan Nieder <jrnieder@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

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>

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>

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>

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>

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>

tag: avoid NULL pointer arithmetic

lookup_blob() etc. can return NULL if the referenced object isn't of the
expected type.  In theory it's wrong to reference the object member in
that case.  In practice it's OK because it's located at offset 0 for all
types, so the pointer arithmetic (NULL + 0) is optimized out by the
compiler.  The issue is reported by Clang's AddressSanitizer, though.

Avoid the ASan error by casting the results of the lookup functions to
struct object pointers.  That works fine with NULL pointers as well.  We
already rely on the object member being first in all object types in
other places in the code.

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

coccinelle: remove parentheses that become unnecessary

Transformations that hide multiplications can end up with an pair of
parentheses that is no longer needed.  E.g. with a rule like this:

  @@
  expression E;
  @@
  - E * 2
  + double(E)

... we might get a patch like this:

  - x = (a + b) * 2;
  + x = double((a + b));

Add a pair of parentheses to the preimage side of such rules.
Coccinelle will generate patches that remove them if they are present,
and it will still match expressions that lack them.

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

fast-import: checkpoint: dump branches/tags/marks even if object_count==0

The checkpoint command cycles packfiles if object_count != 0, a sensible
test or there would be no pack files to write. Since 820b931012, the
command also dumps branches, tags and marks, but still conditionally.
However, it is possible for a command stream to modify refs or create
marks without creating any new objects.

For example, reset a branch (and keep fast-import running):

$ git fast-import
reset refs/heads/master
from refs/heads/master^

checkpoint

but refs/heads/master remains unchanged.

Other example: a commit command that re-creates an object that already
exists in the object database.

The man page also states that checkpoint "updates the refs" and that
"placing a progress command immediately after a checkpoint will inform
the reader when the checkpoint has been completed and it can safely
access the refs that fast-import updated". This wasn't always true
without this patch.

This fix unconditionally calls dump_{branches,tags,marks}() for all
checkpoint commands. dump_branches() and dump_tags() are cheap to call
in the case of a no-op.

Add tests to t9300 that observe the (non-packfiles) effects of
checkpoint.

Signed-off-by: Eric Rannaud <e@nanocritical.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

poll.c: always set revents, even if to zero

Match what is done to pfd[i].revents when compute_revents() returns
0 to the upstream gnulib's commit d42461c3 ("poll: fixes for large
fds", 2015-02-20).  The revents field is set to 0, without
incrementing the value rc to be returned from the function.  The
original code left the field to whatever random value the field was
initialized to.

This fixes occasional hangs in git-upload-pack on HPE NonStop.

Signed-off-by: Randall S. Becker <randall.becker@nexbridge.ca>
Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

doc: correct command formatting

Leaving spaces around the `-delimeters for commands means asciidoc fails
to parse them as the start of a literal string.  Remove an extraneous
space that is causing a literal to not be formatted as such.

Signed-off-by: Adam Dinwoodie <adam@dinwoodie.org>
Acked-by: Andreas Heiduk <asheiduk@gmail.com>
Reviewed-by: Jonathan Nieder <jrnieder@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

validate_headref: use get_oid_hex for detached HEADs

If a candidate HEAD isn't a symref, we check that it
contains a viable sha1. But in a post-sha1 world, we should
be checking whether it has any plausible object-id.

We can do that by switching to get_oid_hex().

Note that both before and after this patch, we only check
for a plausible object id at the start of the file, and then
call that good enough.  We ignore any content _after_ the
hex, so a string like:

  0123456789012345678901234567890123456789 foo

is accepted. Though we do put extra bytes like this into
some pseudorefs (e.g., FETCH_HEAD), we don't typically do so
with HEAD. We could tighten this up by using parse_oid_hex(),
like:

  if (!parse_oid_hex(buffer, &oid, &end) &&
      *end++ == '\n' && *end == '\0')
          return 0;

But we're probably better to remain on the loose side. We're
just checking here for a plausible-looking repository
directory, so heuristics are acceptable (if we really want
to be meticulous, we should use the actual ref code to parse
HEAD).

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

validate_headref: use skip_prefix for symref parsing

Since the previous commit guarantees that our symref buffer
is NUL-terminated, we can just use skip_prefix() and friends
to parse it. This is shorter and saves us having to deal
with magic numbers and keeping the "len" counter up to date.

While we're at it, let's name the rather obscure "buf" to
"refname", since that is the thing we are parsing with it.

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

validate_headref: NUL-terminate HEAD buffer

When we are checking to see if we have a git repo, we peek
into the HEAD file and see if it's a plausible symlink,
symref, or detached HEAD.

For the latter two, we read the contents with read_in_full(),
which means they aren't NUL-terminated. The symref check is
careful to respect the length we got, but the sha1 check
will happily parse up to 40 bytes, even if we read fewer.

E.g.,:

  echo 1234 >.git/HEAD
  git rev-parse

will parse 36 uninitialized bytes from our stack buffer.

This isn't a big deal in practice. Our buffer is 256 bytes,
so we know we'll never read outside of it. The worst case is
that the uninitialized bytes look like valid hex, and we
claim a bogus HEAD file is valid. The chances of this
happening randomly are quite slim, but let's be careful.

One option would be to check that "len == 41" before feeding
the buffer to get_sha1_hex(). But we'd like to eventually
prepare for a world with variable-length hashes. Let's
NUL-terminate as soon as we've read the buffer (we already
even leave a spare byte to do so!). That fixes this problem
without depending on the size of an object id.

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

docs: improve discoverability of exclude pathspec

The ability to exclude paths with a negative pathspec is not mentioned
in the man pages for git grep and other commands where it might be
useful.

Add an example and a pointer to the pathspec glossary entry in the man
page for git grep to help the user to discover this ability.

Add similar pointers from the git-add and git-status man pages.

Additionally,

- Add a test for the behaviour when multiple exclusions are present.
- Add a test for the ^ alias.
- Improve name of existing test.
- Improve grammar in glossary description of the exclude pathspec.

Helped-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Manav Rathi <mnvrth@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

doc: camelCase the config variables to improve readability

References to multi-word configuration variable names in our
documentation must consistently use camelCase to highlight where
the word boundaries are, even though these are treated case
insensitively.

Fix a few places that spell them in all lowercase, which makes
them harder to read.

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

merge-strategies: avoid implying that "-s theirs" exists

The description of `-Xours` merge option has a parenthetical note
that tells the readers that it is very different from `-s ours`,
which is correct, but the description of `-Xtheirs` that follows it
carelessly says "this is the opposite of `ours`", giving a false
impression that the readers also need to be warned that it is very
different from `-s theirs`, which in reality does not even exist.

Clarify it a bit to avoid misleading readers.

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

mailinfo: don't decode invalid =XY quoted-printable sequences

Decode =XY in quoted-printable segments only if X and Y are hexadecimal
digits, otherwise just copy them.  That's at least better than
interpreting negative results from hexval() as a character.

Reported-by: Jeff King <peff@peff.net>
Signed-off-by: Rene Scharfe <l.s.r@web.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

userdiff: fix HTML hunk header regexp

Current HTML header regexp doesn't match headers without attributes.

So it fails to match <h1>...</h1>, while <h1 class="smth">...</h1> matches.

Make attributes optional to fix this.  The regexp is still far from
perfect, but now it at least handles the common case.

Signed-off-by: Ilya Kantor <iliakan@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

doc: put literal block delimiter around table

The git-read-tree manpage has a table that is meant to
be shown with its spacing exactly as it is in the source. We
mark it as a "literal paragraph" by indenting each line by
at least one space. This renders OK with asciidoc for both
the HTML and manpage versions.

But there are two problems when we render it with
asciidoctor.

The first is that some lines mix tabs and spaces.  Even if
asciidoctor is correctly configured for 8-space tabs, it
seems to handle this case differently, soaking up some of
the initial literal-paragraph spaces and mis-aligning the
table text.

The second problem is that the table uses blank lines to
group rows. But as blank lines separate paragraphs in
asciidoc, this actually means that each chunk of the table
is rendered in its own pre-formatted <div> block. This
happens even with vanilla asciidoc, but there's no visible
result because the literal paragraphs aren't styled in any
special way. But with asciidoctor (or at least the styles
used on git-scm.com), literal paragraphs are styled with a
different background.  This breaks the table into a visually
distracting sequence of chunks.

We can fix both by adding a literal-paragraph block
delimiter. That turns the whole table into a single block
(for both implementations) and causes asciidoctor to render
the indentation as it is in the source.

Reported-at: https://github.com/git/git-scm.com/issues/1023
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

Git 2.14.2

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

Sync with 2.13.6

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

Git 2.13.6

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

Sync with 2.12.5

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

Git 2.12.5

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

Sync with 2.11.4

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