2 years agoMerge branch 'ma/t7005-bash-workaround' into maint
Junio C Hamano [Wed, 21 Nov 2018 13:58:04 +0000 (22:58 +0900)]
Merge branch 'ma/t7005-bash-workaround' into maint

Test fix.

* ma/t7005-bash-workaround:
  t7005-editor: quote filename to fix whitespace-issue

2 years agoMerge branch 'jc/how-to-document-api' into maint
Junio C Hamano [Wed, 21 Nov 2018 13:58:03 +0000 (22:58 +0900)]
Merge branch 'jc/how-to-document-api' into maint

Doc update.

* jc/how-to-document-api:
  CodingGuidelines: document the API in *.h files

2 years agoMerge branch 'mw/doc-typofixes' into maint
Junio C Hamano [Wed, 21 Nov 2018 13:58:03 +0000 (22:58 +0900)]
Merge branch 'mw/doc-typofixes' into maint


* mw/doc-typofixes:
  docs: typo: s/isimilar/similar/
  docs: graph: remove unnecessary `graph_update()' call
  docs: typo: s/go/to/

2 years agoMerge branch 'rs/sequencer-oidset-insert-avoids-dups' into maint
Junio C Hamano [Wed, 21 Nov 2018 13:58:02 +0000 (22:58 +0900)]
Merge branch 'rs/sequencer-oidset-insert-avoids-dups' into maint

Code clean-up.

* rs/sequencer-oidset-insert-avoids-dups:
  sequencer: use return value of oidset_insert()

2 years agoMerge branch 'ma/mailing-list-address-in-git-help' into maint
Junio C Hamano [Wed, 21 Nov 2018 13:58:02 +0000 (22:58 +0900)]
Merge branch 'ma/mailing-list-address-in-git-help' into maint

Doc update.

* ma/mailing-list-address-in-git-help:
  git doc: direct bug reporters to mailing list archive

2 years agoMerge branch 'nd/packobjectshook-doc-fix' into maint
Junio C Hamano [Wed, 21 Nov 2018 13:58:01 +0000 (22:58 +0900)]
Merge branch 'nd/packobjectshook-doc-fix' into maint

Doc update.

* nd/packobjectshook-doc-fix:
  config.txt: correct the note about uploadpack.packObjectsHook

2 years agoMerge branch 'ma/t1400-undebug-test' into maint
Junio C Hamano [Wed, 21 Nov 2018 13:58:01 +0000 (22:58 +0900)]
Merge branch 'ma/t1400-undebug-test' into maint

Test fix.

* ma/t1400-undebug-test:
  t1400: drop debug `echo` to actually execute `test`

2 years agoMerge branch 'ma/commit-graph-docs' into maint
Junio C Hamano [Wed, 21 Nov 2018 13:58:00 +0000 (22:58 +0900)]
Merge branch 'ma/commit-graph-docs' into maint

Doc update.

* ma/commit-graph-docs:
  Doc: refer to the "commit-graph file" with dash
  git-commit-graph.txt: refer to "*commit*-graph file"
  git-commit-graph.txt: typeset more in monospace
  git-commit-graph.txt: fix bullet lists

2 years agoMerge branch 'dz/credential-doc-url-matching-rules' into maint
Junio C Hamano [Wed, 21 Nov 2018 13:58:00 +0000 (22:58 +0900)]
Merge branch 'dz/credential-doc-url-matching-rules' into maint

Doc update.

* dz/credential-doc-url-matching-rules:
  doc: clarify gitcredentials path component matching

2 years agoMerge branch 'jk/check-everything-connected-is-long-gone' into maint
Junio C Hamano [Wed, 21 Nov 2018 13:57:59 +0000 (22:57 +0900)]
Merge branch 'jk/check-everything-connected-is-long-gone' into maint

Comment fix.

* jk/check-everything-connected-is-long-gone:
  receive-pack: update comment with check_everything_connected

2 years agoMerge branch 'fe/doc-updates' into maint
Junio C Hamano [Wed, 21 Nov 2018 13:57:58 +0000 (22:57 +0900)]
Merge branch 'fe/doc-updates' into maint

Doc updates.

* fe/doc-updates:
  git-describe.1: clarify that "human readable" is also git-readable
  git-column.1: clarify initial description, provide examples
  git-archimport.1: specify what kind of Arch we're talking about

2 years agoMerge branch 'tg/t5551-with-curl-7.61.1' into maint
Junio C Hamano [Wed, 21 Nov 2018 13:57:58 +0000 (22:57 +0900)]
Merge branch 'tg/t5551-with-curl-7.61.1' into maint

Test update.
Supersedes tz/t5551-with-curl-7.61.1 topic

* tg/t5551-with-curl-7.61.1:
  t5551: compare sorted cookies files
  t5551: move setup code inside test_expect blocks

2 years agoMerge branch 'tq/refs-internal-comment-fix' into maint
Junio C Hamano [Wed, 21 Nov 2018 13:57:58 +0000 (22:57 +0900)]
Merge branch 'tq/refs-internal-comment-fix' into maint

Fix for typo in a sample code in comment.

* tq/refs-internal-comment-fix:
  refs: docstring typo

2 years agoMerge branch 'sg/split-index-test' into maint
Junio C Hamano [Wed, 21 Nov 2018 13:57:57 +0000 (22:57 +0900)]
Merge branch 'sg/split-index-test' into maint

Test updates.

* sg/split-index-test:
  t0090: disable GIT_TEST_SPLIT_INDEX for the test checking split index
  t1700-split-index: drop unnecessary 'grep'

2 years agoMerge branch 'bw/protocol-v2' into maint
Junio C Hamano [Wed, 21 Nov 2018 13:57:57 +0000 (22:57 +0900)]
Merge branch 'bw/protocol-v2' into maint

Doc fix.

* bw/protocol-v2:
  config: document value 2 for protocol.version

2 years agoMerge branch 'sb/string-list-remove-unused' into maint
Junio C Hamano [Wed, 21 Nov 2018 13:57:56 +0000 (22:57 +0900)]
Merge branch 'sb/string-list-remove-unused' into maint

Code clean-up.

* sb/string-list-remove-unused:
  string-list: remove unused function print_string_list

2 years agoMerge branch 'jk/dev-build-format-security' into maint
Junio C Hamano [Wed, 21 Nov 2018 13:57:56 +0000 (22:57 +0900)]
Merge branch 'jk/dev-build-format-security' into maint

Build tweak to help developers.

* jk/dev-build-format-security: add -Wformat-security

2 years agoMerge branch 'sg/t3701-tighten-trace' into maint
Junio C Hamano [Wed, 21 Nov 2018 13:57:55 +0000 (22:57 +0900)]
Merge branch 'sg/t3701-tighten-trace' into maint

Test update.

* sg/t3701-tighten-trace:
  t3701-add-interactive: tighten the check of trace output

2 years agoMerge branch 'en/double-semicolon-fix' into maint
Junio C Hamano [Wed, 21 Nov 2018 13:57:54 +0000 (22:57 +0900)]
Merge branch 'en/double-semicolon-fix' into maint

Code clean-up.

* en/double-semicolon-fix:
  Remove superfluous trailing semicolons

2 years agoMerge branch 'tb/void-check-attr' into maint
Junio C Hamano [Wed, 21 Nov 2018 13:57:54 +0000 (22:57 +0900)]
Merge branch 'tb/void-check-attr' into maint

Code clean-up.

* tb/void-check-attr:
  Make git_check_attr() a void function

2 years agoMerge branch 'sg/doc-trace-appends' into maint
Junio C Hamano [Wed, 21 Nov 2018 13:57:54 +0000 (22:57 +0900)]
Merge branch 'sg/doc-trace-appends' into maint


* sg/doc-trace-appends:
  Documentation/git.txt: clarify that GIT_TRACE=/path appends

2 years agoMerge branch 'tg/conflict-marker-size' into maint
Junio C Hamano [Wed, 21 Nov 2018 13:57:53 +0000 (22:57 +0900)]
Merge branch 'tg/conflict-marker-size' into maint

Developer aid.

* tg/conflict-marker-size:
  .gitattributes: add conflict-marker-size for relevant files

2 years agoMerge branch 'ts/doc-build-manpage-xsl-quietly' into maint
Junio C Hamano [Wed, 21 Nov 2018 13:57:53 +0000 (22:57 +0900)]
Merge branch 'ts/doc-build-manpage-xsl-quietly' into maint

Build tweak.

* ts/doc-build-manpage-xsl-quietly:
  Documentation/Makefile: make manpage-base-url.xsl generation quieter

2 years agoMerge branch 'jk/detect-truncated-zlib-input' into maint
Junio C Hamano [Wed, 21 Nov 2018 13:57:52 +0000 (22:57 +0900)]
Merge branch 'jk/detect-truncated-zlib-input' into maint

A regression in Git 2.12 era made "git fsck" fall into an infinite
loop while processing truncated loose objects.

* jk/detect-truncated-zlib-input:
  cat-file: handle streaming failures consistently
  check_stream_sha1(): handle input underflow
  t1450: check large blob in trailing-garbage test

2 years agoMerge branch 'sg/test-verbose-log' into maint
Junio C Hamano [Wed, 21 Nov 2018 13:57:52 +0000 (22:57 +0900)]
Merge branch 'sg/test-verbose-log' into maint

Our test scripts can now take the '-V' option as a synonym for the
'--verbose-log' option.

* sg/test-verbose-log:
  test-lib: introduce the '-V' short option for '--verbose-log'

2 years agoMerge branch 'ss/travis-ci-force-vm-mode' into maint
Junio C Hamano [Wed, 21 Nov 2018 13:57:52 +0000 (22:57 +0900)]
Merge branch 'ss/travis-ci-force-vm-mode' into maint

The "container" mode of TravisCI is going away.  Our .travis.yml
file is getting prepared for the transition.

* ss/travis-ci-force-vm-mode:
  travis-ci: no longer use containers

2 years agoMerge branch 'md/exclude-promisor-objects-fix' into maint
Junio C Hamano [Wed, 21 Nov 2018 13:57:52 +0000 (22:57 +0900)]
Merge branch 'md/exclude-promisor-objects-fix' into maint

Operations on promisor objects make sense in the context of only a
small subset of the commands that internally use the revisions
machinery, but the "--exclude-promisor-objects" option were taken
and led to nonsense results by commands like "log", to which it
didn't make much sense.  This has been corrected.

* md/exclude-promisor-objects-fix:
  exclude-promisor-objects: declare when option is allowed
  Documentation/git-log.txt: do not show --exclude-promisor-objects

2 years agoMerge branch 'js/shallow-and-fetch-prune' into maint
Junio C Hamano [Wed, 21 Nov 2018 13:57:51 +0000 (22:57 +0900)]
Merge branch 'js/shallow-and-fetch-prune' into maint

"git repack" in a shallow clone did not correctly update the
shallow points in the repository, leading to a repository that
does not pass fsck.

* js/shallow-and-fetch-prune:
  repack -ad: prune the list of shallow commits
  shallow: offer to prune only non-existing entries
  repack: point out a bug handling stale shallow info

2 years agoMerge branch 'jc/receive-deny-current-branch-fix' into maint
Junio C Hamano [Wed, 21 Nov 2018 13:57:51 +0000 (22:57 +0900)]
Merge branch 'jc/receive-deny-current-branch-fix' into maint

The receive.denyCurrentBranch=updateInstead codepath kicked in even
when the push should have been rejected due to other reasons, such
as it does not fast-forward or the update-hook rejects it, which
has been corrected.

* jc/receive-deny-current-branch-fix:
  receive: denyCurrentBranch=updateinstead should not blindly update

2 years agoMerge branch 'js/diff-notice-has-drive-prefix' into maint
Junio C Hamano [Wed, 21 Nov 2018 13:57:50 +0000 (22:57 +0900)]
Merge branch 'js/diff-notice-has-drive-prefix' into maint

Under certain circumstances, "git diff D:/a/b/c D:/a/b/d" on
Windows would strip initial parts from the paths because they
were not recognized as absolute, which has been corrected.

* js/diff-notice-has-drive-prefix:
  diff: don't attempt to strip prefix from absolute Windows paths

2 years agoMerge branch 'js/pack-objects-mutex-init-fix' into maint
Junio C Hamano [Wed, 21 Nov 2018 13:57:50 +0000 (22:57 +0900)]
Merge branch 'js/pack-objects-mutex-init-fix' into maint

A mutex used in "git pack-objects" were not correctly initialized
and this caused "git repack" to dump core on Windows.

* js/pack-objects-mutex-init-fix:
  pack-objects (mingw): initialize `packing_data` mutex in the correct spot
  pack-objects (mingw): demonstrate a segmentation fault with large deltas
  pack-objects: fix typo 'detla' -> 'delta'

2 years agoMerge branch 'jk/run-command-notdot' into maint
Junio C Hamano [Wed, 21 Nov 2018 13:57:50 +0000 (22:57 +0900)]
Merge branch 'jk/run-command-notdot' into maint

The implementation of run_command() API on the UNIX platforms had a
bug that caused a command not on $PATH to be found in the current

* jk/run-command-notdot:
  run-command: mark path lookup errors with ENOENT

2 years agoMerge branch 'np/log-graph-octopus-fix' into maint
Junio C Hamano [Wed, 21 Nov 2018 13:57:49 +0000 (22:57 +0900)]
Merge branch 'np/log-graph-octopus-fix' into maint

"git log --graph" showing an octopus merge sometimes miscounted the
number of display columns it is consuming to show the merge and its
parent commits, which has been corrected.

* np/log-graph-octopus-fix:
  log: fix coloring of certain octopus merge shapes

2 years agoMerge branch 'sg/split-index-racefix' into maint
Junio C Hamano [Wed, 21 Nov 2018 13:57:48 +0000 (22:57 +0900)]
Merge branch 'sg/split-index-racefix' into maint

The codepath to support the experimental split-index mode had
remaining "racily clean" issues fixed.

* sg/split-index-racefix:
  split-index: BUG() when cache entry refers to non-existing shared entry
  split-index: smudge and add racily clean cache entries to split index
  split-index: don't compare cached data of entries already marked for split index
  split-index: count the number of deleted entries
  t1700-split-index: date back files to avoid racy situations
  split-index: add tests to demonstrate the racy split index problem
  t1700-split-index: document why FSMONITOR is disabled in this test script

2 years agoMerge branch 'jt/non-blob-lazy-fetch' into maint
Junio C Hamano [Wed, 21 Nov 2018 13:57:48 +0000 (22:57 +0900)]
Merge branch 'jt/non-blob-lazy-fetch' into maint

A partial clone that is configured to lazily fetch missing objects
will on-demand issue a "git fetch" request to the originating
repository to fill not-yet-obtained objects.  The request has been
optimized for requesting a tree object (and not the leaf blob
objects contained in it) by telling the originating repository that
no blobs are needed.

* jt/non-blob-lazy-fetch:
  fetch-pack: exclude blobs when lazy-fetching trees
  fetch-pack: avoid object flags if no_dependents

2 years agoMerge branch 'sm/show-superproject-while-conflicted' into maint
Junio C Hamano [Wed, 21 Nov 2018 13:57:48 +0000 (22:57 +0900)]
Merge branch 'sm/show-superproject-while-conflicted' into maint

A corner-case bugfix.

* sm/show-superproject-while-conflicted:
  rev-parse: --show-superproject-working-tree should work during a merge

2 years agoMerge branch 'en/status-multiple-renames-to-the-same-target-fix' into maint
Junio C Hamano [Wed, 21 Nov 2018 13:57:48 +0000 (22:57 +0900)]
Merge branch 'en/status-multiple-renames-to-the-same-target-fix' into maint

The code in "git status" sometimes hit an assertion failure.  This
was caused by a structure that was reused without cleaning the data
used for the first run, which has been corrected.

* en/status-multiple-renames-to-the-same-target-fix:
  commit: fix erroneous BUG, 'multiple renames on the same target? how?'

2 years agoMerge branch 'jn/mailmap-update' into maint
Junio C Hamano [Wed, 21 Nov 2018 13:57:47 +0000 (22:57 +0900)]
Merge branch 'jn/mailmap-update' into maint

The mailmap file update.

* jn/mailmap-update:
  mailmap: consistently normalize brian m. carlson's name

2 years agoMerge branch 'ds/commit-graph-with-grafts' into maint
Junio C Hamano [Wed, 21 Nov 2018 13:57:47 +0000 (22:57 +0900)]
Merge branch 'ds/commit-graph-with-grafts' into maint

The recently introduced commit-graph auxiliary data is incompatible
with mechanisms such as replace & grafts that "breaks" immutable
nature of the object reference relationship.  Disable optimizations
based on its use (and updating existing commit-graph) when these
incompatible features are in use in the repository.

* ds/commit-graph-with-grafts:
  commit-graph: close_commit_graph before shallow walk
  commit-graph: not compatible with uninitialized repo
  commit-graph: not compatible with grafts
  commit-graph: not compatible with replace objects
  test-repository: properly init repo
  commit-graph: update design document
  refs.c: upgrade for_each_replace_ref to be a each_repo_ref_fn callback
  refs.c: migrate internal ref iteration to pass thru repository argument

2 years agoMerge branch 'tg/range-diff-corner-case-fix' into maint
Junio C Hamano [Wed, 21 Nov 2018 13:57:46 +0000 (22:57 +0900)]
Merge branch 'tg/range-diff-corner-case-fix' into maint

Recently added "range-diff" had a corner-case bug to cause it
segfault, which has been corrected.

* tg/range-diff-corner-case-fix:
  linear-assignment: fix potential out of bounds memory access

2 years agoMerge branch 'en/update-ref-no-deref-stdin' into maint
Junio C Hamano [Wed, 21 Nov 2018 13:57:46 +0000 (22:57 +0900)]
Merge branch 'en/update-ref-no-deref-stdin' into maint

"git update-ref" learned to make both "--no-deref" and "--stdin"
work at the same time.

* en/update-ref-no-deref-stdin:
  update-ref: allow --no-deref with --stdin
  update-ref: fix type of update_flags variable to match its usage

2 years agoMerge branch 'ms/remote-error-message-update' into maint
Junio C Hamano [Wed, 21 Nov 2018 13:57:46 +0000 (22:57 +0900)]
Merge branch 'ms/remote-error-message-update' into maint

Update error messages given by "git remote" and make them consistent.

* ms/remote-error-message-update:
  builtin/remote: quote remote name on error to display empty name

2 years agoMerge branch 'jt/lazy-object-fetch-fix' into maint
Junio C Hamano [Wed, 21 Nov 2018 13:57:45 +0000 (22:57 +0900)]
Merge branch 'jt/lazy-object-fetch-fix' into maint

The code to backfill objects in lazily cloned repository did not
work correctly, which has been corrected.

* jt/lazy-object-fetch-fix:
  fetch-object: set exact_oid when fetching
  fetch-object: unify fetch_object[s] functions

2 years agoMerge branch 'en/sequencer-empty-edit-result-aborts' into maint
Junio C Hamano [Wed, 21 Nov 2018 13:57:45 +0000 (22:57 +0900)]
Merge branch 'en/sequencer-empty-edit-result-aborts' into maint

"git rebase" etc. in Git 2.19 fails to abort when given an empty
commit log message as result of editing, which has been corrected.

* en/sequencer-empty-edit-result-aborts:
  sequencer: fix --allow-empty-message behavior, make it smarter

2 years agoMerge branch 'nd/attr-pathspec-fix' into maint
Junio C Hamano [Wed, 21 Nov 2018 13:57:45 +0000 (22:57 +0900)]
Merge branch 'nd/attr-pathspec-fix' into maint

"git add ':(attr:foo)'" is not supported and is supposed to be
rejected while the command line arguments are parsed, but we fail
to reject such a command line upfront.

* nd/attr-pathspec-fix:
  add: do not accept pathspec magic 'attr'

2 years agoMerge branch 'en/rerere-multi-stage-1-fix' into maint
Junio C Hamano [Wed, 21 Nov 2018 13:57:44 +0000 (22:57 +0900)]
Merge branch 'en/rerere-multi-stage-1-fix' into maint

A corner case bugfix in "git rerere" code.

* en/rerere-multi-stage-1-fix:
  rerere: avoid buffer overrun
  t4200: demonstrate rerere segfault on specially crafted merge

2 years agoMerge branch 'js/mingw-o-append' into maint
Junio C Hamano [Wed, 21 Nov 2018 13:57:44 +0000 (22:57 +0900)]
Merge branch 'js/mingw-o-append' into maint

Further fix for O_APPEND emulation on Windows

* js/mingw-o-append:
  mingw: fix mingw_open_append to work with named pipes
  t0051: test GIT_TRACE to a windows named pipe

2 years agoMerge branch 'jk/reopen-tempfile-truncate' into maint
Junio C Hamano [Wed, 21 Nov 2018 13:57:43 +0000 (22:57 +0900)]
Merge branch 'jk/reopen-tempfile-truncate' into maint

Fix for a long-standing bug that leaves the index file corrupt when
it shrinks during a partial commit.

* jk/reopen-tempfile-truncate:
  reopen_tempfile(): truncate opened file

2 years agoMerge branch 'bp/mv-submodules-with-fsmonitor' into maint
Junio C Hamano [Wed, 21 Nov 2018 13:57:43 +0000 (22:57 +0900)]
Merge branch 'bp/mv-submodules-with-fsmonitor' into maint

When fsmonitor is in use, after operation on submodules updates
.gitmodules, we lost track of the fact that we did so and relied on
stale fsmonitor data.

* bp/mv-submodules-with-fsmonitor:
  git-mv: allow submodules and fsmonitor to work together

2 years agoMerge branch 'js/rebase-i-autosquash-fix' into maint
Junio C Hamano [Wed, 21 Nov 2018 13:57:42 +0000 (22:57 +0900)]
Merge branch 'js/rebase-i-autosquash-fix' into maint

"git rebase -i" did not clear the state files correctly when a run
of "squash/fixup" is aborted and then the user manually amended the
commit instead, which has been corrected.

* js/rebase-i-autosquash-fix:
  rebase -i: be careful to wrap up fixup/squash chains
  rebase -i --autosquash: demonstrate a problem skipping the last squash

2 years agoMerge branch 'jk/trailer-fixes' into maint
Junio C Hamano [Wed, 21 Nov 2018 13:57:41 +0000 (22:57 +0900)]
Merge branch 'jk/trailer-fixes' into maint

"git interpret-trailers" and its underlying machinery had a buggy
code that attempted to ignore patch text after commit log message,
which triggered in various codepaths that will always get the log
message alone and never get such an input.

* jk/trailer-fixes:
  append_signoff: use size_t for string offsets
  sequencer: ignore "---" divider when parsing trailers
  pretty, ref-filter: format %(trailers) with no_divider option
  interpret-trailers: allow suppressing "---" divider
  interpret-trailers: tighten check for "---" patch boundary
  trailer: pass process_trailer_opts to trailer_info_get()
  trailer: use size_t for iterating trailer list
  trailer: use size_t for string offsets

2 years agoAdjust for 2.19.x series
Junio C Hamano [Wed, 31 Oct 2018 04:12:12 +0000 (13:12 +0900)]
Adjust for 2.19.x series

* jk/detect-truncated-zlib-input
  cat-file: handle streaming failures consistently
  check_stream_sha1(): handle input underflow
  t1450: check large blob in trailing-garbage test

2 years agocat-file: handle streaming failures consistently
Jeff King [Tue, 30 Oct 2018 23:23:38 +0000 (19:23 -0400)]
cat-file: handle streaming failures consistently

There are three ways to convince cat-file to stream a blob:

  - cat-file -p $blob

  - cat-file blob $blob

  - echo $batch | cat-file --batch

In the first two, we simply exit with the error code of
streaw_blob_to_fd(). That means that an error will cause us
to exit with "-1" (which we try to avoid) without printing
any kind of error message (which is confusing to the user).

Instead, let's match the third case, which calls die() on an
error. Unfortunately we cannot be more specific, as
stream_blob_to_fd() does not tell us whether the problem was
on reading (e.g., a corrupt object) or on writing (e.g.,
ENOSPC). That might be an opportunity for future work, but
for now we will at least exit with a sane message and exit

Signed-off-by: Jeff King <>
Signed-off-by: Junio C Hamano <>
2 years agocheck_stream_sha1(): handle input underflow
Jeff King [Tue, 30 Oct 2018 23:23:12 +0000 (19:23 -0400)]
check_stream_sha1(): handle input underflow

This commit fixes an infinite loop when fscking large
truncated loose objects.

The check_stream_sha1() function takes an mmap'd loose
object buffer and streams 4k of output at a time, checking
its sha1. The loop quits when we've output enough bytes (we
know the size from the object header), or when zlib tells us
anything except Z_OK or Z_BUF_ERROR.

The latter is expected because zlib may run out of room in
our 4k buffer, and that is how it tells us to process the
output and loop again.

But Z_BUF_ERROR also covers another case: one in which zlib
cannot make forward progress because it needs more _input_.
This should never happen in this loop, because though we're
streaming the output, we have the entire deflated input
available in the mmap'd buffer. But since we don't check
this case, we'll just loop infinitely if we do see a
truncated object, thinking that zlib is asking for more
output space.

It's tempting to fix this by checking stream->avail_in as
part of the loop condition (and quitting if all of our bytes
have been consumed). But that assumes that once zlib has
consumed the input, there is nothing left to do.  That's not
necessarily the case: it may have read our input into its
internal state, but still have bytes to output.

Instead, let's continue on Z_BUF_ERROR only when we see the
case we're expecting: the previous round filled our output
buffer completely. If it didn't (and we still saw
Z_BUF_ERROR), we know something is wrong and should break
out of the loop.

The bug comes from commit f6371f9210 (sha1_file: add
read_loose_object() function, 2017-01-13), which
reimplemented some of the existing loose object functions.
So it's worth checking if this bug was inherited from any of
those. The answers seems to be no. The two obvious
candidates are both OK:

  1. unpack_sha1_rest(); this doesn't need to loop on
     Z_BUF_ERROR at all, since it allocates the expected
     output buffer in advance (which we can't do since we're
     explicitly streaming here)

  2. check_object_signature(); the streaming path relies on
     the istream interface, which uses read_istream_loose()
     for this case. That function uses a similar "is our
     output buffer full" check with Z_BUF_ERROR (which is
     where I stole it from for this patch!)

Reported-by: Ævar Arnfjörð Bjarmason <>
Helped-by: Junio C Hamano <>
Signed-off-by: Jeff King <>
Signed-off-by: Junio C Hamano <>
2 years agot1450: check large blob in trailing-garbage test
Jeff King [Tue, 30 Oct 2018 23:18:51 +0000 (19:18 -0400)]
t1450: check large blob in trailing-garbage test

Commit cce044df7f (fsck: detect trailing garbage in all
object types, 2017-01-13) added two tests of trailing
garbage in a loose object file: one with a commit and one
with a blob. The point of having two is that blobs would
follow a different code path that streamed the contents,
instead of loading it into a buffer as usual.

At the time, merely being a blob was enough to trigger the
streaming code path. But since 7ac4f3a007 (fsck: actually
fsck blob data, 2018-05-02), we now only stream blobs that
are actually large. So since then, the streaming code path
is not tested at all for this case.

We can restore the original intent of the test by tweaking
core.bigFileThreshold to make our small blob seem large.
There's no easy way to externally verify that we followed
the streaming code path, but I did check before/after using
a temporary debug statement.

Signed-off-by: Jeff King <>
Signed-off-by: Junio C Hamano <>
2 years agotest-lib: introduce the '-V' short option for '--verbose-log'
SZEDER Gábor [Mon, 29 Oct 2018 12:13:59 +0000 (13:13 +0100)]
test-lib: introduce the '-V' short option for '--verbose-log'

'--verbose-log' is one of the most useful and thus most frequently
used test options, but due to its length it's a pain to type on the
command line.

Let's introduce the corresponding short option '-V' to save some

Signed-off-by: SZEDER Gábor <>
Signed-off-by: Junio C Hamano <>
2 years agotravis-ci: no longer use containers
Sebastian Staudt [Thu, 25 Oct 2018 17:41:45 +0000 (19:41 +0200)]
travis-ci: no longer use containers

Travis CI will soon deprecate the container-based infrastructure
enabled by `sudo: false` in ce59dffb34190e780be2fa9f449f842cadee9753.

More info:

Signed-off-by: Sebastian Staudt <>
Signed-off-by: Junio C Hamano <>
2 years agorepack -ad: prune the list of shallow commits
Johannes Schindelin [Wed, 24 Oct 2018 15:56:13 +0000 (08:56 -0700)]
repack -ad: prune the list of shallow commits

`git repack` can drop unreachable commits without further warning,
making the corresponding entries in `.git/shallow` invalid, which causes
serious problems when deepening the branches.

One scenario where unreachable commits are dropped by `git repack` is
when a `git fetch --prune` (or even a `git fetch` when a ref was
force-pushed in the meantime) can make a commit unreachable that was
reachable before.

Therefore it is not safe to assume that a `git repack -adlf` will keep
unreachable commits alone (under the assumption that they had not been
packed in the first place, which is an assumption at least some of Git's
code seems to make).

This is particularly important to keep in mind when looking at the
`.git/shallow` file: if any commits listed in that file become
unreachable, it is not a problem, but if they go missing, it *is* a
problem. One symptom of this problem is that a deepening fetch may now
fail with

fatal: error in object: unshallow <commit-hash>

To avoid this problem, let's prune the shallow list in `git repack` when
the `-d` option is passed, unless `-A` is passed, too (which would force
the now-unreachable objects to be turned into loose objects instead of
being deleted). Additionally, we also need to take `--keep-reachable`
and `--unpack-unreachable=<date>` into account.

Note: an alternative solution discussed during the review of this patch
was to teach `git fetch` to simply ignore entries in .git/shallow if the
corresponding commits do not exist locally. A quick test, however,
revealed that the .git/shallow file is written during a shallow *clone*,
in which case the commits do not exist, either, but the "shallow" line
*does* need to be sent. Therefore, this approach would be a lot more
finicky than the approach presented by the this patch.

Signed-off-by: Johannes Schindelin <>
Signed-off-by: Junio C Hamano <>
2 years agoshallow: offer to prune only non-existing entries
Johannes Schindelin [Wed, 24 Oct 2018 15:56:12 +0000 (08:56 -0700)]
shallow: offer to prune only non-existing entries

The `prune_shallow()` function wants a full reachability check to be
completed before it goes to work, to ensure that all unreachable entries
are removed from the shallow file.

However, in the upcoming patch we do not even want to go that far. We
really only need to remove entries corresponding to pruned commits, i.e.
to commits that no longer exist.

Let's support that use case.

Rather than extending the signature of `prune_shallow()` to accept
another Boolean, let's turn it into a bit field and declare constants,
for readability.

Signed-off-by: Johannes Schindelin <>
Signed-off-by: Junio C Hamano <>
2 years agorepack: point out a bug handling stale shallow info
Johannes Schindelin [Wed, 24 Oct 2018 15:56:10 +0000 (08:56 -0700)]
repack: point out a bug handling stale shallow info

A `git fetch --prune` can turn previously-reachable objects unreachable,
even commits that are in the `shallow` list. A subsequent `git repack
-ad` will then unceremoniously drop those unreachable commits, and the
`shallow` list will become stale. This means that when we try to fetch
with a larger `--depth` the next time, we may end up with:

fatal: error in object: unshallow <commit-hash>

Reported by Alejandro Pauly.

Signed-off-by: Johannes Schindelin <>
Signed-off-by: Junio C Hamano <>
2 years agot0061: adjust to test-tool transition
Junio C Hamano [Thu, 25 Oct 2018 02:41:09 +0000 (11:41 +0900)]
t0061: adjust to test-tool transition

2 years agorun-command: mark path lookup errors with ENOENT
Jeff King [Wed, 24 Oct 2018 07:38:00 +0000 (03:38 -0400)]
run-command: mark path lookup errors with ENOENT

Since commit e3a434468f (run-command: use the
async-signal-safe execv instead of execvp, 2017-04-19),
prepare_cmd() does its own PATH lookup for any commands we
run (on non-Windows platforms).

However, its logic does not match the old execvp call when
we fail to find a matching entry in the PATH. Instead of
feeding the name directly to execv, execvp would consider
that an ENOENT error. By continuing and passing the name
directly to execv, we effectively behave as if "." was
included at the end of the PATH. This can have confusing and
even dangerous results.

The fix itself is pretty straight-forward. There's a new
test in t0061 to cover this explicitly, and I've also added
a duplicate of the ENOENT test to ensure that we return the
correct errno for this case.

Signed-off-by: Jeff King <>
Signed-off-by: Junio C Hamano <>
2 years agoexclude-promisor-objects: declare when option is allowed
Matthew DeVore [Tue, 23 Oct 2018 01:13:42 +0000 (18:13 -0700)]
exclude-promisor-objects: declare when option is allowed

The --exclude-promisor-objects option causes some funny behavior in at
least two commands: log and blame. It causes a BUG crash:

$ git log --exclude-promisor-objects
BUG: revision.c:2143: exclude_promisor_objects can only be used
when fetch_if_missing is 0

Fix this such that the option is treated like any other unknown option.
The commands that must support it are limited, so declare in those
commands that the flag is supported. In particular:


The commands were found by searching for logic which parses
--exclude-promisor-objects outside of revision.c. Extra logic outside of
revision.c is needed because fetch_if_missing must be turned on before
revision.c sees the option or it will BUG-crash. The above list is
supported by the fact that no other command is introspectively invoked
by another command passing --exclude-promisor-object.

Signed-off-by: Matthew DeVore <>
Signed-off-by: Junio C Hamano <>
2 years agoDocumentation/git-log.txt: do not show --exclude-promisor-objects
Matthew DeVore [Tue, 23 Oct 2018 01:13:41 +0000 (18:13 -0700)]
Documentation/git-log.txt: do not show --exclude-promisor-objects

Do not suggest that --exclude-promisor-objects is supported by git-log,
since it currently BUG-crashes and it's not necessary to support it.
Options that control behavior for promisor objects should be limited to
a small number of commands.

Signed-off-by: Matthew DeVore <>
Signed-off-by: Junio C Hamano <>
2 years agodiff: don't attempt to strip prefix from absolute Windows paths
Johannes Sixt [Fri, 19 Oct 2018 16:58:07 +0000 (18:58 +0200)]
diff: don't attempt to strip prefix from absolute Windows paths

git diff can be invoked with absolute paths. Typically, this triggers
the --no-index case. Then the absolute paths remain in the file names
that are printed in the output.

There is one peculiarity, though: When the command is invoked from a
a sub-directory in a repository, then it is attempted to strip the
sub-directory from the beginning of relative paths. Yet, to detect a
relative path the code just checks for an initial forward slash.
This mistakes a Windows style path like "D:/base" as a relative path
and the output looks like this, for example:

  D:\dir\test\one>git -P diff --numstat D:\dir\base D:\dir\diff
  1       1       ir/{base => diff}/1.txt

where the correct output should be

  D:\dir\test\one>git -P diff --numstat D:\dir\base D:\dir\diff
  1       1       D:/dir/{base => diff}/1.txt

If the sub-directory where 'git diff' is invoked is sufficiently deep
that the prefix becomes longer than the path to be printed, then the
subsequent code accesses the path out of bounds.

Use is_absolute_path() to detect Windows style absolute paths.

One might wonder whether the check for a directory separator that
is visible in the patch context should be changed from == '/' to
is_dir_sep() or not. It turns out not to be necessary. That code
only ever investigates paths that have undergone pathspec
normalization, after which there are only forward slashes even on

Signed-off-by: Johannes Sixt <>
Signed-off-by: Junio C Hamano <>
2 years agoreceive: denyCurrentBranch=updateinstead should not blindly update
Junio C Hamano [Fri, 19 Oct 2018 04:57:35 +0000 (13:57 +0900)]
receive: denyCurrentBranch=updateinstead should not blindly update

The handling of receive.denyCurrentBranch=updateInstead was added to
a switch statement that handles other values of the variable, but
all the other case arms only checked a condition to reject the
attempted push, or let later logic in the same function to still
intervene, so that a push that does not fast-forward (which is
checked after the switch statement in question) is still rejected.

But the handling of updateInstead incorrectly took immediate effect,
without giving other checks a chance to intervene.

Instead of calling update_worktree() that causes the side effect
immediately, just note the fact that we will need to call the
function later, and first give other checks a chance to reject the
request.  After the update-hook gets a chance to reject the push
(which happens as the last step in a series of checks), call
update_worktree() when we earlier detected the need to.

Reported-by: Rajesh Madamanchi
Signed-off-by: Junio C Hamano <>
2 years agopack-objects (mingw): initialize `packing_data` mutex in the correct spot
Johannes Schindelin [Tue, 16 Oct 2018 21:02:52 +0000 (14:02 -0700)]
pack-objects (mingw): initialize `packing_data` mutex in the correct spot

In 9ac3f0e5b3e4 (pack-objects: fix performance issues on packing large
deltas, 2018-07-22), a mutex was introduced that is used to guard the
call to set the delta size. This commit even added code to initialize
it, but at an incorrect spot: in `init_threaded_search()`, while the
call to `oe_set_delta_size()` (and hence to `packing_data_lock()`) can
happen in the call chain `check_object()` <- `get_object_details()` <-
`prepare_pack()` <- `cmd_pack_objects()`, which is long before the
`prepare_pack()` function calls `ll_find_deltas()` (which initializes
the threaded search).

Another tell-tale that the mutex was initialized in an incorrect spot is
that the function to initialize it lives in builtin/, while the code
that uses the mutex is defined in a libgit.a header file.

Let's use a more appropriate function: `prepare_packing_data()`, which
not only lives in libgit.a, but *has* to be called before the
`packing_data` struct is used that contains that mutex.

This fixes

Signed-off-by: Johannes Schindelin <>
Signed-off-by: Junio C Hamano <>
2 years agopack-objects (mingw): demonstrate a segmentation fault with large deltas
Johannes Schindelin [Tue, 16 Oct 2018 21:02:50 +0000 (14:02 -0700)]
pack-objects (mingw): demonstrate a segmentation fault with large deltas

There is a problem in the way 9ac3f0e5b3e4 (pack-objects: fix
performance issues on packing large deltas, 2018-07-22) initializes that
mutex in the `packing_data` struct. The problem manifests in a
segmentation fault on Windows, when a mutex (AKA critical section) is
accessed without being initialized. (With pthreads, you apparently do
not really have to initialize them?)

This was reported in

Signed-off-by: Johannes Schindelin <>
Signed-off-by: Junio C Hamano <>
2 years agopack-objects: fix typo 'detla' -> 'delta'
Johannes Schindelin [Tue, 16 Oct 2018 21:02:49 +0000 (14:02 -0700)]
pack-objects: fix typo 'detla' -> 'delta'

Signed-off-by: Johannes Schindelin <>
Signed-off-by: Junio C Hamano <>
2 years agolog: fix coloring of certain octopus merge shapes
Noam Postavsky [Sun, 2 Sep 2018 00:07:16 +0000 (20:07 -0400)]
log: fix coloring of certain octopus merge shapes

For octopus merges where the first parent edge immediately merges into
the next column to the left, the number of columns should be one less
than the usual case.

First parent to the left case:

| *-.
| |\ \
|/ / /

The usual case:

| *-.
| |\ \
| | | *

Also refactor the code to iterate over columns rather than dashes,
building from an initial patch suggested by Jeff King.

Signed-off-by: Noam Postavsky <>
Reviewed-by: Jeff King <>
Signed-off-by: Junio C Hamano <>
2 years agosplit-index: BUG() when cache entry refers to non-existing shared entry
SZEDER Gábor [Thu, 11 Oct 2018 09:53:57 +0000 (11:53 +0200)]
split-index: BUG() when cache entry refers to non-existing shared entry

When the split index feature is in use, then a cache entry is:

  - either only present in the split index, in which case its 'index'
    field must be 0,

  - or it should refer to an existing entry in the shared index, i.e.
    the 'index' field can't be greater than the size of the shared

If a cache entry were to refer to a non-existing entry in the shared
index, then that's a sign of something being wrong in the index state,
either as a result of a bug in dealing with the split/shared index
entries, or perhaps a (potentially unrelated) memory corruption issue.

prepare_to_write_split_index() already has a condition to catch cache
entries with such bogus 'index' field, but instead of calling BUG() it
just sets cache entry's 'index = 0', and the entry will then be
written to the new split index.

Don't write a new index file from bogus index state, and call BUG()
upon encountering an cache entry referring to a non-existing shared
index entry.

Running the test suite repeatedly with 'GIT_TEST_SPLIT_INDEX=yes'
doesn't trigger this condition.

Signed-off-by: SZEDER Gábor <>
Signed-off-by: Junio C Hamano <>
2 years agosplit-index: smudge and add racily clean cache entries to split index
SZEDER Gábor [Thu, 11 Oct 2018 09:43:09 +0000 (11:43 +0200)]
split-index: smudge and add racily clean cache entries to split index

Ever since the split index feature was introduced [1], refreshing a
split index is prone to a variant of the classic racy git problem.

Consider the following sequence of commands updating the split index
when the shared index contains a racily clean cache entry, i.e. an
entry whose cached stat data matches with the corresponding file in
the worktree and the cached mtime matches that of the index:

  echo "cached content" >file
  git update-index --split-index --add file
  echo "dirty worktree" >file    # size stays the same!
  # ... wait ...
  git update-index --add other-file

Normally, when a non-split index is updated, then do_write_index()
(the function responsible for writing all kinds of indexes, "regular",
split, and shared) recognizes racily clean cache entries, and writes
them with smudged stat data, i.e. with file size set to 0.  When
subsequent git commands read the index, they will notice that the
smudged stat data doesn't match with the file in the worktree, and
then go on to check the file's content and notice its dirtiness.

In the above example, however, in the second 'git update-index'
prepare_to_write_split_index() decides which cache entries stored only
in the shared index should be replaced in the new split index.  Alas,
this function never looks out for racily clean cache entries, and
since the file's stat data in the worktree hasn't changed since the
shared index was written, it won't be replaced in the new split index.
Consequently, do_write_index() doesn't even get this racily clean
cache entry, and can't smudge its stat data.  Subsequent git commands
will then see that the index has more recent mtime than the file and
that the (not smudged) cached stat data still matches with the file in
the worktree, and, ultimately, will erroneously consider the file

Modify prepare_to_write_split_index() to recognize racily clean cache
entries, and mark them to be added to the split index.  Note that
there are two places where it should check raciness: first those cache
entries that are only stored in the shared index, and then those that
have been copied by unpack_trees() from the shared index while it
constructed a new index.  This way do_write_index() will get these
racily clean cache entries as well, and will then write them with
smudged stat data to the new split index.

This change makes all tests in '' pass, so
flip the two 'test_expect_failure' tests to success.  Also add the '#'
(as in nr. of trial) to those tests' description that were omitted
when the tests expected failure.

Note that after this change if the index is split when it contains a
racily clean cache entry, then a smudged cache entry will be written
both to the new shared and to the new split indexes.  This doesn't
affect regular git commands: as far as they are concerned this is just
an entry in the split index replacing an outdated entry in the shared
index.  It did affect a few tests in '', though,
because they actually check which entries are stored in the split
index; a previous patch in this series has already made the necessary
adjustments in 't1700'.  And racily clean cache entries and index
splitting are rare enough to not worry about the resulting duplicated
smudged cache entries, and the additional complexity required to
prevent them is not worth it.

Several tests failed occasionally when the test suite was run with
'GIT_TEST_SPLIT_INDEX=yes'.  Here are those that I managed to trace
back to this racy split index problem, starting with those failing
more frequently, with a link to a failing Travis CI build job for
each.  The highlighted line [2] shows when the racy file was written,
which is not always in the failing test but in a preceeding setup

There might be others, e.g. perhaps '' and
others using '', but I couldn't confirm yet.

[1] In the branch leading to the merge commit v2.1.0-rc0~45 (Merge
    branch 'nd/split-index', 2014-07-16).

[2] Note that those highlighted lines are in the 'after failure' fold,
    and your browser might unhelpfully fold it up before you could
    take a good look.

Signed-off-by: SZEDER Gábor <>
Signed-off-by: Junio C Hamano <>
2 years agosplit-index: don't compare cached data of entries already marked for split index
SZEDER Gábor [Thu, 11 Oct 2018 09:43:08 +0000 (11:43 +0200)]
split-index: don't compare cached data of entries already marked for split index

When unpack_trees() constructs a new index, it copies cache entries
from the original index [1].  prepare_to_write_split_index() has to
deal with this, and it has a dedicated code path for copied entries
that are present in the shared index, where it compares the cached
data in the corresponding copied and original entries.  If the cached
data matches, then they are considered the same; if it differs, then
the copied entry will be marked for inclusion as a replacement entry
in the just about to be written split index by setting the

However, a cache entry already has its CE_UPDATE_IN_BASE flag set upon
reading the split index, if the entry already has a replacement entry
there, or upon refreshing the cached stat data, if the corresponding
file was modified.  The state of this flag is then preserved when
unpack_trees() copies a cache entry from the shared index.

So modify prepare_to_write_split_index() to check the copied cache
entries' CE_UPDATE_IN_BASE flag first, and skip the thorough
comparison of cached data if the flag is already set.  Those couple of
lines comparing the cached data would then have too many levels of
indentation, so extract them into a helper function.

Note that comparing the cached data in copied and original entries in
the shared index might actually be entirely unnecessary.  In theory
all code paths refreshing the cached stat data of an entry in the
shared index should set the CE_UPDATE_IN_BASE flag in that entry, and
unpack_trees() should preserve this flag when copying cache entries.
This means that the cached data is only ever changed if the
CE_UPDATE_IN_BASE flag is set as well.  Our test suite seems to
confirm this: instrumenting the conditions in question and running the
test suite repeatedly with 'GIT_TEST_SPLIT_INDEX=yes' showed that the
cached data in a copied entry differs from the data in the shared
entry only if its CE_UPDATE_IN_BASE flag is indeed set.

In practice, however, our test suite doesn't have 100% coverage,
GIT_TEST_SPLIT_INDEX is inherently random, and I certainly can't claim
to possess complete understanding of what goes on in unpack_trees()...
Therefore I kept the comparison of the cached data when
CE_UPDATE_IN_BASE is not set, just in case that an unnoticed or future
code path were to accidentally miss setting this flag upon refreshing
the cached stat data or unpack_trees() were to drop this flag while
copying a cache entry.

[1] Note that when unpack_trees() constructs the new index and decides
    that a cache entry should now refer to different content than what
    was recorded in the original index (e.g. 'git read-tree -m
    HEAD^'), then that can't really be considered a copy of the
    original, but rather the creation of a new entry.  Notably and
    pertinent to the split index feature, such a new entry doesn't
    have a reference to the original's shared index entry anymore,
    i.e. its 'index' field is set to 0.  Consequently, such an entry
    is treated by prepare_to_write_split_index() as an entry not
    present in the shared index and it will be added to the new split
    index, while the original entry will be marked as deleted, and
    neither the above discussion nor the changes in this patch apply
    to them.

Signed-off-by: SZEDER Gábor <>
Signed-off-by: Junio C Hamano <>
2 years agosplit-index: count the number of deleted entries
SZEDER Gábor [Thu, 11 Oct 2018 09:43:07 +0000 (11:43 +0200)]
split-index: count the number of deleted entries

'struct split_index' contains the field 'nr_deletions', whose name
with the 'nr_' prefix suggests that it contains the number of deleted
cache entries.  However, barring its initialization to 0, this field
is only ever set to 1, indicating that there is at least one deleted
entry, but not the number of deleted entries.  Luckily, this doesn't
cause any issues (other than confusing the reader, that is), because
the only place reading this field uses it in the same sense, i.e.: 'if

To avoid confusion, we could either rename this field to something
like 'has_deletions' to make its name match its role, or make it a
counter of deleted cache entries to match its name.

Let's make it a counter, to keep it in sync with the related field
'nr_replacements', which does contain the number of replaced cache
entries.  This will also give developers debugging the split index
code easy access to the number of deleted cache entries.

Signed-off-by: SZEDER Gábor <>
Signed-off-by: Junio C Hamano <>
2 years agot1700-split-index: date back files to avoid racy situations
SZEDER Gábor [Thu, 11 Oct 2018 09:43:06 +0000 (11:43 +0200)]
t1700-split-index: date back files to avoid racy situations

'' checks that the index was split correctly under
various circumstances and that all the different ways to turn the
split index feature on and off work correctly.  To do so, most of its
tests use 'test-tool dump-split-index' to see which files have their
cache entries in the split index.  All these tests assume that all
cache entries are written to the shared index (called "base"
throughout these tests) when a new shared index is created.  This is
an implementation detail: most git commands (basically all except 'git
update-index') don't care or know at all about split index or whether
a cache entry is stored in the split or shared index.

As demonstrated in the previous patch, refreshing a split index is
prone to a variant of the classic racy git issue.  The next patch will
fix this issue, but while doing so it will also slightly change this
behaviour: only cache entries with mtime in the past will be written
only to the newly created shared index, but racily clean cache entries
will be written to the new split index (with smudged stat data).

While this upcoming change won't at all affect any git commands, it
will violate the above mentioned assumption of 't1700's tests.  Since
these tests create or modify files and create or refresh the split
index in rapid succession, there are plenty of racily clean cache
entries to be dealt with, which will then be written to the new split
indexes, and, ultimately, will cause several tests in 't1700' to fail.

Let's prepare '' for this upcoming change and
modify its tests to avoid racily clean files by backdating the mtime
of any file modifications (and since a lot of tests create or modify
files, encapsulate it into a helper function).

Signed-off-by: SZEDER Gábor <>
Signed-off-by: Junio C Hamano <>
2 years agosplit-index: add tests to demonstrate the racy split index problem
SZEDER Gábor [Thu, 11 Oct 2018 09:43:05 +0000 (11:43 +0200)]
split-index: add tests to demonstrate the racy split index problem

Ever since the split index feature was introduced [1], refreshing a
split index is prone to a variant of the classic racy git problem.
There are a couple of unrelated tests in the test suite that
occasionally fail when run with 'GIT_TEST_SPLIT_INDEX=yes', but
'', the only test script focusing solely on split
index, has never noticed this issue, because it only cares about how
the index is split under various circumstances and all the different
ways to turn the split index feature on and off.

Add a dedicated test script '' to exercise
the split index feature in racy situations as well; kind of a
" for split index" but with modern style (the tests
do everything in &&-chained list of commands in 'test_expect_...'
blocks, and use 'test_cmp' for more informative output on failure).

The tests cover the following sequences of index splitting, updating,
and racy file modifications, with the last two cases demonstrating the
racy split index problem:

  1. Split the index while adding a racily clean file:

       echo "cached content" >file
       git update-index --split-index --add file
       echo "dirty worktree" >file    # size stays the same

     This case already works properly.  Even though the cache entry's
     stat data matches with the modifid file in the worktree,
     subsequent git commands will notice that the (split) index and
     the file have the same mtime, and then will go on to check the
     file's content and notice its dirtiness.

  2. Add a racily clean file to an already split index:

       git update-index --split-index
       echo "cached content" >file
       git update-index --add file
       echo "dirty worktree" >file

     This case already works properly.  After the second 'git
     update-index' writes the newly added file's cache entry to the
     new split index, it basically works in the same way as case #1.

  3. Split the index when it (i.e. the not yet splitted index)
     contains a racily clean cache entry, i.e. an entry whose cached
     stat data matches with the corresponding file in the worktree and
     the cached mtime matches that of the index:

       echo "cached content" >file
       git update-index --add file
       echo "dirty worktree" >file
       # ... wait ...
       git update-index --split-index --add other-file

     This case already works properly.  The shared index is written by
     do_write_index(), i.e. the same function that is responsible for
     writing "regular" and split indexes as well.  This function
     cleverly notices the racily clean cache entry, and writes the
     entry to the new shared index with smudged stat data, i.e. file
     size set to 0.  When subsequent git commands read the index, they
     will notice that the smudged stat data doesn't match with the
     file in the worktree, and then go on to check the file's content
     and notice its dirtiness.

  4. Update the split index when it contains a racily clean cache

       git update-index --split-index
       echo "cached content" >file
       git update-index --add file
       echo "dirty worktree" >file
       # ... wait ...
       git update-index --add other-file

     This case already works properly.  After the second 'git
     update-index' the newly added file's cache entry is only stored
     in the split index.  If a cache entry is present in the split
     index (even if it is a replacement of an outdated entry in the
     shared index), then it will always be included in the new split
     index on subsequent split index updates (until the file is
     removed or a new shared index is written), independently from
     whether the entry is racily clean or not.  When do_write_index()
     writes the new split index, it notices the racily clean cache
     entry, and smudges its stat date.  Subsequent git commands
     reading the index will notice the smudged stat data and then go
     on to check the file's content and notice its dirtiness.

  5. Update the split index when a racily clean cache entry is stored
     only in the shared index:

       echo "cached content" >file
       git update-index --split-index --add file
       echo "dirty worktree" >file
       # ... wait ...
       git update-index --add other-file

     This case fails due to the racy split index problem.  In the
     second 'git update-index' prepare_to_write_split_index() decides,
     among other things, which cache entries stored only in the shared
     index should be replaced in the new split index.  Alas, this
     function never looks out for racily clean cache entries, and
     since the file's stat data in the worktree hasn't changed since
     the shared index was written, the entry won't be replaced in the
     new split index.  Consequently, do_write_index() doesn't even get
     this racily clean cache entry, and can't smudge its stat data.
     Subsequent git commands will then see that the index has more
     recent mtime than the file and that the (not smudged) cached stat
     data still matches with the file in the worktree, and,
     ultimately, will erroneously consider the file clean.

  6. Update the split index after unpack_trees() copied a racily clean
     cache entry from the shared index:

       echo "cached content" >file
       git update-index --split-index --add file
       echo "dirty worktree" >file
       # ... wait ...
       git read-tree -m HEAD

     This case fails due to the racy split index problem.  This
     basically fails for the same reason as case #5 above, but there
     is one important difference, which warrants the dedicated test.
     While that second 'git update-index' in case #5 updates
     index_state in place, in this case 'git read-tree -m' calls
     unpack_trees(), which throws out the entire index, and constructs
     a new one from the (potentially updated) copies of the original's
     cache entries.  Consequently, when prepare_to_write_split_index()
     gets to work on this reconstructed index, it takes a different
     code path than in case #5 when deciding which cache entries in
     the shared index should be replaced.  The result is the same,
     though: the racily clean cache entry goes unnoticed, it isn't
     added to the split index with smudged stat data, and subsequent
     git commands will then erroneously consider the file clean.

Note that in the last two 'test_expect_failure' cases I omitted the
'#' (as in nr. of trial) from the tests' description on purpose for
now, as it breakes the TAP output [2]; it will be added at the end of
the series, when those two tests will be flipped to

[1] In the branch leading to the merge commit v2.1.0-rc0~45 (Merge
    branch 'nd/split-index', 2014-07-16).

[2] In the TAP output a '#' should separate the test's description
    from the TODO directive emitted by 'test_expect_failure'.  The
    additional '#' in "#$trial" interferes with this, the test harness
    won't recognize the TODO directive, and will report that those
    tests failed unexpectedly.

Signed-off-by: SZEDER Gábor <>
Signed-off-by: Junio C Hamano <>
2 years agodocs: typo: s/isimilar/similar/
Michael Witten [Sat, 6 Oct 2018 04:20:22 +0000 (04:20 +0000)]
docs: typo: s/isimilar/similar/

Signed-off-by: Michael Witten <>
Signed-off-by: Junio C Hamano <>
2 years agodocs: graph: remove unnecessary `graph_update()' call
Michael Witten [Sat, 6 Oct 2018 04:20:16 +0000 (04:20 +0000)]
docs: graph: remove unnecessary `graph_update()' call

The sample code calls `get_revision()' followed by `graph_update()',
but the documentation and source code indicate that `get_revision()'
already calls `graph_update()' for you.

Signed-off-by: Michael Witten <>
Signed-off-by: Junio C Hamano <>
2 years agodocs: typo: s/go/to/
Michael Witten [Sat, 6 Oct 2018 04:20:09 +0000 (04:20 +0000)]
docs: typo: s/go/to/

Signed-off-by: Michael Witten <>
Signed-off-by: Junio C Hamano <>
2 years agofetch-pack: exclude blobs when lazy-fetching trees
Jonathan Tan [Wed, 3 Oct 2018 23:04:53 +0000 (16:04 -0700)]
fetch-pack: exclude blobs when lazy-fetching trees

A partial clone with missing trees can be obtained using "git clone
--filter=tree:none <repo>". In such a repository, when a tree needs to
be lazily fetched, any tree or blob it directly or indirectly references
is fetched as well, regardless of whether the original command required
those objects, or if the local repository already had some of them.

This is because the fetch protocol, which the lazy fetch uses, does not
allow clients to request that only the wanted objects be sent, which
would be the ideal solution. This patch implements a partial solution:
specify the "blob:none" filter, somewhat reducing the fetch payload.

This change has no effect when lazily fetching blobs (due to how filters
work). And if lazily fetching a commit (such repositories are difficult
to construct and is not a use case we support very well, but it is
possible), referenced commits and trees are still fetched - only the
blobs are not fetched.

The necessary code change is done in fetch_pack() instead of somewhere
closer to where the "filter" instruction is written to the wire so that
only one part of the code needs to be changed in order for users of all
protocol versions to benefit from this optimization.

Signed-off-by: Jonathan Tan <>
Signed-off-by: Junio C Hamano <>
2 years agofetch-pack: avoid object flags if no_dependents
Jonathan Tan [Wed, 3 Oct 2018 23:04:52 +0000 (16:04 -0700)]
fetch-pack: avoid object flags if no_dependents

When fetch_pack() is invoked as part of another Git command (due to a
lazy fetch from a partial clone, for example), it uses object flags that
may already be used by the outer Git command.

The commit that introduced the lazy fetch feature (88e2f9ed8e
("introduce fetch-object: fetch one promisor object", 2017-12-05)) tried
to avoid this overlap, but it did not avoid it totally. It was
successful in avoiding writing COMPLETE, but did not avoid reading
COMPLETE, and did not avoid writing and reading ALTERNATE.

Ensure that no flags are written or read by fetch_pack() in the case
where it is used to perform a lazy fetch. To do this, it is sufficient
to avoid checking completeness of wanted refs (unnecessary in the case
of lazy fetches), and to avoid negotiation-related work (in the current
implementation, already, no negotiation is performed). After that was
done, the lack of overlap was verified by checking all direct and
indirect usages of COMPLETE and ALTERNATE - that they are read or
written only if no_dependents is false.

There are other possible solutions to this issue:

 (1) Split fetch-pack.{c,h} into a flag-using part and a non-flag-using
     part, and whenever no_dependents is set, only use the
     non-flag-using part.
 (2) Make fetch_pack() be able to be used with arbitrary repository
     objects. fetch_pack() should then create its own repository object
     based on the given repository object, with its own object
     hashtable, so that the flags do not conflict.

(1) is possible but invasive - some functions would need to be split;
and such invasiveness would potentially be unnecessary if we ever were
to need (2) anyway. (2) would be useful if we were to support, say,
submodules that were partial clones themselves, but I don't know when or
if the Git project plans to support those.

Signed-off-by: Jonathan Tan <>
Signed-off-by: Junio C Hamano <>
2 years agosequencer: use return value of oidset_insert()
René Scharfe [Wed, 3 Oct 2018 13:06:49 +0000 (15:06 +0200)]
sequencer: use return value of oidset_insert()

oidset_insert() returns 1 if the object ID is already in the set and
doesn't add it again, or 0 if it hadn't been present.  Make use of that
fact instead of checking with an extra oidset_contains() call.

Signed-off-by: Rene Scharfe <>
Acked-by: Johannes Schindelin <>
Signed-off-by: Junio C Hamano <>
2 years agoconfig.txt: correct the note about uploadpack.packObjectsHook
Nguyễn Thái Ngọc Duy [Sat, 29 Sep 2018 06:50:56 +0000 (08:50 +0200)]
config.txt: correct the note about uploadpack.packObjectsHook

Document for uploadpack.packObjectsHook is added in [1] and consists
of two paragraphs, the second one is quite important about where this
variable can stay.

When the paragraph about uploadpack.allowFilter is added in [2], it's
added in between the two paragraphs. This makes the "this is non-repo
level config" note incorrectly apply to allowFilter instead of
packObjectsHook. Move allowFilter paragraph down to fix this.

[1] 20b20a22f8 (upload-pack: provide a hook for running pack-objects -

[2] 10ac85c785 (upload-pack: add object filtering for partial clone -

Signed-off-by: Nguyễn Thái Ngọc Duy <>
Signed-off-by: Junio C Hamano <>
2 years agogit doc: direct bug reporters to mailing list archive
Jonathan Nieder [Fri, 28 Sep 2018 21:20:49 +0000 (14:20 -0700)]
git doc: direct bug reporters to mailing list archive

The mailing list archive can help a user encountering a bug to tell
whether a recent regression has already been reported and whether a
longstanding bug has already had some discussion to start their

Based-on-patch-by: Martin Ågren <>
Improved-by: Junio C Hamano <>
Improved-by: Jonathan Nieder <>
Signed-off-by: Jonathan Nieder <>
Signed-off-by: Junio C Hamano <>
2 years agoCodingGuidelines: document the API in *.h files
Junio C Hamano [Fri, 28 Sep 2018 16:50:14 +0000 (09:50 -0700)]
CodingGuidelines: document the API in *.h files

It makes it harder to let the API description and the reality drift
apart if the doc is kept close to the implementation or the header
of the API.  We have been slowly migrating API docs out of the
Documentation/technical/api-* to *.h files, and the development
community generally considers that how inline docs in strbuf.h is
done the best current practice.

We recommend documenting in the header over documenting near the
implementation to encourage people to write the docs that are
readable without peeking at the implemention.

Helped-by: Jeff King <>
Signed-off-by: Junio C Hamano <>
2 years agot7005-editor: quote filename to fix whitespace-issue
Alexander Pyhalov [Wed, 26 Sep 2018 16:14:11 +0000 (18:14 +0200)]
t7005-editor: quote filename to fix whitespace-issue

Commit 4362da078e (t7005-editor: get rid of the SPACES_IN_FILENAMES
prereq, 2018-05-14) removed code for detecting whether spaces in
filenames work. Since we rely on spaces throughout the test suite
("trash directory.t1234-foo"), testing whether we can use the filename
"e" was redundant and unnecessary.

In simplifying the code, though, this introduced a regression around how
spaces are handled, not in the /name/ of the editor script, but /in/ the
script itself. The script just does `echo space >$1`, where $1 is for
example "/foo/t/trash directory.t7005-editor/.git/COMMIT_EDITMSG".

With most shells, or with Bash in posix mode, $1 will not be subjected
to field splitting. But if we invoke Bash directly, which will happen if
we build Git with SHELL_PATH=/bin/bash, it will detect and complain
about an "ambiguous redirect". More details can be found in [1], thanks
to SZEDER Gábor.

Make sure that the editor script quotes "$1" to remove the ambiguity.


Signed-off-by: Alexander Pyhalov <>
Commit-message-by: Martin Ågren <>
Signed-off-by: Martin Ågren <>
Signed-off-by: Junio C Hamano <>
2 years agorev-parse: --show-superproject-working-tree should work during a merge
Sam McKelvie [Thu, 27 Sep 2018 18:10:54 +0000 (11:10 -0700)]
rev-parse: --show-superproject-working-tree should work during a merge

Invoking 'git rev-parse --show-superproject-working-tree' exits with

    "fatal: BUG: returned path string doesn't match cwd?"

when the superproject has an unmerged entry for the current submodule,
instead of displaying the superproject's working tree.

The problem is due to the fact that when a merge of the submodule reference
is in progress, "git ls-files --stage —full-name <submodule-relative-path>”
returns three seperate entries for the submodule (one for each stage) rather
than a single entry; e.g.,

  $ git ls-files --stage --full-name submodule-child-test
  160000 dbbd2766fa330fa741ea59bb38689fcc2d283ac5 1       submodule-child-test
  160000 f174d1dbfe863a59692c3bdae730a36f2a788c51 2       submodule-child-test
  160000 e6178f3a58b958543952e12824aa2106d560f21d 3       submodule-child-test

The code in get_superproject_working_tree() expected exactly one entry to
be returned; this patch makes it use the first entry if multiple entries
are returned.

Test t1500-rev-parse is extended to cover this case.

Signed-off-by: Sam McKelvie <>
Reviewed-by: Stefan Beller <>
Signed-off-by: Junio C Hamano <>
2 years agot1400: drop debug `echo` to actually execute `test`
Martin Ågren [Fri, 28 Sep 2018 15:43:59 +0000 (17:43 +0200)]
t1400: drop debug `echo` to actually execute `test`

Instead of running `test "foo" = "$(bar)"`, we prefix the whole thing
with `echo`. Comparing to nearby tests makes it clear that this is just
debug leftover. This line has actually been modified four times since it
was introduced in e52290428b (General ref log reading improvements.,
2006-05-19) and the `echo` has always survived. Let's finally drop it.

This script could need some more cleanups. This is just an immediate fix
so that we actually test what we intend to.

All other hits for `git grep "\<echo test " -- t/` seem fine. They want
to create some input or expected output data.

Signed-off-by: Martin Ågren <>
Signed-off-by: Junio C Hamano <>
2 years agot1700-split-index: document why FSMONITOR is disabled in this test script
SZEDER Gábor [Fri, 28 Sep 2018 16:24:54 +0000 (18:24 +0200)]
t1700-split-index: document why FSMONITOR is disabled in this test script

Signed-off-by: SZEDER Gábor <>
Signed-off-by: Junio C Hamano <>
2 years agoDoc: refer to the "commit-graph file" with dash
Martin Ågren [Thu, 27 Sep 2018 19:12:22 +0000 (21:12 +0200)]
Doc: refer to the "commit-graph file" with dash

The file processed by `git commit-graph` is referred to as the
"commit-graph file", also with a dash. We have a few references to the
"commit graph file", though, without the dash. These occur in
git-commit-graph.txt as well as in Doc/technical/commit-graph.txt. Fix

Do not change the references to the "commit graph" (without "... file")
as a data structure.

Signed-off-by: Martin Ågren <>
Reviewed-by: Derrick Stolee <>
Signed-off-by: Junio C Hamano <>
2 years agogit-commit-graph.txt: refer to "*commit*-graph file"
Martin Ågren [Thu, 27 Sep 2018 19:12:21 +0000 (21:12 +0200)]
git-commit-graph.txt: refer to "*commit*-graph file"

This document sometimes refers to the "commit-graph file" as just "the
graph file". This saves a couple of words here and there at the risk of
confusion. In particular, the documentation for `git commit-graph read`
appears to suggest that there are indeed different types of graph files.

Let's just write out the full name everywhere.

The full name, by the way, is not the dash-less "commit graph file".
Use the dashed form. (The next commit will fix the remaining few
instances of the "commit graph file" in this document.)

Signed-off-by: Martin Ågren <>
Reviewed-by: Derrick Stolee <>
Signed-off-by: Junio C Hamano <>
2 years agogit-commit-graph.txt: typeset more in monospace
Martin Ågren [Thu, 27 Sep 2018 19:12:20 +0000 (21:12 +0200)]
git-commit-graph.txt: typeset more in monospace

While we're here, fix an instance of "folder" to be "directory".

Signed-off-by: Martin Ågren <>
Reviewed-by: Derrick Stolee <>
Signed-off-by: Junio C Hamano <>
2 years agogit-commit-graph.txt: fix bullet lists
Martin Ågren [Thu, 27 Sep 2018 19:12:19 +0000 (21:12 +0200)]
git-commit-graph.txt: fix bullet lists

We have a couple of bullet items which span multiple lines, and where we
have prefixed each line with a `*`. (This might be the result of a text
editor trying to help.) This results in each line being typeset as a
separate bullet item. Drop the extra `*`.

Signed-off-by: Martin Ågren <>
Reviewed-by: Derrick Stolee <>
Signed-off-by: Junio C Hamano <>
2 years agodoc: clarify gitcredentials path component matching
David Zych [Wed, 26 Sep 2018 22:23:11 +0000 (22:23 +0000)]
doc: clarify gitcredentials path component matching

The gitcredentials documentation implied that the config file's
"pattern" URL might include a path component, but did not explain that
it must match exactly (potentially leaving readers with the false hope
that it would support a more flexible prefix match).

Signed-off-by: David Zych <>
Signed-off-by: Junio C Hamano <>
2 years agocommit: fix erroneous BUG, 'multiple renames on the same target? how?'
Elijah Newren [Thu, 27 Sep 2018 17:36:57 +0000 (10:36 -0700)]
commit: fix erroneous BUG, 'multiple renames on the same target? how?'

builtin/commit.c:prepare_to_commit() can call run_status() twice if
using the editor, including status, and the user attempts to record a
non-merge empty commit without explicit --allow-empty.  If there is also
a rename involved as well (due to using 'git add -N'), then a BUG in
wt-status.c is triggered:

  BUG: wt-status.c:476: multiple renames on the same target? how?

The reason we hit this bug is that both run_status() calls use the same
struct wt_status * (named s), and s->change is not freed between runs.
Changes are inserted into s with string_list_insert, which usually means
that the second run just recomputes all the same results and overwrites
what was computed the first time.  However, ever since commit
176ea7479309 ("wt-status.c: handle worktree renames", 2017-12-27),
wt-status started checking for renames and copies but also added a
preventative check that d->rename_status wasn't already set and output a
BUG message if it was.  The problem isn't that there are multiple rename
targets to a single path as the error implies, the problem is that 's'
is not freed/cleared between the two run_status() calls.

Ever since commit dc6b1d92ca9c ("wt-status: use settings from
git_diff_ui_config", 2018-05-04), which stopped hardcoding
DIFF_DETECT_RENAME and allowed users to ask for copy detection, this bug
has also been triggerable with a copy instead of a rename.

Fix the bug by clearing s->change.  A better change might be to clean up
all of s between the two run_status() calls.  A good first step towards
such a goal might be writing a function to free the necessary fields in
the wt_status * struct; a cursory glance at the code suggests all of its
allocated data is probably leaked.  However, doing all that cleanup is a
bigger task for someone else interested to tackle; just fix the bug for

Reported-by: Andrea Stacchiotti <>
Signed-off-by: Elijah Newren <>
Signed-off-by: Junio C Hamano <>
2 years agoGit 2.19.1 v2.19.1
Junio C Hamano [Thu, 27 Sep 2018 18:52:33 +0000 (11:52 -0700)]
Git 2.19.1

Signed-off-by: Junio C Hamano <>
2 years agoSync with 2.18.1
Junio C Hamano [Thu, 27 Sep 2018 18:50:45 +0000 (11:50 -0700)]
Sync with 2.18.1

* maint-2.18:
  Git 2.18.1
  Git 2.17.2
  fsck: detect submodule paths starting with dash
  fsck: detect submodule urls starting with dash
  Git 2.16.5
  Git 2.15.3
  Git 2.14.5
  submodule-config: ban submodule paths that start with a dash
  submodule-config: ban submodule urls that start with dash
  submodule--helper: use "--" to signal end of clone options

2 years agoGit 2.18.1 v2.18.1
Junio C Hamano [Thu, 27 Sep 2018 18:48:19 +0000 (11:48 -0700)]
Git 2.18.1

Signed-off-by: Junio C Hamano <>
2 years agoSync with 2.17.2
Junio C Hamano [Thu, 27 Sep 2018 18:45:01 +0000 (11:45 -0700)]
Sync with 2.17.2

* maint-2.17:
  Git 2.17.2
  fsck: detect submodule paths starting with dash
  fsck: detect submodule urls starting with dash
  Git 2.16.5
  Git 2.15.3
  Git 2.14.5
  submodule-config: ban submodule paths that start with a dash
  submodule-config: ban submodule urls that start with dash
  submodule--helper: use "--" to signal end of clone options

2 years agoGit 2.17.2 v2.17.2
Junio C Hamano [Thu, 27 Sep 2018 18:44:07 +0000 (11:44 -0700)]
Git 2.17.2

Signed-off-by: Junio C Hamano <>