git/git.git
2 years agorevision.c: generation-based topo-order algorithm
Derrick Stolee [Thu, 1 Nov 2018 13:46:22 +0000 (13:46 +0000)]
revision.c: generation-based topo-order algorithm

The current --topo-order algorithm requires walking all
reachable commits up front, topo-sorting them, all before
outputting the first value. This patch introduces a new
algorithm which uses stored generation numbers to
incrementally walk in topo-order, outputting commits as
we go. This can dramatically reduce the computation time
to write a fixed number of commits, such as when limiting
with "-n <N>" or filling the first page of a pager.

When running a command like 'git rev-list --topo-order HEAD',
Git performed the following steps:

1. Run limit_list(), which parses all reachable commits,
   adds them to a linked list, and distributes UNINTERESTING
   flags. If all unprocessed commits are UNINTERESTING, then
   it may terminate without walking all reachable commits.
   This does not occur if we do not specify UNINTERESTING
   commits.

2. Run sort_in_topological_order(), which is an implementation
   of Kahn's algorithm. It first iterates through the entire
   set of important commits and computes the in-degree of each
   (plus one, as we use 'zero' as a special value here). Then,
   we walk the commits in priority order, adding them to the
   priority queue if and only if their in-degree is one. As
   we remove commits from this priority queue, we decrement the
   in-degree of their parents.

3. While we are peeling commits for output, get_revision_1()
   uses pop_commit on the full list of commits computed by
   sort_in_topological_order().

In the new algorithm, these three steps correspond to three
different commit walks. We run these walks simultaneously,
and advance each only as far as necessary to satisfy the
requirements of the 'higher order' walk. We know when we can
pause each walk by using generation numbers from the commit-
graph feature.

Recall that the generation number of a commit satisfies:

* If the commit has at least one parent, then the generation
  number is one more than the maximum generation number among
  its parents.

* If the commit has no parent, then the generation number is one.

There are two special generation numbers:

* GENERATION_NUMBER_INFINITY: this value is 0xffffffff and
  indicates that the commit is not stored in the commit-graph and
  the generation number was not previously calculated.

* GENERATION_NUMBER_ZERO: this value (0) is a special indicator
  to say that the commit-graph was generated by a version of Git
  that does not compute generation numbers (such as v2.18.0).

Since we use generation_numbers_enabled() before using the new
algorithm, we do not need to worry about GENERATION_NUMBER_ZERO.
However, the existence of GENERATION_NUMBER_INFINITY implies the
following weaker statement than the usual we expect from
generation numbers:

    If A and B are commits with generation numbers gen(A) and
    gen(B) and gen(A) < gen(B), then A cannot reach B.

Thus, we will walk in each of our stages until the "maximum
unexpanded generation number" is strictly lower than the
generation number of a commit we are about to use.

The walks are as follows:

1. EXPLORE: using the explore_queue priority queue (ordered by
   maximizing the generation number), parse each reachable
   commit until all commits in the queue have generation
   number strictly lower than needed. During this walk, update
   the UNINTERESTING flags as necessary.

2. INDEGREE: using the indegree_queue priority queue (ordered
   by maximizing the generation number), add one to the in-
   degree of each parent for each commit that is walked. Since
   we walk in order of decreasing generation number, we know
   that discovering an in-degree value of 0 means the value for
   that commit was not initialized, so should be initialized to
   two. (Recall that in-degree value "1" is what we use to say a
   commit is ready for output.) As we iterate the parents of a
   commit during this walk, ensure the EXPLORE walk has walked
   beyond their generation numbers.

3. TOPO: using the topo_queue priority queue (ordered based on
   the sort_order given, which could be commit-date, author-
   date, or typical topo-order which treats the queue as a LIFO
   stack), remove a commit from the queue and decrement the
   in-degree of each parent. If a parent has an in-degree of
   one, then we add it to the topo_queue. Before we decrement
   the in-degree, however, ensure the INDEGREE walk has walked
   beyond that generation number.

The implementations of these walks are in the following methods:

* explore_walk_step and explore_to_depth
* indegree_walk_step and compute_indegrees_to_depth
* next_topo_commit and expand_topo_walk

These methods have some patterns that may seem strange at first,
but they are probably carry-overs from their equivalents in
limit_list and sort_in_topological_order.

One thing that is missing from this implementation is a proper
way to stop walking when the entire queue is UNINTERESTING, so
this implementation is not enabled by comparisions, such as in
'git rev-list --topo-order A..B'. This can be updated in the
future.

In my local testing, I used the following Git commands on the
Linux repository in three modes: HEAD~1 with no commit-graph,
HEAD~1 with a commit-graph, and HEAD with a commit-graph. This
allows comparing the benefits we get from parsing commits from
the commit-graph and then again the benefits we get by
restricting the set of commits we walk.

Test: git rev-list --topo-order -100 HEAD
HEAD~1, no commit-graph: 6.80 s
HEAD~1, w/ commit-graph: 0.77 s
  HEAD, w/ commit-graph: 0.02 s

Test: git rev-list --topo-order -100 HEAD -- tools
HEAD~1, no commit-graph: 9.63 s
HEAD~1, w/ commit-graph: 6.06 s
  HEAD, w/ commit-graph: 0.06 s

This speedup is due to a few things. First, the new generation-
number-enabled algorithm walks commits on order of the number of
results output (subject to some branching structure expectations).
Since we limit to 100 results, we are running a query similar to
filling a single page of results. Second, when specifying a path,
we must parse the root tree object for each commit we walk. The
previous benefits from the commit-graph are entirely from reading
the commit-graph instead of parsing commits. Since we need to
parse trees for the same number of commits as before, we slow
down significantly from the non-path-based query.

For the test above, I specifically selected a path that is changed
frequently, including by merge commits. A less-frequently-changed
path (such as 'README') has similar end-to-end time since we need
to walk the same number of commits (before determining we do not
have 100 hits). However, get the benefit that the output is
presented to the user as it is discovered, much the same as a
normal 'git log' command (no '--topo-order'). This is an improved
user experience, even if the command has the same runtime.

Helped-by: Jeff King <peff@peff.net>
Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2 years agocommit/revisions: bookkeeping before refactoring
Derrick Stolee [Thu, 1 Nov 2018 13:46:21 +0000 (13:46 +0000)]
commit/revisions: bookkeeping before refactoring

There are a few things that need to move around a little before
making a big refactoring in the topo-order logic:

1. We need access to record_author_date() and
   compare_commits_by_author_date() in revision.c. These are used
   currently by sort_in_topological_order() in commit.c.

2. Moving these methods to commit.h requires adding an author_date_slab
   declaration to commit.h. Consumers will need their own implementation.

3. The add_parents_to_list() method in revision.c performs logic
   around the UNINTERESTING flag and other special cases depending
   on the struct rev_info. Allow this method to ignore a NULL 'list'
   parameter, as we will not be populating the list for our walk.
   Also rename the method to the slightly more generic name
   process_parents() to make clear that this method does more than
   add to a list (and no list is required anymore).

Helped-by: Jeff King <peff@peff.net>
Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2 years agorevision.c: begin refactoring --topo-order logic
Derrick Stolee [Thu, 1 Nov 2018 13:46:20 +0000 (13:46 +0000)]
revision.c: begin refactoring --topo-order logic

When running 'git rev-list --topo-order' and its kin, the topo_order
setting in struct rev_info implies the limited setting. This means
that the following things happen during prepare_revision_walk():

* revs->limited implies we run limit_list() to walk the entire
  reachable set. There are some short-cuts here, such as if we
  perform a range query like 'git rev-list COMPARE..HEAD' and we
  can stop limit_list() when all queued commits are uninteresting.

* revs->topo_order implies we run sort_in_topological_order(). See
  the implementation of that method in commit.c. It implies that
  the full set of commits to order is in the given commit_list.

These two methods imply that a 'git rev-list --topo-order HEAD'
command must walk the entire reachable set of commits _twice_ before
returning a single result.

If we have a commit-graph file with generation numbers computed, then
there is a better way. This patch introduces some necessary logic
redirection when we are in this situation.

In v2.18.0, the commit-graph file contains zero-valued bytes in the
positions where the generation number is stored in v2.19.0 and later.
Thus, we use generation_numbers_enabled() to check if the commit-graph
is available and has non-zero generation numbers.

When setting revs->limited only because revs->topo_order is true,
only do so if generation numbers are not available. There is no
reason to use the new logic as it will behave similarly when all
generation numbers are INFINITY or ZERO.

In prepare_revision_walk(), if we have revs->topo_order but not
revs->limited, then we trigger the new logic. It breaks the logic
into three pieces, to fit with the existing framework:

1. init_topo_walk() fills a new struct topo_walk_info in the rev_info
   struct. We use the presence of this struct as a signal to use the
   new methods during our walk. In this patch, this method simply
   calls limit_list() and sort_in_topological_order(). In the future,
   this method will set up a new data structure to perform that logic
   in-line.

2. next_topo_commit() provides get_revision_1() with the next topo-
   ordered commit in the list. Currently, this simply pops the commit
   from revs->commits.

3. expand_topo_walk() provides get_revision_1() with a way to signal
   walking beyond the latest commit. Currently, this calls
   add_parents_to_list() exactly like the old logic.

While this commit presents method redirection for performing the
exact same logic as before, it allows the next commit to focus only
on the new logic.

Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2 years agotest-reach: add rev-list tests
Derrick Stolee [Thu, 1 Nov 2018 13:46:19 +0000 (13:46 +0000)]
test-reach: add rev-list tests

The rev-list command is critical to Git's functionality. Ensure it
works in the three commit-graph environments constructed in
t6600-test-reach.sh. Here are a few important types of rev-list
operations:

* Basic: git rev-list --topo-order HEAD
* Range: git rev-list --topo-order compare..HEAD
* Ancestry: git rev-list --topo-order --ancestry-path compare..HEAD
* Symmetric Difference: git rev-list --topo-order compare...HEAD

Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2 years agotest-reach: add run_three_modes method
Derrick Stolee [Thu, 1 Nov 2018 13:46:18 +0000 (13:46 +0000)]
test-reach: add run_three_modes method

The 'test_three_modes' method assumes we are using the 'test-tool
reach' command for our test. However, we may want to use the data
shape of our commit graph and the three modes (no commit-graph,
full commit-graph, partial commit-graph) for other git commands.

Split test_three_modes to be a simple translation on a more general
run_three_modes method that executes the given command and tests
the actual output to the expected output.

Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2 years agoprio-queue: add 'peek' operation
Derrick Stolee [Thu, 1 Nov 2018 13:46:17 +0000 (13:46 +0000)]
prio-queue: add 'peek' operation

When consuming a priority queue, it can be convenient to inspect
the next object that will be dequeued without actually dequeueing
it. Our existing library did not have such a 'peek' operation, so
add it as prio_queue_peek().

Add a reference-level comparison in t/helper/test-prio-queue.c
so this method is exercised by t0009-prio-queue.sh. Further, add
a test that checks the behavior when the compare function is NULL
(i.e. the queue becomes a stack).

Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2 years agoInitial batch post 2.19
Junio C Hamano [Mon, 17 Sep 2018 21:15:00 +0000 (14:15 -0700)]
Initial batch post 2.19

2 years agoMerge branch 'nd/bisect-show-list-fix'
Junio C Hamano [Mon, 17 Sep 2018 20:54:00 +0000 (13:54 -0700)]
Merge branch 'nd/bisect-show-list-fix'

Debugging aid update.

* nd/bisect-show-list-fix:
  bisect.c: make show_list() build again

2 years agoMerge branch 'ab/fetch-tags-noclobber'
Junio C Hamano [Mon, 17 Sep 2018 20:54:00 +0000 (13:54 -0700)]
Merge branch 'ab/fetch-tags-noclobber'

The rules used by "git push" and "git fetch" to determine if a ref
can or cannot be updated were inconsistent; specifically, fetching
to update existing tags were allowed even though tags are supposed
to be unmoving anchoring points.  "git fetch" was taught to forbid
updates to existing tags without the "--force" option.

* ab/fetch-tags-noclobber:
  fetch: stop clobbering existing tags without --force
  fetch: document local ref updates with/without --force
  push doc: correct lies about how push refspecs work
  push doc: move mention of "tag <tag>" later in the prose
  push doc: remove confusing mention of remote merger
  fetch tests: add a test for clobbering tag behavior
  push tests: use spaces in interpolated string
  push tests: make use of unused $1 in test description
  fetch: change "branch" to "reference" in --force -h output

2 years agoMerge branch 'es/worktree-forced-ops-fix'
Junio C Hamano [Mon, 17 Sep 2018 20:53:59 +0000 (13:53 -0700)]
Merge branch 'es/worktree-forced-ops-fix'

Fix a bug in which the same path could be registered under multiple
worktree entries if the path was missing (for instance, was removed
manually).  Also, as a convenience, expand the number of cases in
which --force is applicable.

* es/worktree-forced-ops-fix:
  doc-diff: force worktree add
  worktree: delete .git/worktrees if empty after 'remove'
  worktree: teach 'remove' to override lock when --force given twice
  worktree: teach 'move' to override lock when --force given twice
  worktree: teach 'add' to respect --force for registered but missing path
  worktree: disallow adding same path multiple times
  worktree: prepare for more checks of whether path can become worktree
  worktree: generalize delete_git_dir() to reduce code duplication
  worktree: move delete_git_dir() earlier in file for upcoming new callers
  worktree: don't die() in library function find_worktree()

2 years agoMerge branch 'sg/doc-trace-appends'
Junio C Hamano [Mon, 17 Sep 2018 20:53:59 +0000 (13:53 -0700)]
Merge branch 'sg/doc-trace-appends'

Docfix.

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

2 years agoMerge branch 'jk/diff-rendered-docs'
Junio C Hamano [Mon, 17 Sep 2018 20:53:59 +0000 (13:53 -0700)]
Merge branch 'jk/diff-rendered-docs'

Dev doc update.

* jk/diff-rendered-docs:
  Revert "doc/Makefile: drop doc-diff worktree and temporary files on "make clean""
  doc/Makefile: drop doc-diff worktree and temporary files on "make clean"
  doc-diff: add --clean mode to remove temporary working gunk
  doc-diff: fix non-portable 'man' invocation
  doc-diff: always use oids inside worktree
  SubmittingPatches: mention doc-diff

2 years agoMerge branch 'jk/patch-corrupted-delta-fix'
Junio C Hamano [Mon, 17 Sep 2018 20:53:58 +0000 (13:53 -0700)]
Merge branch 'jk/patch-corrupted-delta-fix'

Malformed or crafted data in packstream can make our code attempt
to read or write past the allocated buffer and abort, instead of
reporting an error, which has been fixed.

* jk/patch-corrupted-delta-fix:
  t5303: use printf to generate delta bases
  patch-delta: handle truncated copy parameters
  patch-delta: consistently report corruption
  patch-delta: fix oob read
  t5303: test some corrupt deltas
  test-delta: read input into a heap buffer

2 years agoMerge branch 'ds/commit-graph-tests'
Junio C Hamano [Mon, 17 Sep 2018 20:53:58 +0000 (13:53 -0700)]
Merge branch 'ds/commit-graph-tests'

We can now optionally run tests with commit-graph enabled.

* ds/commit-graph-tests:
  commit-graph: define GIT_TEST_COMMIT_GRAPH

2 years agoMerge branch 'jk/pack-objects-with-bitmap-fix'
Junio C Hamano [Mon, 17 Sep 2018 20:53:58 +0000 (13:53 -0700)]
Merge branch 'jk/pack-objects-with-bitmap-fix'

Hotfix of the base topic.

* jk/pack-objects-with-bitmap-fix:
  pack-bitmap: drop "loaded" flag
  traverse_bitmap_commit_list(): don't free result
  t5310: test delta reuse with bitmaps
  bitmap_has_sha1_in_uninteresting(): drop BUG check

2 years agoMerge branch 'rs/mailinfo-format-flowed'
Junio C Hamano [Mon, 17 Sep 2018 20:53:57 +0000 (13:53 -0700)]
Merge branch 'rs/mailinfo-format-flowed'

"git mailinfo" used in "git am" learned to make a best-effort
recovery of a patch corrupted by MUA that sends text/plain with
format=flawed option.

* rs/mailinfo-format-flowed:
  mailinfo: support format=flowed

2 years agoMerge branch 'jk/cocci'
Junio C Hamano [Mon, 17 Sep 2018 20:53:57 +0000 (13:53 -0700)]
Merge branch 'jk/cocci'

spatch transformation to replace boolean uses of !hashcmp() to
newly introduced oideq() is added, and applied, to regain
performance lost due to support of multiple hash algorithms.

* jk/cocci:
  show_dirstat: simplify same-content check
  read-cache: use oideq() in ce_compare functions
  convert hashmap comparison functions to oideq()
  convert "hashcmp() != 0" to "!hasheq()"
  convert "oidcmp() != 0" to "!oideq()"
  convert "hashcmp() == 0" to hasheq()
  convert "oidcmp() == 0" to oideq()
  introduce hasheq() and oideq()
  coccinelle: use <...> for function exclusion

2 years agoMerge branch 'tg/rerere-doc-updates'
Junio C Hamano [Mon, 17 Sep 2018 20:53:56 +0000 (13:53 -0700)]
Merge branch 'tg/rerere-doc-updates'

Clarify a part of technical documentation for rerere.

* tg/rerere-doc-updates:
  rerere: add note about files with existing conflict markers
  rerere: mention caveat about unmatched conflict markers

2 years agoMerge branch 'es/format-patch-rangediff'
Junio C Hamano [Mon, 17 Sep 2018 20:53:56 +0000 (13:53 -0700)]
Merge branch 'es/format-patch-rangediff'

"git format-patch" learned a new "--range-diff" option to explain
the difference between this version and the previous attempt in
the cover letter (or after the tree-dashes as a comment).

* es/format-patch-rangediff:
  format-patch: allow --range-diff to apply to a lone-patch
  format-patch: add --creation-factor tweak for --range-diff
  format-patch: teach --range-diff to respect -v/--reroll-count
  format-patch: extend --range-diff to accept revision range
  format-patch: add --range-diff option to embed diff in cover letter
  range-diff: relieve callers of low-level configuration burden
  range-diff: publish default creation factor
  range-diff: respect diff_option.file rather than assuming 'stdout'

2 years agoMerge branch 'es/format-patch-interdiff'
Junio C Hamano [Mon, 17 Sep 2018 20:53:55 +0000 (13:53 -0700)]
Merge branch 'es/format-patch-interdiff'

"git format-patch" learned a new "--interdiff" option to explain
the difference between this version and the previous atttempt in
the cover letter (or after the tree-dashes as a comment).

* es/format-patch-interdiff:
  format-patch: allow --interdiff to apply to a lone-patch
  log-tree: show_log: make commentary block delimiting reusable
  interdiff: teach show_interdiff() to indent interdiff
  format-patch: teach --interdiff to respect -v/--reroll-count
  format-patch: add --interdiff option to embed diff in cover letter
  format-patch: allow additional generated content in make_cover_letter()

2 years agoMerge branch 'cc/delta-islands'
Junio C Hamano [Mon, 17 Sep 2018 20:53:55 +0000 (13:53 -0700)]
Merge branch 'cc/delta-islands'

Lift code from GitHub to restrict delta computation so that an
object that exists in one fork is not made into a delta against
another object that does not appear in the same forked repository.

* cc/delta-islands:
  pack-objects: move 'layer' into 'struct packing_data'
  pack-objects: move tree_depth into 'struct packing_data'
  t5320: tests for delta islands
  repack: add delta-islands support
  pack-objects: add delta-islands support
  pack-objects: refactor code into compute_layer_order()
  Add delta-islands.{c,h}

2 years agoMerge branch 'jk/trailer-fixes'
Junio C Hamano [Mon, 17 Sep 2018 20:53:54 +0000 (13:53 -0700)]
Merge branch 'jk/trailer-fixes'

"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 agoMerge branch 'sb/range-diff-colors'
Junio C Hamano [Mon, 17 Sep 2018 20:53:54 +0000 (13:53 -0700)]
Merge branch 'sb/range-diff-colors'

The color output support for recently introduced "range-diff"
command got tweaked a bit.

* sb/range-diff-colors:
  range-diff: indent special lines as context
  range-diff: make use of different output indicators
  diff.c: add --output-indicator-{new, old, context}
  diff.c: rewrite emit_line_0 more understandably
  diff.c: omit check for line prefix in emit_line_0
  diff: use emit_line_0 once per line
  diff.c: add set_sign to emit_line_0
  diff.c: reorder arguments for emit_line_ws_markup
  diff.c: simplify caller of emit_line_0
  t3206: add color test for range-diff --dual-color
  test_decode_color: understand FAINT and ITALIC

2 years agoMerge branch 'jk/pack-delta-reuse-with-bitmap'
Junio C Hamano [Mon, 17 Sep 2018 20:53:53 +0000 (13:53 -0700)]
Merge branch 'jk/pack-delta-reuse-with-bitmap'

When creating a thin pack, which allows objects to be made into a
delta against another object that is not in the resulting pack but
is known to be present on the receiving end, the code learned to
take advantage of the reachability bitmap; this allows the server
to send a delta against a base beyond the "boundary" commit.

* jk/pack-delta-reuse-with-bitmap:
  pack-objects: reuse on-disk deltas for thin "have" objects
  pack-bitmap: save "have" bitmap from walk
  t/perf: add perf tests for fetches from a bitmapped server
  t/perf: add infrastructure for measuring sizes
  t/perf: factor out percent calculations
  t/perf: factor boilerplate out of test_perf

2 years agoMerge branch 'nd/unpack-trees-with-cache-tree'
Junio C Hamano [Mon, 17 Sep 2018 20:53:53 +0000 (13:53 -0700)]
Merge branch 'nd/unpack-trees-with-cache-tree'

The unpack_trees() API used in checking out a branch and merging
walks one or more trees along with the index.  When the cache-tree
in the index tells us that we are walking a tree whose flattened
contents is known (i.e. matches a span in the index), as linearly
scanning a span in the index is much more efficient than having to
open tree objects recursively and listing their entries, the walk
can be optimized, which is done in this topic.

* nd/unpack-trees-with-cache-tree:
  Document update for nd/unpack-trees-with-cache-tree
  cache-tree: verify valid cache-tree in the test suite
  unpack-trees: add missing cache invalidation
  unpack-trees: reuse (still valid) cache-tree from src_index
  unpack-trees: reduce malloc in cache-tree walk
  unpack-trees: optimize walking same trees with cache-tree
  unpack-trees: add performance tracing
  trace.h: support nested performance tracing

2 years agoMerge branch 'ds/reachable'
Junio C Hamano [Mon, 17 Sep 2018 20:53:52 +0000 (13:53 -0700)]
Merge branch 'ds/reachable'

The code for computing history reachability has been shuffled,
obtained a bunch of new tests to cover them, and then being
improved.

* ds/reachable:
  commit-reach: correct accidental #include of C file
  commit-reach: use can_all_from_reach
  commit-reach: make can_all_from_reach... linear
  commit-reach: replace ref_newer logic
  test-reach: test commit_contains
  test-reach: test can_all_from_reach_with_flags
  test-reach: test reduce_heads
  test-reach: test get_merge_bases_many
  test-reach: test is_descendant_of
  test-reach: test in_merge_bases
  test-reach: create new test tool for ref_newer
  commit-reach: move can_all_from_reach_with_flags
  upload-pack: generalize commit date cutoff
  upload-pack: refactor ok_to_give_up()
  upload-pack: make reachable() more generic
  commit-reach: move commit_contains from ref-filter
  commit-reach: move ref_newer from remote.c
  commit.h: remove method declarations
  commit-reach: move walk methods from commit.c

2 years agoMerge branch 'sb/submodule-update-in-c'
Junio C Hamano [Mon, 17 Sep 2018 20:53:51 +0000 (13:53 -0700)]
Merge branch 'sb/submodule-update-in-c'

"git submodule update" is getting rewritten piece-by-piece into C.

* sb/submodule-update-in-c:
  submodule--helper: introduce new update-module-mode helper
  submodule--helper: replace connect-gitdir-workingtree by ensure-core-worktree
  builtin/submodule--helper: factor out method to update a single submodule
  builtin/submodule--helper: store update_clone information in a struct
  builtin/submodule--helper: factor out submodule updating
  git-submodule.sh: rename unused variables
  git-submodule.sh: align error reporting for update mode to use path

2 years agoMerge branch 'tg/rerere'
Junio C Hamano [Mon, 17 Sep 2018 20:53:51 +0000 (13:53 -0700)]
Merge branch 'tg/rerere'

Fixes to "git rerere" corner cases, especially when conflict
markers cannot be parsed in the file.

* tg/rerere:
  rerere: recalculate conflict ID when unresolved conflict is committed
  rerere: teach rerere to handle nested conflicts
  rerere: return strbuf from handle path
  rerere: factor out handle_conflict function
  rerere: only return whether a path has conflicts or not
  rerere: fix crash with files rerere can't handle
  rerere: add documentation for conflict normalization
  rerere: mark strings for translation
  rerere: wrap paths in output in sq
  rerere: lowercase error messages
  rerere: unify error messages when read_cache fails

2 years agoMerge branch 'ds/multi-pack-index'
Junio C Hamano [Mon, 17 Sep 2018 20:53:50 +0000 (13:53 -0700)]
Merge branch 'ds/multi-pack-index'

When there are too many packfiles in a repository (which is not
recommended), looking up an object in these would require
consulting many pack .idx files; a new mechanism to have a single
file that consolidates all of these .idx files is introduced.

* ds/multi-pack-index: (32 commits)
  pack-objects: consider packs in multi-pack-index
  midx: test a few commands that use get_all_packs
  treewide: use get_all_packs
  packfile: add all_packs list
  midx: fix bug that skips midx with alternates
  midx: stop reporting garbage
  midx: mark bad packed objects
  multi-pack-index: store local property
  multi-pack-index: provide more helpful usage info
  midx: clear midx on repack
  packfile: skip loading index if in multi-pack-index
  midx: prevent duplicate packfile loads
  midx: use midx in approximate_object_count
  midx: use existing midx when writing new one
  midx: use midx in abbreviation calculations
  midx: read objects from multi-pack-index
  config: create core.multiPackIndex setting
  midx: write object offsets
  midx: write object id fanout chunk
  midx: write object ids in a chunk
  ...

2 years agoMerge branch 'jk/branch-l-1-repurpose'
Junio C Hamano [Mon, 17 Sep 2018 20:53:50 +0000 (13:53 -0700)]
Merge branch 'jk/branch-l-1-repurpose'

Updated plan to repurpose the "-l" option to "git branch".

* jk/branch-l-1-repurpose:
  doc/git-branch: remove obsolete "-l" references
  branch: make "-l" a synonym for "--list"

2 years agoMerge branch 'tg/conflict-marker-size'
Junio C Hamano [Mon, 17 Sep 2018 20:53:49 +0000 (13:53 -0700)]
Merge branch 'tg/conflict-marker-size'

Developer aid.

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

2 years agoMerge branch 'ts/doc-build-manpage-xsl-quietly'
Junio C Hamano [Mon, 17 Sep 2018 20:53:49 +0000 (13:53 -0700)]
Merge branch 'ts/doc-build-manpage-xsl-quietly'

Build tweak.

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

2 years agoMerge branch 'jk/rev-list-stdin-noop-is-ok'
Junio C Hamano [Mon, 17 Sep 2018 20:53:48 +0000 (13:53 -0700)]
Merge branch 'jk/rev-list-stdin-noop-is-ok'

"git rev-list --stdin </dev/null" used to be an error; it now shows
no output without an error.  "git rev-list --stdin --default HEAD"
still falls back to the given default when nothing is given on the
standard input.

* jk/rev-list-stdin-noop-is-ok:
  rev-list: make empty --stdin not an error

2 years agoMerge branch 'bp/checkout-new-branch-optim'
Junio C Hamano [Mon, 17 Sep 2018 20:53:48 +0000 (13:53 -0700)]
Merge branch 'bp/checkout-new-branch-optim'

"git checkout -b newbranch [HEAD]" should not have to do as much as
checking out a commit different from HEAD.  An attempt is made to
optimize this special case.

* bp/checkout-new-branch-optim:
  checkout: optimize "git checkout -b <new_branch>"

2 years agoMerge branch 'sg/t1404-update-ref-test-timeout'
Junio C Hamano [Mon, 17 Sep 2018 20:53:47 +0000 (13:53 -0700)]
Merge branch 'sg/t1404-update-ref-test-timeout'

An attempt to unflake a test a bit.

* sg/t1404-update-ref-test-timeout:
  t1404: increase core.packedRefsTimeout to avoid occasional test failure

2 years agoMerge branch 'nd/clone-case-smashing-warning'
Junio C Hamano [Mon, 17 Sep 2018 20:53:47 +0000 (13:53 -0700)]
Merge branch 'nd/clone-case-smashing-warning'

Running "git clone" against a project that contain two files with
pathnames that differ only in cases on a case insensitive
filesystem would result in one of the files lost because the
underlying filesystem is incapable of holding both at the same
time.  An attempt is made to detect such a case and warn.

* nd/clone-case-smashing-warning:
  clone: report duplicate entries on case-insensitive filesystems

2 years agoMerge branch 'mk/http-backend-content-length'
Junio C Hamano [Mon, 17 Sep 2018 20:53:46 +0000 (13:53 -0700)]
Merge branch 'mk/http-backend-content-length'

Test update.

* mk/http-backend-content-length:
  http-backend test: make empty CONTENT_LENGTH test more realistic

2 years agoRevert "doc/Makefile: drop doc-diff worktree and temporary files on "make clean""
Junio C Hamano [Mon, 17 Sep 2018 19:46:18 +0000 (12:46 -0700)]
Revert "doc/Makefile: drop doc-diff worktree and temporary files on "make clean""

This reverts commit 6f924265a0bf6efa677e9a684cebdde958e5ba06, which
started to require that we have an executable git available in order
to say "make clean", which gives us a chicken-and-egg problem.

Having to have Git installed, or be in a repository, in order to be
able to run an optional "doc-diff" tool is fine.  Requiring either
in order to run "make clean" is a different story.

Reported by Jonathan Nieder <jrnieder@gmail.com>.

2 years agohttp-backend test: make empty CONTENT_LENGTH test more realistic
Max Kirillov [Tue, 11 Sep 2018 20:33:36 +0000 (23:33 +0300)]
http-backend test: make empty CONTENT_LENGTH test more realistic

This is a test of smart HTTP, so it should use the smart HTTP endpoints
(e.g. /info/refs?service=git-receive-pack), not dumb HTTP (HEAD).

Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
Signed-off-by: Max Kirillov <max@max630.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2 years agoGit 2.19 v2.19.0
Junio C Hamano [Mon, 10 Sep 2018 17:41:56 +0000 (10:41 -0700)]
Git 2.19

Signed-off-by: Junio C Hamano <gitster@pobox.com>
2 years agoMerge tag 'l10n-2.19.0-rnd2' of git://github.com/git-l10n/git-po
Junio C Hamano [Mon, 10 Sep 2018 17:41:11 +0000 (10:41 -0700)]
Merge tag 'l10n-2.19.0-rnd2' of git://github.com/git-l10n/git-po

l10n for Git 2.19.0 round 2

* tag 'l10n-2.19.0-rnd2' of git://github.com/git-l10n/git-po:
  l10n: zh_CN: for git v2.19.0 l10n round 1 to 2
  l10n: bg.po: Updated Bulgarian translation (3958t)
  l10n: vi.po(3958t): updated Vietnamese translation v2.19.0 round 2
  l10n: es.po v2.19.0 round 2
  l10n: fr.po v2.19.0 rnd 2
  l10n: fr.po v2.19.0 rnd 1
  l10n: fr: fix a message seen in git bisect
  l10n: sv.po: Update Swedish translation (3958t0f0u)
  l10n: git.pot: v2.19.0 round 2 (3 new, 5 removed)
  l10n: ru.po: update Russian translation
  l10n: git.pot: v2.19.0 round 1 (382 new, 30 removed)
  l10n: de.po: translate 108 new messages
  l10n: zh_CN: review for git 2.18.0
  l10n: sv.po: Update Swedish translation(3608t0f0u)

2 years agoMerge branch 'jn/submodule-core-worktree-revert'
Junio C Hamano [Mon, 10 Sep 2018 17:38:58 +0000 (10:38 -0700)]
Merge branch 'jn/submodule-core-worktree-revert'

* jn/submodule-core-worktree-revert:
  Revert "Merge branch 'sb/submodule-core-worktree'"

2 years agoMerge branch 'mk/http-backend-content-length'
Junio C Hamano [Mon, 10 Sep 2018 17:29:16 +0000 (10:29 -0700)]
Merge branch 'mk/http-backend-content-length'

The earlier attempt barfed when given a CONTENT_LENGTH that is
set to an empty string.  RFC 3875 is fairly clear that in this
case we should not read any message body, but we've been reading
through to the EOF in previous versions (which did not even pay
attention to the environment variable), so keep that behaviour for
now in this late update.

* mk/http-backend-content-length:
  http-backend: allow empty CONTENT_LENGTH

2 years agol10n: zh_CN: for git v2.19.0 l10n round 1 to 2
Jiang Xin [Tue, 21 Aug 2018 00:40:05 +0000 (08:40 +0800)]
l10n: zh_CN: for git v2.19.0 l10n round 1 to 2

Translate 382 new messages (3958t0f0u) for git 2.19.0.

Signed-off-by: Jiang Xin <worldhello.net@gmail.com>
2 years agoMerge branch 'master' of git://github.com/alshopov/git-po
Jiang Xin [Sun, 9 Sep 2018 11:05:41 +0000 (19:05 +0800)]
Merge branch 'master' of git://github.com/alshopov/git-po

* 'master' of git://github.com/alshopov/git-po:
  l10n: bg.po: Updated Bulgarian translation (3958t)

2 years agol10n: bg.po: Updated Bulgarian translation (3958t)
Alexander Shopov [Thu, 9 Aug 2018 15:04:10 +0000 (17:04 +0200)]
l10n: bg.po: Updated Bulgarian translation (3958t)

Signed-off-by: Alexander Shopov <ash@kambanaria.org>
2 years agoRevert "Merge branch 'sb/submodule-core-worktree'"
Jonathan Nieder [Sat, 8 Sep 2018 00:09:46 +0000 (17:09 -0700)]
Revert "Merge branch 'sb/submodule-core-worktree'"

This reverts commit 7e25437d35a70791b345872af202eabfb3e1a8bc, reversing
changes made to 00624d608cc69bd62801c93e74d1ea7a7ddd6598.

v2.19.0-rc0~165^2~1 (submodule: ensure core.worktree is set after
update, 2018-06-18) assumes an "absorbed" submodule layout, where the
submodule's Git directory is in the superproject's .git/modules/
directory and .git in the submodule worktree is a .git file pointing
there.  In particular, it uses $GIT_DIR/modules/$name to find the
submodule to find out whether it already has core.worktree set, and it
uses connect_work_tree_and_git_dir if not, resulting in

fatal: could not open sub/.git for writing

The context behind that patch: v2.19.0-rc0~165^2~2 (submodule: unset
core.worktree if no working tree is present, 2018-06-12) unsets
core.worktree when running commands like "git checkout
--recurse-submodules" to switch to a branch without the submodule.  If
a user then uses "git checkout --no-recurse-submodules" to switch back
to a branch with the submodule and runs "git submodule update", this
patch is needed to ensure that commands using the submodule directly
are aware of the path to the worktree.

It is late in the release cycle, so revert the whole 3-patch series.
We can try again later for 2.20.

Reported-by: Allan Sandfeld Jensen <allan.jensen@qt.io>
Helped-by: Stefan Beller <sbeller@google.com>
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2 years agohttp-backend: allow empty CONTENT_LENGTH
Max Kirillov [Fri, 7 Sep 2018 03:36:07 +0000 (06:36 +0300)]
http-backend: allow empty CONTENT_LENGTH

According to RFC3875, empty environment variable is equivalent to unset,
and for CONTENT_LENGTH it should mean zero body to read.

However, unset CONTENT_LENGTH is also used for chunked encoding to indicate
reading until EOF. At least, the test "large fetch-pack requests can be split
across POSTs" from t5551 starts faliing, if unset or empty CONTENT_LENGTH is
treated as zero length body. So keep the existing behavior as much as possible.

Add a test for the case.

Reported-By: Jelmer Vernooij <jelmer@jelmer.uk>
Signed-off-by: Max Kirillov <max@max630.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2 years agol10n: vi.po(3958t): updated Vietnamese translation v2.19.0 round 2
Tran Ngoc Quan [Fri, 7 Sep 2018 06:41:08 +0000 (13:41 +0700)]
l10n: vi.po(3958t): updated Vietnamese translation v2.19.0 round 2

Signed-off-by: Tran Ngoc Quan <vnwildman@gmail.com>
2 years agol10n: es.po v2.19.0 round 2
Christopher Diaz Riveros [Thu, 6 Sep 2018 09:27:56 +0000 (04:27 -0500)]
l10n: es.po v2.19.0 round 2

Signed-off-by: Christopher Diaz Riveros <chrisadr@gentoo.org>
2 years agoMerge branch 'fr_2.19.0_rnd1' of git://github.com/jnavila/git
Jiang Xin [Thu, 6 Sep 2018 01:17:55 +0000 (09:17 +0800)]
Merge branch 'fr_2.19.0_rnd1' of git://github.com/jnavila/git

* 'fr_2.19.0_rnd1' of git://github.com/jnavila/git:
  l10n: fr.po v2.19.0 rnd 2
  l10n: fr.po v2.19.0 rnd 1
  l10n: fr: fix a message seen in git bisect

2 years agol10n: fr.po v2.19.0 rnd 2
Jean-Noël Avila [Wed, 5 Sep 2018 20:19:13 +0000 (22:19 +0200)]
l10n: fr.po v2.19.0 rnd 2

Signed-off-by: Jean-Noël Avila <jn.avila@free.fr>
2 years agol10n: fr.po v2.19.0 rnd 1
Jean-Noël Avila [Thu, 23 Aug 2018 20:50:52 +0000 (22:50 +0200)]
l10n: fr.po v2.19.0 rnd 1

Signed-off-by: Jean-Noël Avila <jn.avila@free.fr>
2 years agol10n: fr: fix a message seen in git bisect
Raphaël Hertzog [Wed, 4 Jul 2018 15:43:56 +0000 (17:43 +0200)]
l10n: fr: fix a message seen in git bisect

"cette" can be only be used before a word (like in "cette bouteille" for
"this bottle"), but here "this" refers to the current step and we have
to use "ceci" in French.

Signed-off-by: Raphaël Hertzog <hertzog@debian.org>
2 years agodoc-diff: force worktree add
Jeff King [Thu, 30 Aug 2018 07:54:31 +0000 (03:54 -0400)]
doc-diff: force worktree add

We avoid re-creating our temporary worktree if it's already
there. But we may run into a situation where the worktree
has been deleted, but an entry still exists in
$GIT_DIR/worktrees.

Older versions of git-worktree would annoyingly create a
series of duplicate entries. Recent versions now detect and
prevent this, allowing you to override with "-f". Since we
know that the worktree in question was just our temporary
workspace, it's safe for us to always pass "-f".

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2 years agol10n: sv.po: Update Swedish translation (3958t0f0u)
Peter Krefting [Tue, 4 Sep 2018 21:34:09 +0000 (22:34 +0100)]
l10n: sv.po: Update Swedish translation (3958t0f0u)

Signed-off-by: Peter Krefting <peter@softwolves.pp.se>
2 years agoGit 2.19-rc2 v2.19.0-rc2
Junio C Hamano [Tue, 4 Sep 2018 21:33:27 +0000 (14:33 -0700)]
Git 2.19-rc2

Signed-off-by: Junio C Hamano <gitster@pobox.com>
2 years agoMerge branch 'es/chain-lint-more'
Junio C Hamano [Tue, 4 Sep 2018 21:31:40 +0000 (14:31 -0700)]
Merge branch 'es/chain-lint-more'

The test linter code has learned that the end of here-doc mark
"EOF" can be quoted in a double-quote pair, not just in a
single-quote pair.

* es/chain-lint-more:
  chainlint: match "quoted" here-doc tags

2 years agoMerge branch 'ab/portable-more'
Junio C Hamano [Tue, 4 Sep 2018 21:31:40 +0000 (14:31 -0700)]
Merge branch 'ab/portable-more'

Portability fix.

* ab/portable-more:
  tests: fix non-portable iconv invocation
  tests: fix non-portable "${var:-"str"}" construct
  tests: fix and add lint for non-portable grep --file
  tests: fix version-specific portability issue in Perl JSON
  tests: use shorter labels in chainlint.sed for AIX sed
  tests: fix comment syntax in chainlint.sed for AIX sed
  tests: fix and add lint for non-portable seq
  tests: fix and add lint for non-portable head -c N

2 years agoMerge branch 'es/freebsd-iconv-portability'
Junio C Hamano [Tue, 4 Sep 2018 21:31:39 +0000 (14:31 -0700)]
Merge branch 'es/freebsd-iconv-portability'

Build fix.

* es/freebsd-iconv-portability:
  config.mak.uname: resolve FreeBSD iconv-related compilation warning

2 years agoMerge branch 'ds/commit-graph-lockfile-fix'
Junio C Hamano [Tue, 4 Sep 2018 21:31:39 +0000 (14:31 -0700)]
Merge branch 'ds/commit-graph-lockfile-fix'

"git merge-base" in 2.19-rc1 has performance regression when the
(experimental) commit-graph feature is in use, which has been
mitigated.

* ds/commit-graph-lockfile-fix:
  commit: don't use generation numbers if not needed

2 years agoMerge branch 'en/directory-renames-nothanks'
Junio C Hamano [Tue, 4 Sep 2018 21:31:38 +0000 (14:31 -0700)]
Merge branch 'en/directory-renames-nothanks'

Recent addition of "directory rename" heuristics to the
merge-recursive backend makes the command susceptible to false
positives and false negatives.  In the context of "git am -3",
which does not know about surrounding unmodified paths and thus
cannot inform the merge machinery about the full trees involved,
this risk is particularly severe.  As such, the heuristic is
disabled for "git am -3" to keep the machinery "more stupid but
predictable".

* en/directory-renames-nothanks:
  am: avoid directory rename detection when calling recursive merge machinery
  merge-recursive: add ability to turn off directory rename detection
  t3401: add another directory rename testcase for rebase and am

2 years agoMerge branch 'pw/rebase-i-author-script-fix'
Junio C Hamano [Tue, 4 Sep 2018 21:31:38 +0000 (14:31 -0700)]
Merge branch 'pw/rebase-i-author-script-fix'

Recent "git rebase -i" update started to write bogusly formatted
author-script, with a matching broken reading code.  These are
fixed.

* pw/rebase-i-author-script-fix:
  sequencer: fix quoting in write_author_script
  sequencer: handle errors from read_author_ident()

2 years agoDocumentation/git.txt: clarify that GIT_TRACE=/path appends
SZEDER Gábor [Tue, 4 Sep 2018 00:05:44 +0000 (02:05 +0200)]
Documentation/git.txt: clarify that GIT_TRACE=/path appends

The current wording of the description of GIT_TRACE=/path/to/file
("... will try to write the trace messages into it") might be
misunderstood as "overwriting"; at least I interpreted it that way on
a cursory first read.

State it more explicitly that the trace messages are appended.

Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2 years agobisect.c: make show_list() build again
Nguyễn Thái Ngọc Duy [Sun, 2 Sep 2018 07:42:50 +0000 (09:42 +0200)]
bisect.c: make show_list() build again

This function only compiles when DEBUG_BISECT is 1, which is often not
the case. As a result there are two commits [1] [2] that break it but
the breakages went unnoticed because the code did not compile by
default. Update the function and include the new header file to make this
function build again.

In order to stop this from happening again, the function is now
compiled unconditionally but exits early unless DEBUG_BISECT is
non-zero. A smart compiler generates no extra code (not even a
function call). But even if it does not, this function does not seem
to be in a hot path that the extra cost becomes a big problem.

[1] bb408ac95d (bisect.c: use commit-slab for commit weight instead of
    commit->util - 2018-05-19)

[2] cbd53a2193 (object-store: move object access functions to
    object-store.h - 2018-05-15)

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2 years agopack-bitmap: drop "loaded" flag
Jeff King [Sat, 1 Sep 2018 07:50:57 +0000 (03:50 -0400)]
pack-bitmap: drop "loaded" flag

In the early days of the bitmap code, there was a single
static bitmap_index struct that was used behind the scenes,
and any bitmap-related functions could lazily check
bitmap_git.loaded to see if they needed to read the on-disk
data.

But since 3ae5fa0768 (pack-bitmap: remove bitmap_git global
variable, 2018-06-07), the caller is responsible for the
lifetime of the bitmap_index struct, and we return it from
prepare_bitmap_git() and prepare_bitmap_walk(), both of
which load the on-disk data (or return NULL).

So outside of these functions, it's not possible to have a
bitmap_index for which the loaded flag is not true. Nor is
it possible to accidentally pass an already-loaded
bitmap_index to the loading function (which is static-local
to the file).

We can drop this unnecessary and confusing flag.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2 years agotraverse_bitmap_commit_list(): don't free result
Jeff King [Sat, 1 Sep 2018 07:49:48 +0000 (03:49 -0400)]
traverse_bitmap_commit_list(): don't free result

Since it was introduced in fff42755ef (pack-bitmap: add
support for bitmap indexes, 2013-12-21), this function has
freed the result after traversing it. That is an artifact of
the early days of the bitmap code, when we had a single
static "struct bitmap_index". Back then, it was intended
that you would do:

  prepare_bitmap_walk(&revs);
  traverse_bitmap_commit_list(&revs);

Since the actual bitmap_index struct was totally behind the
scenes, it was convenient for traverse_bitmap_commit_list()
to clean it up, clearing the way for another traversal.

But since 3ae5fa0768 (pack-bitmap: remove bitmap_git global
variable, 2018-06-07), the caller explicitly manages the
bitmap_index struct itself, like this:

  b = prepare_bitmap_walk(&revs);
  traverse_bitmap_commit_list(b, &revs);
  free_bitmap_index(b);

It no longer makes sense to auto-free the result after the
traversal. If you want to do another traversal, you'd just
create a new bitmap_index. And while nobody tries to call
traverse_bitmap_commit_list() twice, the fact that it throws
away the result might be surprising, and is better avoided.

Note that in the "old" way it was possible for two walks to
amortize the cost of opening the on-disk .bitmap file (since
it was stored in the global bitmap_index), but we lost that
in 3ae5fa0768. However, no code actually does this, so it's
not worth addressing now. The solution might involve a new:

  reset_bitmap_walk(b, &revs);

call. Or we might even attach the bitmap data to its
matching packed_git struct, so that multiple
prepare_bitmap_walk() calls could use it. That can wait
until somebody actually has need of the optimization (and
until then, we'll do the correct, unsurprising thing).

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2 years agot5310: test delta reuse with bitmaps
Jeff King [Sat, 1 Sep 2018 07:48:13 +0000 (03:48 -0400)]
t5310: test delta reuse with bitmaps

Commit 6a1e32d532 (pack-objects: reuse on-disk deltas for
thin "have" objects, 2018-08-21) taught pack-objects a new
optimization trick. Since this wasn't meant to change
user-visible behavior, but only produce smaller packs more
quickly, testing focused on t/perf/p5311.

However, since people don't run perf tests very often, we
should make sure that the feature is exercised in the
regular test suite. This patch does so.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2 years agobitmap_has_sha1_in_uninteresting(): drop BUG check
Jeff King [Sat, 1 Sep 2018 07:44:48 +0000 (03:44 -0400)]
bitmap_has_sha1_in_uninteresting(): drop BUG check

Commit 30cdc33fba (pack-bitmap: save "have" bitmap from
walk, 2018-08-21) introduced a new function for looking at
the "have" side of a bitmap walk. Because it only makes
sense to do so after we've finished the walk, we added an
extra safety assertion, making sure that bitmap_git->result
is non-NULL.

However, this safety is misguided. It was trying to catch
the case where we had called prepare_bitmap_walk() to give
us a "struct bitmap_index", but had not yet called
traverse_bitmap_commit_list() to walk it. But all of the
interesting computation (including setting up the result and
"have" bitmaps) happens in the first function! The latter
function only delivers the result to a callback function.

So the case we were worried about is impossible; if you get
a non-NULL result from prepare_bitmap_walk(), then its
"have" field will be fully formed.

But much worse, traverse_bitmap_commit_list() actually frees
the result field as it finishes. Which means that this
assertion is worse than useless: it's almost guaranteed to
trigger!

Our test suite didn't catch this because the function isn't
actually exercised at all. The only caller comes from
6a1e32d532 (pack-objects: reuse on-disk deltas for thin
"have" objects, 2018-08-21), and that's triggered only when
you fetch or push history that contains an object with a
base that is found deep in history. Our test suite fetches
and pushes either don't use bitmaps, or use too-small
example repositories. But any reasonably-sized real-world
push or fetch (with bitmaps) would trigger this.

This patch drops the harmful assertion and tweaks the
docstring for the function to make the precondition clear.
The tests need to be improved to exercise this new
pack-objects feature, but we'll do that in a separate
commit.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2 years agol10n: git.pot: v2.19.0 round 2 (3 new, 5 removed)
Jiang Xin [Tue, 4 Sep 2018 00:51:58 +0000 (08:51 +0800)]
l10n: git.pot: v2.19.0 round 2 (3 new, 5 removed)

Generate po/git.pot from v2.19.0-rc1 for git v2.19.0 l10n round 2.

Signed-off-by: Jiang Xin <worldhello.net@gmail.com>
2 years agoMerge branch 'master' of git://github.com/git-l10n/git-po
Jiang Xin [Tue, 4 Sep 2018 00:49:54 +0000 (08:49 +0800)]
Merge branch 'master' of git://github.com/git-l10n/git-po

* 'master' of git://github.com/git-l10n/git-po:
  l10n: ru.po: update Russian translation
  l10n: git.pot: v2.19.0 round 1 (382 new, 30 removed)
  l10n: de.po: translate 108 new messages
  l10n: zh_CN: review for git 2.18.0
  l10n: sv.po: Update Swedish translation(3608t0f0u)

2 years agofetch: stop clobbering existing tags without --force
Ævar Arnfjörð Bjarmason [Fri, 31 Aug 2018 20:10:04 +0000 (20:10 +0000)]
fetch: stop clobbering existing tags without --force

Change "fetch" to treat "+" in refspecs (aka --force) to mean we
should clobber a local tag of the same name.

This changes the long-standing behavior of "fetch" added in
853a3697dc ("[PATCH] Multi-head fetch.", 2005-08-20). Before this
change, all tag fetches effectively had --force enabled. See the
git-fetch-script code in fast_forward_local() with the comment:

    > Tags need not be pointing at commits so there is no way to
    > guarantee "fast-forward" anyway.

That commit and the rest of the history of "fetch" shows that the
"+" (--force) part of refpecs was only conceived for branch updates,
while tags have accepted any changes from upstream unconditionally and
clobbered the local tag object. Changing this behavior has been
discussed as early as 2011[1].

The current behavior doesn't make sense to me, it easily results in
local tags accidentally being clobbered. We could namespace our tags
per-remote and not locally populate refs/tags/*, but as with my
97716d217c ("fetch: add a --prune-tags option and fetch.pruneTags
config", 2018-02-09) it's easier to work around the current
implementation than to fix the root cause.

So this change implements suggestion #1 from Jeff's 2011 E-Mail[1],
"fetch" now only clobbers the tag if either "+" is provided as part of
the refspec, or if "--force" is provided on the command-line.

This also makes it nicely symmetrical with how "tag" itself works when
creating tags. I.e. we refuse to clobber any existing tags unless
"--force" is supplied. Now we can refuse all such clobbering, whether
it would happen by clobbering a local tag with "tag", or by fetching
it from the remote with "fetch".

Ref updates outside refs/{tags,heads/* are still still not symmetrical
with how "git push" works, as discussed in the recently changed
pull-fetch-param.txt documentation. This change brings the two
divergent behaviors more into line with one another. I don't think
there's any reason "fetch" couldn't fully converge with the behavior
used by "push", but that's a topic for another change.

One of the tests added in 31b808a032 ("clone --single: limit the fetch
refspec to fetched branch", 2012-09-20) is being changed to use
--force where a clone would clobber a tag. This changes nothing about
the existing behavior of the test.

1. https://public-inbox.org/git/20111123221658.GA22313@sigill.intra.peff.net/

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2 years agofetch: document local ref updates with/without --force
Ævar Arnfjörð Bjarmason [Fri, 31 Aug 2018 20:10:03 +0000 (20:10 +0000)]
fetch: document local ref updates with/without --force

Refer to the new git-push(1) documentation about when ref updates are
and aren't allowed with and without --force, noting how "git-fetch"
differs from the behavior of "git-push".

Perhaps it would be better to split this all out into a new
gitrefspecs(7) man page, or present this information using tables.

In lieu of that, this is accurate, and fixes a big omission in the
existing refspec docs.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2 years agopush doc: correct lies about how push refspecs work
Ævar Arnfjörð Bjarmason [Fri, 31 Aug 2018 20:10:02 +0000 (20:10 +0000)]
push doc: correct lies about how push refspecs work

There's complex rules governing whether a push is allowed to take
place depending on whether we're pushing to refs/heads/*, refs/tags/*
or refs/not-that/*. See is_branch() in refs.c, and the various
assertions in refs/files-backend.c. (e.g. "trying to write non-commit
object %s to branch '%s'").

This documentation has never been quite correct, but went downhill
after dbfeddb12e ("push: require force for refs under refs/tags/",
2012-11-29) when we started claiming that <dst> couldn't be a tag
object, which is incorrect. After some of the logic in that patch was
changed in 256b9d70a4 ("push: fix "refs/tags/ hierarchy cannot be
updated without --force"", 2013-01-16) the docs weren't updated, and
we've had some version of documentation that confused whether <src>
was a tag or not with whether <dst> would accept either an annotated
tag object or the commit it points to.

This makes the intro somewhat more verbose & complex, perhaps we
should have a shorter description here and split the full complexity
into a dedicated section. Very few users will find themselves needing
to e.g. push blobs or trees to refs/custom-namespace/* (or blobs or
trees at all), and that could be covered separately as an advanced
topic.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2 years agopush doc: move mention of "tag <tag>" later in the prose
Ævar Arnfjörð Bjarmason [Fri, 31 Aug 2018 20:10:01 +0000 (20:10 +0000)]
push doc: move mention of "tag <tag>" later in the prose

This change will be followed-up with a subsequent change where I'll
change both sides of this mention of "tag <tag>" to be something
that's best read without interruption.

To make that change smaller, let's move this mention of "tag <tag>" to
the end of the "<refspec>..." section, it's now somewhere in the
middle.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2 years agopush doc: remove confusing mention of remote merger
Ævar Arnfjörð Bjarmason [Fri, 31 Aug 2018 20:10:00 +0000 (20:10 +0000)]
push doc: remove confusing mention of remote merger

Saying that "git push <remote> <src>:<dst>" won't push a merger of
<src> and <dst> to <dst> is clear from the rest of the context here,
so mentioning it is redundant, furthermore the mention of "EXAMPLES
below" isn't specific or useful.

This phrase was originally added in 149f6ddfb3 ("Docs: Expand
explanation of the use of + in git push refspecs.", 2009-02-19), as
can be seen in that change the point of the example being cited was to
show that force pushing can leave unreferenced commits on the
remote. It's enough that we explain that in its own section, it
doesn't need to be mentioned here.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2 years agofetch tests: add a test for clobbering tag behavior
Ævar Arnfjörð Bjarmason [Fri, 31 Aug 2018 20:09:59 +0000 (20:09 +0000)]
fetch tests: add a test for clobbering tag behavior

The test suite only incidentally (and unintentionally) tested for the
current behavior of eager tag clobbering on "fetch". This is a
followup to 380efb65df ("push tests: assert re-pushing annotated
tags", 2018-07-31) which tests for it explicitly.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2 years agopush tests: use spaces in interpolated string
Ævar Arnfjörð Bjarmason [Fri, 31 Aug 2018 20:09:58 +0000 (20:09 +0000)]
push tests: use spaces in interpolated string

The quoted -m'msg' option would mean the same as -mmsg when passed
through the test_force_push_tag helper. Let's instead use a string
with spaces in it, to have a working example in case we need to pass
other whitespace-delimited arguments to git-tag.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2 years agopush tests: make use of unused $1 in test description
Ævar Arnfjörð Bjarmason [Fri, 31 Aug 2018 20:09:57 +0000 (20:09 +0000)]
push tests: make use of unused $1 in test description

Fix up a logic error in 380efb65df ("push tests: assert re-pushing
annotated tags", 2018-07-31), where the $tag_type_description variable
was assigned to but never used, unlike in the subsequently added
companion test for fetches in 2d216a7ef6 ("fetch tests: add a test for
clobbering tag behavior", 2018-04-29).

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2 years agofetch: change "branch" to "reference" in --force -h output
Ævar Arnfjörð Bjarmason [Fri, 31 Aug 2018 20:09:56 +0000 (20:09 +0000)]
fetch: change "branch" to "reference" in --force -h output

The -h output has been referring to the --force command as forcing the
overwriting of local branches, but since "fetch" more generally
fetches all sorts of references in all refs/ namespaces, let's talk
about forcing the update of a a "reference" instead.

This wording was initially introduced in 8320199873 ("Rewrite
builtin-fetch option parsing to use parse_options().", 2007-12-04).

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2 years agoconfig.mak.uname: resolve FreeBSD iconv-related compilation warning
Eric Sunshine [Fri, 31 Aug 2018 08:33:42 +0000 (04:33 -0400)]
config.mak.uname: resolve FreeBSD iconv-related compilation warning

OLD_ICONV has long been needed by FreeBSD so config.mak.uname defines
it unconditionally. However, recent versions do not need it, and its
presence results in compilation warnings. Resolve this issue by defining
OLD_ICONV only for older FreeBSD versions.

Specifically, revision r281550[1], which is part of FreeBSD 11, removed
the need for OLD_ICONV, and r282275[2] back-ported that change to 10.2.
Versions prior to 10.2 do need it.

[1] https://github.com/freebsd/freebsd/commit/b0813ee288f64f677a2cebf7815754b027a8215b
[2] https://github.com/freebsd/freebsd/commit/b709ec868adb5170d09bc5a66b18d0e0d5987ab6

[es: commit message; tweak version check to distinguish 10.x versions]

Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
Signed-off-by: Eric Sunshine <sunshine@sunshineco.com>
Reviewed-by: Jonathan Nieder <jrnieder@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2 years agodoc/Makefile: drop doc-diff worktree and temporary files on "make clean"
Eric Sunshine [Fri, 31 Aug 2018 06:33:18 +0000 (02:33 -0400)]
doc/Makefile: drop doc-diff worktree and temporary files on "make clean"

doc-diff creates a temporary working tree (git-worktree) and generates a
bunch of temporary files which it does not remove since they act as a
cache to speed up subsequent runs. Although doc-diff's working tree and
generated files are not strictly build products of the Makefile (which,
itself, never runs doc-diff), as a convenience, update "make clean" to
clean up doc-diff's working tree and generated files along with other
development detritus normally removed by "make clean".

Signed-off-by: Eric Sunshine <sunshine@sunshineco.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2 years agodoc-diff: add --clean mode to remove temporary working gunk
Eric Sunshine [Fri, 31 Aug 2018 06:33:17 +0000 (02:33 -0400)]
doc-diff: add --clean mode to remove temporary working gunk

As part of its operation, doc-diff creates a bunch of temporary
working files and holds onto them in order to speed up subsequent
invocations. These files are never deleted. Moreover, it creates a
temporary working tree (via git-wortkree) which likewise never gets
removed.

Without knowing the implementation details of the tool, a user may not
know how to clean up manually afterward. Worse, the user may find it
surprising and alarming to discover a working tree which s/he did not
create explicitly.

To address these issues, add a --clean mode which removes the
temporary working tree and deletes all generated files.

Signed-off-by: Eric Sunshine <sunshine@sunshineco.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2 years agodoc-diff: fix non-portable 'man' invocation
Eric Sunshine [Fri, 31 Aug 2018 06:33:16 +0000 (02:33 -0400)]
doc-diff: fix non-portable 'man' invocation

doc-diff invokes 'man' with the -l option to force "local" mode,
however, neither MacOS nor FreeBSD recognize this option. On those
platforms, if the argument to 'man' contains a slash, it is
automatically interpreted as a file specification, so a "local"-like
mode is not needed. And, it turns out, 'man' which does support -l
falls back to enabling -l automatically if it can't otherwise find a
manual entry corresponding to the argument. Since doc-diff always
passes an absolute path of the nroff source file to 'man', the -l
option kicks in anyhow, despite not being specified explicitly.
Therefore, make the invocation portable to the various platforms by
simply dropping -l.

Signed-off-by: Eric Sunshine <sunshine@sunshineco.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2 years agodoc/git-branch: remove obsolete "-l" references
Jeff King [Thu, 30 Aug 2018 20:04:53 +0000 (16:04 -0400)]
doc/git-branch: remove obsolete "-l" references

The previous commit switched "-l" to meaning "--list", but a
few vestiges of its prior meaning as "--create-reflog"
remained:

  - the synopsis mentioned "-l" when creating a new branch;
    we can drop this entirely, as it has been the default
    for years

  - the --list command mentions the unfortunate "-l"
    confusion, but we've now fixed that

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2 years agot5303: use printf to generate delta bases
Jeff King [Thu, 30 Aug 2018 19:13:39 +0000 (15:13 -0400)]
t5303: use printf to generate delta bases

The exact byte count of the delta base file is important.
The test-delta helper will feed it to patch_delta(), which
will barf if it doesn't match the size byte given in the
delta. Using "echo" may end up with unexpected line endings
on some platforms (e.g,. "\r\n" instead of just "\n").

This actually wouldn't cause the test to fail (since we
already expect test-delta to complain about these bogus
deltas), but would mean that we're not exercising the code
we think we are.

Let's use printf instead (which we already trust to give us
byte-perfect output when we generate the deltas).

While we're here, let's tighten the 5-byte result size used
in the "truncated copy parameters" test. This just needs to
have enough room to attempt to parse the bogus copy command,
meaning 2 is sufficient. Using 5 was arbitrary and just
copied from the base size; since those no longer match, it's
simply confusing. Let's use a more meaningful number.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2 years agocommit: don't use generation numbers if not needed
Derrick Stolee [Thu, 30 Aug 2018 12:58:09 +0000 (05:58 -0700)]
commit: don't use generation numbers if not needed

In 3afc679b "commit: use generations in paint_down_to_common()",
the queue in paint_down_to_common() was changed to use a priority
order based on generation number before commit date. This served
two purposes:

 1. When generation numbers are present, the walk guarantees
    correct topological relationships, regardless of clock skew in
    commit dates.

 2. It enables short-circuiting the walk when the min_generation
    parameter is added in d7c1ec3e "commit: add short-circuit to
    paint_down_to_common()". This short-circuit helps commands
    like 'git branch --contains' from needing to walk to a merge
    base when we know the result is false.

The commit message for 3afc679b includes the following sentence:

    This change does not affect the number of commits that are
    walked during the execution of paint_down_to_common(), only
    the order that those commits are inspected.

This statement is incorrect. Because it changes the order in which
the commits are inspected, it changes the order they are added to
the queue, and hence can change the number of loops before the
queue_has_nonstale() method returns true.

This change makes a concrete difference depending on the topology
of the commit graph. For instance, computing the merge-base between
consecutive versions of the Linux kernel has no effect for versions
after v4.9, but 'git merge-base v4.8 v4.9' presents a performance
regression:

    v2.18.0: 0.122s
v2.19.0-rc1: 0.547s
       HEAD: 0.127s

To determine that this was simply an ordering issue, I inserted
a counter within the while loop of paint_down_to_common() and
found that the loop runs 167,468 times in v2.18.0 and 635,579
times in v2.19.0-rc1.

The topology of this case can be described in a simplified way
here:

  v4.9
   |  \
   |   \
  v4.8  \
   | \   \
   |  \   |
  ...  A  B
   |  /  /
   | /  /
   |/__/
   C

Here, the "..." means "a very long line of commits". By generation
number, A and B have generation one more than C. However, A and B
have commit date higher than most of the commits reachable from
v4.8. When the walk reaches v4.8, we realize that it has PARENT1
and PARENT2 flags, so everything it can reach is marked as STALE,
including A. B has only the PARENT1 flag, so is not STALE.

When paint_down_to_common() is run using
compare_commits_by_commit_date, A and B are removed from the queue
early and C is inserted into the queue. At this point, C and the
rest of the queue entries are marked as STALE. The loop then
terminates.

When paint_down_to_common() is run using
compare_commits_by_gen_then_commit_date, B is removed from the
queue only after the many commits reachable from v4.8 are explored.
This causes the loop to run longer. The reason for this regression
is simple: the queue order is intended to not explore a commit
until everything that _could_ reach that commit is explored. From
the information gathered by the original ordering, we have no
guarantee that there is not a commit D reachable from v4.8 that
can also reach B. We gained absolute correctness in exchange for
a performance regression.

The performance regression is probably the worse option, since
these incorrect results in paint_down_to_common() are rare. The
topology required for the performance regression are less rare,
but still require multiple merge commits where the parents differ
greatly in generation number. In our example above, the commit A
is as important as the commit B to demonstrate the problem, since
otherwise the commit C will sit in the queue as non-stale just as
long in both orders.

The solution provided uses the min_generation parameter to decide
if we should use generation numbers in our ordering. When
min_generation is equal to zero, it means that the caller has no
known cutoff for the walk, so we should rely on our commit-date
heuristic as before; this is the case with merge_bases_many().
When min_generation is non-zero, then the caller knows a valuable
cutoff for the short-circuit mechanism; this is the case with
remove_redundant() and in_merge_bases_many().

Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2 years agopatch-delta: handle truncated copy parameters
Jeff King [Thu, 30 Aug 2018 07:12:52 +0000 (03:12 -0400)]
patch-delta: handle truncated copy parameters

When we see a delta command instructing us to copy bytes
from the base, we have to read the offset and size from the
delta stream. We do this without checking whether we're at
the end of the stream, meaning we may read past the end of
the buffer.

In practice this isn't exploitable in any interesting way
because:

  1. Deltas are always in packfiles, so we have at least a
     20-byte trailer that we'll end up reading.

  2. The worst case is that we try to perform a nonsense
     copy from the base object into the result, based on
     whatever was in the pack stream next. In most cases
     this will simply fail due to our bounds-checks against
     the base or the result.

     But even if you carefully constructed a pack stream for
     which it succeeds, it wouldn't perform any delta
     operation that you couldn't have simply included in a
     non-broken form.

But obviously it's poor form to read past the end of the
buffer we've been given. Unfortunately there's no easy way
to do a single length check, since the number of bytes we
need depends on the number of bits set in the initial
command byte. So we'll just check each byte as we parse. We
can hide the complexity in a macro; it's ugly, but not as
ugly as writing out each individual conditional.

Signed-off-by: Jeff King <peff@peff.net>
Reviewed-by: Nicolas Pitre <nico@fluxnic.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2 years agopatch-delta: consistently report corruption
Jann Horn [Thu, 30 Aug 2018 07:10:26 +0000 (03:10 -0400)]
patch-delta: consistently report corruption

When applying a delta, if we see an opcode that cannot be
fulfilled (e.g., asking to write more bytes than the
destination has left), we break out of our parsing loop but
don't signal an explicit error. We rely on the sanity check
after the loop to see if we have leftover delta bytes or
didn't fill our result buffer.

This can silently ignore corruption when the delta buffer
ends with a bogus command and the destination buffer is
already full. Instead, let's jump into the error handler
directly when we see this case.

Note that the tests also cover the "bad opcode" case, which
already handles this correctly.

Signed-off-by: Jann Horn <jannh@google.com>
Signed-off-by: Jeff King <peff@peff.net>
Reviewed-by: Nicolas Pitre <nico@fluxnic.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2 years agopatch-delta: fix oob read
Jann Horn [Thu, 30 Aug 2018 07:09:45 +0000 (03:09 -0400)]
patch-delta: fix oob read

If `cmd` is in the range [0x01,0x7f] and `cmd > top-data`, the
`memcpy(out, data, cmd)` can copy out-of-bounds data from after `delta_buf`
into `dst_buf`.

This is not an exploitable bug because triggering the bug increments the
`data` pointer beyond `top`, causing the `data != top` sanity check after
the loop to trigger and discard the destination buffer - which means that
the result of the out-of-bounds read is never used for anything.

Signed-off-by: Jann Horn <jannh@google.com>
Signed-off-by: Jeff King <peff@peff.net>
Reviewed-by: Nicolas Pitre <nico@fluxnic.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2 years agot5303: test some corrupt deltas
Jeff King [Thu, 30 Aug 2018 07:09:32 +0000 (03:09 -0400)]
t5303: test some corrupt deltas

We don't have any tests that specifically check boundary
cases in patch_delta(). It obviously gets exercised by tests
which read from packfiles, but it's hard to create packfiles
with bogus deltas.

So let's cover some obvious boundary cases:

  1. commands that overflow the result buffer

     a. literal content from the delta

     b. copies from a base

  2. commands where the source isn't large enough

     a. literal content from a truncated delta

     b. copies that need more bytes than the base has

  3. copy commands who parameters are truncated

And indeed, we have problems with both 2a and 3. I've marked
these both as expect_failure, though note that because they
involve reading past the end of a buffer, they will
typically only be caught when run under valgrind or ASan.

There's one more test here, too, which just applies a basic
delta. Since all of the other tests expect failure and we
don't otherwise use "test-tool delta" in the test suite,
this gives a sanity check that the tool works at all.

These are based on an earlier patch by Jann Horn
<jannh@google.com>.

Signed-off-by: Jeff King <peff@peff.net>
Reviewed-by: Nicolas Pitre <nico@fluxnic.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2 years agotest-delta: read input into a heap buffer
Jeff King [Thu, 30 Aug 2018 07:07:52 +0000 (03:07 -0400)]
test-delta: read input into a heap buffer

We currently read the input to test-delta by mmap()-ing it.
However, memory-checking tools like valgrind and ASan are
less able to detect reads/writes past the end of an mmap'd
buffer, because the OS is likely to give us extra bytes to
pad out the final page size. So instead, let's read into a
heap buffer.

As a bonus, this also makes it possible to write tests with
empty bases, as mmap() will complain about a zero-length
map.

This is based on a patch by Jann Horn <jannh@google.com>
which actually aligned the data at the end of a page, and
followed it with another page marked with mprotect(). That
would detect problems even without a tool like ASan, but it
was significantly more complex and may have introduced
portability problems. By comparison, this approach pushes
the complexity onto existing memory-checking tools.

Note that this could be done even more simply by using
strbuf_read_file(), but that would defeat the purpose:
strbufs generally overallocate (and at the very least
include a trailing NUL which we do not care about), which
would defeat most memory checkers.

Signed-off-by: Jeff King <peff@peff.net>
Reviewed-by: Nicolas Pitre <nico@fluxnic.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2 years agodoc-diff: always use oids inside worktree
Jeff King [Thu, 30 Aug 2018 08:12:03 +0000 (04:12 -0400)]
doc-diff: always use oids inside worktree

The doc-diff script immediately resolves its two endpoints
to actual object ids, so that we can reuse cached results
even if they appear under a different name. But we still use
the original name the user fed us when running "git
checkout" in our temporary worktree. This can lead to
confusing results:

  - the namespace inside the worktree is different than the
    one outside. In particular, "./doc-diff origin HEAD"
    will resolve HEAD inside the worktree, whose detached
    HEAD will be pointing at origin! As a result, such a
    diff would always be empty.

  - worse, we will store this result under the oid we got by
    resolving HEAD in the main worktree, thus polluting our
    cache

  - we didn't pass --detach, which meant that using a branch
    name would cause us to actually check out that branch,
    making it unavailable to other worktrees.

We can solve this by feeding the already-resolved object id
to git-checkout. That naturally forces a detached HEAD, but
just to make clear our expectation, let's explicitly pass
--detach.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2 years agoworktree: delete .git/worktrees if empty after 'remove'
Eric Sunshine [Tue, 28 Aug 2018 21:20:26 +0000 (17:20 -0400)]
worktree: delete .git/worktrees if empty after 'remove'

For cleanliness, "git worktree prune" deletes the .git/worktrees
directory if it is empty after pruning is complete.

For consistency, make "git worktree remove <path>" likewise delete
.git/worktrees if it is empty after the removal.

Signed-off-by: Eric Sunshine <sunshine@sunshineco.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2 years agoworktree: teach 'remove' to override lock when --force given twice
Eric Sunshine [Tue, 28 Aug 2018 21:20:25 +0000 (17:20 -0400)]
worktree: teach 'remove' to override lock when --force given twice

For consistency with "add -f -f" and "move -f -f" which override
the lock on a worktree, allow "remove -f -f" to do so, as well, as a
convenience.

Signed-off-by: Eric Sunshine <sunshine@sunshineco.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2 years agoworktree: teach 'move' to override lock when --force given twice
Eric Sunshine [Tue, 28 Aug 2018 21:20:24 +0000 (17:20 -0400)]
worktree: teach 'move' to override lock when --force given twice

For consistency with "add -f -f", which allows a missing but locked
worktree path to be re-used, allow "move -f -f" to override a lock,
as well, as a convenience.

Signed-off-by: Eric Sunshine <sunshine@sunshineco.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2 years agoworktree: teach 'add' to respect --force for registered but missing path
Eric Sunshine [Tue, 28 Aug 2018 21:20:23 +0000 (17:20 -0400)]
worktree: teach 'add' to respect --force for registered but missing path

For safety, "git worktree add <path>" will refuse to add a new
worktree at <path> if <path> is already associated with a worktree
entry, even if <path> is missing (for instance, has been deleted or
resides on non-mounted removable media or network share). The typical
way to re-create a worktree at <path> in such a situation is either to
prune all "broken" entries ("git worktree prune") or to selectively
remove the worktree entry manually ("git worktree remove <path>").

However, neither of these approaches ("prune" nor "remove") is
especially convenient, and they may be unsuitable for scripting when a
tool merely wants to re-use a worktree if it exists or create it from
scratch if it doesn't (much as a tool might use "mkdir -p" to re-use
or create a directory).

Therefore, teach 'add' to respect --force as a convenient way to
re-use a path already associated with a worktree entry if the path is
non-existent. For a locked worktree, require --force to be specified
twice.

Signed-off-by: Eric Sunshine <sunshine@sunshineco.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2 years agoworktree: disallow adding same path multiple times
Eric Sunshine [Tue, 28 Aug 2018 21:20:22 +0000 (17:20 -0400)]
worktree: disallow adding same path multiple times

A given path should only ever be associated with a single registered
worktree. This invariant is enforced by refusing to create a new
worktree at a given path if that path already exists. For example:

    $ git worktree add -q --detach foo
    $ git worktree add -q --detach foo
    fatal: 'foo' already exists

However, the check can be fooled, and the invariant broken, if the
path is missing. Continuing the example:

    $ rm -fr foo
    $ git worktree add -q --detach foo
    $ git worktree list
    ...      eadebfe [master]
    .../foo  eadebfe (detached HEAD)
    .../foo  eadebfe (detached HEAD)

This "corruption" leads to the unfortunate situation in which the
worktree can not be removed:

    $ git worktree remove foo
    fatal: validation failed, cannot remove working tree: '.../foo'
    does not point back to '.git/worktrees/foo'

Nor can the bogus entry be pruned:

    $ git worktree prune -v
    $ git worktree list
    ...      eadebfe [master]
    .../foo  eadebfe (detached HEAD)
    .../foo  eadebfe (detached HEAD)

without first deleting the worktree directory manually:

    $ rm -fr foo
    $ git worktree prune -v
    Removing .../foo: gitdir file points to non-existent location
    Removing .../foo1: gitdir file points to non-existent location
    $ git worktree list
    ...  eadebfe [master]

or by manually deleting the worktree entry in .git/worktrees.

To address this problem, upgrade "git worktree add" validation to
allow worktree creation only if the given path is not already
associated with an existing worktree (even if the path itself is
non-existent), thus preventing such bogus worktree entries from being
created in the first place.

Reported-by: Jeff King <peff@peff.net>
Signed-off-by: Eric Sunshine <sunshine@sunshineco.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2 years agoworktree: prepare for more checks of whether path can become worktree
Eric Sunshine [Tue, 28 Aug 2018 21:20:21 +0000 (17:20 -0400)]
worktree: prepare for more checks of whether path can become worktree

Certain conditions must be met for a path to be a valid candidate as the
location of a new worktree; for instance, the path must not exist or
must be an empty directory. Although the number of conditions is small,
new conditions will soon be added so factor out the existing checks into
a separate function to avoid further bloating add_worktree().

Signed-off-by: Eric Sunshine <sunshine@sunshineco.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2 years agoworktree: generalize delete_git_dir() to reduce code duplication
Eric Sunshine [Tue, 28 Aug 2018 21:20:20 +0000 (17:20 -0400)]
worktree: generalize delete_git_dir() to reduce code duplication

prune_worktrees() and delete_git_dir() both remove worktree
administrative entries from .git/worktrees, and their implementations
are nearly identical. The only difference is that prune_worktrees() is
also capable of removing a bogus non-worktree-related file from
.git/worktrees.

Simplify by extending delete_git_dir() to handle the little bit of
extra functionality needed by prune_worktrees(), and drop the
effectively duplicate code from the latter.

Signed-off-by: Eric Sunshine <sunshine@sunshineco.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>