Merge branch 'nr/diff-highlight-indent-fix'

Code cleanup.

* nr/diff-highlight-indent-fix:
  diff-highlight: fix a whitespace nit

Merge branch 'mb/clarify-zsh-completion-doc'

The installation instruction for zsh completion script (in
contrib/) has been a bit improved.

* mb/clarify-zsh-completion-doc:
  completion: clarify installation instruction for zsh

path.c: don't call the match function without value in trie_find()

'logs/refs' is not a working tree-specific path, but since commit
b9317d55a3 (Make sure refs/rewritten/ is per-worktree, 2019-03-07)
'git rev-parse --git-path' has been returning a bogus path if a
trailing '/' is present:

  $ git -C WT/ rev-parse --git-path logs/refs --git-path logs/refs/
  /home/szeder/src/git/.git/logs/refs
  /home/szeder/src/git/.git/worktrees/WT/logs/refs/

We use a trie data structure to efficiently decide whether a path
belongs to the common dir or is working tree-specific.  As it happens
b9317d55a3 triggered a bug that is as old as the trie implementation
itself, added in 4e09cf2acf (path: optimize common dir checking,
2015-08-31).

  - According to the comment describing trie_find(), it should only
    call the given match function 'fn' for a "/-or-\0-terminated
    prefix of the key for which the trie contains a value".  This is
    not true: there are three places where trie_find() calls the match
    function, but one of them is missing the check for value's
    existence.

  - b9317d55a3 added two new keys to the trie: 'logs/refs/rewritten'
    and 'logs/refs/worktree', next to the already existing
    'logs/refs/bisect'.  This resulted in a trie node with the path
    'logs/refs/', which didn't exist before, and which doesn't have a
    value attached.  A query for 'logs/refs/' finds this node and then
    hits that one callsite of the match function which doesn't check
    for the value's existence, and thus invokes the match function
    with NULL as value.

  - When the match function check_common() is invoked with a NULL
    value, it returns 0, which indicates that the queried path doesn't
    belong to the common directory, ultimately resulting the bogus
    path shown above.

Add the missing condition to trie_find() so it will never invoke the
match function with a non-existing value.  check_common() will then no
longer have to check that it got a non-NULL value, so remove that
condition.

I believe that there are no other paths that could cause similar bogus
output.  AFAICT the only other key resulting in the match function
being called with a NULL value is 'co' (because of the keys 'common'
and 'config').  However, as they are not in a directory that belongs
to the common directory the resulting working tree-specific path is
expected.

Signed-off-by: SZEDER Gábor <redacted>
Signed-off-by: Junio C Hamano <redacted>

path.c: clarify two field names in 'struct common_dir'

An array of 'struct common_dir' instances is used to specify whether
various paths in $GIT_DIR are specific to a worktree, or are common,
i.e. belong to main worktree.  The names of two fields in this
struct are somewhat confusing or ambigious:

  - The path is recorded in the struct's 'dirname' field, even though
    several entries are regular files e.g. 'gc.pid', 'packed-refs',
    etc.

    Rename this field to 'path' to reduce confusion.

  - The field 'exclude' tells whether the path is excluded...  from
    where?  Excluded from the common dir or from the worktree?  It
    means the former, but it's ambigious.

    Rename this field to 'is_common' to make it unambigious what it
    means.  This, however, means the exact opposite of what 'exclude'
    meant, so we have to negate the field's value in all entries as
    well.

The diff is best viewed with '--color-words'.

Signed-off-by: SZEDER Gábor <redacted>
Signed-off-by: Junio C Hamano <redacted>

path.c: mark 'logs/HEAD' in 'common_list' as file

'logs/HEAD', i.e. HEAD's reflog, is a file, but its entry in
'common_list' has the 'is_dir' bit set.

Unset that bit to make it consistent with what 'logs/HEAD' is supposed
to be.

This doesn't make a difference in behavior: check_common() is the only
function that looks at the 'is_dir' bit, and that function either
returns 0, or '!exclude', which for 'logs/HEAD' results in 0 as well.

Signed-off-by: SZEDER Gábor <redacted>
Signed-off-by: Junio C Hamano <redacted>

path.c: clarify trie_find()'s in-code comment

A fairly long comment describes trie_find()'s behavior and shows
examples, but it's slightly incomplete/inaccurate.  Update this
comment to specify how trie_find() handles a negative return value
from the given match function.

Furthermore, update the list of examples to include not only two but
three levels of path components.  This makes the examples slightly
more complicated, but it can illustrate the behavior in more corner
cases.

Finally, basically everything refers to the data stored for a key as
"value", with two confusing exceptions:

  - The type definition of the match function calls its corresponding
    parameter 'data'.
    Rename that parameter to 'value'.  (check_common(), the only
    function of this type already calls it 'value').

  - The table of examples above trie_find() has a "val from node"
    column, which has nothing to do with the value stored in the trie:
    it's a "prefix of the key for which the trie contains a value"
    that led to that node.
    Rename that column header to "prefix to node".

Note that neither the original nor the updated description and
examples correspond 100% to the current implementation, because the
implementation is a bit buggy, but the comment describes the desired
behavior.  The bug will be fixed in the last patch of this series.

Signed-off-by: SZEDER Gábor <redacted>
Signed-off-by: Junio C Hamano <redacted>

Documentation: mention more worktree-specific exceptions

If a directory in $GIT_DIR is overridden when $GIT_COMMON_DIR is set,
then usually all paths within that directory are overridden as well.
There are a couple of exceptions, though, and two of them, namely
'refs/rewritten' and 'logs/HEAD' are not mentioned in
'gitrepository-layout'.  Document them as well.

Signed-off-by: SZEDER Gábor <redacted>
Signed-off-by: Junio C Hamano <redacted>

multi-pack-index: add [--[no-]progress] option.

Add the --[no-]progress option to git multi-pack-index.
Pass the MIDX_PROGRESS flag to the subcommand functions
when progress should be displayed by multi-pack-index.
The progress feature was added to 'verify' in 144d703
("multi-pack-index: report progress during 'verify'", 2018-09-13)
but some subcommands were not updated to display progress, and
the ability to opt-out was overlooked.

Signed-off-by: William Baker <redacted>
Signed-off-by: Junio C Hamano <redacted>

midx: honor the MIDX_PROGRESS flag in midx_repack

Update midx_repack to only display progress when
the MIDX_PROGRESS flag is set.

Signed-off-by: William Baker <redacted>
Signed-off-by: Junio C Hamano <redacted>

midx: honor the MIDX_PROGRESS flag in verify_midx_file

Update verify_midx_file to only display progress when
the MIDX_PROGRESS flag is set.

Signed-off-by: William Baker <redacted>
Signed-off-by: Junio C Hamano <redacted>

midx: add progress to expire_midx_packs

Add progress to expire_midx_packs.  Progress is
displayed when the MIDX_PROGRESS flag is set.

Signed-off-by: William Baker <redacted>
Signed-off-by: Junio C Hamano <redacted>

midx: add progress to write_midx_file

Add progress to write_midx_file.  Progress is displayed
when the MIDX_PROGRESS flag is set.

Signed-off-by: William Baker <redacted>
Signed-off-by: Junio C Hamano <redacted>

midx: add MIDX_PROGRESS flag

Add the MIDX_PROGRESS flag and update the
write|verify|expire|repack functions in midx.h
to accept a flags parameter.  The MIDX_PROGRESS
flag indicates whether the caller of the function
would like progress information to be displayed.
This patch only changes the method prototypes
and does not change the functionality. The
functionality change will be handled by a later patch.

Signed-off-by: William Baker <redacted>
Signed-off-by: Junio C Hamano <redacted>

ci(osx): use new location of the `perforce` cask

The Azure Pipelines builds are failing for macOS due to a change in the
location of the perforce cask. The command outputs the following error:

    + brew install caskroom/cask/perforce
    Error: caskroom/cask was moved. Tap homebrew/cask-cask instead.

So let's try to call `brew cask install perforce` first (which is what
that error message suggests, in a most round-about way).

Prior to 672f51cb we used to install the 'perforce' package with 'brew
install perforce' (note: no 'cask' in there). The justification for
672f51cb was that the command 'brew install perforce' simply stopped
working, after Homebrew folks decided that it's better to move the
'perforce' package to a "cask". Their justification for this move was
that 'brew install perforce' "can fail due to a checksum mismatch ...",
and casks can be installed without checksum verification. And indeed,
both 'brew cask install perforce' and 'brew install
caskroom/cask/perforce' printed something along the lines of:

  ==> No checksum defined for Cask perforce, skipping verification

It is unclear why 672f51cb used 'brew install caskroom/cask/perforce'
instead of 'brew cask install perforce'. It appears (by running both
commands on old Travis CI macOS images) that both commands worked all
the same already back then.

In any case, as the error message at the top of this commit message
shows, 'brew install caskroom/cask/perforce' has stopped working
recently, but 'brew cask install perforce' still does, so let's use
that.

CI servers are typically fresh virtual machines, but not always. To
accommodate for that, let's try harder if `brew cask install perforce`
fails, by specifically pulling the latest `master` of the
`homebrew-cask` repository.

This will still fail, of course, when `homebrew-cask` falls behind
Perforce's release schedule. But once it is updated, we can now simply
re-run the failed jobs and they will pick up that update.

As for updating `homebrew-cask`: the beginnings of automating this in
https://dev.azure.com/gitgitgadget/git/_build?definitionId=11&_a=summary
will be finished once the next Perforce upgrade comes around.

Helped-by: SZEDER Gábor <redacted>
Signed-off-by: Johannes Schindelin <redacted>
Signed-off-by: Derrick Stolee <redacted>
Signed-off-by: Junio C Hamano <redacted>

t604[236]: do not run setup in separate tests

Transform the setup "tests" to setup functions, and have the actual
tests call the setup functions.  Advantages:

  * Should make life easier for people working with webby CI/PR builds
    who have to abuse mice (and their own index finger as well) in
    order to switch from viewing one testcase to another.  Sounds
    awful; hopefully this will improve things for them.

  * Improves re-runnability: any failed test in any of these three
    files can now be re-run in isolation, e.g.
       ./t6042* --ver --imm -x --run=21
    whereas before it would require two tests to be specified to the
    --run argument, the other needing to be picked out as the relevant
    setup test from one or two tests before.

  * Importantly, this still keeps the "setup" and "test" sections
    somewhat separate to make it easier for readers to discern what is
    just ancillary setup and what the intent of the test is.

Signed-off-by: Elijah Newren <redacted>
Signed-off-by: Junio C Hamano <redacted>

merge-recursive: fix merging a subdirectory into the root directory

We allow renaming all entries in e.g. a directory named z/ into a
directory named y/ to be detected as a z/ -> y/ rename, so that if the
other side of history adds any files to the directory z/ in the mean
time, we can provide the hint that they should be moved to y/.

There is no reason to not allow 'y/' to be the root directory, but the
code did not handle that case correctly.  Add a testcase and the
necessary special checks to support this case.

Signed-off-by: Elijah Newren <redacted>
Signed-off-by: Junio C Hamano <redacted>

merge-recursive: clean up get_renamed_dir_portion()

Dscho noted a few things making this function hard to follow.
Restructure it a bit and add comments to make it easier to follow.  The
restructurings include:

  * There was a special case if-check at the end of the function
    checking whether someone just renamed a file within its original
    directory, meaning that there could be no directory rename involved.
    That check was slightly convoluted; it could be done in a more
    straightforward fashion earlier in the function, and can be done
    more cheaply too (no call to strncmp).

  * The conditions for advancing end_of_old and end_of_new before
    calling strchr were both confusing and unnecessary.  If either
    points at a '/', then they need to be advanced in order to find the
    next '/'.  If either doesn't point at a '/', then advancing them one
    char before calling strchr() doesn't hurt.  So, just rip out the
    if conditions and advance both before calling strchr().

Signed-off-by: Elijah Newren <redacted>
Signed-off-by: Junio C Hamano <redacted>

doc: am --show-current-patch gives an entire e-mail message

The existing wording gives an impression that it only gives the
contents of the $GIT_DIR/rebase-apply/patch file, i.e. the patch
proper, but the option actually emits the entire e-mail message
being processed (iow, one of the output files from "git mailsplit").

Signed-off-by: Junio C Hamano <redacted>

t7419: change test_must_fail to ! for grep

According to t/README, test_must_fail() should only be used to test for
failure in Git commands. Replace the invocations of
`test_must_fail grep` with `! grep`.

Signed-off-by: Denton Liu <redacted>
Signed-off-by: Junio C Hamano <redacted>

t4014: make output-directory tests self-contained

As noted by Gábor in [1], the new tests in edefc31873 ("format-patch:
create leading components of output directory", 2019-10-11) cannot be
run independently. Fix this.

[1] https://public-inbox.org/git/20191011144650.GM29845@szeder.dev/

Signed-off-by: Bert Wesarg <redacted>
Signed-off-by: Junio C Hamano <redacted>

Merge branch 'js/azure-pipelines-msvc'

* js/azure-pipelines-msvc:
  ci(visual-studio): actually run the tests in parallel
  ci(visual-studio): use strict compile flags, and optimization

ci(visual-studio): actually run the tests in parallel

Originally, the CI/PR builds that build and test using Visual Studio
were implemented imitating `linux-clang`, i.e. still using the
`Makefile`-based build infrastructure.

Later (but still before the patches made their way into git.git's
`master`), however, this was changed to generate Visual Studio project
files and build the binaries using `MSBuild`, as this reflects more
accurately how Visual Studio users would want to build Git (internally,
Visual Studio uses `MSBuild`, or at least something very similar).

During that transition, we needed to implement a new way to run the test
suite in parallel, as Visual Studio users typically will only have a Git
Bash available (which does not ship with `make` nor with support for
`prove`): we simply implemented a new test helper to run the test suite.

This helper even knows how to run the tests in parallel, but due to a
mistake on this developer's part, it was never turned on in the CI/PR
builds. This results in 2x-3x longer run times of the test phase.

Let's use the `--jobs=10` option to fix this.

Signed-off-by: Johannes Schindelin <redacted>
Signed-off-by: Junio C Hamano <redacted>

ci(visual-studio): use strict compile flags, and optimization

To make full use of the work that went into the Visual Studio build &
test jobs in our CI/PR builds, let's turn on strict compiler flags. This
will give us the benefit of Visual C's compiler warnings (which, at
times, seem to catch things that GCC does not catch, and vice versa).

While at it, also turn on optimization; It does not make sense to
produce binaries with debug information, and we can use any ounce of
speed that we get (because the test suite is particularly slow on
Windows, thanks to the need to run inside a Unix shell, which
requires us to use the POSIX emulation layer provided by MSYS2).

Signed-off-by: Johannes Schindelin <redacted>
Signed-off-by: Junio C Hamano <redacted>

l10n: it.po: update the Italian translation for Git 2.24.0

Signed-off-by: Alessandro Menti <redacted>

Merge branch 'master' of github.com:jnavila/git into git-po-master

* 'master' of github.com:jnavila/git:
  l10n: fr 2.24.0 rnd 1

Merge branch 'master' of github.com:alshopov/git-po into git-po-master

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

l10n: fr 2.24.0 rnd 1

Signed-off-by: Jean-Noël Avila <redacted>

Merge remote-tracking branch 'git-po/master' into git-po-master

* git-po/master:
  l10n: sv.po: Update Swedish translation (4674t0f0u)
  l10n: Update Catalan translation

l10n: git.pot: v2.24.0 round 1 (35 new, 16 removed)

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

Signed-off-by: Jiang Xin <redacted>

userdiff: fix some corner cases in dts regex

While reviewing some dts diffs recently I noticed that the hunk header
logic was failing to find the containing node. This is because the regex
doesn't consider properties that may span multiple lines, i.e.

property = <something>,
   <something_else>;

and it got hung up on comments inside nodes that look like the root node
because they start with '/*'. Add tests for these cases and update the
regex to find them. Maybe detecting the root node is too complicated but
forcing it to be a backslash with any amount of whitespace up to an open
bracket seemed OK. I tried to detect that a comment is in-between the
two parts but I wasn't happy so I just dropped it.

Cc: Rob Herring <redacted>
Cc: Frank Rowand <redacted>
Signed-off-by: Stephen Boyd <redacted>
Reviewed-by: Johannes Sixt <redacted>
Signed-off-by: Junio C Hamano <redacted>

builtin/blame.c: constants into bit shift format

We are looking at bitfield constants, and elsewhere in the Git source
code, such cases are handled via bit shift operators rather than octal
numbers, which also makes it easier to spot holes in the range
(if, say, 1<<5 was missing, it is easier to spot it between 1<<4
and 1<<6 than it is to spot a missing 040 between a 020 and a 0100).

Signed-off-by: Hariom Verma <redacted>
Signed-off-by: Junio C Hamano <redacted>

rebase: hide --preserve-merges option

Since --preserve-merges has been deprecated in favour of
--rebase-merges, mark this option as hidden so it no longer shows up in
the usage and completions.

Signed-off-by: Denton Liu <redacted>
Signed-off-by: Junio C Hamano <redacted>

Doc: Bundle file usage

Improve the command description, including paragraph spacing.

Git URLs can accept bundle files for fetch, pull and clone, include
in that section. Include git clone in the bundle usage description.
Correct the quoting of <git-rev-list-args>.

Detail the <git-rev-list-args> options for cloning a complete repo.

Signed-off-by: Philip Oakley <redacted>
Signed-off-by: Junio C Hamano <redacted>

parse_tag_buffer(): treat NULL tag pointer as parse error

When parsing a tag, we may end up with a NULL "tagged" field when
there's a type mismatch (e.g., the tag claims to point to object X as a
commit, but we previously saw X as a blob in the same process), but we
do not otherwise indicate a parse failure to the caller.

This is similar to the case discussed in the previous commit, where a
commit could end up with a NULL tree field: while slightly convenient
for callers who want to overlook a corrupt object, it means that normal
callers have to explicitly deal with this case (rather than just relying
on the return code from parsing). And most don't, leading to segfault
fixes like the one in c77722b3ea (use get_tagged_oid(), 2019-09-05).

Let's address this more centrally, by returning an error code from the
parse itself, which most callers would already notice (adventurous
callers are free to ignore the error and continue looking at the
struct).

This also covers the case where the tag contains a nonsensical "type"
field (there we produced a user-visible error but still returned success
to the caller; now we'll produce a slightly better message and return an
error).

Signed-off-by: Jeff King <redacted>
Signed-off-by: Junio C Hamano <redacted>

parse_commit_buffer(): treat lookup_tree() failure as parse error

If parsing a commit yields a valid tree oid, but we've seen that same
oid as a non-tree in the same process, the resulting commit struct will
end up with a NULL tree pointer, but not otherwise report a parsing
failure.

That's perhaps convenient for callers which want to operate on even
partially corrupt commits (e.g., by still looking at the parents). But
it leaves a potential trap for most callers, who now have to manually
check for a NULL tree. Most do not, and it's likely that there are
possible segfaults lurking. I say "possible" because there are many
candidates, and I don't think it's worth following through on
reproducing them when we can just fix them all in one spot. And
certainly we _have_ seen real-world cases, such as the one fixed by
806278dead (commit-graph.c: handle corrupt/missing trees, 2019-09-05).

Note that we can't quite drop the check in the caller added by that
commit yet, as there's some subtlety with repeated parsings (which will
be addressed in a future commit).

Signed-off-by: Jeff King <redacted>
Signed-off-by: Junio C Hamano <redacted>

parse_commit_buffer(): treat lookup_commit() failure as parse error

While parsing the parents of a commit, if we are able to parse an actual
oid but lookup_commit() fails on it (because we previously saw it in
this process as a different object type), we silently omit the parent
and do not report any error to the caller.

The caller has no way of knowing this happened, because even an empty
parent list is a valid parse result. As a result, it's possible to fool
our "rev-list" connectivity check into accepting a corrupted set of
objects.

There's a test for this case already in t6102, but unfortunately it has
a slight error. It creates a broken commit with a parent line pointing
to a blob, and then checks that rev-list notices the problem in two
cases:

  1. the "lone" case: we traverse the broken commit by itself (here we
     try to actually load the blob from disk and find out that it's not
     a commit)

  2. the "seen" case: we parse the blob earlier in the process, and then
     when calling lookup_commit() we realize immediately that it's not a
     commit

The "seen" variant for this test mistakenly parsed another commit
instead of the blob, meaning that we were actually just testing the
"lone" case again. Changing that reveals the breakage (and shows that
this fixes it).

Signed-off-by: Jeff King <redacted>
Signed-off-by: Junio C Hamano <redacted>

test-progress: fix test failures on big-endian systems

In 't0500-progress-display.sh' all tests running 'test-tool progress
--total=<N>' fail on big-endian systems, e.g. like this:

  + test-tool progress --total=3 Working hard
  [...]
  + test_i18ncmp expect out
  --- expect 2019-10-18 23:07:54.765523916 +0000
  +++ out 2019-10-18 23:07:54.773523916 +0000
  @@ -1,4 +1,2 @@
  -Working hard:  33% (1/3)<CR>
  -Working hard:  66% (2/3)<CR>
  -Working hard: 100% (3/3)<CR>
  -Working hard: 100% (3/3), done.
  +Working hard:   0% (1/12884901888)<CR>
  +Working hard:   0% (3/12884901888), done.

The reason for that bogus value is that '--total's parameter is parsed
via parse-options's OPT_INTEGER into a uint64_t variable [1], so the
two bits of 3 end up in the "wrong" bytes on big-endian systems
(12884901888 = 0x300000000).

Change the type of that variable from uint64_t to int, to match what
parse-options expects; in the tests of the progress output we won't
use values that don't fit into an int anyway.

[1] start_progress() expects the total number as an uint64_t, that's
    why I chose the same type when declaring the variable holding the
    value given on the command line.

Signed-off-by: SZEDER Gábor <redacted>
[jpag: Debian unstable/ppc64 (big-endian)]
Tested-By: John Paul Adrian Glaubitz <redacted>
[tz: Fedora s390x (big-endian)]
Tested-By: Todd Zullinger <redacted>
Signed-off-by: Junio C Hamano <redacted>

l10n: bg.po: Updated Bulgarian translation (4693)

Synced with 2.24-rc0

Signed-off-by: Alexander Shopov <redacted>

completion: clarify installation instruction for zsh

The original comment does not describe type of ~/.zsh/_git explicitly
and zsh does not warn or fail if a user create it as a dictionary.
So unexperienced users could be misled by the original comment.

There is a small update to clarify it.

Helped-by: Johannes Schindelin <redacted>
Helped-by: Junio C Hamano <redacted>
Signed-off-by: Maxim Belsky <redacted>
Signed-off-by: Junio C Hamano <redacted>

Git 2.24-rc0

Signed-off-by: Junio C Hamano <redacted>

Merge branch 'rs/remote-curl-use-argv-array'

Code cleanup.

* rs/remote-curl-use-argv-array:
  remote-curl: use argv_array in parse_push()

Merge branch 'rs/column-use-utf8-strnwidth'

Code cleanup.

* rs/column-use-utf8-strnwidth:
  column: use utf8_strnwidth() to strip out ANSI color escapes

Merge branch 'rs/http-push-simplify'

Code cleanup.

* rs/http-push-simplify:
  http-push: simplify deleting a list item

Merge branch 'jj/stash-reset-only-toplevel'

"git stash save" lost local changes to submodules, which has been
corrected.

* jj/stash-reset-only-toplevel:
  stash: avoid recursive hard reset on submodules

Merge branch 'bw/format-patch-o-create-leading-dirs'

"git format-patch -o <outdir>" did an equivalent of "mkdir <outdir>"
not "mkdir -p <outdir>", which is being corrected.

* bw/format-patch-o-create-leading-dirs:
  format-patch: create leading components of output directory

Merge branch 'bb/compat-util-comment-fix'

Code cleanup.

* bb/compat-util-comment-fix:
  git-compat-util: fix documentation syntax

Merge branch 'bb/utf8-wcwidth-cleanup'

Code cleanup.

* bb/utf8-wcwidth-cleanup:
  utf8: use ARRAY_SIZE() in git_wcwidth()

Merge branch 'dl/allow-running-cocci-verbosely'

Dev support update.

* dl/allow-running-cocci-verbosely:
  Makefile: respect $(V) in %.cocci.patch target

Merge branch 'dl/compat-cleanup'

Code formatting micronit fix.

* dl/compat-cleanup:
  pthread.h: manually align parameter lists

Merge branch 'ta/t1308-typofix'

Test fix.

* ta/t1308-typofix:
  t1308-config-set: fix a test that has a typo

Merge branch 'js/doc-stash-save'

Doc clarification.

* js/doc-stash-save:
  doc(stash): clarify the description of `save`

grep: avoid leak of chartables in PCRE2

94da9193a6 ("grep: add support for PCRE v2", 2017-06-01) introduced
a small memory leak visible with valgrind in t7813.

Complete the creation of a PCRE2 specific variable that was missing from
the original change and free the generated table just like it is done
for PCRE1.

Signed-off-by: Carlo Marcelo Arenas Belón <redacted>
Signed-off-by: Johannes Schindelin <redacted>
Signed-off-by: Junio C Hamano <redacted>

grep: make PCRE2 aware of custom allocator

94da9193a6 (grep: add support for PCRE v2, 2017-06-01) didn't include
a way to override the system allocator, and so it is incompatible with
custom allocators (e.g. nedmalloc). This problem became obvious when we
tried to plug a memory leak by `free()`ing a data structure allocated by
PCRE2, triggering a segfault in Windows (where we use nedmalloc by
default).

PCRE2 requires the use of a general context to override the allocator
and therefore, there is a lot more code needed than in PCRE1, including
a couple of wrapper functions.

Extend the grep API with a "destructor" that could be called to cleanup
any objects that were created and used globally.

Update `builtin/grep.c` to use that new API, but any other future users
should make sure to have matching `grep_init()`/`grep_destroy()` calls
if they are using the pattern matching functionality.

Move some of the logic that was before done per thread (in the workers)
into an earlier phase to avoid degrading performance, but as the use of
PCRE2 with custom allocators is better understood it is expected more of
its functions will be instructed to use the custom allocator as well as
was done in the original code[1] this work was based on.

[1] https://public-inbox.org/git/3397e6797f872aedd18c6d795f4976e1c579514b.1565005867.git.gitgitgadget@gmail.com/

Reported-by: Johannes Schindelin <redacted>
Signed-off-by: Carlo Marcelo Arenas Belón <redacted>
Signed-off-by: Johannes Schindelin <redacted>
Signed-off-by: Junio C Hamano <redacted>

grep: make PCRE1 aware of custom allocator

63e7e9d8b6 ("git-grep: Learn PCRE", 2011-05-09) didn't include a way
to override the system alocator, and so it is incompatible with
USE_NED_ALLOCATOR as reported by Dscho[1] (in similar code from PCRE2)

Note that nedmalloc, as well as other custom allocators like jemalloc
and mi-malloc, can be configured at runtime (via `LD_PRELOAD`),
therefore we cannot know at compile time whether a custom allocator is
used or not.

Make the minimum change possible to ensure this combination is supported
by extending `grep_init()` to set the PCRE1 specific functions to Git's
idea of `malloc()` and `free()` and therefore making sure all
allocations are done inside PCRE1 with the same allocator than the rest
of Git.

This change has negligible performance impact: PCRE needs to allocate
memory once per program run for the character table and for each pattern
compilation. These are both rare events compared to matching patterns
against lines. Actual measurements[2] show that the impact is lost in
the noise.

[1] https://public-inbox.org/git/pull.306.git.gitgitgadget@gmail.com
[2] https://public-inbox.org/git/7f42007f-911b-c570-17f6-1c6af0429586@web.de

Signed-off-by: Carlo Marcelo Arenas Belón <redacted>
Signed-off-by: Johannes Schindelin <redacted>
Signed-off-by: Junio C Hamano <redacted>

notes: fix minimum number of parameters to "copy" subcommand

The builtin/notes.c::copy() function is prepared to handle either
one or two arguments given from the command line; when one argument
is given, to-obj defaults to HEAD.

bbb1b8a3 ("notes: check number of parameters to "git notes copy"",
2010-06-28) tried to make sure "git notes copy" (with *no* other
argument) does not dereference NULL by checking the number of
parameters, but it incorrectly insisted that we need two arguments,
instead of either one or two.  This disabled the defaulting to-obj
to HEAD.

Correct it.

Signed-off-by: Doan Tran Cong Danh <redacted>
Signed-off-by: Junio C Hamano <redacted>

t3301: test diagnose messages for too few/many paramters

Commit bbb1b8a35a ("notes: check number of parameters to "git notes
copy"", 2010-06-28) added a test for too many or too few of
parameters provided to `git notes copy'.

However, the test only ensures that the command will fail but it
doesn't really check if it fails because of number of parameters.

If we accidentally lifted the check inside our code base, the test
may still have failed because the provided parameter is not a valid
ref.

Correct it.

Signed-off-by: Doan Tran Cong Danh <redacted>
Signed-off-by: Junio C Hamano <redacted>

remote-curl: pass on atomic capability to remote side

When pushing more than one reference with the --atomic option, the
server is supposed to perform a single atomic transaction to update the
references, leaving them either all to succeed or all to fail.  This
works fine when pushing locally or over SSH, but when pushing over HTTP,
we fail to pass the atomic capability to the remote side.  In fact, we
have not reported this capability to any remote helpers during the life
of the feature.

Now normally, things happen to work nevertheless, since we actually
check for most types of failures, such as non-fast-forward updates, on
the client side, and just abort the entire attempt.  However, if the
server side reports a problem, such as the inability to lock a ref, the
transaction isn't atomic, because we haven't passed the appropriate
capability over and the remote side has no way of knowing that we wanted
atomic behavior.

Fix this by passing the option from the transport code through to remote
helpers, and from the HTTP remote helper down to send-pack.  With this
change, we can detect if the server side rejects the push and report
back appropriately.  Note the difference in the messages: the remote
side reports "atomic transaction failed", while our own checking rejects
pushes with the message "atomic push failed".

Document the atomic option in the remote helper documentation, so other
implementers can implement it if they like.

Signed-off-by: brian m. carlson <redacted>
Signed-off-by: Junio C Hamano <redacted>

graph: fix coloring of octopus dashes

In 04005834ed ("log: fix coloring of certain octopus merge shapes",
2018-09-01) there is a fix for the coloring of dashes following an
octopus merge. It makes a distinction between the case where all parents
introduce a new column, versus the case where the first parent collapses
into an existing column:

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

The latter case means that the columns for the merge parents begin one
place to the left in the `new_columns` array compared to the former
case.

However, the implementation only works if the commit's parents are kept
in order as they map onto the visual columns, as we get the colors by
iterating over `new_columns` as we print the dashes. In general, the
commit's parents can arbitrarily merge with existing columns, and change
their ordering in the process.

For example, in the following diagram, the number of each column
indicates which commit parent appears in each column.

        | | *---.
        | | |\ \ \
        | | |/ / /
        | |/| | /
        | |_|_|/
        |/| | |
        3 1 0 2

If the columns are colored (red, green, yellow, blue), then the dashes
will currently be colored yellow and blue, whereas they should be blue
and red.

To fix this, we need to look up each column in the `mapping` array,
which before the `GRAPH_COLLAPSING` state indicates which logical column
is displayed in each visual column. This implementation is simpler as it
doesn't have any edge cases, and it also handles how left-skewed first
parents are now displayed:

        | *-.
        |/|\ \
        | | | |
        0 1 2 3

The color of the first dashes is always the color found in `mapping` two
columns to the right of the commit symbol. Because commits are displayed
after all edges have been collapsed together and the visual columns
match the logical ones, we can find the visual offset of the commit
symbol using `commit_index`.

Signed-off-by: James Coglan <redacted>
Signed-off-by: Junio C Hamano <redacted>

graph: flatten edges that fuse with their right neighbor

When a merge commit is printed and its final parent is the same commit
that occupies the column to the right of the merge, this results in a
kink in the displayed edges:

        * |
        |\ \
        | |/
        | *

Graphs containing these shapes can be hard to read, as the expansion to
the right followed immediately by collapsing back to the left creates a
lot of zig-zagging edges, especially when many columns are present.

We can improve this by eliminating the zig-zag and having the merge's
final parent edge fuse immediately with its neighbor:

        * |
        |\|
        | *

This reduces the horizontal width for the current commit by 2, and
requires one less row, making the graph display more compact. Taken in
combination with other graph-smoothing enhancements, it greatly
compresses the space needed to display certain histories:

        *
        |\
        | *                       *
        | |\                      |\
        | | *                     | *
        | | |                     | |\
        | |  \                    | | *
        | *-. \                   | * |
        | |\ \ \        =>        |/|\|
        |/ / / /                  | | *
        | | | /                   | * |
        | | |/                    | |/
        | | *                     * /
        | * |                     |/
        | |/                      *
        * |
        |/
        *

One of the test cases here cannot be correctly rendered in Git v2.23.0;
it produces this output following commit E:

        | | *-. \   5_E
        | | |\ \ \
        | |/ / / /
        | | | / _
        | |_|/
        |/| |

The new implementation makes sure that the rightmost edge in this
history is not left dangling as above.

Signed-off-by: James Coglan <redacted>
Signed-off-by: Junio C Hamano <redacted>

graph: smooth appearance of collapsing edges on commit lines

When a graph contains edges that are in the process of collapsing to the
left, but those edges cross a commit line, the effect is that the edges
have a jagged appearance:

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

We already takes steps to smooth edges like this when they're expanding;
when an edge appears to the right of a merge commit marker on a
GRAPH_COMMIT line immediately following a GRAPH_POST_MERGE line, we
render it as a `\`:

        * \
        |\ \
        | * \
        | |\ \

We can make a similar improvement to collapsing edges, making them
easier to follow and giving the overall graph a feeling of increased
symmetry:

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

To do this, we introduce a new special case for edges on GRAPH_COMMIT
lines that immediately follow a GRAPH_COLLAPSING line. By retaining a
copy of the `mapping` array used to render the GRAPH_COLLAPSING line in
the `old_mapping` array, we can determine that an edge is collapsing
through the GRAPH_COMMIT line and should be smoothed.

Signed-off-by: James Coglan <redacted>
Signed-off-by: Junio C Hamano <redacted>

graph: rename `new_mapping` to `old_mapping`

The change I'm about to make requires being able to inspect the mapping
array that was used to render the last GRAPH_COLLAPSING line while
rendering a GRAPH_COMMIT line. The `new_mapping` array currently exists
as a pre-allocated space for computing the next `mapping` array during
`graph_output_collapsing_line()`, but we can repurpose it to let us see
the previous `mapping` state.

To support this use it will make more sense if this array is named
`old_mapping`, as it will contain the mapping data for the previous line
we rendered, at the point we're rendering a commit line.

Signed-off-by: James Coglan <redacted>
Signed-off-by: Junio C Hamano <redacted>

graph: commit and post-merge lines for left-skewed merges

Following the introduction of "left-skewed" merges, which are merges
whose first parent fuses with another edge to its left, we have some
more edge cases to deal with in the display of commit and post-merge
lines.

The current graph code handles the following cases for edges appearing
to the right of the commit (*) on commit lines. A 2-way merge is usually
followed by vertical lines:

        | | |
        | * |
        | |\ \

An octopus merge (more than two parents) is always followed by edges
sloping to the right:

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

A 2-way merge is followed by a right-sloping edge if the commit line
immediately follows a post-merge line for a commit that appears in the
same column as the current commit, or any column to the left of that:

        | *             | * |
        | |\            | |\ \
        | * \           | | * \
        | |\ \          | | |\ \

This commit introduces the following new cases for commit lines. If a
2-way merge skews to the left, then the edges to its right are always
vertical lines, even if the commit follows a post-merge line:

        | | |           | |\
        | * |           | * |
        |/| |           |/| |

A commit with 3 parents that skews left is followed by vertical edges:

        | | |
        | * |
        |/|\ \

If a 3-way left-skewed merge commit appears immediately after a
post-merge line, then it may be followed the right-sloping edges, just
like a 2-way merge that is not skewed.

        | |\
        | * \
        |/|\ \

Octopus merges with 4 or more parents that skew to the left will always
be followed by right-sloping edges, because the existing columns need to
expand around the merge.

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

On post-merge lines, usually all edges following the current commit
slope to the right:

        | * | |
        | |\ \ \

However, if the commit is a left-skewed 2-way merge, the edges to its
right remain vertical. We also need to display a space after the
vertical line descending from the commit marker, whereas this line would
normally be followed by a backslash.

        | * | |
        |/| | |

If a left-skewed merge has more than 2 parents, then the edges to its
right are still sloped as they bend around the edges introduced by the
merge.

        | * | |
        |/|\ \ \

To handle these new cases, we need to know not just how many parents
each commit has, but how many new columns it adds to the display; this
quantity is recorded in the `edges_added` field for the current commit,
and `prev_edges_added` field for the previous commit.

Here, "column" refers to visual columns, not the logical columns of the
`columns` array. This is because even if all the commit's parents end up
fusing with existing edges, they initially introduce distinct edges in
the commit and post-merge lines before those edges collapse. For
example, a 3-way merge whose 2nd and 3rd parents fuse with existing
edges still introduces 2 visual columns that affect the display of edges
to their right.

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

This merge does not introduce any *logical* columns; there are 4 edges
before and after this commit once all edges have collapsed. But it does
initially introduce 2 new edges that need to be accommodated by the
edges to their right.

Signed-off-by: James Coglan <redacted>
Signed-off-by: Junio C Hamano <redacted>

graph: tidy up display of left-skewed merges

Currently, when we display a merge whose first parent is already present
in a column to the left of the merge commit, we display the first parent
as a vertical pipe `|` in the GRAPH_POST_MERGE line and then immediately
enter the GRAPH_COLLAPSING state. The first-parent line tracks to the
left and all the other parent lines follow it; this creates a "kink" in
those lines:

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

This change tidies the display of such commits such that if the first
parent appears to the left of the merge, we render it as a `/` and the
second parent as a `|`. This reduces the horizontal and vertical space
needed to render the merge, and makes the resulting lines easier to
read.

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

If the first parent is separated from the merge by several columns, a
horizontal line is drawn in a similar manner to how the GRAPH_COLLAPSING
state displays the line.

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

This effect is applied to both "normal" two-parent merges, and to
octopus merges. It also reduces the vertical space needed for pre-commit
lines, as the merge occupies one less column than usual.

        Before:         After:

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

Signed-off-by: James Coglan <redacted>
Signed-off-by: Junio C Hamano <redacted>

graph: example of graph output that can be simplified

The commits following this one introduce a series of improvements to the
layout of graphs, tidying up a few edge cases, namely:

- merge whose first parent fuses with an existing column to the left
- merge whose last parent fuses with its immediate neighbor on the right
- edges that collapse to the left above and below a commit line

This test case exemplifies these cases and provides a motivating example
of the kind of history I'm aiming to clear up.

The first parent of merge E is the same as the parent of H, so those
edges fuse together.

        * H
        |
        | *-.   E
        | |\ \
        |/ / /
        |
        * B

We can "skew" the display of this merge so that it doesn't introduce
additional columns that immediately collapse:

        * H
        |
        | *   E
        |/|\
        |
        * B

The last parent of E is D, the same as the parent of F which is the edge
to the right of the merge.

            * F
            |
             \
          *-. \   E
          |\ \ \
         / / / /
            | /
            |/
            * D

The two edges leading to D could be fused sooner: rather than expanding
the F edge around the merge and then letting the edges collapse, the F
edge could fuse with the E edge in the post-merge line:

            * F
            |
             \
          *-. | E
          |\ \|
         / / /
            |
            * D

If this is combined with the "skew" effect above, we get a much cleaner
graph display for these edges:

            * F
            |
          * | E
         /|\|
            |
            * D

Finally, the edge leading from C to A appears jagged as it passes
through the commit line for B:

        | * | C
        | |/
        * | B
        |/
        * A

This can be smoothed out so that such edges are easier to read:

        | * | C
        | |/
        * / B
        |/
        * A

Signed-off-by: James Coglan <redacted>
Signed-off-by: Junio C Hamano <redacted>

graph: extract logic for moving to GRAPH_PRE_COMMIT state

This computation is repeated in a couple of places and I need to add
another condition to it to implement a further improvement to the graph
rendering, so I'm extracting this into a function.

Signed-off-by: James Coglan <redacted>
Signed-off-by: Junio C Hamano <redacted>

graph: remove `mapping_idx` and `graph_update_width()`

There's a duplication of logic between `graph_insert_into_new_columns()`
and `graph_update_width()`. `graph_insert_into_new_columns()` is called
repeatedly by `graph_update_columns()` with an `int *` that tracks the
offset into the `mapping` array where we should write the next value.
Each call to `graph_insert_into_new_columns()` effectively pushes one
column index and one "null" value (-1) onto the `mapping` array and
therefore increments `mapping_idx` by 2.

`graph_update_width()` duplicates this process: the `width` of the graph
is essentially the initial width of the `mapping` array before edges
begin collapsing. The `graph_update_width()` function's logic
effectively works out how many times `graph_insert_into_new_columns()`
was called based on the relationship of the current commit to the rest
of the graph.

I'm about to make some changes that make the assignment of values into
the `mapping` array more complicated. Rather than make
`graph_update_width()` more complicated at the same time, we can simply
remove this function and use `graph->width` to track the offset into the
`mapping` array as we're building it. This removes the duplication and
makes sure that `graph->width` is the same as the visual width of the
`mapping` array once `graph_update_columns()` is complete.

Signed-off-by: James Coglan <redacted>
Signed-off-by: Junio C Hamano <redacted>

graph: reduce duplication in `graph_insert_into_new_columns()`

I will shortly be making some changes to this function and so am trying
to simplify it. It currently contains some duplicated logic; both
branches the function can take assign the commit's column index into
the `mapping` array and increment `mapping_index`.

Here I change the function so that the only conditional behaviour is
that it appends the commit to `new_columns` if it's not present. All
manipulation of `mapping` now happens on a single code path.

Signed-off-by: James Coglan <redacted>
Signed-off-by: Junio C Hamano <redacted>

graph: reuse `find_new_column_by_commit()`

I will shortly be making some changes to
`graph_insert_into_new_columns()` and so am trying to simplify it. One
possible simplification is that we can extract the loop for finding the
element in `new_columns` containing the given commit.

`find_new_column_by_commit()` contains a very similar loop but it
returns a `struct column *` rather than an `int` offset into the array.
Here I'm introducing a version that returns `int` and using that in
`graph_insert_into_new_columns()` and `graph_output_post_merge_line()`.

Signed-off-by: James Coglan <redacted>
Signed-off-by: Junio C Hamano <redacted>

graph: handle line padding in `graph_next_line()`

Now that the display width of graph lines is implicitly tracked via the
`graph_line` interface, the calls to `graph_pad_horizontally()` no
longer need to be located inside the individual output functions, where
the character counting was previously being done.

All the functions called by `graph_next_line()` generate a line of
output, then call `graph_pad_horizontally()`, and finally change the
graph state if necessary. As padding is the final change to the output
done by all these functions, it can be removed from all of them and done
in `graph_next_line()` instead.

I've also moved the guard in `graph_output_padding_line()` that checks
the graph has a commit; this function is only called by
`graph_next_line()` and we must not pad the `graph_line` if no commit is
set.

Signed-off-by: James Coglan <redacted>
Signed-off-by: Junio C Hamano <redacted>

graph: automatically track display width of graph lines

All the output functions called by `graph_next_line()` currently keep
track of how many printable chars they've written to the buffer, before
calling `graph_pad_horizontally()` to pad the line with spaces. Some
functions do this by incrementing a counter whenever they write to the
buffer, and others do it by encoding an assumption about how many chars
are written, as in:

    graph_pad_horizontally(graph, sb, graph->num_columns * 2);

This adds a fair amount of noise to the functions' logic and is easily
broken if one forgets to increment the right counter or update the
calculations used for padding.

To make this easier to use, I'm introducing a new struct called
`graph_line` that wraps a `strbuf` and keeps count of its display width
implicitly. `graph_next_line()` wraps this around the `struct strbuf *`
it's given and passes a `struct graph_line *` to the output functions,
which use its interface.

The `graph_line` interface wraps the `strbuf_addch()`,
`strbuf_addchars()` and `strbuf_addstr()` functions, and adds the
`graph_line_write_column()` function for adding a single character with
color formatting. The `graph_pad_horizontally()` function can then use
the `width` field from the struct rather than taking a character count
as a parameter.

Signed-off-by: James Coglan <redacted>
Signed-off-by: Junio C Hamano <redacted>

fetch-pack: write fetched refs to .promisor

The specification of promisor packfiles (in partial-clone.txt) states
that the .promisor files that accompany packfiles do not matter (just
like .keep files), so whenever a packfile is fetched from the promisor
remote, Git has been writing empty .promisor files. But these files
could contain more useful information.

So instead of writing empty files, write the refs fetched to these
files. This makes it easier to debug issues with partial clones, as we
can identify what refs (and their associated hashes) were fetched at the
time the packfile was downloaded, and if necessary, compare those hashes
against what the promisor remote reports now.

This is implemented by teaching fetch-pack to write its own non-empty
.promisor file whenever it knows the name of the pack's lockfile. This
covers the case wherein the user runs "git fetch" with an internal
protocol or HTTP protocol v2 (fetch_refs_via_pack() in transport.c sets
lock_pack) and with HTTP protocol v0/v1 (fetch_git() in remote-curl.c
passes "--lock-pack" to "fetch-pack").

Signed-off-by: Jonathan Tan <redacted>
Acked-by: Josh Steadmon <redacted>
Signed-off-by: Junio C Hamano <redacted>

sequencer: run post-commit hook

Prior to commit 356ee4659b ("sequencer: try to commit without forking
'git commit'", 2017-11-24) the sequencer would always run the
post-commit hook after each pick or revert as it forked `git commit` to
create the commit. The conversion to committing without forking `git
commit` omitted to call the post-commit hook after creating the commit.

Signed-off-by: Phillip Wood <redacted>
Signed-off-by: Junio C Hamano <redacted>

move run_commit_hook() to libgit and use it there

This function was declared in commit.h but was implemented in
builtin/commit.c so was not part of libgit. Move it to libgit so we can
use it in the sequencer. This simplifies the implementation of
run_prepare_commit_msg_hook() and will be used in the next commit.

Signed-off-by: Phillip Wood <redacted>
Signed-off-by: Junio C Hamano <redacted>

sequencer.h fix placement of #endif

Commit 65850686cf ("rebase -i: rewrite write_basic_state() in C",
2018-08-28) accidentially added new function declarations after
the #endif at the end of the include guard.

Signed-off-by: Phillip Wood <redacted>
Signed-off-by: Junio C Hamano <redacted>

t3404: remove uneeded calls to set_fake_editor

Some tests were calling set_fake_editor to ensure they had a sane no-op
editor set. Now that all the editor setting is done in subshells these
tests can rely on EDITOR=: and so do not need to call set_fake_editor.

Also add a test at the end to detect any future additions messing with
the exported value of $EDITOR.

Signed-off-by: Phillip Wood <redacted>
Signed-off-by: Junio C Hamano <redacted>

t3404: set $EDITOR in subshell

As $EDITOR is exported setting it in one test affects all subsequent
tests. Avoid this by always setting it in a subshell. This commit leaves
20 calls to set_fake_editor that are not in subshells as they can
safely be removed in the next commit once all the other editor setting
is done inside subshells.

I have moved the call to set_fake_editor in some tests so it comes
immediately before the call to 'git rebase' to avoid moving unrelated
commands into the subshell. In one case ('rebase -ix with
--autosquash') the call to set_fake_editor is moved past an invocation
of 'git rebase'. This is safe as that invocation of 'git rebase'
requires EDITOR=: or EDITOR=fake-editor.sh without FAKE_LINES being
set which will be the case as the preceding tests either set their
editor in a subshell or call set_fake_editor without setting FAKE_LINES.

In a one test ('auto-amend only edited commits after "edit"') a call
to test_tick are now in a subshell. I think this is OK as it is there
to set the date for the next commit which is executed in the same
subshell rather than updating GIT_COMMITTER_DATE for later tests (the
next test calls test_tick before doing anything else).

Signed-off-by: Phillip Wood <redacted>
Signed-off-by: Junio C Hamano <redacted>

t3404: remove unnecessary subshell

Neither of the commands executed in the subshell change any shell
variables or the current directory so there is no need for them to be
executed in a subshell.

Signed-off-by: Phillip Wood <redacted>
Signed-off-by: Junio C Hamano <redacted>

format-patch: teach --cover-from-description option

Before, when format-patch generated a cover letter, only the body would
be populated with a branch's description while the subject would be
populated with placeholder text. However, users may want to have the
subject of their cover letter automatically populated in the same way.

Teach format-patch to accept the `--cover-from-description` option and
corresponding `format.coverFromDescription` config, allowing users to
populate different parts of the cover letter (including the subject
now).

Signed-off-by: Denton Liu <redacted>
Signed-off-by: Junio C Hamano <redacted>

format-patch: use enum variables

Before, `thread` and `config_cover_letter` were defined as ints even
though they behaved as enums. Define actual enums and change these
variables to use these new definitions.

Signed-off-by: Denton Liu <redacted>
Signed-off-by: Junio C Hamano <redacted>

format-patch: replace erroneous and condition

Commit 30984ed2e9 (format-patch: support deep threading, 2009-02-19),
introduced the following lines:

#define THREAD_SHALLOW 1

[...]

thread = git_config_bool(var, value) && THREAD_SHALLOW;

Since git_config_bool() returns a bool, the trailing `&& THREAD_SHALLOW`
is a no-op. Replace this errorneous and condition with a ternary
statement so that it is clear what the configured value is when a
boolean is given.

Signed-off-by: Denton Liu <redacted>
Signed-off-by: Junio C Hamano <redacted>

diff-highlight: fix a whitespace nit

This changes the indent from
  "<tab><sp><sp><sp><sp><sp><sp><sp><sp>"
to
  "<tab><tab>"
so that the statement lines up with the rest of the block.

Signed-off-by: Norman Rasmussen <redacted>
Acked-by: Jeff King <redacted>
Signed-off-by: Junio C Hamano <redacted>

Ninth batch

Signed-off-by: Junio C Hamano <redacted>

Merge branch 'jk/coc'

Code-of-conduct document.

* jk/coc:
  CODE_OF_CONDUCT: mention individual project-leader emails
  add a Code of Conduct document

Merge branch 'js/trace2-fetch-push'

Dev support.

* js/trace2-fetch-push:
  transport: push codepath can take arbitrary repository
  push: add trace2 instrumentation
  fetch: add trace2 instrumentation

Merge branch 'jt/push-avoid-lazy-fetch'

Performance hack.

* jt/push-avoid-lazy-fetch:
  send-pack: never fetch when checking exclusions

Merge branch 'dl/format-patch-doc-test-cleanup'

test cleanup.

* dl/format-patch-doc-test-cleanup:
  t4014: treat rev-list output as the expected value

Merge branch 'js/xdiffi-comment-updates'

Comment update.

* js/xdiffi-comment-updates:
  xdiffi: fix typos and touch up comments

Merge branch 'dl/t0000-skip-test-test'

test update.

* dl/t0000-skip-test-test:
  t0000: cover GIT_SKIP_TESTS blindspots

Merge branch 'tg/range-diff-output-update'

"git range-diff" failed to handle mode-only change, which has been
corrected.

* tg/range-diff-output-update:
  range-diff: don't segfault with mode-only changes

Merge branch 'gs/sq-quote-buf-pretty'

Pretty-printed command line formatter (used in e.g. reporting the
command being run by the tracing API) had a bug that lost an
argument that is an empty string, which has been corrected.

* gs/sq-quote-buf-pretty:
  sq_quote_buf_pretty: don't drop empty arguments

Merge branch 'ew/hashmap'

Code clean-up of the hashmap API, both users and implementation.

* ew/hashmap:
  hashmap_entry: remove first member requirement from docs
  hashmap: remove type arg from hashmap_{get,put,remove}_entry
  OFFSETOF_VAR macro to simplify hashmap iterators
  hashmap: introduce hashmap_free_entries
  hashmap: hashmap_{put,remove} return hashmap_entry *
  hashmap: use *_entry APIs for iteration
  hashmap_cmp_fn takes hashmap_entry params
  hashmap_get{,_from_hash} return "struct hashmap_entry *"
  hashmap: use *_entry APIs to wrap container_of
  hashmap_get_next returns "struct hashmap_entry *"
  introduce container_of macro
  hashmap_put takes "struct hashmap_entry *"
  hashmap_remove takes "const struct hashmap_entry *"
  hashmap_get takes "const struct hashmap_entry *"
  hashmap_add takes "struct hashmap_entry *"
  hashmap_get_next takes "const struct hashmap_entry *"
  hashmap_entry_init takes "struct hashmap_entry *"
  packfile: use hashmap_entry in delta_base_cache_entry
  coccicheck: detect hashmap_entry.hash assignment
  diff: use hashmap_entry_init on moved_entry.ent

Merge branch 'js/trace2-cap-max-output-files'

The trace2 output, when sending them to files in a designated
directory, can populate the directory with too many files; a
mechanism is introduced to set the maximum number of files and
discard further logs when the maximum is reached.

* js/trace2-cap-max-output-files:
  trace2: write discard message to sentinel files
  trace2: discard new traces if target directory has too many files
  docs: clarify trace2 version invariants
  docs: mention trace2 target-dir mode in git-config

Merge branch 'am/t0028-utf16-tests'

Test fixes.

* am/t0028-utf16-tests:
  t0028: add more tests
  t0028: fix test for UTF-16-LE-BOM

Merge branch 'dl/octopus-graph-bug'

"git log --graph" for an octopus merge is sometimes colored
incorrectly, which is demonstrated and documented but not yet
fixed.

* dl/octopus-graph-bug:
  t4214: demonstrate octopus graph coloring failure
  t4214: explicitly list tags in log
  t4214: generate expect in their own test cases
  t4214: use test_merge
  test-lib: let test_merge() perform octopus merges

Merge branch 'en/fast-imexport-nested-tags'

Updates to fast-import/export.

* en/fast-imexport-nested-tags:
  fast-export: handle nested tags
  t9350: add tests for tags of things other than a commit
  fast-export: allow user to request tags be marked with --mark-tags
  fast-export: add support for --import-marks-if-exists
  fast-import: add support for new 'alias' command
  fast-import: allow tags to be identified by mark labels
  fast-import: fix handling of deleted tags
  fast-export: fix exporting a tag and nothing else

Merge branch 'js/azure-pipelines-msvc'

CI updates.

* js/azure-pipelines-msvc:
  ci: also build and test with MS Visual Studio on Azure Pipelines
  ci: really use shallow clones on Azure Pipelines
  tests: let --immediate and --write-junit-xml play well together
  test-tool run-command: learn to run (parts of) the testsuite
  vcxproj: include more generated files
  vcxproj: only copy `git-remote-http.exe` once it was built
  msvc: work around a bug in GetEnvironmentVariable()
  msvc: handle DEVELOPER=1
  msvc: ignore some libraries when linking
  compat/win32/path-utils.h: add #include guards
  winansi: use FLEX_ARRAY to avoid compiler warning
  msvc: avoid using minus operator on unsigned types
  push: do not pretend to return `int` from `die_push_simple()`

Merge branch 'gs/commit-graph-trace-with-cmd'

Dev support.

* gs/commit-graph-trace-with-cmd:
  commit-graph: emit trace2 cmd_mode for each sub-command

Merge branch 'js/fetch-jobs'

"git fetch --jobs=<n>" allowed <n> parallel jobs when fetching
submodules, but this did not apply to "git fetch --multiple" that
fetches from multiple remote repositories.  It now does.

* js/fetch-jobs:
  fetch: let --jobs=<n> parallelize --multiple, too

Merge branch 'en/merge-recursive-cleanup'

The merge-recursive machiery is one of the most complex parts of
the system that accumulated cruft over time.  This large series
cleans up the implementation quite a bit.

* en/merge-recursive-cleanup: (26 commits)
  merge-recursive: fix the fix to the diff3 common ancestor label
  merge-recursive: fix the diff3 common ancestor label for virtual commits
  merge-recursive: alphabetize include list
  merge-recursive: add sanity checks for relevant merge_options
  merge-recursive: rename MERGE_RECURSIVE_* to MERGE_VARIANT_*
  merge-recursive: split internal fields into a separate struct
  merge-recursive: avoid losing output and leaking memory holding that output
  merge-recursive: comment and reorder the merge_options fields
  merge-recursive: consolidate unnecessary fields in merge_options
  merge-recursive: move some definitions around to clean up the header
  merge-recursive: rename merge_options argument to opt in header
  merge-recursive: rename 'mrtree' to 'result_tree', for clarity
  merge-recursive: use common name for ancestors/common/base_list
  merge-recursive: fix some overly long lines
  cache-tree: share code between functions writing an index as a tree
  merge-recursive: don't force external callers to do our logging
  merge-recursive: remove useless parameter in merge_trees()
  merge-recursive: exit early if index != head
  Ensure index matches head before invoking merge machinery, round N
  merge-recursive: remove another implicit dependency on the_repository
  ...

remote-curl: use argv_array in parse_push()

Use argv_array to build an array of strings instead of open-coding it.
This simplifies the code a bit.

We also need to make the specs parameter of push(), push_dav() and
push_git() const to match the argv member of the argv_array.  That's
fine, as all three only actually read from the specs array anyway.

Signed-off-by: René Scharfe <redacted>
Signed-off-by: Junio C Hamano <redacted>