git/git.git
3 years agoMerge branch 'sb/test-cmp-expect-actual'
Junio C Hamano [Wed, 11 Oct 2017 05:52:23 +0000 (14:52 +0900)]
Merge branch 'sb/test-cmp-expect-actual'

Test tweak.

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

3 years agoMerge branch 'jk/refs-df-conflict'
Junio C Hamano [Wed, 11 Oct 2017 05:52:23 +0000 (14:52 +0900)]
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

3 years agoMerge branch 'rs/rs-mailmap'
Junio C Hamano [Wed, 11 Oct 2017 05:52:23 +0000 (14:52 +0900)]
Merge branch 'rs/rs-mailmap'

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

3 years agoMerge branch 'rs/fsck-null-return-from-lookup'
Junio C Hamano [Wed, 11 Oct 2017 05:52:23 +0000 (14:52 +0900)]
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()

3 years agoMerge branch 'jk/sha1-loose-object-info-fix'
Junio C Hamano [Wed, 11 Oct 2017 05:52:22 +0000 (14:52 +0900)]
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

3 years agoMerge branch 'hn/string-list-doc'
Junio C Hamano [Wed, 11 Oct 2017 05:52:22 +0000 (14:52 +0900)]
Merge branch 'hn/string-list-doc'

Docfix.

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

3 years agoMerge branch 'tb/show-trailers-in-ref-filter'
Junio C Hamano [Wed, 11 Oct 2017 05:52:22 +0000 (14:52 +0900)]
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

3 years agoMerge branch 'jt/oidmap'
Junio C Hamano [Wed, 11 Oct 2017 05:52:22 +0000 (14:52 +0900)]
Merge branch 'jt/oidmap'

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

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

3 years agoMerge branch 'jr/hash-migration-plan-doc'
Junio C Hamano [Wed, 11 Oct 2017 05:52:22 +0000 (14:52 +0900)]
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

3 years agoMerge branch 'js/rebase-i-final'
Junio C Hamano [Mon, 9 Oct 2017 09:59:16 +0000 (18:59 +0900)]
Merge branch 'js/rebase-i-final'

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

3 years agoi18n: add a missing space in message
Jean-Noel Avila [Sun, 8 Oct 2017 12:18:39 +0000 (14:18 +0200)]
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>
3 years agoPrepare for -rc1
Junio C Hamano [Sat, 7 Oct 2017 07:29:03 +0000 (16:29 +0900)]
Prepare for -rc1

Signed-off-by: Junio C Hamano <gitster@pobox.com>
3 years agoMerge branch 'tb/ref-filter-empty-modifier'
Junio C Hamano [Sat, 7 Oct 2017 07:27:56 +0000 (16:27 +0900)]
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

3 years agoMerge branch 'ks/verify-filename-non-option-error-message-tweak'
Junio C Hamano [Sat, 7 Oct 2017 07:27:55 +0000 (16:27 +0900)]
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

3 years agoMerge branch 'ks/branch-tweak-error-message-for-extra-args'
Junio C Hamano [Sat, 7 Oct 2017 07:27:55 +0000 (16:27 +0900)]
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

3 years agoMerge branch 'jk/ui-color-always-to-auto'
Junio C Hamano [Sat, 7 Oct 2017 07:27:55 +0000 (16:27 +0900)]
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

3 years agoMerge branch 'ma/builtin-unleak'
Junio C Hamano [Sat, 7 Oct 2017 07:27:55 +0000 (16:27 +0900)]
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

3 years agoMerge branch 'rb/compat-poll-fix'
Junio C Hamano [Sat, 7 Oct 2017 07:27:55 +0000 (16:27 +0900)]
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

3 years agoMerge branch 'tg/memfixes'
Junio C Hamano [Sat, 7 Oct 2017 07:27:54 +0000 (16:27 +0900)]
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

3 years agoMerge branch 'sb/branch-avoid-repeated-strbuf-release'
Junio C Hamano [Sat, 7 Oct 2017 07:27:54 +0000 (16:27 +0900)]
Merge branch 'sb/branch-avoid-repeated-strbuf-release'

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

3 years agoMerge branch 'rs/qsort-s'
Junio C Hamano [Sat, 7 Oct 2017 07:27:53 +0000 (16:27 +0900)]
Merge branch 'rs/qsort-s'

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

3 years agoMerge branch 'jn/strbuf-doc-re-reuse'
Junio C Hamano [Sat, 7 Oct 2017 07:27:53 +0000 (16:27 +0900)]
Merge branch 'jn/strbuf-doc-re-reuse'

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

3 years agoMerge branch 'tb/delimit-pretty-trailers-args-with-comma'
Junio C Hamano [Sat, 7 Oct 2017 07:27:52 +0000 (16:27 +0900)]
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 ","

3 years agotests: fix diff order arguments in test_cmp
Stefan Beller [Fri, 6 Oct 2017 19:00:06 +0000 (12:00 -0700)]
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>
3 years agorefs_resolve_ref_unsafe: handle d/f conflicts for writes
Jeff King [Fri, 6 Oct 2017 14:42:17 +0000 (10:42 -0400)]
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>
3 years agot3308: create a real ref directory/file conflict
Jeff King [Fri, 6 Oct 2017 14:38:30 +0000 (10:38 -0400)]
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>
3 years agoapi-argv-array.txt: remove broken link to string-list API
Todd Zullinger [Fri, 6 Oct 2017 03:14:56 +0000 (23:14 -0400)]
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>
3 years agosha1_loose_object_info: handle errors from unpack_sha1_rest
Jeff King [Thu, 5 Oct 2017 05:59:52 +0000 (01:59 -0400)]
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>
3 years ago.mailmap: normalize name for René Scharfe
René Scharfe [Thu, 5 Oct 2017 19:41:31 +0000 (21:41 +0200)]
.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>
3 years agofsck: handle NULL return of lookup_blob() and lookup_tree()
René Scharfe [Thu, 5 Oct 2017 19:41:26 +0000 (21:41 +0200)]
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>
3 years agoGit 2.15-rc0 v2.15.0-rc0
Junio C Hamano [Thu, 5 Oct 2017 04:49:07 +0000 (13:49 +0900)]
Git 2.15-rc0

Signed-off-by: Junio C Hamano <gitster@pobox.com>
3 years agoMerge branch 'ar/request-pull-phrasofix'
Junio C Hamano [Thu, 5 Oct 2017 04:48:21 +0000 (13:48 +0900)]
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

3 years agoMerge branch 'rs/run-command-use-alloc-array'
Junio C Hamano [Thu, 5 Oct 2017 04:48:20 +0000 (13:48 +0900)]
Merge branch 'rs/run-command-use-alloc-array'

Code clean-up.

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

3 years agoMerge branch 'sb/git-clang-format'
Junio C Hamano [Thu, 5 Oct 2017 04:48:20 +0000 (13:48 +0900)]
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

3 years agoMerge branch 'rs/use-free-and-null'
Junio C Hamano [Thu, 5 Oct 2017 04:48:20 +0000 (13:48 +0900)]
Merge branch 'rs/use-free-and-null'

Code clean-up.

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

3 years agoMerge branch 'rs/tag-null-pointer-arith-fix'
Junio C Hamano [Thu, 5 Oct 2017 04:48:19 +0000 (13:48 +0900)]
Merge branch 'rs/tag-null-pointer-arith-fix'

Code clean-up.

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

3 years agoMerge branch 'rs/cocci-de-paren-call-params'
Junio C Hamano [Thu, 5 Oct 2017 04:48:19 +0000 (13:48 +0900)]
Merge branch 'rs/cocci-de-paren-call-params'

Code clean-up.

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

3 years agoMerge branch 'rs/cleanup-strbuf-users'
Junio C Hamano [Thu, 5 Oct 2017 04:48:19 +0000 (13:48 +0900)]
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()

3 years agoMerge branch 'rs/resolve-ref-optional-result'
Junio C Hamano [Thu, 5 Oct 2017 04:48:19 +0000 (13:48 +0900)]
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

3 years agoMerge branch 'er/fast-import-dump-refs-on-checkpoint'
Junio C Hamano [Thu, 5 Oct 2017 04:48:19 +0000 (13:48 +0900)]
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

3 years agoref-filter.c: pass empty-string as NULL to atom parsers
Taylor Blau [Mon, 2 Oct 2017 16:10:34 +0000 (09:10 -0700)]
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>
3 years agostrbuf doc: reuse after strbuf_release is fine
Jonathan Nieder [Wed, 4 Oct 2017 02:39:54 +0000 (19:39 -0700)]
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>
3 years agobranch: reset instead of release a strbuf
Stefan Beller [Tue, 3 Oct 2017 22:17:40 +0000 (15:17 -0700)]
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>
3 years agosub-process: use child_process.args instead of child_process.argv
Johannes Sixt [Tue, 3 Oct 2017 20:24:57 +0000 (22:24 +0200)]
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>
3 years agohttp-push: fix construction of hex value from path
Thomas Gummerer [Tue, 3 Oct 2017 19:57:12 +0000 (20:57 +0100)]
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>
3 years agopath.c: fix uninitialized memory access
Jeff King [Tue, 3 Oct 2017 23:30:40 +0000 (19:30 -0400)]
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>
3 years agotest-stringlist: avoid buffer underrun when sorting nothing
René Scharfe [Tue, 3 Oct 2017 14:36:40 +0000 (16:36 +0200)]
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>
3 years agosetup: update error message to be more meaningful
Kaartic Sivaraam [Mon, 2 Oct 2017 17:30:02 +0000 (23:00 +0530)]
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>
3 years agobranch: change the error messages to be more meaningful
Kaartic Sivaraam [Mon, 21 Aug 2017 13:36:08 +0000 (19:06 +0530)]
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>
3 years agoMerge branch 'jk/ui-color-always-to-auto-maint' into jk/ui-color-always-to-auto
Junio C Hamano [Wed, 4 Oct 2017 03:03:05 +0000 (12:03 +0900)]
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

3 years agot7301: use test_terminal to check color
Jeff King [Tue, 3 Oct 2017 13:44:58 +0000 (09:44 -0400)]
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>
3 years agot4015: use --color with --color-moved
Jeff King [Tue, 3 Oct 2017 13:41:51 +0000 (09:41 -0400)]
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>
3 years agocolor: make "always" the same as "auto" in config
Jeff King [Tue, 3 Oct 2017 13:46:06 +0000 (09:46 -0400)]
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>
3 years agoprovide --color option for all ref-filter users
Jeff King [Tue, 3 Oct 2017 13:45:47 +0000 (09:45 -0400)]
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>
3 years agot3205: use --color instead of color.branch=always
Jeff King [Tue, 3 Oct 2017 13:45:18 +0000 (09:45 -0400)]
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>
3 years agot3203: drop "always" color test
Jeff King [Tue, 3 Oct 2017 13:44:39 +0000 (09:44 -0400)]
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>
3 years agot6006: drop "always" color config tests
Jeff King [Tue, 3 Oct 2017 13:44:27 +0000 (09:44 -0400)]
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>
3 years agot7502: use diff.noprefix for --verbose test
Jeff King [Tue, 3 Oct 2017 13:43:47 +0000 (09:43 -0400)]
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>
3 years agot7508: use test_terminal for color output
Jeff King [Tue, 3 Oct 2017 13:43:29 +0000 (09:43 -0400)]
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>
3 years agot3701: use test-terminal to collect color output
Jeff King [Tue, 3 Oct 2017 13:42:15 +0000 (09:42 -0400)]
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>
3 years agot4015: prefer --color to -c color.diff=always
Jeff King [Tue, 3 Oct 2017 13:40:19 +0000 (09:40 -0400)]
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>
3 years agotest-terminal: set TERM=vt100
Jeff King [Tue, 3 Oct 2017 13:39:34 +0000 (09:39 -0400)]
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>
3 years agoThe twelfth batch for 2.15
Junio C Hamano [Tue, 3 Oct 2017 06:50:31 +0000 (15:50 +0900)]
The twelfth batch for 2.15

Signed-off-by: Junio C Hamano <gitster@pobox.com>
3 years agoMerge branch 'bw/git-clang-format'
Junio C Hamano [Tue, 3 Oct 2017 06:42:50 +0000 (15:42 +0900)]
Merge branch 'bw/git-clang-format'

Adjust clang-format penalty parameters.

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

3 years agoMerge branch 'ad/doc-markup-fix'
Junio C Hamano [Tue, 3 Oct 2017 06:42:50 +0000 (15:42 +0900)]
Merge branch 'ad/doc-markup-fix'

Docfix.

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

3 years agoMerge branch 'mh/mmap-packed-refs'
Junio C Hamano [Tue, 3 Oct 2017 06:42:50 +0000 (15:42 +0900)]
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
  ...

3 years agoMerge branch 'mr/doc-negative-pathspec'
Junio C Hamano [Tue, 3 Oct 2017 06:42:50 +0000 (15:42 +0900)]
Merge branch 'mr/doc-negative-pathspec'

Doc updates.

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

3 years agoMerge branch 'sb/submodule-diff-header-fix'
Junio C Hamano [Tue, 3 Oct 2017 06:42:49 +0000 (15:42 +0900)]
Merge branch 'sb/submodule-diff-header-fix'

Error message tweak.

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

3 years agoMerge branch 'sb/diff-color-move'
Junio C Hamano [Tue, 3 Oct 2017 06:42:49 +0000 (15:42 +0900)]
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

3 years agoMerge branch 'sb/test-submodule-update-config'
Junio C Hamano [Tue, 3 Oct 2017 06:42:49 +0000 (15:42 +0900)]
Merge branch 'sb/test-submodule-update-config'

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

3 years agoMerge branch 'jk/validate-headref-fix'
Junio C Hamano [Tue, 3 Oct 2017 06:42:49 +0000 (15:42 +0900)]
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

3 years agoMerge branch 'jk/read-in-full'
Junio C Hamano [Tue, 3 Oct 2017 06:42:49 +0000 (15:42 +0900)]
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

3 years agoMerge branch 'jk/no-optional-locks'
Junio C Hamano [Tue, 3 Oct 2017 06:42:48 +0000 (15:42 +0900)]
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

3 years agoMerge branch 'hn/string-list-doc'
Junio C Hamano [Tue, 3 Oct 2017 06:42:48 +0000 (15:42 +0900)]
Merge branch 'hn/string-list-doc'

Doc reorg.

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

3 years agoMerge branch 'hn/path-ownership-comment'
Junio C Hamano [Tue, 3 Oct 2017 06:42:48 +0000 (15:42 +0900)]
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

3 years agoMerge branch 'hn/submodule-comment'
Junio C Hamano [Tue, 3 Oct 2017 06:42:48 +0000 (15:42 +0900)]
Merge branch 'hn/submodule-comment'

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

3 years agoMerge branch 'sd/branch-copy'
Junio C Hamano [Tue, 3 Oct 2017 06:42:48 +0000 (15:42 +0900)]
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

3 years agoMerge branch 'bc/rev-parse-parseopt-fix'
Junio C Hamano [Tue, 3 Oct 2017 06:42:47 +0000 (15:42 +0900)]
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

3 years agoMerge branch 'js/rebase-i-final'
Junio C Hamano [Tue, 3 Oct 2017 06:42:47 +0000 (15:42 +0900)]
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

3 years agorequest-pull: capitalise "Git" to make it a proper noun
Ann T Ropea [Tue, 3 Oct 2017 00:08:38 +0000 (00:08 +0000)]
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>
3 years agorun-command: use ALLOC_ARRAY
René Scharfe [Sun, 1 Oct 2017 15:14:31 +0000 (17:14 +0200)]
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>
3 years agoref-filter.c: parse trailers arguments with %(contents) atom
Taylor Blau [Mon, 2 Oct 2017 05:25:24 +0000 (22:25 -0700)]
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>
3 years agoref-filter.c: use trailer_opts to format trailers
Taylor Blau [Mon, 2 Oct 2017 05:25:23 +0000 (22:25 -0700)]
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>
3 years agot6300: refactor %(trailers) tests
Taylor Blau [Mon, 2 Oct 2017 05:25:22 +0000 (22:25 -0700)]
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>
3 years agodoc: use "`<literal>`"-style quoting for literal strings
Taylor Blau [Sun, 1 Oct 2017 16:18:50 +0000 (09:18 -0700)]
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>
3 years agodoc: 'trailers' is the preferred way to format trailers
Taylor Blau [Mon, 2 Oct 2017 05:25:20 +0000 (22:25 -0700)]
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>
3 years agot4205: unfold across multiple lines
Taylor Blau [Mon, 2 Oct 2017 05:25:19 +0000 (22:25 -0700)]
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>
3 years agoclang-format: add a comment about the meaning/status of the
Stephan Beyer [Sun, 1 Oct 2017 23:37:14 +0000 (08:37 +0900)]
clang-format: add a comment about the meaning/status of the

Having a .clang-format file in a project can be understood in a way that
code has to be in the style defined by the .clang-format file, i.e., you
just have to run clang-format over all code and you are set.

This unfortunately is not yet the case in the Git project, as the
format file is still work in progress.  Explain it with a comment in
the beginning of the file.

Additionally, the working clang-format version is mentioned because the
config directives change from time to time (in a compatibility-breaking way).

Signed-off-by: Stephan Beyer <s-beyer@gmx.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
3 years agorepository: use FREE_AND_NULL
René Scharfe [Sun, 1 Oct 2017 14:44:46 +0000 (16:44 +0200)]
repository: use FREE_AND_NULL

Use the macro FREE_AND_NULL to release allocated objects and clear their
pointers.  This is shorter and documents the intent better by combining
the two related operations into one.

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

Signed-off-by: Rene Scharfe <l.s.r@web.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
3 years agotag: avoid NULL pointer arithmetic
René Scharfe [Sun, 1 Oct 2017 14:45:13 +0000 (16:45 +0200)]
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>
3 years agograph: use strbuf_addchars() to add spaces
René Scharfe [Sun, 1 Oct 2017 14:45:45 +0000 (16:45 +0200)]
graph: use strbuf_addchars() to add spaces

strbuf_addf() can be used to add a specific number of space characters
by using the format "%*s" with an empty string and specifying the
desired width.  Use strbuf_addchars() instead as it's shorter, makes the
intent clearer and is a bit more efficient.

Signed-off-by: Rene Scharfe <l.s.r@web.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
3 years agouse strbuf_addstr() for adding strings to strbufs
René Scharfe [Sun, 1 Oct 2017 14:44:20 +0000 (16:44 +0200)]
use strbuf_addstr() for adding strings to strbufs

Use strbuf_addstr() instead of strbuf_addf() for adding strings.  That's
simpler and makes the intent clearer.

Patch generated by Coccinelle and contrib/coccinelle/strbuf.cocci;
adjusted indentation in refs/packed-backend.c manually.

Signed-off-by: Rene Scharfe <l.s.r@web.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
3 years agopath: use strbuf_add_real_path()
René Scharfe [Sun, 1 Oct 2017 14:44:06 +0000 (16:44 +0200)]
path: use strbuf_add_real_path()

Avoid a string copy to a static buffer by using strbuf_add_real_path()
instead of combining strbuf_addstr() and real_path().

Patch generated by Coccinelle and contrib/coccinelle/strbuf.cocci.

Signed-off-by: Rene Scharfe <l.s.r@web.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
3 years agobuiltin/: add UNLEAKs
Martin Ågren [Sun, 1 Oct 2017 17:42:08 +0000 (19:42 +0200)]
builtin/: add UNLEAKs

Add some UNLEAKs where we are about to return from `cmd_*`. UNLEAK the
variables in the same order as we've declared them. While addressing
`msg` in builtin/tag.c, convert the existing `strbuf_release()` calls as
well.

Signed-off-by: Martin Ågren <martin.agren@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
3 years agococcinelle: remove parentheses that become unnecessary
René Scharfe [Sun, 1 Oct 2017 15:12:08 +0000 (17:12 +0200)]
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>
3 years agopretty.c: delimit "%(trailers)" arguments with ","
Taylor Blau [Sun, 1 Oct 2017 16:18:47 +0000 (09:18 -0700)]
pretty.c: delimit "%(trailers)" arguments with ","

In preparation for adding consistent "%(trailers)" atom options to
`git-for-each-ref(1)`'s "--format" argument, change "%(trailers)" in
pretty.c to separate sub-arguments with a ",", instead of a ":".

Multiple sub-arguments are given either as "%(trailers:unfold,only)" or
"%(trailers:only,unfold)".

This change disambiguates between "top-level" arguments, and arguments
given to the trailers atom itself. It is consistent with the behavior of
"%(upstream)" and "%(push)" atoms.

Signed-off-by: Taylor Blau <me@ttaylorr.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
3 years agorefs: pass NULL to resolve_refdup() if hash is not needed
René Scharfe [Sun, 1 Oct 2017 07:29:03 +0000 (09:29 +0200)]
refs: pass NULL to resolve_refdup() if hash is not needed

This allows us to get rid of several write-only variables.

Signed-off-by: Rene Scharfe <l.s.r@web.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
3 years agorefs: pass NULL to refs_resolve_refdup() if hash is not needed
René Scharfe [Sun, 1 Oct 2017 07:28:50 +0000 (09:28 +0200)]
refs: pass NULL to refs_resolve_refdup() if hash is not needed

This gets us rid of a write-only variable.

Signed-off-by: Rene Scharfe <l.s.r@web.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
3 years agooidmap: map with OID as key
Jonathan Tan [Fri, 29 Sep 2017 22:54:22 +0000 (15:54 -0700)]
oidmap: map with OID as key

This is similar to using the hashmap in hashmap.c, but with an
easier-to-use API. In particular, custom entry comparisons no longer
need to be written, and lookups can be done without constructing a
temporary entry structure.

This is implemented as a thin wrapper over the hashmap API. In
particular, this means that there is an additional 4-byte overhead due
to the fact that the first 4 bytes of the hash is redundantly stored.
For now, I'm taking the simpler approach, but if need be, we can
reimplement oidmap without affecting the callers significantly.

oidset has been updated to use oidmap.

Signed-off-by: Jonathan Tan <jonathantanmy@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
3 years agoclang-format: adjust line break penalties
Johannes Schindelin [Fri, 29 Sep 2017 18:26:44 +0000 (20:26 +0200)]
clang-format: adjust line break penalties

We really, really, really want to limit the columns to 80 per line: One
of the few consistent style comments on the Git mailing list is that the
lines should not have more than 80 columns/line (even if 79 columns/line
would make more sense, given that the code is frequently viewed as diff,
and diffs adding an extra character).

The penalty of 5 for excess characters is way too low to guarantee that,
though, as pointed out by Brandon Williams.

From the existing clang-format examples and documentation, it appears
that 100 is a penalty deemed appropriate for Stuff You Really Don't
Want, so let's assign that as the penalty for "excess characters", i.e.
overly long lines.

While at it, adjust the penalties further: we are actually not that keen
on preventing new line breaks within comments or string literals, so the
penalty of 100 seems awfully high.

Likewise, we are not all that adamant about keeping line breaks away
from assignment operators (a lot of Git's code breaks immediately after
the `=` character just to keep that 80 columns/line limit).

We do frown a little bit more about functions' return types being on
their own line than the penalty 0 would suggest, so this was adjusted,
too.

Finally, we do not particularly fancy breaking before the first parameter
in a call, but if it keeps the line shorter than 80 columns/line, that's
what we do, so lower the penalty for breaking before a call's first
parameter, but not quite as much as introducing new line breaks to
comments.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Reviewed-by: Jonathan Nieder <jrnieder@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>