git
3 years agopull: get rid of unnecessary global variable
Junio C Hamano [Mon, 14 Dec 2020 17:05:41 +0000 (09:05 -0800)] 
pull: get rid of unnecessary global variable

It is easy enough to do, and gives a more descriptive name to the
variable that is scoped in a more focused way.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
3 years agostyle: do not "break" in switch() after "return"
Ævar Arnfjörð Bjarmason [Tue, 15 Dec 2020 23:50:27 +0000 (00:50 +0100)] 
style: do not "break" in switch() after "return"

Remove this unreachable code. It was found by SunCC, it's found by a
non-fatal warning emitted by SunCC. It's one of the things it's more
vehement about than GCC & Clang.

It complains about a lot of other similarly unreachable code, e.g. a
BUG(...) without a "return", and a "return 0" after a long if/else,
both of whom have "return" statements. Those are also genuine
redundancies to a compiler, but arguably make the code a bit easier to
read & less fragile to maintain.

These return/break cases are just unnecessary however, and as seen
here the surrounding code just did a plain "return" without a "break"
already.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
3 years agocompat-util: pretend that stub setitimer() always succeeds
Junio C Hamano [Tue, 15 Dec 2020 21:26:17 +0000 (13:26 -0800)] 
compat-util: pretend that stub setitimer() always succeeds

When 15b52a44 (compat-util: type-check parameters of no-op
replacement functions, 2020-08-06) turned a handful of no-op
C-preprocessor macros into static inline functions to give the
callers a better type checking for their parameters, it forgot
to return anything from the stubbed out setitimer() function,
even though the function was defined to return an int just like the
real thing.

Since the original C-preprocessor macro implementation was to just
turn the call to the function an empty statement, we know that the
existing callers do not check the return value from it, and it does
not matter what value we return.  But it is safer to pretend that
the call succeeded by returning 0 than making it fail by returning -1
and clobbering errno with some value.

Reported-by: Randall S. Becker <rsbecker@nexbridge.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
3 years agostrmap: make callers of strmap_remove() to call it in void context
Junio C Hamano [Tue, 15 Dec 2020 21:25:36 +0000 (13:25 -0800)] 
strmap: make callers of strmap_remove() to call it in void context

Two "static inline" functions, both of which return void, call
strmap_remove() and tries to return the value it returns as their
return value, which is just bogus, as strmap_remove() returns void
itself.  Call it in the void context and fall-thru the control to
the end instead.

Reported-by: Randall S. Becker <rsbecker@nexbridge.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
3 years agol10n: sv.po: Update Swedish translation (5038t0f0u)
Peter Krefting [Tue, 15 Dec 2020 20:42:13 +0000 (21:42 +0100)] 
l10n: sv.po: Update Swedish translation (5038t0f0u)

Signed-off-by: Peter Krefting <peter@softwolves.pp.se>
3 years agol10n: git.pot: v2.30.0 round 1 (70 new, 45 removed)
Jiang Xin [Tue, 15 Dec 2020 08:27:56 +0000 (16:27 +0800)] 
l10n: git.pot: v2.30.0 round 1 (70 new, 45 removed)

Generate po/git.pot from v2.30.0-rc0 for git v2.30.0 l10n round 1.

Signed-off-by: Jiang Xin <worldhello.net@gmail.com>
3 years agodoc: mention Python 3.x supports
Đoàn Trần Công Danh [Thu, 10 Dec 2020 14:30:17 +0000 (21:30 +0700)] 
doc: mention Python 3.x supports

Commit 0b4396f068, (git-p4: make python2.7 the oldest supported version,
2019-12-13) pointed out that git-p4 uses Python 2.7-or-later features
in the code.

In addition, git-p4 gained enough support for Python 3 from
6cec21a82f, (git-p4: encode/decode communication with p4 for
python3, 2019-12-13).

Let's update our documentation to reflect that fact.

Signed-off-by: Đoàn Trần Công Danh <congdanhqx@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
3 years agoGit 2.30-rc0 v2.30.0-rc0
Junio C Hamano [Mon, 14 Dec 2020 18:30:05 +0000 (10:30 -0800)] 
Git 2.30-rc0

Signed-off-by: Junio C Hamano <gitster@pobox.com>
3 years agoMerge branch 'js/t5526-with-no-particular-primary-branch-name'
Junio C Hamano [Mon, 14 Dec 2020 18:21:38 +0000 (10:21 -0800)] 
Merge branch 'js/t5526-with-no-particular-primary-branch-name'

Test update.

* js/t5526-with-no-particular-primary-branch-name:
  t5526: drop the prereq expecting the default branch name `main`
  t5526: avoid depending on a specific default branch name

3 years agoMerge branch 'js/cmake-extra-built-ins-fix'
Junio C Hamano [Mon, 14 Dec 2020 18:21:38 +0000 (10:21 -0800)] 
Merge branch 'js/cmake-extra-built-ins-fix'

VSbuild fix.

* js/cmake-extra-built-ins-fix:
  cmake: determine list of extra built-ins dynamically

3 years agoMerge branch 'da/vs-build-iconv-fix'
Junio C Hamano [Mon, 14 Dec 2020 18:21:38 +0000 (10:21 -0800)] 
Merge branch 'da/vs-build-iconv-fix'

Build update.

* da/vs-build-iconv-fix:
  ci(vs-build): stop passing the iconv library location explicitly

3 years agoMerge branch 'jk/multi-line-indent-style-fix'
Junio C Hamano [Mon, 14 Dec 2020 18:21:38 +0000 (10:21 -0800)] 
Merge branch 'jk/multi-line-indent-style-fix'

Style fix.

* jk/multi-line-indent-style-fix:
  style: indent multiline "if" conditions to align

3 years agoMerge branch 'jk/check-config-parsing-error-in-upload-pack'
Junio C Hamano [Mon, 14 Dec 2020 18:21:37 +0000 (10:21 -0800)] 
Merge branch 'jk/check-config-parsing-error-in-upload-pack'

Tighten error checking in the codepath that responds to "git fetch".

* jk/check-config-parsing-error-in-upload-pack:
  upload-pack: propagate return value from object filter config callback

3 years agoMerge branch 'ae/doc-reproducible-html'
Junio C Hamano [Mon, 14 Dec 2020 18:21:37 +0000 (10:21 -0800)] 
Merge branch 'ae/doc-reproducible-html'

Newer versions of xsltproc can assign IDs in HTML documents it
generates in a consistent manner.  Use the feature to help format
HTML version of the user manual reproducibly.

* ae/doc-reproducible-html:
  doc: make HTML manual reproducible

3 years agoMerge branch 'so/glossary-branch-is-not-necessarily-active'
Junio C Hamano [Mon, 14 Dec 2020 18:21:36 +0000 (10:21 -0800)] 
Merge branch 'so/glossary-branch-is-not-necessarily-active'

The glossary described a branch as an "active" line of development,
which is misleading---a stale and non-moving branch is still a
branch.

* so/glossary-branch-is-not-necessarily-active:
  glossary: improve "branch" definition

3 years agoMerge branch 'fc/atmark-in-refspec'
Junio C Hamano [Mon, 14 Dec 2020 18:21:36 +0000 (10:21 -0800)] 
Merge branch 'fc/atmark-in-refspec'

"@" sometimes worked (e.g. "git push origin @:there") as a part of
a refspec element, but "git push origin @" did not work, which has
been corrected.

* fc/atmark-in-refspec:
  refspec: make @ a synonym of HEAD
  tests: push: trivial cleanup
  tests: push: improve cleanup of HEAD tests

3 years agoMerge branch 'dd/help-autocorrect-never'
Junio C Hamano [Mon, 14 Dec 2020 18:21:36 +0000 (10:21 -0800)] 
Merge branch 'dd/help-autocorrect-never'

"git $cmd $args", when $cmd is not a recognised subcommand, by
default tries to see if $cmd is a typo of an existing subcommand
and optionally executes the corrected command if there is only one
possibility, depending on the setting of help.autocorrect; the
users can now disable the whole thing, including the cycles spent
to find a likely typo, by setting the configuration variable to
'never'.

* dd/help-autocorrect-never:
  help.c: help.autocorrect=never means "do not compute suggestions"

3 years agopull: give the advice for choosing rebase/merge much later
Felipe Contreras [Sat, 12 Dec 2020 16:52:07 +0000 (10:52 -0600)] 
pull: give the advice for choosing rebase/merge much later

Eventually we want to be omit the advice when we can fast-forward
in which case there is no reason to require the user to choose
between rebase or merge.

In order to do so, we need to delay giving the advice up to the
point where we can check if we can fast-forward or not.

Additionally, config_get_rebase() was probably never its true home.

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
3 years agopull: refactor fast-forward check
Felipe Contreras [Sat, 12 Dec 2020 16:52:06 +0000 (10:52 -0600)] 
pull: refactor fast-forward check

We would like to be able to make this check before the decision to
rebase is made in a future step.  Besides, using a separate helper
makes the code easier to follow.

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
3 years agoindex-format.txt: document v2 format of file system monitor extension
Jeff Hostetler [Mon, 14 Dec 2020 13:33:42 +0000 (13:33 +0000)] 
index-format.txt: document v2 format of file system monitor extension

Update the documentation of the file system monitor extension to
describe version 2.

The format was extended to support opaque tokens in:
56c6910028 fsmonitor: change last update timestamp on the index_state to opaque token

Signed-off-by: Jeff Hostetler <jeffhost@microsoft.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
3 years agodocs: multi-pack-index: remove note about future 'verify' work
Johannes Berg [Sun, 13 Dec 2020 10:13:40 +0000 (11:13 +0100)] 
docs: multi-pack-index: remove note about future 'verify' work

This was implemented in the 'git multi-pack-index' command and
merged in 468b3221 (Merge branch 'ds/multi-pack-verify',
2018-10-10).

And there's no 'git midx' command.

Signed-off-by: Johannes Berg <johannes@sipsolutions.net>
Reviewed-by: Derrick Stolee <dstolee@microsoft.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
3 years agoinit: provide useful advice about init.defaultBranch
Johannes Schindelin [Fri, 11 Dec 2020 11:36:57 +0000 (11:36 +0000)] 
init: provide useful advice about init.defaultBranch

To give ample warning for users wishing to override Git's the fall-back
for an unconfigured `init.defaultBranch` (in case we decide to change it
in a future Git version), let's introduce some advice that is shown upon
`git init` when that value is not set.

Note: two test cases in Git's test suite want to verify that the
`stderr` output of `git init` is empty. It is now necessary to suppress
the advice, we now do that via the `init.defaultBranch` setting. While
not strictly necessary, we also set this to `false` in
`test_create_repo()`.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
3 years agoget_default_branch_name(): prepare for showing some advice
Johannes Schindelin [Fri, 11 Dec 2020 11:36:56 +0000 (11:36 +0000)] 
get_default_branch_name(): prepare for showing some advice

We are about to introduce a message giving users running `git init` some
advice about `init.defaultBranch`. This will necessarily be done in
`repo_default_branch_name()`.

Not all code paths want to show that advice, though. In particular, the
`git clone` codepath _specifically_ asks for `init_db()` to be quiet,
via the `INIT_DB_QUIET` flag.

In preparation for showing users above-mentioned advice, let's change
the function signature of `get_default_branch_name()` to accept the
parameter `quiet`.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
3 years agobranch -m: allow renaming a yet-unborn branch
Johannes Schindelin [Fri, 11 Dec 2020 11:36:55 +0000 (11:36 +0000)] 
branch -m: allow renaming a yet-unborn branch

In one of the next commits, we would like to give users some advice
regarding the initial branch name, and how to modify it.

To that end, it would be good if `git branch -m <name>` worked in a
freshly initialized repository without any commits. Let's make it so.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
3 years agoinit: document `init.defaultBranch` better
Johannes Schindelin [Fri, 11 Dec 2020 11:36:54 +0000 (11:36 +0000)] 
init: document `init.defaultBranch` better

Our documentation does not mention any future plan to change 'master' to
other value. It is a good idea to document this, though.

Initial-patch-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
3 years agomerge-ort: add modify/delete handling and delayed output processing
Elijah Newren [Thu, 3 Dec 2020 15:59:46 +0000 (15:59 +0000)] 
merge-ort: add modify/delete handling and delayed output processing

The focus here is on adding a path_msg() which will queue up
warning/conflict/notice messages about the merge for later processing,
storing these in a pathname -> strbuf map.  It might seem like a big
change, but it really just is:

  * declaration of necessary map with some comments
  * initialization and recording of data
  * a bunch of code to iterate over the map at print/free time
  * at least one caller in order to avoid an error about having an
    unused function (which we provide in the form of implementing
    modify/delete conflict handling).

At this stage, it is probably not clear why I am opting for delayed
output processing.  There are multiple reasons:

  1. Merges are supposed to abort if they would overwrite dirty changes
     in the working tree.  We cannot correctly determine whether changes
     would be overwritten until both rename detection has occurred and
     full processing of entries with the renames has finalized.
     Warning/conflict/notice messages come up at intermediate codepaths
     along the way, so unless we want spurious conflict/warning messages
     being printed when the merge will be aborted anyway, we need to
     save these messages and only print them when relevant.

  2. There can be multiple messages for a single path, and we want all
     messages for a give path to appear together instead of having them
     grouped by conflict/warning type.  This was a problem already with
     merge-recursive.c but became even more important due to the
     splitting apart of conflict types as discussed in the commit
     message for 1f3c9ba707 ("t6425: be more flexible with rename/delete
     conflict messages", 2020-08-10)

  3. Some callers might want to avoid showing the output in certain
     cases, such as if the end result is a clean merge.  Rebases have
     typically done this.

  4. Some callers might not want the output to go to stdout or even
     stderr, but might want to do something else with it entirely.
     For example, a --remerge-diff option to `git show` or `git log
     -p` that remerges on the fly and diffs merge commits against the
     remerged version would benefit from stdout/stderr not being
     written to in the standard form.

Signed-off-by: Elijah Newren <newren@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
3 years agomerge-ort: add die-not-implemented stub handle_content_merge() function
Elijah Newren [Thu, 3 Dec 2020 15:59:45 +0000 (15:59 +0000)] 
merge-ort: add die-not-implemented stub handle_content_merge() function

This simplistic and weird-looking patch is here to facilitate future
patch submissions.  Adding this stub allows rename detection code to
reference it in one patch series, while a separate patch series can
define the implementation, and then both series can merge cleanly and
work nicely together at that point.

Signed-off-by: Elijah Newren <newren@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
3 years agomerge-ort: add function grouping comments
Elijah Newren [Thu, 3 Dec 2020 15:59:44 +0000 (15:59 +0000)] 
merge-ort: add function grouping comments

Commit b658536f59 ("merge-ort: add some high-level algorithm structure",
2020-10-27) added high-level structure of the ort merge algorithm.  As
we have added more and more functions, that high-level structure has
been slightly obscured.  Since functions are still grouped according to
this high-level structure, add comments denoting sections where all the
functions are specifically tied to a piece of the high-level structure.

This function groupings include a few sub-divisions of the original
high-level structure, including some sub-divisions that are yet to be
submitted.  Each has (or will have) several functions all serving as
helpers to one or two main functions for each section.

As an added bonus, the comments will serve to provide a small textual
separation between nearby sections and allow the next three patch series
to be submitted independently and merge cleanly.

Signed-off-by: Elijah Newren <newren@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
3 years agomerge-ort: add a paths_to_free field to merge_options_internal
Elijah Newren [Thu, 3 Dec 2020 15:59:43 +0000 (15:59 +0000)] 
merge-ort: add a paths_to_free field to merge_options_internal

This field will be used in future patches to allow removal of paths from
opt->priv->paths.

Signed-off-by: Elijah Newren <newren@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
3 years agomerge-ort: add a path_conflict field to merge_options_internal
Elijah Newren [Thu, 3 Dec 2020 15:59:42 +0000 (15:59 +0000)] 
merge-ort: add a path_conflict field to merge_options_internal

This field is not yet used, but will be used by both the rename handling
code, and the conflict type handling code in process_entry().

Signed-off-by: Elijah Newren <newren@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
3 years agomerge-ort: add a clear_internal_opts helper
Elijah Newren [Thu, 3 Dec 2020 15:59:41 +0000 (15:59 +0000)] 
merge-ort: add a clear_internal_opts helper

Move most of merge_finalize() into a new helper function,
clear_internal_opts().  This is a step to facilitate recursive merges,
as well as some future optimizations.

Signed-off-by: Elijah Newren <newren@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
3 years agomerge-ort: add a few includes
Elijah Newren [Thu, 3 Dec 2020 15:59:40 +0000 (15:59 +0000)] 
merge-ort: add a few includes

Include blob.h for definition of blob_type, and commit-reach.h for
declarations of get_merge_bases() and in_merge_bases().  While none of
these are used yet, we want to avoid cross-dependencies in the next
three series of patches for merge-ort and merge them at the end; adding
these "#include"s now avoids textual conflicts.

Signed-off-by: Elijah Newren <newren@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
3 years agomerge-ort: free data structures in merge_finalize()
Elijah Newren [Sun, 13 Dec 2020 08:04:27 +0000 (08:04 +0000)] 
merge-ort: free data structures in merge_finalize()

Signed-off-by: Elijah Newren <newren@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
3 years agomerge-ort: add implementation of record_conflicted_index_entries()
Elijah Newren [Sun, 13 Dec 2020 08:04:26 +0000 (08:04 +0000)] 
merge-ort: add implementation of record_conflicted_index_entries()

After checkout(), the working tree has the appropriate contents, and the
index matches the working copy.  That means that all unmodified and
cleanly merged files have correct index entries, but conflicted entries
need to be updated.

We do this by looping over the conflicted entries, marking the existing
index entry for the path with CE_REMOVE, adding new higher order staged
for the path at the end of the index (ignoring normal index sort order),
and then at the end of the loop removing the CE_REMOVED-marked cache
entries and sorting the index.

Signed-off-by: Elijah Newren <newren@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
3 years agotree: enable cmp_cache_name_compare() to be used elsewhere
Elijah Newren [Sun, 13 Dec 2020 08:04:25 +0000 (08:04 +0000)] 
tree: enable cmp_cache_name_compare() to be used elsewhere

Signed-off-by: Elijah Newren <newren@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
3 years agomerge-ort: add implementation of checkout()
Elijah Newren [Sun, 13 Dec 2020 08:04:24 +0000 (08:04 +0000)] 
merge-ort: add implementation of checkout()

Since merge-ort creates a tree for its output, when there are no
conflicts, updating the working tree and index is as simple as using the
unpack_trees() machinery with a twoway_merge (i.e. doing the equivalent
of a "checkout" operation).

If there were conflicts in the merge, then since the tree we created
included all the conflict markers, then using the unpack_trees machinery
in this manner will still update the working tree correctly.  Further,
all index entries corresponding to cleanly merged files will also be
updated correctly by this procedure.  Index entries corresponding to
conflicted entries will appear as though the user had run "git add -u"
after the merge to accept all files as-is with conflict markers.

Thus, after running unpack_trees(), there needs to be a separate step
for updating the entries in the index corresponding to conflicted files.
This will be the job for the function record_conflicted_index_entris(),
which will be implemented in a subsequent commit.

Signed-off-by: Elijah Newren <newren@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
3 years agomerge-ort: basic outline for merge_switch_to_result()
Elijah Newren [Sun, 13 Dec 2020 08:04:23 +0000 (08:04 +0000)] 
merge-ort: basic outline for merge_switch_to_result()

This adds a basic implementation for merge_switch_to_result(), though
just in terms of a few new empty functions that will be defined in
subsequent commits.

Signed-off-by: Elijah Newren <newren@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
3 years agomerge-ort: step 3 of tree writing -- handling subdirectories as we go
Elijah Newren [Sun, 13 Dec 2020 08:04:22 +0000 (08:04 +0000)] 
merge-ort: step 3 of tree writing -- handling subdirectories as we go

Our order for processing of entries means that if we have a tree of
files that looks like
   Makefile
   src/moduleA/foo.c
   src/moduleA/bar.c
   src/moduleB/baz.c
   src/moduleB/umm.c
   tokens.txt

Then we will process paths in the order of the leftmost column below.  I
have added two additional columns that help explain the algorithm that
follows; the 2nd column is there to remind us we have oid & mode info we
are tracking for each of these paths (which differs between the paths
which I'm not representing well here), and the third column annotates
the parent directory of the entry:
   tokens.txt               <version_info>    ""
   src/moduleB/umm.c        <version_info>    src/moduleB
   src/moduleB/baz.c        <version_info>    src/moduleB
   src/moduleB              <version_info>    src
   src/moduleA/foo.c        <version_info>    src/moduleA
   src/moduleA/bar.c        <version_info>    src/moduleA
   src/moduleA              <version_info>    src
   src                      <version_info>    ""
   Makefile                 <version_info>    ""

When the parent directory changes, if it's a subdirectory of the previous
parent directory (e.g. "" -> src/moduleB) then we can just keep appending.
If the parent directory differs from the previous parent directory and is
not a subdirectory, then we should process that directory.

So, for example, when we get to this point:
   tokens.txt               <version_info>    ""
   src/moduleB/umm.c        <version_info>    src/moduleB
   src/moduleB/baz.c        <version_info>    src/moduleB

and note that the next entry (src/moduleB) has a different parent than
the last one that isn't a subdirectory, we should write out a tree for it
   100644 blob <HASH> umm.c
   100644 blob <HASH> baz.c

then pop all the entries under that directory while recording the new
hash for that directory, leaving us with
   tokens.txt               <version_info>        ""
   src/moduleB              <new version_info>    src

This process repeats until at the end we get to
   tokens.txt               <version_info>        ""
   src                      <new version_info>    ""
   Makefile                 <version_info>        ""

and then we can write out the toplevel tree.  Since we potentially have
entries in our string_list corresponding to multiple different toplevel
directories, e.g. a slightly different repository might have:
   whizbang.txt             <version_info>        ""
   tokens.txt               <version_info>        ""
   src/moduleD              <new version_info>    src
   src/moduleC              <new version_info>    src
   src/moduleB              <new version_info>    src
   src/moduleA/foo.c        <version_info>        src/moduleA
   src/moduleA/bar.c        <version_info>        src/moduleA

When src/moduleA is popped off, we need to know that the "last
directory" reverts back to src, and how many entries in our string_list
are associated with that parent directory.  So I use an auxiliary
offsets string_list which would have (parent_directory,offset)
information of the form
   ""             0
   src            2
   src/moduleA    5

Whenever I write out a tree for a subdirectory, I set versions.nr to
the final offset value and then decrement offsets.nr...and then add
an entry to versions with a hash for the new directory.

The idea is relatively simple, there's just a lot of accounting to
implement this.

Signed-off-by: Elijah Newren <newren@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
3 years agomerge-ort: step 2 of tree writing -- function to create tree object
Elijah Newren [Sun, 13 Dec 2020 08:04:21 +0000 (08:04 +0000)] 
merge-ort: step 2 of tree writing -- function to create tree object

Create a new function, write_tree(), which will take a list of
basenames, modes, and oids for a single directory and create a tree
object in the object-store.  We do not yet have just basenames, modes,
and oids for just a single directory (we have a mixture of entries from
all directory levels in the hierarchy) so we still die() before the
current call to write_tree(), but the next patch will rectify that.

Signed-off-by: Elijah Newren <newren@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
3 years agomerge-ort: step 1 of tree writing -- record basenames, modes, and oids
Elijah Newren [Sun, 13 Dec 2020 08:04:20 +0000 (08:04 +0000)] 
merge-ort: step 1 of tree writing -- record basenames, modes, and oids

As a step towards transforming the processed path->conflict_info entries
into an actual tree object, start recording basenames, modes, and oids
in a dir_metadata structure.  Subsequent commits will make use of this
to actually write a tree.

Signed-off-by: Elijah Newren <newren@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
3 years agomerge-ort: have process_entries operate in a defined order
Elijah Newren [Sun, 13 Dec 2020 08:04:19 +0000 (08:04 +0000)] 
merge-ort: have process_entries operate in a defined order

We want to handle paths below a directory before needing to handle the
directory itself.  Also, we want to handle the directory immediately
after the paths below it, so we can't use simple lexicographic ordering
from strcmp (which would insert foo.txt between foo and foo/file.c).
Copy string_list_df_name_compare() from merge-recursive.c, and set up a
string list of paths sorted by that function so that we can iterate in
the desired order.

Signed-off-by: Elijah Newren <newren@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
3 years agomerge-ort: add a preliminary simple process_entries() implementation
Elijah Newren [Sun, 13 Dec 2020 08:04:18 +0000 (08:04 +0000)] 
merge-ort: add a preliminary simple process_entries() implementation

Add a process_entries() implementation that just loops over the paths
and processes each one individually with an auxiliary process_entry()
call.  Add a basic process_entry() as well, which handles several cases
but leaves a few of the more involved ones with die-not-implemented
messages.  Also, although process_entries() is supposed to create a
tree, it does not yet have code to do so -- except in the special case
of merging completely empty trees.

Signed-off-by: Elijah Newren <newren@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
3 years agomerge-ort: avoid recursing into identical trees
Elijah Newren [Sun, 13 Dec 2020 08:04:17 +0000 (08:04 +0000)] 
merge-ort: avoid recursing into identical trees

When all three trees have the same oid, there is no need to recurse into
these trees to find that all files within them happen to match.  We can
just record any one of the trees as the resolution of merging that
particular path.

Immediately resolving trees for other types of trivial tree merges (such
as one side matches the merge base, or the two sides match each other)
would prevent us from detecting renames for some paths, and thus prevent
us from doing three-way content merges for those paths whose renames we
did not detect.

Signed-off-by: Elijah Newren <newren@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
3 years agomerge-ort: record stage and auxiliary info for every path
Elijah Newren [Sun, 13 Dec 2020 08:04:16 +0000 (08:04 +0000)] 
merge-ort: record stage and auxiliary info for every path

Create a helper function, setup_path_info(), which can be used to record
all the information we want in a merged_info or conflict_info.  While
there is currently only one caller of this new function, and some of its
particular parameters are fixed, future callers of this function will be
added later.

Signed-off-by: Elijah Newren <newren@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
3 years agomerge-ort: compute a few more useful fields for collect_merge_info
Elijah Newren [Sun, 13 Dec 2020 08:04:15 +0000 (08:04 +0000)] 
merge-ort: compute a few more useful fields for collect_merge_info

Signed-off-by: Elijah Newren <newren@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
3 years agomerge-ort: avoid repeating fill_tree_descriptor() on the same tree
Elijah Newren [Sun, 13 Dec 2020 08:04:14 +0000 (08:04 +0000)] 
merge-ort: avoid repeating fill_tree_descriptor() on the same tree

Three-way merges, by their nature, are going to often have two or more
trees match at a given subdirectory.  We can avoid calling
fill_tree_descriptor() on the same tree by checking when these trees
match.  Noting when various oids match will also be useful in other
calculations and optimizations as well.

Signed-off-by: Elijah Newren <newren@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
3 years agomerge-ort: implement a very basic collect_merge_info()
Elijah Newren [Sun, 13 Dec 2020 08:04:13 +0000 (08:04 +0000)] 
merge-ort: implement a very basic collect_merge_info()

This does not actually collect any necessary info other than the
pathnames involved, since it just allocates an all-zero conflict_info
and stuffs that into paths.  However, it invokes the traverse_trees()
machinery to walk over all the paths and sets up the basic
infrastructure we need.

I have left out a few obvious optimizations to try to make this patch as
short and obvious as possible.  A subsequent patch will add some of
those back in with some more useful data fields before we introduce a
patch that actually sets up the conflict_info fields.

Signed-off-by: Elijah Newren <newren@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
3 years agomerge-ort: add an err() function similar to one from merge-recursive
Elijah Newren [Sun, 13 Dec 2020 08:04:12 +0000 (08:04 +0000)] 
merge-ort: add an err() function similar to one from merge-recursive

Various places in merge-recursive used an err() function when it hit
some kind of unrecoverable error.  That code was from the reusable bits
of merge-recursive.c that we liked, such as merge_3way, writing object
files to the object store, reading blobs from the object store, etc.  So
create a similar function to allow us to port that code over, and use it
for when we detect problems returned from collect_merge_info()'s
traverse_trees() call, which we will be adding next.

While we are at it, also add more documentation for the "clean" field
from struct merge_result, particularly since the name suggests a boolean
but it is not quite one and this is our first non-boolean usage.

Signed-off-by: Elijah Newren <newren@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
3 years agomerge-ort: use histogram diff
Elijah Newren [Sun, 13 Dec 2020 08:04:11 +0000 (08:04 +0000)] 
merge-ort: use histogram diff

In my cursory investigation, histogram diffs are about 2% slower than
Myers diffs.  Others have probably done more detailed benchmarks.  But,
in short, histogram diffs have been around for years and in a number of
cases provide obviously better looking diffs where Myers diffs are
unintelligible but the performance hit has kept them from becoming the
default.

However, there are real merge bugs we know about that have triggered on
git.git and linux.git, which I don't have a clue how to address without
the additional information that I believe is provided by histogram
diffs.  See the following:

https://lore.kernel.org/git/20190816184051.GB13894@sigill.intra.peff.net/
https://lore.kernel.org/git/CABPp-BHvJHpSJT7sdFwfNcPn_sOXwJi3=o14qjZS3M8Rzcxe2A@mail.gmail.com/
https://lore.kernel.org/git/CABPp-BGtez4qjbtFT1hQoREfcJPmk9MzjhY5eEq1QhXT23tFOw@mail.gmail.com/

I don't like mismerges.  I really don't like silent mismerges.  While I
am sometimes willing to make performance and correctness tradeoff, I'm
much more interested in correctness in general.  I want to fix the above
bugs.  I have not yet started doing so, but I believe histogram diff at
least gives me an angle.  Unfortunately, I can't rely on using the
information from histogram diff unless it's in use.  And it hasn't been
used because of a few percentage performance hit.

In testcases I have looked at, merge-ort is _much_ faster than
merge-recursive for non-trivial merges/rebases/cherry-picks.  As such,
this is a golden opportunity to switch out the underlying diff algorithm
(at least the one used by the merge machinery; git-diff and git-log are
separate questions); doing so will allow me to get additional data and
improved diffs, and I believe it will help me fix the above bugs at some
point in the future.

Signed-off-by: Elijah Newren <newren@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
3 years agomerge-ort: port merge_start() from merge-recursive
Elijah Newren [Sun, 13 Dec 2020 08:04:10 +0000 (08:04 +0000)] 
merge-ort: port merge_start() from merge-recursive

merge_start() basically does a bunch of sanity checks, then allocates
and initializes opt->priv -- a struct merge_options_internal.

Most of the sanity checks are usable as-is.  The
allocation/intialization is a bit different since merge-ort has a very
different merge_options_internal than merge-recursive, but the idea is
the same.

The weirdest part here is that merge-ort and merge-recursive use the
same struct merge_options, even though merge_options has a number of
fields that are oddly specific to merge-recursive's internal
implementation and don't even make sense with merge-ort's high-level
design (e.g. buffer_output, which merge-ort has to always do).  I reused
the same data structure because:
  * most the fields made sense to both merge algorithms
  * making a new struct would have required making new enums or somehow
    externalizing them, and that was getting messy.
  * it simplifies converting the existing callers by not having to
    have different code paths for merge_options setup.

I also marked detect_renames as ignored.  We can revisit that later, but
in short: merge-recursive allowed turning off rename detection because
it was sometimes glacially slow.  When you speed something up by a few
orders of magnitude, it's worth revisiting whether that justification is
still relevant.  Besides, if folks find it's still too slow, perhaps
they have a better scaling case than I could find and maybe it turns up
some more optimizations we can add.  If it still is needed as an option,
it is easy to add later.

Signed-off-by: Elijah Newren <newren@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
3 years agomerge-ort: add some high-level algorithm structure
Elijah Newren [Sun, 13 Dec 2020 08:04:09 +0000 (08:04 +0000)] 
merge-ort: add some high-level algorithm structure

merge_ort_nonrecursive_internal() will be used by both
merge_inmemory_nonrecursive() and merge_inmemory_recursive(); let's
focus on it for now.  It involves some setup -- merge_start() --
followed by the following chain of functions:

  collect_merge_info()
    This function will populate merge_options_internal's paths field,
    via a call to traverse_trees() and a new callback that will be added
    later.

  detect_and_process_renames()
    This function will detect renames, and then adjust entries in paths
    to move conflict stages from old pathnames into those for new
    pathnames, so that the next step doesn't have to think about renames
    and just can do three-way content merging and such.

  process_entries()
    This function determines how to take the various stages (versions of
    a file from the three different sides) and merge them, and whether
    to mark the result as conflicted or cleanly merged.  It also writes
    out these merged file versions as it goes to create a tree.

Signed-off-by: Elijah Newren <newren@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
3 years agomerge-ort: setup basic internal data structures
Elijah Newren [Sun, 13 Dec 2020 08:04:08 +0000 (08:04 +0000)] 
merge-ort: setup basic internal data structures

Set up some basic internal data structures.  The only carry-over from
merge-recursive.c is call_depth, though needed_rename_limit will be
added later.

The central piece of data will definitely be the strmap "paths", which
will map every relevant pathname under consideration to either a
merged_info or a conflict_info.  ("conflicted" is a strmap that is a
subset of "paths".)

merged_info contains all relevant information for a non-conflicted
entry.  conflict_info contains a merged_info, plus any additional
information about a conflict such as the higher orders stages involved
and the names of the paths those came from (handy once renames get
involved).  If an entry remains conflicted, the merged_info portion of a
conflict_info will later be filled with whatever version of the file
should be placed in the working directory (e.g. an as-merged-as-possible
variation that contains conflict markers).

Signed-off-by: Elijah Newren <newren@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
3 years agot7900: use --fixed-value in git-maintenance tests
Josh Steadmon [Wed, 9 Dec 2020 19:16:16 +0000 (11:16 -0800)] 
t7900: use --fixed-value in git-maintenance tests

Use --fixed-value in git-config calls in the git-maintenance tests, so
that the tests will continue to work even if the repo path contains
regexp metacharacters.

Signed-off-by: Josh Steadmon <steadmon@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
3 years agopretty format %(trailers): add a "key_value_separator"
Ævar Arnfjörð Bjarmason [Wed, 9 Dec 2020 15:52:08 +0000 (16:52 +0100)] 
pretty format %(trailers): add a "key_value_separator"

Add a "key_value_separator" option to the "%(trailers)" pretty format,
to go along with the existing "separator" argument. In combination
these two options make it trivial to produce machine-readable (e.g. \0
and \0\0-delimited) format output.

As elaborated on in a previous commit which added "keyonly" it was
needlessly tedious to extract structured data from "%(trailers)"
before the addition of this "key_value_separator" option. As seen by
the test being added here extracting this data now becomes trivial.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
3 years agopretty format %(trailers): add a "keyonly"
Ævar Arnfjörð Bjarmason [Wed, 9 Dec 2020 15:52:07 +0000 (16:52 +0100)] 
pretty format %(trailers): add a "keyonly"

Add support for a "keyonly". This allows for easier parsing out of the
key and value. Before if you didn't want to make assumptions about how
the key was formatted. You'd need to parse it out as e.g.:

    --pretty=format:'%H%x00%(trailers:separator=%x00%x00)' \
                       '%x00%(trailers:separator=%x00%x00,valueonly)'

And then proceed to deduce keys by looking at those two and
subtracting the value plus the hardcoded ": " separator from the
non-valueonly %(trailers) line. Now it's possible to simply do:

    --pretty=format:'%H%x00%(trailers:separator=%x00%x00,keyonly)' \
                    '%x00%(trailers:separator=%x00%x00,valueonly)'

Which at least reduces it to a state machine where you get N keys and
correlate them with N values. Even better would be to have a way to
change the ": " delimiter to something easily machine-readable (a key
might contain ": " too). A follow-up change will add support for that.

I don't really have a use-case for just "keyonly" myself. I suppose it
would be useful in some cases as "key=*" matches case-insensitively,
so a plain "keyonly" will give you the variants of the keys you
matched. I'm mainly adding it to fix the inconsistency with
"valueonly".

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
3 years agopretty-format %(trailers): fix broken standalone "valueonly"
Ævar Arnfjörð Bjarmason [Wed, 9 Dec 2020 15:52:06 +0000 (16:52 +0100)] 
pretty-format %(trailers): fix broken standalone "valueonly"

Fix %(trailers:valueonly) being a noop due to on overly eager
optimization in format_trailer_info() which skips custom formatting if
no custom options are given.

When "valueonly" was added in d9b936db522 (pretty: add support for
"valueonly" option in %(trailers), 2019-01-28) we forgot to add it to
the list of options that optimization checks for. See e.g. the
addition of "key" in 250bea0c165 (pretty: allow showing specific
trailers, 2019-01-28) for a similar change where this wasn't missed.

Thus the "valueonly" option in "%(trailers:valueonly)" was a noop and
the output was equivalent to that of a plain "%(trailers)". This
wasn't caught because the tests for it always combined it with other
options.

Fix the bug by adding !opts->value_only to the list. I initially
attempted to make this more future-proof by setting a flag if we got
to ":" in "%(trailers:" in format_commit_one() in pretty.c. However,
"%(trailers:" is also parsed in trailers_atom_parser() in
ref-filter.c.

There is an outstanding patch[1] unify those two, and such a fix, or
other future-proofing, such as changing "process_trailer_options"
flags into a bitfield, would conflict with that effort. Let's instead
do the bare minimum here as this aspect of trailers is being actively
worked on by another series.

Let's also test for a plain "valueonly" without any other options, as
well as "separator". All the other existing options on the pretty.c
path had tests where they were the only option provided. I'm also
keeping a sanity test for "%(trailers:)" being the same as
"%(trailers)". There's no reason to suspect it wouldn't be in the
current implementation, but let's keep it in the interest of black box
testing.

1. https://lore.kernel.org/git/pull.726.git.1599335291.gitgitgadget@gmail.com/

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
3 years agopretty format %(trailers) doc: avoid repetition
Ævar Arnfjörð Bjarmason [Wed, 9 Dec 2020 15:52:05 +0000 (16:52 +0100)] 
pretty format %(trailers) doc: avoid repetition

Change the documentation for the various %(trailers) options so it
isn't repeating part of the documentation for "only" about how boolean
values are handled. Instead, let's split the description of that into
general documentation at the top.

It then suffices to refer to it by listing the options as
"opt[=<BOOL>]". I'm also changing it to upper-case "[=<BOOL>]" from
"[=val]" for consistency with "<SEP>"

It took me a couple of readings to realize that these options were
referring back to the "only" option's treatment of boolean
values.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
3 years agosubmodules: fix of regression on fetching of non-init subsub-repo
Peter Kaestle [Wed, 9 Dec 2020 10:58:44 +0000 (11:58 +0100)] 
submodules: fix of regression on fetching of non-init subsub-repo

A regression has been introduced by a62387b (submodule.c: fetch in
submodules git directory instead of in worktree, 2018-11-28).

The scenario in which it triggers is when one has a repository with a
submodule inside a submodule like this:
superproject/middle_repo/inner_repo

Person A and B have both a clone of it, while Person B is not working
with the inner_repo and thus does not have it initialized in his working
copy.

Now person A introduces a change to the inner_repo and propagates it
through the middle_repo and the superproject.

Once person A pushed the changes and person B wants to fetch them using
"git fetch" at the superproject level, B's git call will return with
error saying:

Could not access submodule 'inner_repo'
Errors during submodule fetch:
         middle_repo

Expectation is that in this case the inner submodule will be recognized
as uninitialized submodule and skipped by the git fetch command.

This used to work correctly before 'a62387b (submodule.c: fetch in
submodules git directory instead of in worktree, 2018-11-28)'.

Starting with a62387b the code wants to evaluate "is_empty_dir()" inside
.git/modules for a directory only existing in the worktree, delivering
then of course wrong return value.

This patch ensures is_empty_dir() is getting the correct path of the
uninitialized submodule by concatenation of the actual worktree and the
name of the uninitialized submodule.

The first attempt to fix this regression, in 1b7ac4e6d4 (submodules:
fix of regression on fetching of non-init subsub-repo, 2020-11-12), by
simply reverting a62387b, resulted in an infinite loop of submodule
fetches in the simpler case of a recursive fetch of a superproject with
uninitialized submodules, and so this commit was reverted in 7091499bc0
(Revert "submodules: fix of regression on fetching of non-init
subsub-repo", 2020-12-02).
To prevent future breakages, also add a regression test for this
scenario.

Signed-off-by: Peter Kaestle <peter.kaestle@nokia.com>
CC: Junio C Hamano <gitster@pobox.com>
CC: Philippe Blain <levraiphilippeblain@gmail.com>
CC: Ralf Thielow <ralf.thielow@gmail.com>
CC: Eric Sunshine <sunshine@sunshineco.us>
Reviewed-by: Philippe Blain <levraiphilippeblain@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
3 years agoMakefile: don't use a versioned temp distribution directory
Ramsay Jones [Tue, 8 Dec 2020 22:36:33 +0000 (22:36 +0000)] 
Makefile: don't use a versioned temp distribution directory

The 'dist' target uses a versioned temp directory, $(GIT_TARNAME), into
which it copies various files added to the distribution tarball. Should
it be necessary to remove this directory in the 'clean' target, since
the name depends on $(GIT_VERSION), the current HEAD must be positioned
on the same commit as when 'make dist' was issued. Otherwise, the target
will fail to remove that directory.

Create an '.dist-tmp-dir' directory and copy the various files into this
now un-versioned directory while creating the distribution tarball. Change
the 'clean' target to remove the '.dist-tmp-dir' directory, instead of the
version dependent $(GIT_TARNAME) directory.

Signed-off-by: Ramsay Jones <ramsay@ramsayjones.plus.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
3 years agoMakefile: don't try to clean old debian build product
Ramsay Jones [Tue, 8 Dec 2020 22:35:27 +0000 (22:35 +0000)] 
Makefile: don't try to clean old debian build product

The 'clean' target includes code to remove an '*.tar.gz' file that
was the by-product of a debian build. This was originally added by
commit 5a571cdd8a (Clean generated files a bit more, to cope with
Debian build droppings., 2005-08-12). However, all support for the
'debian build' was dropped by commit 7d0e65b892 (Retire debian/
directory., 2006-01-06), which seems to have simply forgotten to
remove the 'git-core_$(GIT_VERSION)-*.tar.gz' from the 'clean'
target. Remove it now.

Signed-off-by: Ramsay Jones <ramsay@ramsayjones.plus.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
3 years agogitweb/Makefile: conditionally include ../GIT-VERSION-FILE
Ramsay Jones [Tue, 8 Dec 2020 22:34:28 +0000 (22:34 +0000)] 
gitweb/Makefile: conditionally include ../GIT-VERSION-FILE

The 'clean' target is still noticeably slow on cygwin, despite the
improvements made by previous patches. For example, the second
invocation of 'make clean' below:

  $ make clean >/dev/null 2>&1
  $ make clean
  ...
  make[1]: Entering directory '/home/ramsay/git/gitweb'
  make[2]: Entering directory '/home/ramsay/git'
  make[2]: 'GIT-VERSION-FILE' is up to date.
  make[2]: Leaving directory '/home/ramsay/git'
  ...
  $

has been timed at 10.361s on my laptop (an old core i5-4200M @ 2.50GHz,
8GB RAM, 1TB HDD).

Notice that the 'clean' target is making a nested call to the parent
Makefile to ensure that the GIT-VERSION-FILE is up-to-date. This is to
ensure that the $(GIT_VERSION) make variable is set, once that file had
been included. However, the 'clean' target does not use the $(GIT_VERSION)
variable, directly or indirectly, so it does not have any affect on what
the target removes. Therefore, the time spent on ensuring an up to date
GIT-VERSION-FILE is wasted effort.

In order to eliminate such wasted effort, use the value of the internal
$(MAKECMDGOALS) variable to only '-include ../GIT-VERSION-FILE' when the
target is not 'clean'. (This drops the time down to 8.430s, on my laptop,
giving an improvement of 18.64%).

Signed-off-by: Ramsay Jones <ramsay@ramsayjones.plus.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
3 years agoDocumentation/Makefile: conditionally include ../GIT-VERSION-FILE
Ramsay Jones [Tue, 8 Dec 2020 22:33:05 +0000 (22:33 +0000)] 
Documentation/Makefile: conditionally include ../GIT-VERSION-FILE

The 'clean' target is still noticeably slow on cygwin, despite the
substantial improvement made by the previous patch. For example, the
second invocation of 'make clean' below:

  $ make clean >/dev/null 2>&1
  $ make clean
  ...
  make[1]: Entering directory '/home/ramsay/git/Documentation'
  make[2]: Entering directory '/home/ramsay/git'
  make[2]: 'GIT-VERSION-FILE' is up to date.
  make[2]: Leaving directory '/home/ramsay/git'
  ...
  $

has been timed at 12.364s on my laptop (an old core i5-4200M @ 2.50GHz,
8GB RAM, 1TB HDD).

Notice that the 'clean' target is making a nested call to the parent
Makefile to ensure that the GIT-VERSION-FILE is up-to-date (prior to
the previous patch, there would have been _two_ such invocations).
This is to ensure that the $(GIT_VERSION) make variable is set, once
that file had been included.  However, the 'clean' target does not use
the $(GIT_VERSION) variable, directly or indirectly, so it does not
have any affect on what the target removes. Therefore, the time spent
on ensuring an up to date GIT-VERSION-FILE is wasted effort.

In order to eliminate such wasted effort, use the value of the internal
$(MAKECMDGOALS) variable to only '-include ../GIT-VERSION-FILE' when the
target is not 'clean'. (This drops the time down to 10.361s, on my laptop,
giving an improvement of 16.20%).

Signed-off-by: Ramsay Jones <ramsay@ramsayjones.plus.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
3 years agoDocumentation/Makefile: conditionally include doc.dep
Ramsay Jones [Tue, 8 Dec 2020 22:31:44 +0000 (22:31 +0000)] 
Documentation/Makefile: conditionally include doc.dep

The 'clean' target is noticeably slow on cygwin, even for a 'do-nothing'
invocation of 'make clean'. For example, the second 'make clean' below:

  $ make clean >/dev/null 2>&1
  $ make clean
  GIT_VERSION = 2.29.0
  ...
  make[1]: Entering directory '/home/ramsay/git/Documentation'
      GEN mergetools-list.made
      GEN cmd-list.made
      GEN doc.dep
  ...
  $

has been timed at 23.339s, using git v2.29.0, on my laptop (an old core
i5-4200M @ 2.50GHz, 8GB RAM, 1TB HDD).

Notice that, since the 'doc.dep' file does not exist, make takes the
time (about 8s) to generate several files in order to create the doc.dep
include file. (If an 'include' file is missing, but a target for the
said file is present in the Makefile, make will execute that target
and, if that file now exists, throw away all its internal data and
re-read and re-parse the Makefile). Having spent the time to include
the 'doc.dep' file, the 'clean' target immediately deletes those files.
The document dependencies specified in the 'doc.dep' include file,
expressed as make targets and prerequisites, do not affect what the
'clean' target removes. Therefore, the time spent in generating the
dependencies is completely wasted effort.

In order to eliminate such wasted effort, use the value of the internal
$(MAKECMDGOALS) variable to only '-include doc.dep' when the target is
not 'clean'. (This drops the time down to 12.364s, on my laptop, giving
an improvement of 47.02%).

Signed-off-by: Ramsay Jones <ramsay@ramsayjones.plus.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
3 years agoEleventh batch
Junio C Hamano [Tue, 8 Dec 2020 22:56:00 +0000 (14:56 -0800)] 
Eleventh batch

Signed-off-by: Junio C Hamano <gitster@pobox.com>
3 years agoMerge branch 'fc/zsh-completion'
Junio C Hamano [Tue, 8 Dec 2020 23:11:22 +0000 (15:11 -0800)] 
Merge branch 'fc/zsh-completion'

Hotfix for a recent breakage.

* fc/zsh-completion:
  completion: bash: fix gitk alias regression
  completion: zsh: fix file completion regression

3 years agoMerge branch 'sn/config-doc-typofix'
Junio C Hamano [Tue, 8 Dec 2020 23:11:22 +0000 (15:11 -0800)] 
Merge branch 'sn/config-doc-typofix'

Fix for an old typo.

* sn/config-doc-typofix:
  config.txt: fix a typo (backslash != backquote)

3 years agoMerge branch 'fc/random-cleanup'
Junio C Hamano [Tue, 8 Dec 2020 23:11:21 +0000 (15:11 -0800)] 
Merge branch 'fc/random-cleanup'

Random cleanup.

* fc/random-cleanup:
  gitignore: remove entry for git serve
  gitignore: drop duplicate entry for git-sh-i18n
  tests: lib-functions: trivial style cleanups
  test: completion: fix typos
  .gitignore: remove dangling file
  refspec: trivial cleanup

3 years agoMerge branch 'nm/imap-send-use-default-config'
Junio C Hamano [Tue, 8 Dec 2020 23:11:21 +0000 (15:11 -0800)] 
Merge branch 'nm/imap-send-use-default-config'

"git imap-send" used to ignore configuration variables like
core.askpass; this has been corrected.

* nm/imap-send-use-default-config:
  imap-send: parse default git config

3 years agoMerge branch 'jk/banned'
Junio C Hamano [Tue, 8 Dec 2020 23:11:21 +0000 (15:11 -0800)] 
Merge branch 'jk/banned'

Non-reentrant time-related library functions and ctime/asctime with
awkward calling interfaces are banned from the codebase.

* jk/banned:
  banned.h: mark ctime_r() and asctime_r() as banned
  banned.h: mark non-reentrant gmtime, etc as banned

3 years agoMerge branch 'tb/bugreport-no-localtime'
Junio C Hamano [Tue, 8 Dec 2020 23:11:21 +0000 (15:11 -0800)] 
Merge branch 'tb/bugreport-no-localtime'

Use of non-reentrant localtime() has been removed.

* tb/bugreport-no-localtime:
  builtin/bugreport.c: use thread-safe localtime_r()

3 years agoMerge branch 'rs/maintenance-run-outside-repo'
Junio C Hamano [Tue, 8 Dec 2020 23:11:20 +0000 (15:11 -0800)] 
Merge branch 'rs/maintenance-run-outside-repo'

"git maintenance run/start/stop" needed to be run in a repository
to hold the lockfile they use, but didn't make sure they are
actually in a repository, which has been corrected.

* rs/maintenance-run-outside-repo:
  t7900: fix typo: "test_execpt_success"
  maintenance: fix SEGFAULT when no repository

3 years agoMerge branch 'rs/fetch-pack-invalid-lockfile'
Junio C Hamano [Tue, 8 Dec 2020 23:11:20 +0000 (15:11 -0800)] 
Merge branch 'rs/fetch-pack-invalid-lockfile'

"fetch-pack" could pass NULL pointer to unlink(2) when it sees an
invalid filename; the error checking has been tightened to make
this impossible.

* rs/fetch-pack-invalid-lockfile:
  fetch-pack: disregard invalid pack lockfiles

3 years agoMerge branch 'nk/perf-fsmonitor-cleanup'
Junio C Hamano [Tue, 8 Dec 2020 23:11:20 +0000 (15:11 -0800)] 
Merge branch 'nk/perf-fsmonitor-cleanup'

Test clean-up.

* nk/perf-fsmonitor-cleanup:
  perf/fsmonitor: use test_must_be_empty helper

3 years agoMerge branch 'ma/grep-init-default'
Junio C Hamano [Tue, 8 Dec 2020 23:11:20 +0000 (15:11 -0800)] 
Merge branch 'ma/grep-init-default'

Code clean-up.

* ma/grep-init-default:
  MyFirstObjectWalk: drop `init_walken_defaults()`
  grep: copy struct in one fell swoop
  grep: use designated initializers for `grep_defaults`
  grep: don't set up a "default" repo for grep

3 years agoMerge branch 'js/trace2-session-id'
Junio C Hamano [Tue, 8 Dec 2020 23:11:20 +0000 (15:11 -0800)] 
Merge branch 'js/trace2-session-id'

The transport layer was taught to optionally exchange the session
ID assigned by the trace2 subsystem during fetch/push transactions.

* js/trace2-session-id:
  receive-pack: log received client session ID
  send-pack: advertise session ID in capabilities
  upload-pack, serve: log received client session ID
  fetch-pack: advertise session ID in capabilities
  transport: log received server session ID
  serve: advertise session ID in v2 capabilities
  receive-pack: advertise session ID in v0 capabilities
  upload-pack: advertise session ID in v0 capabilities
  trace2: add a public function for getting the SID
  docs: new transfer.advertiseSID option
  docs: new capability to advertise session IDs

3 years agoMerge branch 'mt/do-not-use-scld-in-working-tree'
Junio C Hamano [Tue, 8 Dec 2020 23:11:19 +0000 (15:11 -0800)] 
Merge branch 'mt/do-not-use-scld-in-working-tree'

"git apply" adjusted the permission bits of working-tree files and
directories according core.sharedRepository setting by mistake and
for a long time, which has been corrected.

* mt/do-not-use-scld-in-working-tree:
  apply: don't use core.sharedRepository to create working tree files

3 years agoMerge branch 'ds/maintenance-part-3'
Junio C Hamano [Tue, 8 Dec 2020 23:11:19 +0000 (15:11 -0800)] 
Merge branch 'ds/maintenance-part-3'

"git maintenance" command had trouble working in a directory whose
pathname contained an ERE metacharacter like '+'.

* ds/maintenance-part-3:
  maintenance: use 'git config --fixed-value'

3 years agoMerge branch 'ds/maintenance-part-2'
Junio C Hamano [Tue, 8 Dec 2020 23:11:19 +0000 (15:11 -0800)] 
Merge branch 'ds/maintenance-part-2'

Test fix.

* ds/maintenance-part-2:
  t7900: speed up expensive test

3 years agoMerge branch 'ds/config-literal-value'
Junio C Hamano [Tue, 8 Dec 2020 23:11:19 +0000 (15:11 -0800)] 
Merge branch 'ds/config-literal-value'

Various subcommands of "git config" that takes value_regex
learn the "--literal-value" option to take the value_regex option
as a literal string.

* ds/config-literal-value:
  config doc: value-pattern is not necessarily a regexp
  config: implement --fixed-value with --get*
  config: plumb --fixed-value into config API
  config: add --fixed-value option, un-implemented
  t1300: add test for --replace-all with value-pattern
  t1300: test "set all" mode with value-pattern
  config: replace 'value_regex' with 'value_pattern'
  config: convert multi_replace to flags

3 years agoMerge branch 'ds/maintenance-part-1'
Junio C Hamano [Tue, 8 Dec 2020 23:11:19 +0000 (15:11 -0800)] 
Merge branch 'ds/maintenance-part-1'

Build consistency fix.

* ds/maintenance-part-1:
  Makefile: mark git-maintenance as a builtin

3 years agoMerge branch 'tb/idx-midx-race-fix'
Junio C Hamano [Tue, 8 Dec 2020 23:11:18 +0000 (15:11 -0800)] 
Merge branch 'tb/idx-midx-race-fix'

Processes that access packdata while the .idx file gets removed
(e.g. while repacking) did not fail or fall back gracefully as they
could.

* tb/idx-midx-race-fix:
  midx.c: protect against disappearing packs
  packfile.c: protect against disappearing indexes

3 years agoMerge branch 'ps/update-ref-multi-transaction'
Junio C Hamano [Tue, 8 Dec 2020 23:11:17 +0000 (15:11 -0800)] 
Merge branch 'ps/update-ref-multi-transaction'

"git update-ref --stdin" learns to take multiple transactions in a
single session.

* ps/update-ref-multi-transaction:
  update-ref: disallow "start" for ongoing transactions
  p1400: use `git-update-ref --stdin` to test multiple transactions
  update-ref: allow creation of multiple transactions
  t1400: avoid touching refs on filesystem

3 years agoMerge branch 'js/add-i-color-fix'
Junio C Hamano [Tue, 8 Dec 2020 23:11:17 +0000 (15:11 -0800)] 
Merge branch 'js/add-i-color-fix'

"git add -i" failed to honor custom colors configured to show
patches, which has been corrected.

* js/add-i-color-fix:
  add -i: verify in the tests that colors can be overridden
  add -p: prefer color.diff.context over color.diff.plain
  add -i (Perl version): color header to match the C version
  add -i (built-in): use the same indentation as the Perl version
  add -p (built-in): do not color the progress indicator separately
  add -i (built-in): use correct names to load color.diff.* config
  add -i (built-in): prevent the `reset` "color" from being configured
  add -i: use `reset_color` consistently
  add -p (built-in): imitate `xdl_format_hunk_hdr()` generating hunk headers
  add -i (built-in): send error messages to stderr
  add -i (built-in): do show an error message for incorrect inputs

3 years agoMerge branch 'jt/trace-error-on-warning'
Junio C Hamano [Tue, 8 Dec 2020 23:11:17 +0000 (15:11 -0800)] 
Merge branch 'jt/trace-error-on-warning'

Like die() and error(), a call to warning() will also trigger a
trace2 event.

* jt/trace-error-on-warning:
  usage: add trace2 entry upon warning()

3 years agopack-bitmap-write: better reuse bitmaps
Derrick Stolee [Tue, 8 Dec 2020 22:05:30 +0000 (17:05 -0500)] 
pack-bitmap-write: better reuse bitmaps

If the old bitmap file contains a bitmap for a given commit, then that
commit does not need help from intermediate commits in its history to
compute its final bitmap. Eject that commit from the walk and insert it
into a separate list of reusable commits that are eventually stored in
the list of commits for computing bitmaps.

This helps the repeat bitmap computation task, even if the selected
commits shift drastically. This helps when a previously-bitmapped commit
exists in the first-parent history of a newly-selected commit. Since we
stop the walk at these commits and we use a first-parent walk, it is
harder to walk "around" these bitmapped commits. It's not impossible,
but we can greatly reduce the computation time for many selected
commits.

             |   runtime (sec)    |   peak heap (GB)   |
             |                    |                    |
             |   from  |   with   |   from  |   with   |
             | scratch | existing | scratch | existing |
  -----------+---------+----------+---------+-----------
  last patch |  88.478 |   53.218 |   2.157 |    2.224 |
  this patch |  86.681 |   16.164 |   2.157 |    2.222 |

Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
Signed-off-by: Taylor Blau <me@ttaylorr.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
3 years agopack-bitmap-write: relax unique revwalk condition
Derrick Stolee [Tue, 8 Dec 2020 22:05:26 +0000 (17:05 -0500)] 
pack-bitmap-write: relax unique revwalk condition

The previous commits improved the bitmap computation process for very
long, linear histories with many refs by removing quadratic growth in
how many objects were walked. The strategy of computing "intermediate
commits" using bitmasks for which refs can reach those commits
partitioned the poset of reachable objects so each part could be walked
exactly once. This was effective for linear histories.

However, there was a (significant) drawback: wide histories with many
refs had an explosion of memory costs to compute the commit bitmasks
during the exploration that discovers these intermediate commits. Since
these wide histories are unlikely to repeat walking objects, the benefit
of walking objects multiple times was not expensive before. But now, the
commit walk *before computing bitmaps* is incredibly expensive.

In an effort to discover a happy medium, this change reduces the walk
for intermediate commits to only the first-parent history. This focuses
the walk on how the histories converge, which still has significant
reduction in repeat object walks. It is still possible to create
quadratic behavior in this version, but it is probably less likely in
realistic data shapes.

Here is some data taken on a fresh clone of the kernel:

             |   runtime (sec)    |   peak heap (GB)   |
             |                    |                    |
             |   from  |   with   |   from  |   with   |
             | scratch | existing | scratch | existing |
  -----------+---------+----------+---------+-----------
    original |  64.044 |   83.241 |   2.088 |    2.194 |
  last patch |  45.049 |   37.624 |   2.267 |    2.334 |
  this patch |  88.478 |   53.218 |   2.157 |    2.224 |

Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
Helped-by: Johannes Schindelin <Johannes.Schindelin@gmx.de>
Signed-off-by: Taylor Blau <me@ttaylorr.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
3 years agopack-bitmap-write: use existing bitmaps
Derrick Stolee [Tue, 8 Dec 2020 22:05:21 +0000 (17:05 -0500)] 
pack-bitmap-write: use existing bitmaps

When constructing new bitmaps, we perform a commit and tree walk in
fill_bitmap_commit() and fill_bitmap_tree(). This walk would benefit
from using existing bitmaps when available. We must track the existing
bitmaps and translate them into the new object order, but this is
generally faster than parsing trees.

In fill_bitmap_commit(), we must reorder thing somewhat. The priority
queue walks commits from newest-to-oldest, which means we correctly stop
walking when reaching a commit with a bitmap. However, if we walk trees
interleaved with the commits, then we might be parsing trees that are
actually part of a re-used bitmap. To avoid over-walking trees, add them
to a LIFO queue and walk them after exploring commits completely.

On git.git, this reduces a second immediate bitmap computation from 2.0s
to 1.0s. On linux.git, we go from 32s to 22s. On chromium's fork
network, we go from 227s to 198s.

Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
Signed-off-by: Taylor Blau <me@ttaylorr.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
3 years agopack-bitmap: factor out 'add_commit_to_bitmap()'
Taylor Blau [Tue, 8 Dec 2020 22:05:14 +0000 (17:05 -0500)] 
pack-bitmap: factor out 'add_commit_to_bitmap()'

'find_objects()' currently needs to interact with the bitmaps khash
pretty closely. To make 'find_objects()' read a little more
straightforwardly, remove some of the khash-level details into a new
function that describes what it does: 'add_commit_to_bitmap()'.

Signed-off-by: Taylor Blau <me@ttaylorr.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
3 years agopack-bitmap: factor out 'bitmap_for_commit()'
Taylor Blau [Tue, 8 Dec 2020 22:05:09 +0000 (17:05 -0500)] 
pack-bitmap: factor out 'bitmap_for_commit()'

A couple of callers within pack-bitmap.c duplicate logic to lookup a
given object id in the bitamps khash. Factor this out into a new
function, 'bitmap_for_commit()' to reduce some code duplication.

Make this new function non-static, since it will be used in later
commits from outside of pack-bitmap.c.

Signed-off-by: Taylor Blau <me@ttaylorr.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
3 years agopack-bitmap-write: ignore BITMAP_FLAG_REUSE
Jeff King [Tue, 8 Dec 2020 22:04:34 +0000 (17:04 -0500)] 
pack-bitmap-write: ignore BITMAP_FLAG_REUSE

The on-disk bitmap format has a flag to mark a bitmap to be "reused".
This is a rather curious feature, and works like this:

  - a run of pack-objects would decide to mark the last 80% of the
    bitmaps it generates with the reuse flag

  - the next time we generate bitmaps, we'd see those reuse flags from
    the last run, and mark those commits as special:

      - we'd be more likely to select those commits to get bitmaps in
        the new output

      - when generating the bitmap for a selected commit, we'd reuse the
        old bitmap as-is (rearranging the bits to match the new pack, of
        course)

However, neither of these behaviors particularly makes sense.

Just because a commit happened to be bitmapped last time does not make
it a good candidate for having a bitmap this time. In particular, we may
choose bitmaps based on how recent they are in history, or whether a ref
tip points to them, and those things will change. We're better off
re-considering fresh which commits are good candidates.

Reusing the existing bitmap _is_ a reasonable thing to do to save
computation. But only reusing exact bitmaps is a weak form of this. If
we have an old bitmap for A and now want a new bitmap for its child, we
should be able to compute that only by looking at trees and that are new
to the child. But this code would consider only exact reuse (which is
perhaps why it was eager to select those commits in the first place).

Furthermore, the recent switch to the reverse-edge algorithm for
generating bitmaps dropped this optimization entirely (and yet still
performs better).

So let's do a few cleanups:

 - drop the whole "reusing bitmaps" phase of generating bitmaps. It's
   not helping anything, and is mostly unused code (or worse, code that
   is using CPU but not doing anything useful)

 - drop the use of the on-disk reuse flag to select commits to bitmap

 - stop setting the on-disk reuse flag in bitmaps we generate (since
   nothing respects it anymore)

We will keep a few innards of the reuse code, which will help us
implement a more capable version of the "reuse" optimization:

 - simplify rebuild_existing_bitmaps() into a function that only builds
   the mapping of bits between the old and new orders, but doesn't
   actually convert any bitmaps

 - make rebuild_bitmap() public; we'll call it lazily to convert bitmaps
   as we traverse (using the mapping created above)

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Taylor Blau <me@ttaylorr.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
3 years agopack-bitmap-write: build fewer intermediate bitmaps
Derrick Stolee [Tue, 8 Dec 2020 22:04:30 +0000 (17:04 -0500)] 
pack-bitmap-write: build fewer intermediate bitmaps

The bitmap_writer_build() method calls bitmap_builder_init() to
construct a list of commits reachable from the selected commits along
with a "reverse graph". This reverse graph has edges pointing from a
commit to other commits that can reach that commit. After computing a
reachability bitmap for a commit, the values in that bitmap are then
copied to the reachability bitmaps across the edges in the reverse
graph.

We can now relax the role of the reverse graph to greatly reduce the
number of intermediate reachability bitmaps we compute during this
reverse walk. The end result is that we walk objects the same number of
times as before when constructing the reachability bitmaps, but we also
spend much less time copying bits between bitmaps and have much lower
memory pressure in the process.

The core idea is to select a set of "important" commits based on
interactions among the sets of commits reachable from each selected commit.

The first technical concept is to create a new 'commit_mask' member in the
bb_commit struct. Note that the selected commits are provided in an
ordered array. The first thing to do is to mark the ith bit in the
commit_mask for the ith selected commit. As we walk the commit-graph, we
copy the bits in a commit's commit_mask to its parents. At the end of
the walk, the ith bit in the commit_mask for a commit C stores a boolean
representing "The ith selected commit can reach C."

As we walk, we will discover non-selected commits that are important. We
will get into this later, but those important commits must also receive
bit positions, growing the width of the bitmasks as we walk. At the true
end of the walk, the ith bit means "the ith _important_ commit can reach
C."

MAXIMAL COMMITS
---------------

We use a new 'maximal' bit in the bb_commit struct to represent whether
a commit is important or not. The term "maximal" comes from the
partially-ordered set of commits in the commit-graph where C >= P if P
is a parent of C, and then extending the relationship transitively.
Instead of taking the maximal commits across the entire commit-graph, we
instead focus on selecting each commit that is maximal among commits
with the same bits on in their commit_mask. This definition is
important, so let's consider an example.

Suppose we have three selected commits A, B, and C. These are assigned
bitmasks 100, 010, and 001 to start. Each of these can be marked as
maximal immediately because they each will be the uniquely maximal
commit that contains their own bit. Keep in mind that that these commits
may have different bitmasks after the walk; for example, if B can reach
C but A cannot, then the final bitmask for C is 011. Even in these
cases, C would still be a maximal commit among all commits with the
third bit on in their masks.

Now define sets X, Y, and Z to be the sets of commits reachable from A,
B, and C, respectively. The intersections of these sets correspond to
different bitmasks:

 * 100: X - (Y union Z)
 * 010: Y - (X union Z)
 * 001: Z - (X union Y)
 * 110: (X intersect Y) - Z
 * 101: (X intersect Z) - Y
 * 011: (Y intersect Z) - X
 * 111: X intersect Y intersect Z

This can be visualized with the following Hasse diagram:

100    010    001
         | \  /   \  / |
         |  \/     \/  |
         |  /\     /\  |
         | /  \   /  \ |
        110    101    011
          \___  |  ___/
              \ | /
               111

Some of these bitmasks may not be represented, depending on the topology
of the commit-graph. In fact, we are counting on it, since the number of
possible bitmasks is exponential in the number of selected commits, but
is also limited by the total number of commits. In practice, very few
bitmasks are possible because most commits converge on a common "trunk"
in the commit history.

With this three-bit example, we wish to find commits that are maximal
for each bitmask. How can we identify this as we are walking?

As we walk, we visit a commit C. Since we are walking the commits in
topo-order, we know that C is visited after all of its children are
visited. Thus, when we get C from the revision walk we inspect the
'maximal' property of its bb_data and use that to determine if C is truly
important. Its commit_mask is also nearly final. If C is not one of the
originally-selected commits, then assign a bit position to C (by
incrementing num_maximal) and set that bit on in commit_mask. See
"MULTIPLE MAXIMAL COMMITS" below for more detail on this.

Now that the commit C is known to be maximal or not, consider each
parent P of C. Compute two new values:

 * c_not_p : true if and only if the commit_mask for C contains a bit
             that is not contained in the commit_mask for P.

 * p_not_c : true if and only if the commit_mask for P contains a bit
             that is not contained in the commit_mask for P.

If c_not_p is false, then P already has all of the bits that C would
provide to its commit_mask. In this case, move on to other parents as C
has nothing to contribute to P's state that was not already provided by
other children of P.

We continue with the case that c_not_p is true. This means there are
bits in C's commit_mask to copy to P's commit_mask, so use bitmap_or()
to add those bits.

If p_not_c is also true, then set the maximal bit for P to one. This means
that if no other commit has P as a parent, then P is definitely maximal.
This is because no child had the same bitmask. It is important to think
about the maximal bit for P at this point as a temporary state: "P is
maximal based on current information."

In contrast, if p_not_c is false, then set the maximal bit for P to
zero. Further, clear all reverse_edges for P since any edges that were
previously assigned to P are no longer important. P will gain all
reverse edges based on C.

The final thing we need to do is to update the reverse edges for P.
These reverse edges respresent "which closest maximal commits
contributed bits to my commit_mask?" Since C contributed bits to P's
commit_mask in this case, C must add to the reverse edges of P.

If C is maximal, then C is a 'closest' maximal commit that contributed
bits to P. Add C to P's reverse_edges list.

Otherwise, C has a list of maximal commits that contributed bits to its
bitmask (and this list is exactly one element). Add all of these items
to P's reverse_edges list. Be careful to ignore duplicates here.

After inspecting all parents P for a commit C, we can clear the
commit_mask for C. This reduces the memory load to be limited to the
"width" of the commit graph.

Consider our ABC/XYZ example from earlier and let's inspect the state of
the commits for an interesting bitmask, say 011. Suppose that D is the
only maximal commit with this bitmask (in the first three bits). All
other commits with bitmask 011 have D as the only entry in their
reverse_edges list. D's reverse_edges list contains B and C.

COMPUTING REACHABILITY BITMAPS
------------------------------

Now that we have our definition, let's zoom out and consider what
happens with our new reverse graph when computing reachability bitmaps.
We walk the reverse graph in reverse-topo-order, so we visit commits
with largest commit_masks first. After we compute the reachability
bitmap for a commit C, we push the bits in that bitmap to each commit D
in the reverse edge list for C. Then, when we finally visit D we already
have the bits for everything reachable from maximal commits that D can
reach and we only need to walk the objects in the set-difference.

In our ABC/XYZ example, when we finally walk for the commit A we only
need to walk commits with bitmask equal to A's bitmask. If that bitmask
is 100, then we are only walking commits in X - (Y union Z) because the
bitmap already contains the bits for objects reachable from (X intersect
Y) union (X intersect Z) (i.e. the bits from the reachability bitmaps
for the maximal commits with bitmasks 110 and 101).

The behavior is intended to walk each commit (and the trees that commit
introduces) at most once while allocating and copying fewer reachability
bitmaps. There is one caveat: what happens when there are multiple
maximal commits with the same bitmask, with respect to the initial set
of selected commits?

MULTIPLE MAXIMAL COMMITS
------------------------

Earlier, we mentioned that when we discover a new maximal commit, we
assign a new bit position to that commit and set that bit position to
one for that commit. This is absolutely important for interesting
commit-graphs such as git/git and torvalds/linux. The reason is due to
the existence of "butterflies" in the commit-graph partial order.

Here is an example of four commits forming a butterfly:

   I    J
   |\  /|
   | \/ |
   | /\ |
   |/  \|
   M    N
    \  /
     |/
     Q

Here, I and J both have parents M and N. In general, these do not need
to be exact parent relationships, but reachability relationships. The
most important part is that M and N cannot reach each other, so they are
independent in the partial order. If I had commit_mask 10 and J had
commit_mask 01, then M and N would both be assigned commit_mask 11 and
be maximal commits with the bitmask 11. Then, what happens when M and N
can both reach a commit Q? If Q is also assigned the bitmask 11, then it
is not maximal but is reachable from both M and N.

While this is not necessarily a deal-breaker for our abstract definition
of finding maximal commits according to a given bitmask, we have a few
issues that can come up in our larger picture of constructing
reachability bitmaps.

In particular, if we do not also consider Q to be a "maximal" commit,
then we will walk commits reachable from Q twice: once when computing
the reachability bitmap for M and another time when computing the
reachability bitmap for N. This becomes much worse if the topology
continues this pattern with multiple butterflies.

The solution has already been mentioned: each of M and N are assigned
their own bits to the bitmask and hence they become uniquely maximal for
their bitmasks. Finally, Q also becomes maximal and thus we do not need
to walk its commits multiple times. The final bitmasks for these commits
are as follows:

  I:10       J:01
   |\        /|
   | \ _____/ |
   | /\____   |
   |/      \  |
   M:111    N:1101
        \  /
       Q:1111

Further, Q's reverse edge list is { M, N }, while M and N both have
reverse edge list { I, J }.

PERFORMANCE MEASUREMENTS
------------------------

Now that we've spent a LOT of time on the theory of this algorithm,
let's show that this is actually worth all that effort.

To test the performance, use GIT_TRACE2_PERF=1 when running
'git repack -abd' in a repository with no existing reachability bitmaps.
This avoids any issues with keeping existing bitmaps to skew the
numbers.

Inspect the "building_bitmaps_total" region in the trace2 output to
focus on the portion of work that is affected by this change. Here are
the performance comparisons for a few repositories. The timings are for
the following versions of Git: "multi" is the timing from before any
reverse graph is constructed, where we might perform multiple
traversals. "reverse" is for the previous change where the reverse graph
has every reachable commit.  Finally "maximal" is the version introduced
here where the reverse graph only contains the maximal commits.

      Repository: git/git
           multi: 2.628 sec
         reverse: 2.344 sec
         maximal: 2.047 sec

      Repository: torvalds/linux
           multi: 64.7 sec
         reverse: 205.3 sec
         maximal: 44.7 sec

So in all cases we've not only recovered any time lost to switching to
the reverse-edge algorithm, but we come out ahead of "multi" in all
cases. Likewise, peak heap has gone back to something reasonable:

      Repository: torvalds/linux
           multi: 2.087 GB
         reverse: 3.141 GB
         maximal: 2.288 GB

While I do not have access to full fork networks on GitHub, Peff has run
this algorithm on the chromium/chromium fork network and reported a
change from 3 hours to ~233 seconds. That network is particularly
beneficial for this approach because it has a long, linear history along
with many tags. The "multi" approach was obviously quadratic and the new
approach is linear.

Helped-by: Jeff King <peff@peff.net>
Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
Helped-by: Johannes Schindelin <Johannes.Schindelin@gmx.de>
Signed-off-by: Taylor Blau <me@ttaylorr.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
3 years agopack-bitmap.c: check reads more aggressively when loading
Taylor Blau [Tue, 8 Dec 2020 22:04:26 +0000 (17:04 -0500)] 
pack-bitmap.c: check reads more aggressively when loading

Before 'load_bitmap_entries_v1()' reads an actual EWAH bitmap, it should
check that it can safely do so by ensuring that there are at least 6
bytes available to be read (four for the commit's index position, and
then two more for the xor offset and flags, respectively).

Likewise, it should check that the commit index it read refers to a
legitimate object in the pack.

The first fix catches a truncation bug that was exposed when testing,
and the second is purely precautionary.

There are some possible future improvements, not pursued here. They are:

  - Computing the correct boundary of the bitmap itself in the caller
    and ensuring that we don't read past it. This may or may not be
    worth it, since in a truncation situation, all bets are off: (is the
    trailer still there and the bitmap entries malformed, or is the
    trailer truncated?). The best we can do is try to read what's there
    as if it's correct data (and protect ourselves when it's obviously
    bogus).

  - Avoid the magic "6" by teaching read_be32() and read_u8() (both of
    which are custom helpers for this function) to check sizes before
    advancing the pointers.

  - Adding more tests in this area. Testing these truncation situations
    are remarkably fragile to even subtle changes in the bitmap
    generation. So, the resulting tests are likely to be quite brittle.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Taylor Blau <me@ttaylorr.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
3 years agopack-bitmap-write: rename children to reverse_edges
Derrick Stolee [Tue, 8 Dec 2020 22:04:22 +0000 (17:04 -0500)] 
pack-bitmap-write: rename children to reverse_edges

The bitmap_builder_init() method walks the reachable commits in
topological order and constructs a "reverse graph" along the way. At the
moment, this reverse graph contains an edge from commit A to commit B if
and only if A is a parent of B. Thus, the name "children" is appropriate
for for this reverse graph.

In the next change, we will repurpose the reverse graph to not be
directly-adjacent commits in the commit-graph, but instead a more
abstract relationship. The previous changes have already incorporated
the necessary updates to fill_bitmap_commit() that allow these edges to
not be immediate children.

Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
Signed-off-by: Taylor Blau <me@ttaylorr.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
3 years agot5310: add branch-based checks
Derrick Stolee [Tue, 8 Dec 2020 22:04:17 +0000 (17:04 -0500)] 
t5310: add branch-based checks

The current rev-list tests that check the bitmap data only work on HEAD
instead of multiple branches. Expand the test cases to handle both
'master' and 'other' branches.

Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
Helped-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Taylor Blau <me@ttaylorr.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
3 years agocommit: implement commit_list_contains()
Derrick Stolee [Tue, 8 Dec 2020 22:04:13 +0000 (17:04 -0500)] 
commit: implement commit_list_contains()

It can be helpful to check if a commit_list contains a commit. Use
pointer equality, assuming lookup_commit() was used.

Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
Signed-off-by: Taylor Blau <me@ttaylorr.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
3 years agobitmap: implement bitmap_is_subset()
Derrick Stolee [Tue, 8 Dec 2020 22:04:08 +0000 (17:04 -0500)] 
bitmap: implement bitmap_is_subset()

The bitmap_is_subset() function checks if the 'self' bitmap contains any
bitmaps that are not on in the 'other' bitmap. Up until this patch, it
had a declaration, but no implementation or callers. A subsequent patch
will want this function, so implement it here.

Helped-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
Signed-off-by: Taylor Blau <me@ttaylorr.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
3 years agopack-bitmap-write: fill bitmap with commit history
Derrick Stolee [Tue, 8 Dec 2020 22:04:03 +0000 (17:04 -0500)] 
pack-bitmap-write: fill bitmap with commit history

The current implementation of bitmap_writer_build() creates a
reachability bitmap for every walked commit. After computing a bitmap
for a commit, those bits are pushed to an in-progress bitmap for its
children.

fill_bitmap_commit() assumes the bits corresponding to objects
reachable from the parents of a commit are already set. This means that
when visiting a new commit, we only have to walk the objects reachable
between it and any of its parents.

A future change to bitmap_writer_build() will relax this condition so
not all parents have their bits set. Prepare for that by having
'fill_bitmap_commit()' walk parents until reaching commits whose bits
are already set. Then, walk the trees for these commits as well.

This has no functional change with the current implementation of
bitmap_writer_build().

Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
Signed-off-by: Taylor Blau <me@ttaylorr.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
3 years agopack-bitmap-write: pass ownership of intermediate bitmaps
Jeff King [Tue, 8 Dec 2020 22:03:59 +0000 (17:03 -0500)] 
pack-bitmap-write: pass ownership of intermediate bitmaps

Our algorithm to generate reachability bitmaps walks through the commit
graph from the bottom up, passing bitmap data from each commit to its
descendants. For a linear stretch of history like:

  A -- B -- C

our sequence of steps is:

  - compute the bitmap for A by walking its trees, etc

  - duplicate A's bitmap as a starting point for B; we can now free A's
    bitmap, since we only needed it as an intermediate result

  - OR in any extra objects that B can reach into its bitmap

  - duplicate B's bitmap as a starting point for C; likewise, free B's
    bitmap

  - OR in objects for C, and so on...

Rather than duplicating bitmaps and immediately freeing the original, we
can just pass ownership from commit to commit. Note that this doesn't
always work:

  - the recipient may be a merge which already has an intermediate
    bitmap from its other ancestor. In that case we have to OR our
    result into it. Note that the first ancestor to reach the merge does
    get to pass ownership, though.

  - we may have multiple children; we can only pass ownership to one of
    them

However, it happens often enough and copying bitmaps is expensive enough
that this provides a noticeable speedup. On a clone of linux.git, this
reduces the time to generate bitmaps from 205s to 70s. This is about the
same amount of time it took to generate bitmaps using our old "many
traversals" algorithm (the previous commit measures the identical
scenario as taking 63s). It unfortunately provides only a very modest
reduction in the peak memory usage, though.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Taylor Blau <me@ttaylorr.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
3 years agopack-bitmap-write: reimplement bitmap writing
Jeff King [Tue, 8 Dec 2020 22:03:55 +0000 (17:03 -0500)] 
pack-bitmap-write: reimplement bitmap writing

The bitmap generation code works by iterating over the set of commits
for which we plan to write bitmaps, and then for each one performing a
traditional traversal over the reachable commits and trees, filling in
the bitmap. Between two traversals, we can often reuse the previous
bitmap result as long as the first commit is an ancestor of the second.
However, our worst case is that we may end up doing "n" complete
complete traversals to the root in order to create "n" bitmaps.

In a real-world case (the shared-storage repo consisting of all GitHub
forks of chromium/chromium), we perform very poorly: generating bitmaps
takes ~3 hours, whereas we can walk the whole object graph in ~3
minutes.

This commit completely rewrites the algorithm, with the goal of
accessing each object only once. It works roughly like this:

  - generate a list of commits in topo-order using a single traversal

  - invert the edges of the graph (so have parents point at their
    children)

  - make one pass in reverse topo-order, generating a bitmap for each
    commit and passing the result along to child nodes

We generate correct results because each node we visit has already had
all of its ancestors added to the bitmap. And we make only two linear
passes over the commits.

We also visit each tree usually only once. When filling in a bitmap, we
don't bother to recurse into trees whose bit is already set in the
bitmap (since we know we've already done so when setting their bit).
That means that if commit A references tree T, none of its descendants
will need to open T again. I say "usually", though, because it is
possible for a given tree to be mentioned in unrelated parts of history
(e.g., cherry-picking to a parallel branch).

So we've accomplished our goal, and the resulting algorithm is pretty
simple to understand. But there are some downsides, at least with this
initial implementation:

  - we no longer reuse the results of any on-disk bitmaps when
    generating. So we'd expect to sometimes be slower than the original
    when bitmaps already exist. However, this is something we'll be able
    to add back in later.

  - we use much more memory. Instead of keeping one bitmap in memory at
    a time, we're passing them up through the graph. So our memory use
    should scale with the graph width (times the size of a bitmap).

So how does it perform?

For a clone of linux.git, generating bitmaps from scratch with the old
algorithm took 63s. Using this algorithm it takes 205s. Which is much
worse, but _might_ be acceptable if it behaved linearly as the size
grew. It also increases peak heap usage by ~1G. That's not impossibly
large, but not encouraging.

On the complete fork-network of torvalds/linux, it increases the peak
RAM usage by 40GB. Yikes. (I forgot to record the time it took, but the
memory usage was too much to consider this reasonable anyway).

On the complete fork-network of chromium/chromium, I ran out of memory
before succeeding. Some back-of-the-envelope calculations indicate it
would need 80+GB to complete.

So at this stage, we've managed to make things much worse. But because
of the way this new algorithm is structured, there are a lot of
opportunities for optimization on top. We'll start implementing those in
the follow-on patches.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Taylor Blau <me@ttaylorr.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
3 years agoewah: add bitmap_dup() function
Jeff King [Tue, 8 Dec 2020 22:03:50 +0000 (17:03 -0500)] 
ewah: add bitmap_dup() function

There's no easy way to make a copy of a bitmap. Obviously a caller can
iterate over the bits and set them one by one in a new bitmap, but we
can go much faster by copying whole words with memcpy().

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