completion: use __gitcomp_builtin for format-patch

This helps format-patch gain completion for a couple new options,
notably --range-diff.

Since send-email completion relies on $__git_format_patch_options
which is now reduced, we need to do something not to regress
send-email completion.

The workaround here is implement --git-completion-helper in
send-email.perl just as a bridge to "format-patch --git-completion-helper".
This is enough to use __gitcomp_builtin on send-email (to take
advantage of caching).

In the end, send-email.perl can probably reuse the same info it passes
to GetOptions() to generate full --git-completion-helper output so
that we don't need to keep track of its options in git-completion.bash
anymore. But that's something for another boring day.

Helped-by: Denton Liu <liu.denton@gmail.com>
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

midx: double-check large object write loop

The write_midx_large_offsets() function takes an array of object
entries, the number of entries in the array (nr_objects), and the number
of entries with large offsets (nr_large_offset). But we never actually
use nr_objects; instead we keep walking down the array and counting down
nr_large_offset until we've seen all of the large entries.

This is correct, but we can be a bit more defensive. If there were ever
a mismatch between nr_large_offset and the actual set of large-offset
objects, we'd walk off the end of the array.

Since we know the size of the array, we can use nr_objects to make sure
we don't walk too far.

Signed-off-by: Jeff King <peff@peff.net>
Reviewed-by: Derrick Stolee <dstolee@microsoft.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

assert NOARG/NONEG behavior of parse-options callbacks

When we define a parse-options callback, the flags we put in the option
struct must match what the callback expects. For example, a callback
which does not handle the "unset" parameter should only be used with
PARSE_OPT_NONEG. But since the callback and the option struct are not
defined next to each other, it's easy to get this wrong (as earlier
patches in this series show).

Fortunately, the compiler can help us here: compiling with
-Wunused-parameters can show us which callbacks ignore their "unset"
parameters (and likewise, ones that ignore "arg" expect to be triggered
with PARSE_OPT_NOARG).

But after we've inspected a callback and determined that all of its
callers use the right flags, what do we do next? We'd like to silence
the compiler warning, but do so in a way that will catch any wrong calls
in the future.

We can do that by actually checking those variables and asserting that
they match our expectations. Because this is such a common pattern,
we'll introduce some helper macros. The resulting messages aren't
as descriptive as we could make them, but the file/line information from
BUG() is enough to identify the problem (and anyway, the point is that
these should never be seen).

Each of the annotated callbacks in this patch triggers
-Wunused-parameters, and was manually inspected to make sure all callers
use the correct options (so none of these BUGs should be triggerable).

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

parse-options: drop OPT_DATE()

There are no users of OPT_DATE except for test-parse-options; its
only caller went away in 27ec394a97 (prune: introduce OPT_EXPIRY_DATE()
and use it, 2013-04-25).

It also has a bug: it does not specify PARSE_OPT_NONEG, but its callback
does not respect the "unset" flag, and will feed NULL to approxidate()
and segfault. Probably this should be marked with NONEG, or the callback
should set the timestamp to some sentinel value (e.g,. "0", or
"(time_t)-1").

But since there are no callers, deleting it means we don't even have to
think about what the right behavior should be.

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

apply: return -1 from option callback instead of calling exit(1)

The option callback for "apply --whitespace" exits with status "1" on
error. It makes more sense for it to just return an error to
parse-options. That code will exit, too, but it will use status "129"
that is customary for option errors.

The exit() dates back to aaf6c447aa (builtin/apply: make
parse_whitespace_option() return -1 instead of die()ing, 2016-08-08).
That commit gives no reason why we'd prefer the current exit status (it
looks like it was just bumping the "die" up a level in the callstack,
but did not go as far as it could have).

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

cat-file: report an error on multiple --batch options

The options callback for --batch and --batch-check detects when the two
mutually incompatible options are used. But it simply returns an error
code to parse-options, meaning the program will quit without any kind of
message to the user.

Instead, let's use error() to print something and return -1. Note that
this flips the error return from 1 to -1, but negative values are more
idiomatic here (and parse-options treats them the same).

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

tag: mark "--message" option with NONEG

We do not allow "--no-message" to work now, as the option callback
returns "-1" when it sees a NULL arg. However, that will cause
parse-options to exit(129) without printing anything further, leaving
the user confused about what happened.

Instead, let's explicitly mark it as PARSE_OPT_NONEG, which will give a
useful error message (and print the usual -h output).

In theory this could be used to override an earlier "-m", but it's not
clear how it would interact with other message options (e.g., would it
also clear data read for "-F"?). Since it's already disabled and nobody
is asking for it, let's punt on that and just improve the error message.

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

show-branch: mark --reflog option as NONEG

Running "git show-branch --no-reflog" will behave as if "--reflog" was
given with no options, which makes no sense.

In theory this option might be used to cancel an earlier "--reflog"
option, but the semantics are not clear. Let's punt on it and just
disallow the broken option.

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

format-patch: mark "--no-numbered" option with NONEG

We have separate parse-options entries for "numbered" and "no-numbered",
which means that we accept "--no-no-numbered". It does not behave
sensibly, though (it ignores the "unset" flag and acts like
"--no-numbered").

We could fix that, but obviously this is silly and unintentional. Let's
just disallow it.

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

status: mark --find-renames option with NONEG

If you run "git status --no-find-renames", it will behave the same as
"--find-renames", because we ignore the "unset" parameter (we see a NULL
"arg", but since the score argument is optional, we just think that the
user did not provide a score).

We already have a separate "--no-renames" to disable renames, so there's
not much point in supporting "--no-find-renames". Let's just flag it as
an error.

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

cat-file: mark batch options with NONEG

Running "cat-file --no-batch" will behave as if "--batch" was given,
since the option callback does not handle the "unset" flag (likewise for
"--no-batch-check").

In theory this might be used to cancel an earlier --batch, but it's not
immediately obvious how that would interact with --batch-check. Let's
just disallow the negated form of both options.

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

pack-objects: mark index-version option as NONEG

Running "git pack-objects --no-index-version" will segfault, since the
callback is not prepared to handle the "unset" flag.

In theory this might be used to counteract an earlier "--index-version",
or override a pack.indexversion config setting. But the semantics aren't
immediately obvious, and it's unlikely anybody wants this. Let's just
disable the broken option for now.

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

ls-files: mark exclude options as NONEG

Running "git ls-files --no-exclude" will currently segfault, as its
option callback does not handle the "unset" parameter.

In theory this could be used to clear the exclude list, but it is not
clear how that would interact with the other exclude options, nor is the
current code capable of clearing the list. Let's just disable the broken
option.

Note that --no-exclude-from will similarly segfault, but
--no-exclude-standard will not. It just silently does the wrong thing
(pretending as if --exclude-standard was specified).

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

am: handle --no-patch-format option

Running "git am --no-patch-format" will currently segfault, since it
tries to parse a NULL argument. Instead, let's have it cancel any
previous --patch-format option.

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

apply: mark include/exclude options as NONEG

The options callback for "git apply --no-include" is not ready to handle
the "unset" parameter, and as a result will segfault when it adds a NULL
argument to the include list (likewise for "--no-exclude").

In theory this might be used to clear the list, but since both
"--include" and "--exclude" add to the same list, it's not immediately
obvious what the semantics should be. Let's punt on that for now and
just disallow the broken options.

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

refresh_index: remove unnecessary calls to preload_index()

With refresh_index() learning to utilize preload_index() to speed up its
operation there is no longer any benefit to having the caller preload the
index first. Remove those unneeded calls by calling read_index() instead of
the preload variant.

There is no measurable performance impact of this patch - the 2nd call to
preload_index() bails out quickly but there is no reason to call it twice.

Signed-off-by: Ben Peart <benpeart@microsoft.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

Clean up pthread_create() error handling

Normally pthread_create() rarely fails. But with new pthreads wrapper,
pthread_create() will return ENOSYS on a system without thread support.

Threaded code _is_ protected by HAVE_THREADS and pthread_create()
should never run in the first place. But the situation could change in
the future and bugs may sneak in. Make sure that all pthread_create()
reports the error cause.

While at there, mark these strings for translation if they aren't.

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

read-cache.c: initialize copy_len to shut up gcc 8

It was reported that when building with NO_PTHREADS=1,
-Wmaybe-uninitialized is triggered. Just initialize the variable from
the beginning to shut the compiler up (because this warning is enabled
in config.dev)

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

read-cache.c: reduce branching based on HAVE_THREADS

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

read-cache.c: remove #ifdef NO_PTHREADS

This is a faithful conversion with no attempt to clean up whatsoever.
Code indentation is left broken. There will be another commit to clean
it up and un-indent if we just indent now. It's just more code noise.

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

pack-objects: remove #ifdef NO_PTHREADS

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

preload-index.c: remove #ifdef NO_PTHREADS

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

grep: clean up num_threads handling

When NO_PTHREADS is still used in this file, we have two separate code
paths for thread and no thread support. The latter will always have
num_threads remain zero while the former uses num_threads zero as
"default number of threads".

With recent changes blur the line between thread and no-thread
support, this num_threads handling becomes a bit strange so let's
redefine it like this:

- num_threads == 0 means default number of threads and should become
positive after all configuration and option parsing is done if
multithread is supported.

- num_threads <= 1 runs no threads. It does not matter if the platform
supports threading or not.

- num_threads > 1 will run multiple threads and is invalid if
HAVE_THREADS is false. pthread API is only used in this case.

PS. a new warning is also added when num_threads is forced back to one
because a thread-incompatible option is specified.

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

grep: remove #ifdef NO_PTHREADS

This is a faithful conversion without attempting to improve
anything. That comes later.

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

attr.c: remove #ifdef NO_PTHREADS

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

name-hash.c: remove #ifdef NO_PTHREADS

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

index-pack: remove #ifdef NO_PTHREADS

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

send-pack.c: move async's #ifdef NO_PTHREADS back to run-command.c

On systems that do not support multithread, start_async() is
implemented with fork(). This implementation details unfortunately
leak out at least in send-pack.c [1].

To keep the code base clean of NO_PTHREADS, move the this #ifdef back
to run-command.c. The new wrapper function async_with_fork() at least
helps suggest that this special "close()" is related to async in fork
mode.

[1] 09c9957cf7 (send-pack: avoid deadlock when pack-object dies early
- 2011-04-25)

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

run-command.h: include thread-utils.h instead of pthread.h

run-command.c may use threads for its async support. But instead of
including pthread.h directly, let's include thread-utils.h.

run-command.c probably never needs the dummy bits in thread-utils.h
when NO_PTHREADS is defined. But this makes sure we have consistent
HAVE_THREADS behavior everywhere. From now on outside compat/,
thread-utils.h is the only place that includes pthread.h

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

xdiff-interface: drop parse_hunk_header()

This function was used only for parsing the hunk headers generated by
xdiff. Now that we can use hunk callbacks to get that information
directly, it has outlived its usefulness.

Note to anyone who wants to resurrect it: the "len" parameter was
totally unused, meaning that the function could read past the end of the
"line" array. In practice this never happened, because we only used it
to parse xdiff's generated header lines. But it would be dangerous to
use it for other cases without fixing this defect.

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

range-diff: use a hunk callback

When we count the lines in a diff, we don't actually care about the
contents of each line. By using a hunk callback, we tell xdiff that it
does not need to even bother generating a hunk header line, saving a
small amount of work.

Arguably we could even ignore the hunk headers completely, since we're
just computing a cost function between patches. But doing it this way
maintains the exact same behavior before and after.

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

diff: convert --check to use a hunk callback

The "diff --check" code needs to know the line number on which each hunk
starts in order to generate its output. We get that now by parsing the
hunk header line generated by xdiff, but it's much simpler to just pass
it directly using a hunk callback.

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

combine-diff: use an xdiff hunk callback

A combined diff has to line up the hunks for all of the individual
pairwise diffs, and thus needs to know their line numbers and sizes. We
get that now by parsing the hunk header line that xdiff generates.
However, now that xdiff supports a hunk callback, we can just use the
values directly.

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

diff: use hunk callback for word-diff

Our word-diff does not look at the -/+ lines generated by xdiff at all
(because they are not real lines to show the user, but just the
tokenized words split into lines). Instead we use the line numbers from
the hunk headers to index our own data structure.

As a result, our xdi_diff_outf() callback throws away all lines except
hunk headers. We can instead use a hunk callback, which has two
benefits:

1. We don't have to re-parse the generated hunk header line, but can
use the passed parameters directly.

2. By setting our line callback to NULL, we can tell xdiff-interface
that it does not even need to bother generating the other lines,
saving a small amount of work.

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

diff: discard hunk headers for patch-ids earlier

We do not include hunk header lines when computing patch-ids, since
the line numbers would create false negatives. Rather than detect and
skip them in our line callback, we can simply tell xdiff to avoid
generating them.

This is similar to the previous commit, but split out because it
actually requires modifying the matching line callback.

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

diff: avoid generating unused hunk header lines

Some callers of xdi_diff_outf() do not look at the generated hunk header
lines at all. By plugging in a no-op hunk callback, this tells xdiff not
to even bother formatting them.

This patch introduces a stock no-op callback and uses it with a few
callers whose line callbacks explicitly ignore hunk headers (because
they look only for +/- lines).

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

poll: use GetTickCount64() to avoid wrap-around issues

The value of timeout starts as an int value, and for this reason it
cannot overflow unsigned long long aka ULONGLONG. The unsigned version
of this initial value is available in orig_timeout. The difference
(orig_timeout - elapsed) cannot wrap around because it is protected by
a conditional (as can be seen in the patch text). Hence, the ULONGLONG
difference can only have values that are smaller than the initial
timeout value and truncation to int cannot overflow.

Signed-off-by: Steve Hoelzer <shoelzer@gmail.com>
[j6t: improved both implementation and log message]
Signed-off-by: Johannes Sixt <j6t@kdbg.org>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

t/t7510-signed-commit.sh: add signing subkey to Eris Discordia key

Add a dedicated signing subkey to the key identified as 'Eris
Discordia', and update tests appropriately. GnuPG will now sign commits
using the dedicated signing subkey, changing the value of %GK and %GF,
and effectively creating a test case for %GF!=%GP.

Signed-off-by: Michał Górny <mgorny@gentoo.org>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

t/t7510-signed-commit.sh: Add %GP to custom format checks

Test %GP in addition to %GF in custom format checks. With current
keyring, both have the same value.

Signed-off-by: Michał Górny <mgorny@gentoo.org>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

tree-walk.c: fix overoptimistic inclusion in :(exclude) matching

tree_entry_interesting() is used for matching pathspec on a tree. The
interesting thing about this function is that, because the tree
entries are known to be sorted, this function can return more than
just "yes, matched" and "no, not matched". It can also say "yes, this
entry is matched and so is the remaining entries in the tree".

This is where I made a mistake when matching exclude pathspec. For
exclude pathspec, we do matching twice, one with positive patterns and
one with negative ones, then a rule table is applied to determine the
final "include or exclude" result. Note that "matched" does not
necessarily mean include. For negative patterns, "matched" means
exclude.

This particular rule is too eager to include everything. Rule 8 says
that "if all entries are positively matched" and the current entry is
not negatively matched (i.e. not excluded), then all entries are
positively matched and therefore included. But this is not true. If
the _current_ entry is not negatively matched, it does not mean the
next one will not be and we cannot conclude right away that all
remaining entries are positively matched and can be included.

Rules 8 and 18 are now updated to be less eager. We conclude that the
current entry is positively matched and included. But we say nothing
about remaining entries. tree_entry_interesting() will be called again
for those entries where we will determine entries individually.

Reported-by: Christophe Bliard <christophe.bliard@trux.info>
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

sequencer.c: remove a stray semicolon

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

git-worktree.txt: correct linkgit command name

Noticed-by: SZEDER Gábor <szeder.dev@gmail.com>
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

build: link with curl-defined linker flags

Adjusting the build process to rely more on curl-config to populate
linker flags instead of manually populating flags based off detected
features.

Originally, a configure-invoked build would check for SSL-support in the
target curl library. If enabled, NEEDS_SSL_WITH_CURL would be set and
used in the Makefile to append additional libraries to link against. As
for systems building solely with make, the defines NEEDS_IDN_WITH_CURL
and NEEDS_SSL_WITH_CURL could be set to indirectly enable respective
linker flags. Since both configure.ac and Makefile already rely on
curl-config utility to provide curl-related build information, adjusting
the respective assets to populate required linker flags using the
utility (unless explicitly configured).

Signed-off-by: James Knight <james.d.knight@live.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

Merge branch 'jc/http-curlver-warnings'

Warning message fix.

* jc/http-curlver-warnings:
http: give curl version warnings consistently

Merge branch 'js/mingw-http-ssl'

On platforms with recent cURL library, http.sslBackend configuration
variable can be used to choose a different SSL backend at runtime.
The Windows port uses this mechanism to switch between OpenSSL and
Secure Channel while talking over the HTTPS protocol.

* js/mingw-http-ssl:
http: when using Secure Channel, ignore sslCAInfo by default
http: add support for disabling SSL revocation checks in cURL
http: add support for selecting SSL backends at runtime

Merge branch 'mg/gpg-fingerprint'

New "--pretty=format:" placeholders %GF and %GP that show the GPG
key fingerprints have been invented.

* mg/gpg-fingerprint:
gpg-interface.c: obtain primary key fingerprint as well
gpg-interface.c: support getting key fingerprint via %GF format
gpg-interface.c: use flags to determine key/signer info presence

Merge branch 'mg/gpg-parse-tighten'

Detect and reject a signature block that has more than one GPG
signature.

* mg/gpg-parse-tighten:
gpg-interface.c: detect and reject multiple signatures on commits

Merge branch 'en/merge-cleanup-more'

Further clean-up of merge-recursive machinery.

* en/merge-cleanup-more:
merge-recursive: avoid showing conflicts with merge branch before HEAD
merge-recursive: improve auto-merging messages with path collisions

add: speed up cmd_add() by utilizing read_cache_preload()

During an "add", a call is made to run_diff_files() which calls
check_removed() for each index-entry. The preload_index() code
distributes some of the costs across multiple threads.

Because the files checked are restricted to pathspec, adding
individual files makes no measurable impact but on a Windows repo
with ~200K files, 'git add .' drops from 6.3 seconds to 3.3 seconds
for a 47% savings.

Signed-off-by: Ben Peart <benpeart@microsoft.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

remote: make add_missing_tags() linear

The add_missing_tags() method currently has quadratic behavior.
This is due to a linear number (based on number of tags T) of
calls to in_merge_bases_many, which has linear performance (based
on number of commits C in the repository).

Replace this O(T * C) algorithm with an O(T + C) algorithm by
using get_reachable_subset(). We ignore the return list and focus
instead on the reachable_flag assigned to the commits we care
about, because we need to interact with the tag ref and not just
the commit object.

Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

test-reach: test get_reachable_subset

The get_reachable_subset() method returns the list of commits in
the 'to' array that are reachable from at least one commit in the
'from' array. Add tests that check this method works in a few
cases:

1. All commits in the 'to' list are reachable. This exercises the
early-termination condition.

2. Some commits in the 'to' list are reachable. This exercises the
loop-termination condition.

3. No commits in the 'to' list are reachable. This exercises the
NULL return condition.

Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

commit-reach: implement get_reachable_subset

The existing reachability algorithms in commit-reach.c focus on
finding merge-bases or determining if all commits in a set X can
reach at least one commit in a set Y. However, for two commits sets
X and Y, we may also care about which commits in Y are reachable
from at least one commit in X.

Implement get_reachable_subset() which answers this question. Given
two arrays of commits, 'from' and 'to', return a commit_list with
every commit from the 'to' array that is reachable from at least
one commit in the 'from' array.

The algorithm is a simple walk starting at the 'from' commits, using
the PARENT2 flag to indicate "this commit has already been added to
the walk queue". By marking the 'to' commits with the PARENT1 flag,
we can determine when we see a commit from the 'to' array. We remove
the PARENT1 flag as we add that commit to the result list to avoid
duplicates.

The order of the resulting list is a reverse of the order that the
commits are discovered in the walk.

There are a couple shortcuts to avoid walking more than we need:

1. We determine the minimum generation number of commits in the
'to' array. We do not walk commits with generation number
below this minimum.

2. We count how many distinct commits are in the 'to' array, and
decrement this count when we discover a 'to' commit during the
walk. If this number reaches zero, then we can terminate the
walk.

Tests will be added using the 'test-tool reach' helper in a
subsequent commit.

Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

send-email: avoid empty transfer encoding header

Fix a small bug introduced by "7a36987ff (send-email: add an auto option
for transfer encoding, 2018-07-14)".

I saw the following message when setting --transfer-encoding for a file
with the same encoding:

$ git send-email --transfer-encoding=8bit example.patch
Use of uninitialized value $xfer_encoding in concatenation (.) or string
at /usr/lib/git-core/git-send-email line 1744.

The new tests are by brian m. carlson.

Signed-off-by: Aaron Lindsay <aaron@aclindsay.com>
Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

pathspec: handle non-terminated strings with :(attr)

The pathspec code always takes names to be matched as a
name/namelen pair, but match_attrs() never looks at namelen,
and just treats "name" like a NUL-terminated string, passing
it to git_check_attr().

This usually works anyway. Every caller passes a
NUL-terminated string, and in all but one the passed-in
length is the same as the length of the string (the
exception is dir_path_match(), which may pass a smaller
length to drop a trailing slash). So we won't currently ever
read random memory, and the one case I found actually
happens to work correctly because the attr code can handle
the trailing slash itself.

But it's still worth addressing, as the function interface
implies that the name does not have to be NUL-terminated,
making this an accident waiting to happen.

Since teaching git_check_attr() to take a ptr/len pair would
be a big refactor, we'll just allocate a new string. We can
do this only when necessary, which avoids paying the cost
for most callers.

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

approxidate: handle pending number for "specials"

The approxidate parser has a table of special keywords like
"yesterday", "noon", "pm", etc. Some of these, like "pm", do
the right thing if we've recently seen a number: "3pm" is
what you'd think.

However, most of them do not look at or modify the
pending-number flag at all, which means a number may "jump"
across a significant keyword and be used unexpectedly. For
example, when parsing:

January 5th noon pm

we'd connect the "5" to "pm", and ignore it as a
day-of-month. This is obviously a bit silly, as "noon"
already implies "pm". And other mis-parsed things are
generally as silly ("January 5th noon, years ago" would
connect the 5 to "years", but probably nobody would type
that).

However, the fix is simple: when we see a keyword like
"noon", we should flush the pending number (as we would if
we hit another number, or the end of the string). In a few
of the specials that actually modify the day, we can simply
throw away the number (saying "Jan 5 yesterday" should not
respect the number at all).

Note that we have to either move or forward-declare the
static pending_number() to make it accessible to these
functions; this patch moves it.

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

rev-list: handle flags for --indexed-objects

When a traversal sees the --indexed-objects option, it adds
all blobs and valid cache-trees from the index to the
traversal using add_index_objects_to_pending(). But that
function totally ignores its flags parameter!

That means that doing:

git rev-list --objects --indexed-objects

and

git rev-list --objects --not --indexed-objects

produce the same output, because we ignore the UNINTERESTING
flag when walking the index in the second example.

Nobody noticed because this feature was added as a way for
tools like repack to increase their coverage of reachable
objects, meaning it would only be used like the first
example above.

But since it's user facing (and because the documentation
describes it "as if the objects are listed on the command
line"), we should make sure the negative case behaves
sensibly.

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

xdiff-interface: provide a separate consume callback for hunks

The previous commit taught xdiff to optionally provide the hunk header
data to a specialized callback. But most users of xdiff actually use our
more convenient xdi_diff_outf() helper, which ensures that our callbacks
are always fed whole lines.

Let's plumb the special hunk-callback through this interface, too. It
will follow the same rule as xdiff when the hunk callback is NULL (i.e.,
continue to pass a stringified hunk header to the line callback). Since
we add NULL to each caller, there should be no behavior change yet.

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

xdiff: provide a separate emit callback for hunks

The xdiff library always emits hunk header lines to our callbacks as
formatted strings like "@@ -a,b +c,d @@\n". This is convenient if we're
going to output a diff, but less so if we actually need to compute using
those numbers, which requires re-parsing the line.

In preparation for moving away from this, let's teach xdiff a new
callback function which gets the broken-out hunk information. To help
callers that don't want to use this new callback, if it's NULL we'll
continue to format the hunk header into a string.

Note that this function renames the "outf" callback to "out_line", as
well. This isn't strictly necessary, but helps in two ways:

1. Now that there are two callbacks, it's nice to use more descriptive
names.

2. Many callers did not zero the emit_callback_data struct, and needed
to be modified to set ecb.out_hunk to NULL. By changing the name of
the existing struct member, that guarantees that any new callers
from in-flight topics will break the build and be examined
manually.

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

t6012: make rev-list tests more interesting

As we are working to rewrite some of the revision-walk machinery,
there could easily be some interesting interactions between the
options that force topological constraints (--topo-order,
--date-order, and --author-date-order) along with specifying a
path.

Add extra tests to t6012-rev-list-simplify.sh to add coverage of
these interactions. To ensure interesting things occur, alter the
repo data shape to have different orders depending on topo-, date-,
or author-date-order.

When testing using GIT_TEST_COMMIT_GRAPH, this assists in covering
the new logic for topo-order walks using generation numbers. The
extra tests can be added indepently.

Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

revision.c: generation-based topo-order algorithm

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

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

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

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

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

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

Recall that the generation number of a commit satisfies:

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

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

There are two special generation numbers:

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

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

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

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

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

The walks are as follows:

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

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

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

The implementations of these walks are in the following methods:

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

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

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

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

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

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

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

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

Helped-by: Jeff King <peff@peff.net>
Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

commit/revisions: bookkeeping before refactoring

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

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

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

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

Helped-by: Jeff King <peff@peff.net>
Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

revision.c: begin refactoring --topo-order logic

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

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

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

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

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

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

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

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

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

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

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

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

Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

test-reach: add rev-list tests

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

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

Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

test-reach: add run_three_modes method

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

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

Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

prio-queue: add 'peek' operation

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

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

Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

travis-ci: install packages in 'ci/install-dependencies.sh'

Ever since we started using Travis CI, we specified the list of
packages to install in '.travis.yml' via the APT addon. While running
our builds on Travis CI's container-based infrastructure we didn't
have another choice, because that environment didn't support 'sudo',
and thus we didn't have permission to install packages ourselves. With
the switch to the VM-based infrastructure in the previous patch we do
get a working 'sudo', so we can install packages by running 'sudo
apt-get -y install ...' as well.

Let's make use of this and install necessary packages in
'ci/install-dependencies.sh', so all the dependencies (i.e. both
packages and "non-packages" (P4 and Git-LFS)) are handled in the same
file. Install gcc-8 only in the 'linux-gcc' build job; so far it has
been unnecessarily installed in the 'linux-clang' build job as well.
Print the versions of P4 and Git-LFS conditionally, i.e. only when
they have been installed; with this change even the static analysis
and documentation build jobs start using 'ci/install-dependencies.sh'
to install packages, and neither of these two build jobs depend on and
thus install those.

This change will presumably be beneficial for the upcoming Azure
Pipelines integration [1]: preliminary versions of that patch series
run a couple of 'apt-get' commands to install the necessary packages
before running 'ci/install-dependencies.sh', but with this patch it
will be sufficient to run only 'ci/install-dependencies.sh'.

[1] https://public-inbox.org/git/1a22efe849d6da79f2c639c62a1483361a130238.1539598316.git.gitgitgadget@gmail.com/

Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

tests: optionally skip `git rebase -p` tests

The `--preserve-merges` mode of the `rebase` command is slated to be
deprecated soon, as the more powerful `--rebase-merges` mode is
available now, and the latter was designed with the express intent to
address the shortcomings of `--preserve-merges`' design (e.g. the
inability to reorder commits in an interactive rebase).

As such, we will eventually even remove the `--preserve-merges` support,
and along with it, its tests.

In preparation for this, and also to allow the Windows phase of our
automated tests to save some well-needed time when running the test
suite, this commit introduces a new prerequisite REBASE_P, which can be
forced to being unmet by setting the environment variable
`GIT_TEST_SKIP_REBASE_P` to any non-empty string.

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

t3418: decouple test cases from a previous `rebase -p` test case

It is in general a good idea for regression test cases to be as
independent of each other as possible (with the one exception of an
initial `setup` test case, which is only a test case in Git's test suite
because it does not have a notion of a fixture or setup).

This patch addresses one particular instance of this principle being
violated: a few test cases in t3418-rebase-continue.sh depend on a side
effect of a test case that verifies a specific `rebase -p` behavior. The
later test cases should, however, still succeed even if the `rebase -p`
test case is skipped.

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

t3404: decouple some test cases from outcomes of previous test cases

Originally, the `--preserve-merges` option of the `git rebase` command
piggy-backed on top of the `--interactive` feature. For that reason, the
early test cases were added to the very same test script that contains
the `git rebase -i` tests: `t3404-rebase-interactive.sh`.

However, since c42abfe7857 (rebase: introduce a dedicated backend for
--preserve-merges, 2018-05-28), the `--preserve-merges` feature got its
own backend, in preparation for converting the rest of the
`--interactive` code to built-in code, written in C rather than shell.

The reason why the `--preserve-merges` feature was not converted at the
same time is that we have something much better now: `--rebase-merges`.
That option intends to supersede `--preserve-merges`, and we will
probably deprecate the latter soon.

Once `--preserve-merges` has been deprecated for a good amount of time,
it will be time to remove it, and along with it, its tests.

In preparation for that, let's make the rest of the test cases in
`t3404-rebase-interactive.sh` independent of the test cases dedicated to
`--preserve-merges`.

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

Eighth batch for 2.20

Signed-off-by: Junio C Hamano <gitster@pobox.com>

rebase: apply cocci patch

Favor oideq() over !oidcmp() when checking for equality.

Signed-off-by: Junio C Hamano <gitster@pobox.com>

Merge branch 'js/rebase-i-shortopt'

"git rebase -i" learned to take 'b' as the short form of 'break'
option in the todo list.

* js/rebase-i-shortopt:
rebase -i: recognize short commands without arguments

Merge branch 'js/rebase-i-break'

"git rebase -i" learned a new insn, 'break', that the user can
insert in the to-do list. Upon hitting it, the command returns
control back to the user.

* js/rebase-i-break:
rebase -i: introduce the 'break' command
rebase -i: clarify what happens on a failed `exec`

Merge branch 'js/rebase-autostash-fix'

"git rebase" that has recently been rewritten in C had a few issues
in its "--autstash" feature, which have been corrected.

* js/rebase-autostash-fix:
rebase --autostash: fix issue with dirty submodules
rebase --autostash: demonstrate a problem with dirty submodules
rebase (autostash): use an explicit OID to apply the stash
rebase (autostash): store the full OID in <state-dir>/autostash
rebase (autostash): avoid duplicate call to state_dir_path()

Merge branch 'cb/printf-empty-format'

Build fix for a topic in flight.

* cb/printf-empty-format:
sequencer: cleanup for gcc warning in non developer mode

Merge branch 'jc/rebase-in-c-5-test-typofix'

Typofix.

* jc/rebase-in-c-5-test-typofix:
rebase: fix typoes in error messages

Merge branch 'pk/rebase-in-c-6-final'

The final step of rewriting "rebase -i" in C.

* pk/rebase-in-c-6-final:
rebase: default to using the builtin rebase

Merge branch 'js/rebase-in-c-5.5-work-with-rebase-i-in-c'

"rebase" that has been rewritten learns the new calling convention
used by "rebase -i" that was rewritten in C, tying the loose end
between two GSoC topics that stomped on each other's toes.

* js/rebase-in-c-5.5-work-with-rebase-i-in-c:
builtin rebase: prepare for builtin rebase -i

Merge branch 'pk/rebase-in-c-5-test'

Rewrite "git rebase" in C.

* pk/rebase-in-c-5-test:
builtin rebase: error out on incompatible option/mode combinations
builtin rebase: use no-op editor when interactive is "implied"
builtin rebase: show progress when connected to a terminal
builtin rebase: fast-forward to onto if it is a proper descendant
builtin rebase: optionally pass custom reflogs to reset_head()
builtin rebase: optionally auto-detect the upstream

Merge branch 'pk/rebase-in-c-4-opts'

Rewrite "git rebase" in C.

* pk/rebase-in-c-4-opts:
builtin rebase: support --root
builtin rebase: add support for custom merge strategies
builtin rebase: support `fork-point` option
merge-base --fork-point: extract libified function
builtin rebase: support --rebase-merges[=[no-]rebase-cousins]
builtin rebase: support `--allow-empty-message` option
builtin rebase: support `--exec`
builtin rebase: support `--autostash` option
builtin rebase: support `-C` and `--whitespace=<type>`
builtin rebase: support `--gpg-sign` option
builtin rebase: support `--autosquash`
builtin rebase: support `keep-empty` option
builtin rebase: support `ignore-date` option
builtin rebase: support `ignore-whitespace` option
builtin rebase: support --committer-date-is-author-date
builtin rebase: support --rerere-autoupdate
builtin rebase: support --signoff
builtin rebase: allow selecting the rebase "backend"

Merge branch 'pk/rebase-in-c-3-acts'

Rewrite "git rebase" in C.

* pk/rebase-in-c-3-acts:
builtin rebase: stop if `git am` is in progress
builtin rebase: actions require a rebase in progress
builtin rebase: support --edit-todo and --show-current-patch
builtin rebase: support --quit
builtin rebase: support --abort
builtin rebase: support --skip
builtin rebase: support --continue

Merge branch 'pk/rebase-in-c-2-basic'

Rewrite "git rebase" in C.

* pk/rebase-in-c-2-basic:
builtin rebase: support `git rebase <upstream> <switch-to>`
builtin rebase: only store fully-qualified refs in `options.head_name`
builtin rebase: start a new rebase only if none is in progress
builtin rebase: support --force-rebase
builtin rebase: try to fast forward when possible
builtin rebase: require a clean worktree
builtin rebase: support the `verbose` and `diffstat` options
builtin rebase: support --quiet
builtin rebase: handle the pre-rebase hook and --no-verify
builtin rebase: support `git rebase --onto A...B`
builtin rebase: support --onto

Merge branch 'ag/rebase-i-in-c'

Rewrite of the remaining "rebase -i" machinery in C.

* ag/rebase-i-in-c:
rebase -i: move rebase--helper modes to rebase--interactive
rebase -i: remove git-rebase--interactive.sh
rebase--interactive2: rewrite the submodes of interactive rebase in C
rebase -i: implement the main part of interactive rebase as a builtin
rebase -i: rewrite init_basic_state() in C
rebase -i: rewrite write_basic_state() in C
rebase -i: rewrite the rest of init_revisions_and_shortrevisions() in C
rebase -i: implement the logic to initialize $revisions in C
rebase -i: remove unused modes and functions
rebase -i: rewrite complete_action() in C
t3404: todo list with commented-out commands only aborts
sequencer: change the way skip_unnecessary_picks() returns its result
sequencer: refactor append_todo_help() to write its message to a buffer
rebase -i: rewrite checkout_onto() in C
rebase -i: rewrite setup_reflog_action() in C
sequencer: add a new function to silence a command, except if it fails
rebase -i: rewrite the edit-todo functionality in C
editor: add a function to launch the sequence editor
rebase -i: rewrite append_todo_help() in C
sequencer: make three functions and an enum from sequencer.c public

Merge branch 'pk/rebase-in-c'

Rewrite of the "rebase" machinery in C.

* pk/rebase-in-c:
builtin/rebase: support running "git rebase <upstream>"
rebase: refactor common shell functions into their own file
rebase: start implementing it as a builtin

fetch: replace string-list used as a look-up table with a hashmap

In find_non_local_tags() helper function (used to implement the
"follow tags"), we use string_list_has_string() on two string lists
as a way to see if a refname has already been processed, etc.

All this code predates more modern in-core lookup API like hashmap;
replace them with two hashmaps and one string list---the hashmaps
are used for look-ups and the string list is to keep the order of
items in the returned result stable (which is the only single thing
hashmap does worse than lookups on string-list).

Similarly, get_ref_map() uses another string-list as a look-up table
for existing refs. Replace it with a hashmap.

Signed-off-by: Junio C Hamano <gitster@pobox.com>

rev-parse: clear --exclude list after 'git rev-parse --all'

Commit [1] added the --exclude option to revision.c. The --all,
--branches, --tags, --remotes, and --glob options clear the exclude
list. Shortly therafter, commit [2] added the same to 'git rev-parse',
but without clearing the exclude list for the --all option.

[1] e7b432c52 ("revision: introduce --exclude=<glob> to tame wildcards", 2013-08-30)
[2] 9dc01bf06 ("rev-parse: introduce --exclude=<glob> to tame wildcards", 2013-11-01)

Signed-off-by: Andreas Gruenbacher <agruenba@redhat.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

fetch-pack: be more precise in parsing v2 response

Each section in a protocol v2 response is followed by either a DELIM
packet (indicating more sections to follow) or a FLUSH packet
(indicating none to follow). But when parsing the "acknowledgments"
section, do_fetch_pack_v2() is liberal in accepting both, but determines
whether to continue reading or not based solely on the contents of the
"acknowledgments" section, not on whether DELIM or FLUSH was read.

There is no issue with a protocol-compliant server, but can result in
confusing error messages when communicating with a server that
serves unexpected additional sections. Consider a server that sends
"new-section" after "acknowledgments":

- client writes request
- client reads the "acknowledgments" section which contains no "ready",
then DELIM
- since there was no "ready", client needs to continue negotiation, and
writes request
- client reads "new-section", and reports to the end user "expected
'acknowledgments', received 'new-section'"

For the person debugging the involved Git implementation(s), the error
message is confusing in that "new-section" was not received in response
to the latest request, but to the first one.

One solution is to always continue reading after DELIM, but in this
case, we can do better. We know from the protocol that "ready" means at
least the packfile section is coming (hence, DELIM) and that no "ready"
means that no sections are to follow (hence, FLUSH). So teach
process_acks() to enforce this.

Signed-off-by: Jonathan Tan <jonathantanmy@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

sequencer: use read_author_script()

Use the new function added in the last commit to read the author
script, updating read_env_script() and read_author_ident(). We now
have a single code path that reads the author script for am and all
flavors of rebase. This changes the behavior of read_env_script() as
previously it would set any environment variables that were in the
author-script file. Now it is an error if the file contains other
variables or any of GIT_AUTHOR_NAME, GIT_AUTHOR_EMAIL and
GIT_AUTHOR_DATE are missing. This is what am and the non interactive
version of rebase have been doing for several years so hopefully it
will not cause a problem for interactive rebase users. The advantage
is that we are reusing existing code from am which uses sq_dequote()
to properly dequote variables. This fixes potential problems with user
edited scripts as read_env_script() which did not track quotes
properly.

This commit also removes the fallback code for checking for a broken
author script after git is upgraded when a rebase is stopped. Now that
the parsing uses sq_dequote() it will reliably return an error if the
quoting is broken and the user will have to abort the rebase and
restart. This isn't ideal but it's a corner case and the detection of
the broken quoting could be confused by user edited author scripts.

Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

add read_author_script() to libgit

Add read_author_script() to sequencer.c based on the implementation in
builtin/am.c and update read_am_author_script() to use
read_author_script(). The sequencer code that reads the author script
will be updated in the next commit.

Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

am: rename read_author_script()

Rename read_author_script() in preparation for adding a shared
read_author_script() function to libgit.

Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

am: improve author-script error reporting

If there are errors in a user edited author-script there was no
indication of what was wrong. This commit adds some specific error messages
depending on the problem. It also relaxes the requirement that the
variables appear in a specific order in the file to match the behavior
of 'rebase --interactive'.

Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

am: don't die in read_author_script()

The caller is already prepared to handle errors returned from this
function so there is no need for it to die if it cannot read the file.

Suggested-by: Eric Sunshine <sunshine@sunshineco.com>
Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

t/helper: add test-submodule-nested-repo-config

Add a test tool to exercise config_from_gitmodules(), in particular for
the case of nested submodules.

Add also a test to document that reading the submoudles config of nested
submodules does not work yet when the .gitmodules file is not in the
working tree but it still in the index.

This is because the git API does not always make it possible access the
object store of an arbitrary repository (see get_oid() usage in
config_from_gitmodules()).

When this git limitation gets fixed the aforementioned use case will be
supported too.

Signed-off-by: Antonio Ospite <ao2@ao2.it>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

submodule: support reading .gitmodules when it's not in the working tree

When the .gitmodules file is not available in the working tree, try
using the content from the index and from the current branch. This
covers the case when the file is part of the repository but for some
reason it is not checked out, for example because of a sparse checkout.

This makes it possible to use at least the 'git submodule' commands
which *read* the gitmodules configuration file without fully populating
the working tree.

Writing to .gitmodules will still require that the file is checked out,
so check for that before calling config_set_in_gitmodules_file_gently.

Add a similar check also in git-submodule.sh::cmd_add() to anticipate
the eventual failure of the "git submodule add" command when .gitmodules
is not safely writeable; this prevents the command from leaving the
repository in a spurious state (e.g. the submodule repository was cloned
but .gitmodules was not updated because
config_set_in_gitmodules_file_gently failed).

Moreover, since config_from_gitmodules() now accesses the global object
store, it is necessary to protect all code paths which call the function
against concurrent access to the global object store. Currently this
only happens in builtin/grep.c::grep_submodules(), so call
grep_read_lock() before invoking code involving
config_from_gitmodules().

Finally, add t7418-submodule-sparse-gitmodules.sh to verify that reading
from .gitmodules succeeds and that writing to it fails when the file is
not checked out.

NOTE: there is one rare case where this new feature does not work
properly yet: nested submodules without .gitmodules in their working
tree. This has been documented with a warning and a test_expect_failure
item in t7814, and in this case the current behavior is not altered: no
config is read.

Signed-off-by: Antonio Ospite <ao2@ao2.it>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

read_istream_pack_non_delta(): document input handling

Twice now we have scratched our heads about why the loose streaming code
needs the protection added by 692f0bc7ae (avoid infinite loop in
read_istream_loose, 2013-03-25), but the similar code in its pack
counterpart does not.

The short answer is that use_pack() will die before it lets us run out
of bytes. Note that this could mean reading garbage (including the
trailing hash) from the packfile in some cases of corruption, but that's
OK. zlib will notice and complain (and if not, certainly the end result
will not match the object hash we expect).

Let's leave a comment this time to document our findings.

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

ls-remote: pass heads/tags prefixes to transport

Unlike its arbitrary text patterns, the --heads and --tags
options to ls-remote are true prefixes. We can pass this
information to the transport code. If the v2 protocol is in
use, that will reduce the size of the ref advertisement.

Note that the test added here succeeds both before and after
the patch. This is an optimization, not a bug-fix; it's just
making sure we didn't break anything.

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

ls-remote: do not send ref prefixes for patterns

Since b4be74105f (ls-remote: pass ref prefixes when requesting a
remote's refs, 2018-03-15), "ls-remote foo" will pass "refs/heads/foo",
"refs/tags/foo", etc to the transport code in an attempt to let the
other side reduce the size of its advertisement.

Unfortunately this is not correct, as ls-remote patterns do not follow
the usual ref lookup rules, and are in fact tail-matched. So we could
find "refs/heads/foo" or "refs/heads/a/much/deeper/foo" or even
"refs/another/hierarchy/foo".

Since we can't pass a prefix and there's not yet a v2 extension for
matching wildcards, we must disable this feature to keep the same
behavior as v1.

Reported-by: Jon Simons <jon@jonsimons.org>
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

Adjust for 2.19.x series

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

cat-file: handle streaming failures consistently

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

- cat-file -p $blob

- cat-file blob $blob

- echo $batch | cat-file --batch

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

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

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

check_stream_sha1(): handle input underflow

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

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

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

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

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

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

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

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

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

Reported-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Helped-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>