Merge branch 'am/mingw-poll-fix' into maint

MinGW's poll() emulation has been improved.

* am/mingw-poll-fix:
  mingw: workaround for hangs when sending STDIN

Merge branch 'hi/gpg-use-check-signature' into maint

"git merge signed-tag" while lacking the public key started to say
"No signature", which was utterly wrong.  This regression has been
reverted.

* hi/gpg-use-check-signature:
  Revert "gpg-interface: prefer check_signature() for GPG verification"

Merge branch 'ds/partial-clone-fixes' into maint

Fix for a bug revealed by a recent change to make the protocol v2
the default.

* ds/partial-clone-fixes:
  partial-clone: avoid fetching when looking for objects
  partial-clone: demonstrate bugs in partial fetch

Merge branch 'en/t3433-rebase-stat-dirty-failure' into maint

The merge-recursive machinery failed to refresh the cache entry for
a merge result in a couple of places, resulting in an unnecessary
merge failure, which has been fixed.

* en/t3433-rebase-stat-dirty-failure:
  merge-recursive: fix the refresh logic in update_file_flags
  t3433: new rebase testcase documenting a stat-dirty-like failure

Merge branch 'en/check-ignore' into maint

"git check-ignore" did not work when the given path is explicitly
marked as not ignored with a negative entry in the .gitignore file.

* en/check-ignore:
  check-ignore: fix documentation and implementation to match

Merge branch 'jk/push-option-doc-markup-fix' into maint

Doc markup fix.

* jk/push-option-doc-markup-fix:
  doc/config/push: use longer "--" line for preformatted example

Merge branch 'jk/doc-diff-parallel' into maint

Update to doc-diff.

* jk/doc-diff-parallel:
  doc-diff: use single-colon rule in rendering Makefile

Merge branch 'jh/notes-fanout-fix' into maint

The code to automatically shrink the fan-out in the notes tree had
an off-by-one bug, which has been killed.

* jh/notes-fanout-fix:
  notes.c: fix off-by-one error when decreasing notes fanout
  t3305: check notes fanout more carefully and robustly

Merge branch 'jk/index-pack-dupfix' into maint

The index-pack code now diagnoses a bad input packstream that
records the same object twice when it is used as delta base; the
code used to declare a software bug when encountering such an
input, but it is an input error.

* jk/index-pack-dupfix:
  index-pack: downgrade twice-resolved REF_DELTA to die()

Merge branch 'js/rebase-i-with-colliding-hash' into maint

"git rebase -i" identifies existing commits in its todo file with
their abbreviated object name, which could become ambigous as it
goes to create new commits, and has a mechanism to avoid ambiguity
in the main part of its execution.  A few other cases however were
not covered by the protection against ambiguity, which has been
corrected.

* js/rebase-i-with-colliding-hash:
  rebase -i: also avoid SHA-1 collisions with missingCommitsCheck
  rebase -i: re-fix short SHA-1 collision
  parse_insn_line(): improve error message when parsing failed

Merge branch 'jk/clang-sanitizer-fixes' into maint

C pedantry ;-) fix.

* jk/clang-sanitizer-fixes:
  obstack: avoid computing offsets from NULL pointer
  xdiff: avoid computing non-zero offset from NULL pointer
  avoid computing zero offsets from NULL pointer
  merge-recursive: use subtraction to flip stage
  merge-recursive: silence -Wxor-used-as-pow warning

Merge branch 'dt/submodule-rm-with-stale-cache' into maint

Running "git rm" on a submodule failed unnecessarily when
.gitmodules is only cache-dirty, which has been corrected.

* dt/submodule-rm-with-stale-cache:
  git rm submodule: succeed if .gitmodules index stat info is zero

Merge branch 'pb/recurse-submodule-in-worktree-fix' into maint

The "--recurse-submodules" option of various subcommands did not
work well when run in an alternate worktree, which has been
corrected.

* pb/recurse-submodule-in-worktree-fix:
  submodule.c: use get_git_dir() instead of get_git_common_dir()
  t2405: clarify test descriptions and simplify test
  t2405: use git -C and test_commit -C instead of subshells
  t7410: rename to t2405-worktree-submodule.sh

Merge branch 'es/outside-repo-errmsg-hints' into maint

An earlier update to show the location of working tree in the error
message did not consider the possibility that a git command may be
run in a bare repository, which has been corrected.

* es/outside-repo-errmsg-hints:
  prefix_path: show gitdir if worktree unavailable
  prefix_path: show gitdir when arg is outside repo

Merge branch 'js/builtin-add-i-cmds' into maint

Minor bugfixes to "git add -i" that has recently been rewritten in C.

* js/builtin-add-i-cmds:
  built-in add -i: accept open-ended ranges again
  built-in add -i: do not try to `patch`/`diff` an empty list of files

prefix_path: show gitdir if worktree unavailable

If there is no worktree at present, we can still hint the user about
Git's current directory by showing them the absolute path to the Git
directory. Even though the Git directory doesn't make it as easy to
locate the worktree in question, it can still help a user figure out
what's going on while developing a script.

This fixes a segmentation fault introduced in e0020b2f
("prefix_path: show gitdir when arg is outside repo", 2020-02-14).

Signed-off-by: Emily Shaffer <emilyshaffer@google.com>
[jc: added minimum tests, with help from Szeder Gábor]
Signed-off-by: Junio C Hamano <gitster@pobox.com>

Revert "gpg-interface: prefer check_signature() for GPG verification"

This reverts commit 72b006f4bfd30b7c5037c163efaf279ab65bea9c, which
breaks the end-user experience when merging a signed tag without
having the public key.  We should report "can't check because we
have no public key", but the code with this change claimed that
there was no signature.

mingw: workaround for hangs when sending STDIN

Explanation
-----------
The problem here is flawed `poll()` implementation. When it tries to
see if pipe can be written without blocking, it eventually calls
`NtQueryInformationFile()` and tests `WriteQuotaAvailable`. However,
the meaning of quota was misunderstood. The value of quota is reduced
when either some data was written to a pipe, *or* there is a pending
read on the pipe. Therefore, if there is a pending read of size >= than
the pipe's buffer size, poll() will think that pipe is not writable and
will hang forever, usually that means deadlocking both pipe users.

I have studied the problem and found that Windows pipes track two values:
`QuotaUsed` and `BytesInQueue`. The code in `poll()` apparently wants to
know `BytesInQueue` instead of quota. Unfortunately, `BytesInQueue` can
only be requested from read end of the pipe, while `poll()` receives
write end.

The git's implementation of `poll()` was copied from gnulib, which also
contains a flawed implementation up to today.

I also had a look at implementation in cygwin, which is also broken in a
subtle way. It uses this code in `pipe_data_available()`:
fpli.WriteQuotaAvailable = (fpli.OutboundQuota - fpli.ReadDataAvailable)
However, `ReadDataAvailable` always returns 0 for the write end of the pipe,
turning the code into an obfuscated version of returning pipe's total
buffer size, which I guess will in turn have `poll()` always say that pipe
is writable. The commit that introduced the code doesn't say anything about
this change, so it could be some debugging code that slipped in.

These are the typical sizes used in git:
0x2000 - default read size in `strbuf_read()`
0x1000 - default read size in CRT, used by `strbuf_getwholeline()`
0x2000 - pipe buffer size in compat\mingw.c

As a consequence, as soon as child process uses `strbuf_read()`,
`poll()` in parent process will hang forever, deadlocking both
processes.

This results in two observable behaviors:
1) If parent process begins sending STDIN quickly (and usually that's
   the case), then first `poll()` will succeed and first block will go
   through. MAX_IO_SIZE_DEFAULT is 8MB, so if STDIN exceeds 8MB, then
   it will deadlock.
2) If parent process waits a little bit for any reason (including OS
   scheduler) and child is first to issue `strbuf_read()`, then it will
   deadlock immediately even on small STDINs.

The problem is illustrated by `git stash push`, which will currently
read the entire patch into memory and then send it to `git apply` via
STDIN. If patch exceeds 8MB, git hangs on Windows.

Possible solutions
------------------
1) Somehow obtain `BytesInQueue` instead of `QuotaUsed`
   I did a pretty thorough search and didn't find any ways to obtain
   the value from write end of the pipe.
2) Also give read end of the pipe to `poll()`
   That can be done, but it will probably invite some dirty code,
   because `poll()`
   * can accept multiple pipes at once
   * can accept things that are not pipes
   * is expected to have a well known signature.
3) Make `poll()` always reply "writable" for write end of the pipe
   Afterall it seems that cygwin (accidentally?) does that for years.
   Also, it should be noted that `pump_io_round()` writes 8MB blocks,
   completely ignoring the fact that pipe's buffer size is only 8KB,
   which means that pipe gets clogged many times during that single
   write. This may invite a deadlock, if child's STDERR/STDOUT gets
   clogged while it's trying to deal with 8MB of STDIN. Such deadlocks
   could be defeated with writing less than pipe's buffer size per
   round, and always reading everything from STDOUT/STDERR before
   starting next round. Therefore, making `poll()` always reply
   "writable" shouldn't cause any new issues or block any future
   solutions.
4) Increase the size of the pipe's buffer
   The difference between `BytesInQueue` and `QuotaUsed` is the size
   of pending reads. Therefore, if buffer is bigger than size of reads,
   `poll()` won't hang so easily. However, I found that for example
   `strbuf_read()` will get more and more hungry as it reads large inputs,
   eventually surpassing any reasonable pipe buffer size.

Chosen solution
---------------
Make `poll()` always reply "writable" for write end of the pipe.
Hopefully one day someone will find a way to implement it properly.

Reproduction
------------
printf "%8388608s" X >large_file.txt
git stash push --include-untracked -- large_file.txt

I have decided not to include this as test to avoid slowing down the
test suite. I don't expect the specific problem to come back, and
chances are that `git stash push` will be reworked to avoid sending the
entire patch via STDIN.

Signed-off-by: Alexandr Miloslavskiy <alexandr.miloslavskiy@syntevo.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

partial-clone: avoid fetching when looking for objects

When using partial clone, find_non_local_tags() in builtin/fetch.c
checks each remote tag to see if its object also exists locally. There
is no expectation that the object exist locally, but this function
nevertheless triggers a lazy fetch if the object does not exist. This
can be extremely expensive when asking for a commit, as we are
completely removed from the context of the non-existent object and
thus supply no "haves" in the request.

6462d5eb9a (fetch: remove fetch_if_missing=0, 2019-11-05) removed a
global variable that prevented these fetches in favor of a bitflag.
However, some object existence checks were not updated to use this flag.

Update find_non_local_tags() to use OBJECT_INFO_SKIP_FETCH_OBJECT in
addition to OBJECT_INFO_QUICK. The _QUICK option only prevents
repreparing the pack-file structures. We need to be extremely careful
about supplying _SKIP_FETCH_OBJECT when we expect an object to not exist
due to updated refs.

This resolves a broken test in t5616-partial-clone.sh.

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

partial-clone: demonstrate bugs in partial fetch

While testing partial clone, I noticed some odd behavior. I was testing
a way of running 'git init', followed by manually configuring the remote
for partial clone, and then running 'git fetch'. Astonishingly, I saw
the 'git fetch' process start asking the server for multiple rounds of
pack-file downloads! When tweaking the situation a little more, I
discovered that I could cause the remote to hang up with an error.

Add two tests that demonstrate these two issues.

In the first test, we find that when fetching with blob filters from
a repository that previously did not have any tags, the 'git fetch
--tags origin' command fails because the server sends "multiple
filter-specs cannot be combined". This only happens when using
protocol v2.

In the second test, we see that a 'git fetch origin' request with
several ref updates results in multiple pack-file downloads. This must
be due to Git trying to fault-in the objects pointed by the refs. What
makes this matter particularly nasty is that this goes through the
do_oid_object_info_extended() method, so there are no "haves" in the
negotiation. This leads the remote to send every reachable commit and
tree from each new ref, providing a quadratic amount of data transfer!
This test is fixed if we revert 6462d5eb9a (fetch: remove
fetch_if_missing=0, 2019-11-05), but that revert causes other test
failures. The real fix will need more care.

The tests are ordered in this way because if I swap the test order the
tag test will succeed instead of fail. I believe this is because somehow
we need the srv.bare repo to not have any tags when we clone, but then
have tags in our next fetch.

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

merge-recursive: fix the refresh logic in update_file_flags

If we need to delete a higher stage entry in the index to place the file
at stage 0, then we'll lose that file's stat information.  In such
situations we may still be able to detect that the file on disk is the
version we want (as noted by our comment in the code:
  /* do not overwrite file if already present */
), but we do still need to update the mtime since we are creating a new
cache_entry for that file.  Update the logic used to determine whether
we refresh a file's mtime.

Signed-off-by: Elijah Newren <newren@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

t3433: new rebase testcase documenting a stat-dirty-like failure

A user discovered a case where they had a stack of 20 simple commits to
rebase, and the rebase would succeed in picking the first commit and
then error out with a pair of "Could not execute the todo command" and
"Your local changes to the following files would be overwritten by
merge" messages.

Their steps actually made use of the -i flag, but I switched it over to
-m to make it simpler to trigger the bug.  With that flag, it bisects
back to commit 68aa495b590d (rebase: implement --merge via the
interactive machinery, 2018-12-11), but that's misleading.  If you
change the -m flag to --keep-empty, then the problem persists and will
bisect back to 356ee4659bb5 (sequencer: try to commit without forking
'git commit', 2017-11-24)

After playing with the testcase for a bit, I discovered that added
--exec "sleep 1" to the command line makes the rebase succeed, making me
suspect there is some kind of discard and reloading of caches that lead
us to believe that something is stat dirty, but I didn't succeed in
digging any further than that.

Signed-off-by: Elijah Newren <newren@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

check-ignore: fix documentation and implementation to match

check-ignore has two different modes, and neither of these modes has an
implementation that matches the documentation.  These modes differ in
whether they just print paths or whether they also print the final
pattern matched by the path.  The fix is different for both modes, so
I'll discuss both separately.

=== First (default) mode ===

The first mode is documented as:

    For each pathname given via the command-line or from a file via
    --stdin, check whether the file is excluded by .gitignore (or other
    input files to the exclude mechanism) and output the path if it is
    excluded.

However, it fails to do this because it did not account for negated
patterns.  Commands other than check-ignore verify exclusion rules via
calling

   ... -> treat_one_path() -> is_excluded() -> last_matching_pattern()

while check-ignore has a call path of the form:

   ... -> check_ignore()                    -> last_matching_pattern()

The fact that the latter does not include the call to is_excluded()
means that it is susceptible to to messing up negated patterns (since
that is the only significant thing is_excluded() adds over
last_matching_pattern()).  Unfortunately, we can't make it just call
is_excluded(), because the same codepath is used by the verbose mode
which needs to know the matched pattern in question.  This brings us
to...

=== Second (verbose) mode ===

The second mode, known as verbose mode, references the first in the
documentation and says:

    Also output details about the matching pattern (if any) for each
    given pathname. For precedence rules within and between exclude
    sources, see gitignore(5).

The "Also" means it will print patterns that match the exclude rules as
noted for the first mode, and also print which pattern matches.  Unless
more information is printed than just pathname and pattern (which is not
done), this definition is somewhat ill-defined and perhaps even
self-contradictory for negated patterns: A path which matches a negated
exclude pattern is NOT excluded and thus shouldn't be printed by the
former logic, while it certainly does match one of the explicit patterns
and thus should be printed by the latter logic.

=== Resolution ==

Since the second mode exists to find out which pattern matches given
paths, and showing the user a pattern that begins with a '!' is
sufficient for them to figure out whether the pattern is excluded, the
existing behavior is desirable -- we just need to update the
documentation to match the implementation (i.e. it is about printing
which pattern is matched by paths, not about showing which paths are
excluded).

For the first or default mode, users just want to know whether a pattern
is excluded.  As such, the existing documentation is desirable; change
the implementation to match the documented behavior.

Finally, also adjust a few tests in t0008 that were caught up by this
discrepancy in how negated paths were handled.

Signed-off-by: Elijah Newren <newren@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

doc-diff: use single-colon rule in rendering Makefile

When rendering the troff manpages to text via "man", we create an ad-hoc
Makefile and feed it to "make". The purpose here is two-fold:

  - reuse results from a prior interrupted render of the same tree

  - use make's -j option to build in parallel

But the second part doesn't seem to work (at least with my version of
GNU make, 4.2.1). It just runs one render at a time.

We use a double-colon "all" rule for each file, like:

  all:: foo
  foo:
    ...actual render recipe...
  all:: bar
  bar:
    ...actual render recipe...
  ...and so on...

And it's this double-colon that seems to inhibit the parallelism. We can
just switch to a regular single-colon rule. Even though we do have
multiple rules for "all" here, we don't have any recipe to execute for
"all" (we only care about triggering its dependencies), so the
distinction is irrelevant.

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

doc/config/push: use longer "--" line for preformatted example

The example for the push.pushOption config tries to create a
preformatted section, but uses only two dashes in its "--" line. In
AsciiDoc this is an "open block", with no type; the lines end up jumbled
because they're formatted as paragraphs. We need four or more dashes to
make it a "listing block" that will respect the linebreaks.

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

Git 2.25.1

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

prefix_path: show gitdir when arg is outside repo

When developing a script, it can be painful to understand why Git thinks
something is outside the current repo, if the current repo isn't what
the user thinks it is. Since this can be tricky to diagnose, especially
in cases like submodules or nested worktrees, let's give the user a hint
about which repository is offended about that path.

Signed-off-by: Emily Shaffer <emilyshaffer@google.com>
Acked-by: brian m. carlson <sandals@crustytoothpaste.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

Merge branch 'js/convert-typofix' into maint

Typofix.

* js/convert-typofix:
  convert: fix typo

Merge branch 'js/ci-squelch-doc-warning' into maint

Squelch unhelpful warning message during documentation build.

* js/ci-squelch-doc-warning:
  ci: ignore rubygems warning in the "Documentation" job

Merge branch 'jb/multi-pack-index-docfix' into maint

Doc fix.

* jb/multi-pack-index-docfix:
  pack-format: correct multi-pack-index description

Merge branch 'ma/diff-doc-clarify-regexp-example' into maint

Doc clarification.

* ma/diff-doc-clarify-regexp-example:
  diff-options.txt: avoid "regex" overload in example

Merge branch 'ms/doc-bundle-format' into maint

Technical details of the bundle format has been documented.
I think this is in a good enough shape.

* ms/doc-bundle-format:
  doc: describe Git bundle format

Merge branch 'es/submodule-fetch-message-fix' into maint

Error message fix.

* es/submodule-fetch-message-fix:
  submodule: add newline on invalid submodule error

Merge branch 'jb/parse-options-message-fix' into maint

Error message fix.

* jb/parse-options-message-fix:
  parse-options: lose an unnecessary space in an error message

Merge branch 'ma/filter-branch-doc-caret' into maint

Doc mark-up updates.

* ma/filter-branch-doc-caret:
  git-filter-branch.txt: wrap "maths" notation in backticks

Merge branch 'km/submodule-doc-use-sm-path' into maint

Docfix.

* km/submodule-doc-use-sm-path:
  submodule foreach: replace $path with $sm_path in example

Merge branch 'pb/do-not-recurse-grep-no-index' into maint

"git grep --no-index" should not get affected by the contents of
the .gitmodules file but when "--recurse-submodules" is given or
the "submodule.recurse" variable is set, it did.  Now these
settings are ignored in the "--no-index" mode.

* pb/do-not-recurse-grep-no-index:
  grep: ignore --recurse-submodules if --no-index is given

Merge branch 'jt/t5616-robustify' into maint

Futureproofing a test not to depend on the current implementation
detail.

* jt/t5616-robustify:
  t5616: make robust to delta base change

Merge branch 'en/fill-directory-fixes-more' into maint

Corner case bugs in "git clean" that stems from a (necessarily for
performance reasons) awkward calling convention in the directory
enumeration API has been corrected.

* en/fill-directory-fixes-more:
  dir: point treat_leading_path() warning to the right place
  dir: restructure in a way to avoid passing around a struct dirent
  dir: treat_leading_path() and read_directory_recursive(), round 2
  clean: demonstrate a bug with pathspecs

Merge branch 'bc/misconception-doc' into maint

Doc updates.

* bc/misconception-doc:
  docs: mention when increasing http.postBuffer is valuable
  doc: dissuade users from trying to ignore tracked files

Merge branch 'bc/author-committer-doc' into maint

Clarify documentation on committer/author identities.

* bc/author-committer-doc:
  doc: provide guidance on user.name format
  docs: expand on possible and recommended user config options
  doc: move author and committer information to git-commit(1)

Merge branch 'ds/refmap-doc' into maint

"git fetch --refmap=" option has got a better documentation.

* ds/refmap-doc:
  fetch: document and test --refmap=""

Merge branch 'bc/actualmente' into maint

Doc grammo fix.

* bc/actualmente:
  docs: use "currently" for the present time

Merge branch 'rt/submodule-i18n' into maint

Comments update.

* rt/submodule-i18n:
  submodule.c: mark more strings for translation

Merge branch 'jk/test-fixes' into maint

Test fixes.

* jk/test-fixes:
  t7800: don't rely on reuse_worktree_file()
  t4018: drop "debugging" cat from hunk-header tests

Merge branch 'jk/asan-build-fix' into maint

Work around test breakages caused by custom regex engine used in
libasan, when address sanitizer is used with more recent versions
of gcc and clang.

* jk/asan-build-fix:
  Makefile: use compat regex with SANITIZE=address

Merge branch 'ds/sparse-cone' into maint

The code recently added in this release to move to the entry beyond
the ones in the same directory in the index in the sparse-cone mode
did not count the number of entries to skip over incorrectly, which
has been corrected.

* ds/sparse-cone:
  .mailmap: fix GGG authoship screwup
  unpack-trees: correctly compute result count

Merge branch 'nd/switch-and-restore' into maint

"git restore --staged" did not correctly update the cache-tree
structure, resulting in bogus trees to be written afterwards, which
has been corrected.

* nd/switch-and-restore:
  restore: invalidate cache-tree when removing entries with --staged

Merge branch 'jk/no-flush-upon-disconnecting-slrpc-transport' into maint

Reduce unnecessary round-trip when running "ls-remote" over the
stateless RPC mechanism.

* jk/no-flush-upon-disconnecting-slrpc-transport:
  transport: don't flush when disconnecting stateless-rpc helper

Merge branch 'hw/tutorial-favor-switch-over-checkout' into maint

Complete an update to tutorial that encourages "git switch" over
"git checkout" that was done only half-way.

* hw/tutorial-favor-switch-over-checkout:
  doc/gitcore-tutorial: fix prose to match example command

Merge branch 'es/unpack-trees-oob-fix' into maint

The code that tries to skip over the entries for the paths in a
single directory using the cache-tree was not careful enough
against corrupt index file.

* es/unpack-trees-oob-fix:
  unpack-trees: watch for out-of-range index position

Merge branch 'bc/run-command-nullness-after-free-fix' into maint

C pedantry ;-) fix.

* bc/run-command-nullness-after-free-fix:
  run-command: avoid undefined behavior in exists_in_PATH

Merge branch 'en/string-list-can-be-custom-sorted' into maint

API-doc update.

* en/string-list-can-be-custom-sorted:
  string-list: note in docs that callers can specify sorting function

Merge branch 'jt/sha1-file-remove-oi-skip-cached' into maint

has_object_file() said "no" given an object registered to the
system via pretend_object_file(), making it inconsistent with
read_object_file(), causing lazy fetch to attempt fetching an
empty tree from promisor remotes.

* jt/sha1-file-remove-oi-skip-cached:
  sha1-file: remove OBJECT_INFO_SKIP_CACHED

Merge branch 'hw/commit-advise-while-rejecting' into maint

"git commit" gives output similar to "git status" when there is
nothing to commit, but without honoring the advise.statusHints
configuration variable, which has been corrected.

* hw/commit-advise-while-rejecting:
  commit: honor advice.statusHints when rejecting an empty commit

convert: fix typo

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

ci: ignore rubygems warning in the "Documentation" job

A recent update in the Linux VM images used by Azure Pipelines surfaced
a new problem in the "Documentation" job. Apparently, this warning
appears 396 times on `stderr` when running `make doc`:

/usr/lib/ruby/vendor_ruby/rubygems/defaults/operating_system.rb:10: warning: constant Gem::ConfigMap is deprecated

This problem was already reported to the `rubygems` project via
https://github.com/rubygems/rubygems/issues/3068.

As there is nothing Git can do about this warning, and as the
"Documentation" job reports this warning as a failure, let's just
silence it and move on.

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

pack-format: correct multi-pack-index description

The description of the multi-pack-index contains a small bug,
if all offsets are < 2^32 then there will be no LOFF chunk,
not only if they're all < 2^31 (since the highest bit is only
needed as the "LOFF-escape" when that's actually needed.)

Correct this, and clarify that in that case only offsets up
to 2^31-1 can be stored in the OOFF chunk.

Signed-off-by: Johannes Berg <johannes@sipsolutions.net>
Acked-by: Derrick Stolee <dstolee@microsoft.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff-options.txt: avoid "regex" overload in example

When we exemplify the difference between `-G` and `-S` (using
`--pickaxe-regex`), we do so using an example diff and git-diff
invocation involving "regexec", "regexp", "regmatch", ...

The example is correct, but we can make it easier to untangle by
avoiding writing "regex.*" unless it's really needed to make our point.

Use some made-up, non-regexy words instead.

Reported-by: Adam Dinwoodie <adam@dinwoodie.org>
Signed-off-by: Martin Ågren <martin.agren@gmail.com>
Reviewed-by: Taylor Blau <me@ttaylorr.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

doc: describe Git bundle format

The bundle format was not documented. Describe the format with ABNF and
explain the meaning of each part.

Signed-off-by: Masaya Suzuki <masayasuzuki@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

submodule: add newline on invalid submodule error

Since 'err' contains output for multiple submodules and is printed all
at once by fetch_populated_submodules(), errors for each submodule
should be newline separated for readability. The same strbuf is added to
with a newline in the other half of the conditional where this error is
detected, so make the two consistent.

Signed-off-by: Emily Shaffer <emilyshaffer@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

parse-options: lose an unnecessary space in an error message

Signed-off-by: Jacques Bodin-Hullin <j.bodinhullin@monsieurbiz.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

index-pack: downgrade twice-resolved REF_DELTA to die()

When we're resolving a REF_DELTA, we compare-and-swap its type from
REF_DELTA to whatever real type the base object has, as discussed in
ab791dd138 (index-pack: fix race condition with duplicate bases,
2014-08-29). If the old type wasn't a REF_DELTA, we consider that a
BUG(). But as discussed in that commit, we might see this case whenever
we try to resolve an object twice, which may happen because we have
multiple copies of the base object.

So this isn't a bug at all, but rather a sign that the input pack is
broken. And indeed, this case is triggered already in t5309.5 and
t5309.6, which create packs with delta cycles and duplicate bases. But
we never noticed because those tests are marked expect_failure.

Those tests were added by b2ef3d9ebb (test index-pack on packs with
recoverable delta cycles, 2013-08-23), which was leaving the door open
for cases that we theoretically _could_ handle. And when we see an
already-resolved object like this, in theory we could keep going after
confirming that the previously resolved child->real_type matches
base->obj->real_type. But:

  - enforcing the "only resolve once" rule here saves us from an
    infinite loop in other parts of the code. If we keep going, then the
    delta cycle in t5309.5 causes us to loop infinitely, as
    find_ref_delta_children() doesn't realize which objects have already
    been resolved. So there would be more changes needed to make this
    case work, and in the meantime we'd be worse off.

  - any pack that triggers this is broken anyway. It either has a
    duplicate base object, or it has a cycle which causes us to bring in
    a duplicate via --fix-thin. In either case, we'd end up rejecting
    the pack in write_idx_file(), which also detects duplicates.

So the tests have little value in documenting what we _could_ be doing
(and have been neglected for 6+ years). Let's switch them to confirming
that we handle this case cleanly (and switch out the BUG() for a more
informative die() so that we do so).

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

notes.c: fix off-by-one error when decreasing notes fanout

As noted in the previous commit, the nature of the fanout heuristic
in the notes code causes the exact point at which we increase or
decrease the notes fanout to vary with the objects being annotated.
Since the object ids generated by the test environment are
deterministic (by design), the notes generated and tested by t3305
are always the same, and we therefore happen to see the same fanout
behavior from one run to the next.

Coincidentally, if we were to change the test environment slightly
(say by making a test commit on an unrelated branch before we start
the t3305 test proper), we not only see the fanout switch happen at
different points, we also manage to trigger a _bug_ in the notes
code where the fanout 1 -> 0 switch is not applied uniformly across
the notes tree, but instead yields a notes tree like this:

  ...
  bdeafb301e44b0e4db0f738a2d2a7beefdb70b70
  bff2d39b4f7122bd4c5caee3de353a774d1e632a
  d3/8ec8f851adf470131178085bfbaab4b12ad2a7
  e0b173960431a3e692ae929736df3c9b73a11d5b
  eb3c3aede523d729990ac25c62a93eb47c21e2e3
  ...

The bug occurs when we are writing out a notes tree with a newly
decreased fanout, and the notes tree contains unexpanded subtrees
that should be consolidated into the parent tree as a consequence of
the decreased fanout):

Subtrees that happen to sit at an _even_ level in the internal notes
16-tree structure (in other words: subtrees whose path - "d3" in the
example above - is unique in the first nibble - i.e. there are no
other note paths that start with "d") are _not_ unpacked as part of
the tree writeout. This error will repeat itself in subsequent note
trees until the subtree is forced to be unpacked. In t3305 this only
happens when the d38ec8f8 note is itself removed from the tree.

The error is not severe (no information is lost, and the notes code
is able to read/decode this tree and manipulate it correctly), but
this is nonetheless a bug in the current implementation that should
be fixed.

That said, fixing the off-by-one error is not without complications:
We must take into account that the load_subtree() call from
for_each_note_helper() (that is now done to correctly unpack the
subtree while we're writing out the notes tree) may end up inserting
unpacked non-notes into the linked list of non_note entries held by
the struct notes_tree. Since we are in the process of writing out the
notes tree, this linked list is currently in the process of being
traversed by write_each_non_note_until(). The unpacked non-notes are
necessarily inserted between the last non-note we wrote out, and the
next non-note to be written. Hence, we cannot simply hold the
next_non_note to write in struct write_each_note_data (as we would
then silently skip these newly inserted notes), but must instead
always follow the ->next pointer from the last non-note we wrote.
(This part was caught by an existing test in t3304.)

Cc: Johannes Schindelin <Johannes.Schindelin@gmx.de>
Cc: Brian M. Carlson <sandals@crustytoothpaste.net>
Signed-off-by: Johan Herland <johan@herland.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

t3305: check notes fanout more carefully and robustly

In short, before this patch, this test script:
 - creates many notes
 - verifies that all notes in the notes tree has a fanout of 1
 - removes most notes
 - verifies that the notes in the notes tree now has a fanout of 0

The fanout verification only happened twice: after creating all the
notes, and after removing most of them.

This patch strengthens the test by checking the fanout after _each_
added/removed note: We assert that the switch from fanout 0 -> 1
happens exactly once while adding notes (and that the switch pervades
the entire notes tree). Likewise, we assert that the switch from
fanout 1 -> 0 happens exactly once while removing notes.

Additionally, we decrease the number of notes left after removal,
from 50 to 15 notes, in order to ensure that fanout 1 -> 0 transition
keeps happening regardless of external factors[1].

[1]: Currently (with the SHA1 hash function and the deterministic
object ids of the test environment) the fanout heuristic in the notes
code happens to switch from 0 -> 1 at 109 notes, and from 1 -> 0 at
59 notes. However, changing the hash function or other external
factors will vary these numbers, and the latter may - in theory - go
as low as 15. For more details, please see the discussion at
https://public-inbox.org/git/20200125230035.136348-4-sandals@crustytoothpaste.net/

Cc: Johannes Schindelin <Johannes.Schindelin@gmx.de>
Cc: Brian M. Carlson <sandals@crustytoothpaste.net>
Signed-off-by: Johan Herland <johan@herland.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

git-filter-branch.txt: wrap "maths" notation in backticks

In this paragraph, we have a few instances of the '^' character, which
we give as "\^". This renders well with AsciiDoc ("^"), but Asciidoctor
renders it literally as "\^". Dropping the backslashes renders fine
with Asciidoctor, but not AsciiDoc...

An earlier version of this patch used "{caret}" instead of "^", which
avoided these escaping problems. The rendering was still so-so, though
-- these expressions end up set as normal text, similarly to when one
provides, e.g., computer code in the middle of running text, without
properly marking it with `backticks` to be monospaced.

As noted by Jeff King, this suggests actually wrapping these
expressions in backticks, setting them in monospace.

The lone "5" could be left as is or wrapped as `5`. Spell it out as
"five" instead -- this generally looks better anyway for small numbers
in the middle of text like this.

Suggested-by: Jeff King <peff@peff.net>
Signed-off-by: Martin Ågren <martin.agren@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

submodule foreach: replace $path with $sm_path in example

f0fd0dc5c5 (submodule foreach: document '$sm_path' instead of '$path',
2018-05-08) updated the documentation to advise callers to favor
$sm_path over the deprecated synonym $path.  However, the example in
that section still uses $path.  Update it to use $sm_path.

Signed-off-by: Kyle Meyer <kyle@kyleam.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

.mailmap: map Yi-Jyun Pan's email

In 13185fd241 (l10n: zh_TW.po: update translation for v2.25.0 round 1,
2019-12-31), the author mistakenly used their GitHub username for
authorship information instead of their real name. However, a commit
with their real name exists prior to this: 9917eca794 (l10n: zh_TW: add
translation for v2.24.0, 2019-11-20).

Map their email to their real name so that these contributions can be
counted together.

Signed-off-by: Denton Liu <liu.denton@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

grep: ignore --recurse-submodules if --no-index is given

Since grep learned to recurse into submodules in 0281e487fd
(grep: optionally recurse into submodules, 2016-12-16),
using --recurse-submodules along with --no-index makes Git
die().

This is unfortunate because if submodule.recurse is set in a user's
~/.gitconfig, invoking `git grep --no-index` either inside or outside
a Git repository results in

    fatal: option not supported with --recurse-submodules

Let's allow using these options together, so that setting submodule.recurse
globally does not prevent using `git grep --no-index`.

Using `--recurse-submodules` should not have any effect if `--no-index`
is used inside a repository, as Git will recurse into the checked out
submodule directories just like into regular directories.

Helped-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Philippe Blain <levraiphilippeblain@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

obstack: avoid computing offsets from NULL pointer

As with the previous two commits, UBSan with clang-11 complains about
computing offsets from a NULL pointer. The failures in t4013 (and
elsewhere) look like this:

  kwset.c:102:23: runtime error: applying non-zero offset 107820859019600 to null pointer
  ...
  not ok 79 - git log -SF master # magic is (not used)

That line is not enlightening:

  ... = obstack_alloc(&kwset->obstack, sizeof (struct trie));

because obstack is implemented almost entirely in macros, and the actual
problem is five macros deep (I temporarily converted them to inline
functions to get better compiler errors, which was tedious but worked
reasonably well).

The actual problem is in these pointer-alignment macros:

  /* If B is the base of an object addressed by P, return the result of
     aligning P to the next multiple of A + 1.  B and P must be of type
     char *.  A + 1 must be a power of 2.  */

  #define __BPTR_ALIGN(B, P, A) ((B) + (((P) - (B) + (A)) & ~(A)))

  /* Similar to _BPTR_ALIGN (B, P, A), except optimize the common case
     where pointers can be converted to integers, aligned as integers,
     and converted back again.  If PTR_INT_TYPE is narrower than a
     pointer (e.g., the AS/400), play it safe and compute the alignment
     relative to B.  Otherwise, use the faster strategy of computing the
     alignment relative to 0.  */

  #define __PTR_ALIGN(B, P, A)                                                \
    __BPTR_ALIGN (sizeof (PTR_INT_TYPE) < sizeof (void *) ? (B) : (char *) 0, \
                  P, A)

If we have a sufficiently-large integer pointer type, then we do the
computation using a NULL pointer constant. That turns __BPTR_ALIGN()
into something like:

  NULL + (P - NULL + A) & ~A

and UBSan is complaining about adding the full value of P to that
initial NULL. We can fix this by doing our math as an integer type, and
then casting the result back to a pointer. The problem case only happens
when we know that the integer type is large enough, so there should be
no issue with truncation.

Another option would be just simplify out all the 0's from
__BPTR_ALIGN() for the NULL-pointer case. That probably wouldn't work
for a platform where the NULL pointer isn't all-zeroes, but Git already
wouldn't work on such a platform (due to our use of memset to set
pointers in structs to NULL). But I tried here to keep as close to the
original as possible.

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

xdiff: avoid computing non-zero offset from NULL pointer

As with the previous commit, clang-11's UBSan complains about computing
offsets from a NULL pointer, causing some tests to fail. In this case,
though, we're actually computing a non-zero offset, which is even more
dubious. From t7810:

  xdiff-interface.c:268:14: runtime error: applying non-zero offset 1 to null pointer
  ...
  not ok 131 - grep -p with userdiff

The problem is our parsing of the funcname config. We count the number
of lines in the string, allocate an array, and then loop over our
allocated entries, parsing each line and moving our cursor to one past
the trailing newline for the next iteration.

But the final line will not generally have a trailing newline (since
it's a config value), and hence we go to one past NULL. In practice this
is OK, since our loop should terminate before we look at the value. But
even computing such an invalid pointer technically violates the
standard.

We can fix it by leaving the pointer at NULL if we're at the end, rather
than one-past. And while we're thinking about it, we can also document
the variant by asserting that our initial line-count matches the
second-pass of parsing.

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

avoid computing zero offsets from NULL pointer

The Undefined Behavior Sanitizer in clang-11 seems to have learned a new
trick: it complains about computing offsets from a NULL pointer, even if
that offset is 0. This causes numerous test failures. For example, from
t1090:

  unpack-trees.c:1355:41: runtime error: applying zero offset to null pointer
  ...
  not ok 6 - in partial clone, sparse checkout only fetches needed blobs

The code in question looks like this:

  struct cache_entry **cache_end = cache + nr;
  ...
  while (cache != cache_end)

and we sometimes pass in a NULL and 0 for "cache" and "nr". This is
conceptually fine, as "cache_end" would be equal to "cache" in this
case, and we wouldn't enter the loop at all. But computing even a zero
offset violates the C standard. And given the fact that UBSan is
noticing this behavior, this might be a potential problem spot if the
compiler starts making unexpected assumptions based on undefined
behavior.

So let's just avoid it, which is pretty easy. In some cases we can just
switch to iterating with a numeric index (as we do in sequencer.c here).
In other cases (like the cache_end one) the use of an end pointer is
more natural; we can keep that by just explicitly checking for the
NULL/0 case when assigning the end pointer.

Note that there are two ways you can write this latter case, checking
for the pointer:

  cache_end = cache ? cache + nr : cache;

or the size:

  cache_end = nr ? cache + nr : cache;

For the case of a NULL/0 ptr/len combo, they are equivalent. But writing
it the second way (as this patch does) has the property that if somebody
were to incorrectly pass a NULL pointer with a non-zero length, we'd
continue to notice and segfault, rather than silently pretending the
length was zero.

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

git rm submodule: succeed if .gitmodules index stat info is zero

The bug was that ie_match_stat() was used to compare if the stat info
for the file was compatible with the stat info in the index, rather
using ie_modified() to check if the file was in fact different from the
version in the index.

A version of this (with deinit instead of rm) was reported here:
https://lore.kernel.org/git/CAPOqYV+C-P9M2zcUBBkD2LALPm4K3sxSut+BjAkZ9T1AKLEr+A@mail.gmail.com/

It seems that in that case, the user's clone command left the index
with empty stat info.  The mailing list was unable to reproduce this.
But we (Two Sigma) hit the bug while using some plumbing commands, so
I'm fixing it.  I manually confirmed that the fix also repairs deinit
in this scenario.

Signed-off-by: David Turner <dturner@twosigma.com>
Reported-by: Thomas Bétous <th.betous@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

.mailmap: fix GGG authoship screwup

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

t5616: make robust to delta base change

Commit 6462d5eb9a ("fetch: remove fetch_if_missing=0", 2019-11-08)
contains a test that relies on having to lazily fetch the delta base of
a blob, but assumes that the tree being fetched (as part of the test) is
sent as a non-delta object. This assumption may not hold in the future;
for example, a change in the length of the object hash might result in
the tree being sent as a delta instead.

Make the test more robust by relying on having to lazily fetch the delta
base of the tree instead, and by making no assumptions on whether the
blobs are sent as delta or non-delta.

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

merge-recursive: use subtraction to flip stage

The flip_stage() helper uses a bit-flipping xor to switch between "2"
and "3". While clever, this relies on a property of those two numbers
that is mostly coincidence. Let's write it as a subtraction; that's more
clear and would extend to other numbers if somebody copies the logic.

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

merge-recursive: silence -Wxor-used-as-pow warning

The merge-recursive code uses stage number constants like this:

  add = &ci->ren1->dst_entry->stages[2 ^ 1];
  ...
  add = &ci->ren2->dst_entry->stages[3 ^ 1];

The xor has the effect of flipping the "1" bit, so that "2 ^ 1" becomes
"3" and "3 ^ 1" becomes "2", which correspond to the "ours" and "theirs"
stages respectively.

Unfortunately, clang-10 and up issue a warning for this code:

  merge-recursive.c:1759:40: error: result of '2 ^ 1' is 3; did you mean '1 << 1' (2)? [-Werror,-Wxor-used-as-pow]
                  add = &ci->ren1->dst_entry->stages[2 ^ 1];
                                                     ~~^~~
                                                     1 << 1
  merge-recursive.c:1759:40: note: replace expression with '0x2 ^ 1' to silence this warning

We could silence it by using 0x2, as the compiler mentions. Or by just
using the constants "2" and "3" directly. But after digging into it, I
do think this bit-flip is telling us something. If we just wrote:

  add = &ci->ren2->dst_entry->stages[2];

for the second one, you might think that "ren2" and "2" correspond. But
they don't. The logic is: ren2 is theirs, which is stage 3, but we
are interested in the opposite side's stage, so flip it to 2.

So let's keep the bit-flipping, but let's also put it behind a named
function, which will make its purpose a bit clearer. This also has the
side effect of suppressing the warning (and an optimizing compiler
should be able to easily turn it into a constant as before).

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

.mailmap: fix erroneous authorship for Johannes Schindelin

In 49e268e23e (mingw: safeguard better against backslashes in file
names, 2020-01-09), the commit author is listed as
"Johannes Schindelin via GitGitGadget <gitgitgadget@gmail.com>", which
is erroneous. Fix the authorship by mapping the erroneous authorship to
his canonical authorship information.

Signed-off-by: Denton Liu <liu.denton@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

rebase -i: also avoid SHA-1 collisions with missingCommitsCheck

When `rebase.missingCommitsCheck` is in effect, we use the backup of the
todo list that was copied just before the user was allowed to edit it.

That backup is, of course, just as susceptible to the hash collision as
the todo list itself: a reworded commit could make a previously
unambiguous short commit ID ambiguous all of a sudden.

So let's not just copy the todo list, but let's instead write out the
backup with expanded commit IDs.

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

rebase -i: re-fix short SHA-1 collision

In 66ae9a57b88 (t3404: rebase -i: demonstrate short SHA-1 collision,
2013-08-23), we added a test case that demonstrated how it is possible
that a previously unambiguous short commit ID could become ambiguous
*during* a rebase.

In 75c69766554 (rebase -i: fix short SHA-1 collision, 2013-08-23), we
fixed that problem simply by writing out the todo list with expanded
commit IDs (except *right* before letting the user edit the todo list,
in which case we shorten them, but we expand them right after the file
was edited).

However, the bug resurfaced as a side effect of 393adf7a6f6 (sequencer:
directly call pick_commits() from complete_action(), 2019-11-24): as of
this commit, the sequencer no longer re-reads the todo list after
writing it out with expanded commit IDs.

The only redeeming factor is that the todo list is already parsed at
that stage, including all the commits corresponding to the commands,
therefore the sequencer can continue even if the internal todo list has
short commit IDs.

That does not prevent problems, though: the sequencer writes out the
`done` and `git-rebase-todo` files incrementally (i.e. overwriting the
todo list with a version that has _short_ commit IDs), and if a merge
conflict happens, or if an `edit` or a `break` command is encountered, a
subsequent `git rebase --continue` _will_ re-read the todo list, opening
an opportunity for the "short SHA-1 collision" bug again.

To avoid that, let's make sure that we do expand the commit IDs in the
todo list as soon as we have parsed it after letting the user edit it.

Additionally, we improve the 'short SHA-1 collide' test case in t3404 to
test specifically for the case where the rebase is resumed. We also
hard-code the expected colliding short SHA-1s, to document the
expectation (and to make it easier on future readers).

Note that we specifically test that the short commit ID is used in the
`git-rebase-todo.tmp` file: this file is created by the fake editor in
the test script and reflects the state that would have been presented to
the user to edit.

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

parse_insn_line(): improve error message when parsing failed

In the case that a `get_oid()` call failed, we showed some rather bogus
part of the line instead of the precise string we sent to said function.
That makes it rather hard for users to understand what is going wrong,
so let's fix that.

While at it, return a negative value from `parse_insn_line()` in case of
an error, as per our convention. This function's only caller,
`todo_list_parse_insn_buffer()`, cares only whether that return value is
non-zero or not, i.e. does not need to be changed.

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

submodule.c: use get_git_dir() instead of get_git_common_dir()

Ever since df56607dff (git-common-dir: make "modules/"
per-working-directory directory, 2014-11-30), submodules in linked worktrees
are cloned to $GIT_DIR/modules, i.e. $GIT_COMMON_DIR/worktrees/<name>/modules.

However, this convention was not followed when the worktree updater commands
checkout, reset and read-tree learned to recurse into submodules. Specifically,
submodule.c::submodule_move_head, introduced in 6e3c1595c6 (update submodules:
add submodule_move_head, 2017-03-14) and submodule.c::submodule_unset_core_worktree,
(re)introduced in 898c2e65b7 (submodule: unset core.worktree if no working tree
is present, 2018-12-14) use get_git_common_dir() instead of get_git_dir()
to get the path of the submodule repository.

This means that, for example, 'git checkout --recurse-submodules <branch>'
in a linked worktree will correctly checkout <branch>, detach the submodule's HEAD
at the commit recorded in <branch> and update the submodule working tree, but the
submodule HEAD that will be moved is the one in $GIT_COMMON_DIR/modules/<name>/,
i.e. the submodule repository of the main superproject working tree.
It will also rewrite the gitfile in the submodule working tree of the linked worktree
to point to $GIT_COMMON_DIR/modules/<name>/.
This leads to an incorrect (and confusing!) state in the submodule working tree
of the main superproject worktree.

Additionally, if switching to a commit where the submodule is not present,
submodule_unset_core_worktree will be called and will incorrectly remove
'core.wortree' from the config file of the submodule in the main superproject worktree,
$GIT_COMMON_DIR/modules/<name>/config.

Fix this by constructing the path to the submodule repository using get_git_dir()
in both submodule_move_head and submodule_unset_core_worktree.

Signed-off-by: Philippe Blain <levraiphilippeblain@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

t2405: clarify test descriptions and simplify test

When 'checkout --to' functionality was moved to 'worktree add', tests were adapted
in f194b1ef6e (tests: worktree: retrofit "checkout --to" tests for "worktree add",
2015-07-06).

The calls were changed to 'worktree add' in this test (then t7410), but the test
descriptions were not updated, keeping 'checkout' instead of using the new
terminology (linked worktrees).

Also, in the test each worktree is created in
$TRASH_DIRECTORY/<leading-directory>/main, where the name of <leading-directory>
carries some information about what behavior each test verifies. This directory
structure is not mandatory for the tests; the worktrees can live next to one
another in the trash directory.

Clarify the tests by using the right terminology, and remove the unnecessary
leading directories such that all superproject worktrees are directly next to one
another in the trash directory.

Signed-off-by: Philippe Blain <levraiphilippeblain@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

t2405: use git -C and test_commit -C instead of subshells

The subshells used in the setup phase of this test are unnecessary.

Remove them by using 'git -C' and 'test_commit -C'.

Signed-off-by: Philippe Blain <levraiphilippeblain@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

t7410: rename to t2405-worktree-submodule.sh

This test was added in df56607dff (git-common-dir: make "modules/"
per-working-directory directory, 2014-11-30), back when the 'git worktree' command
did not exist and 'git checkout --to' was used to create supplementary worktrees.

Since this file contains tests for the interaction of 'git worktree' with
submodules, rename it to t2405-worktree-submodule.sh, following the naming scheme for
tests checking the behavior of various commands with submodules.

Signed-off-by: Philippe Blain <levraiphilippeblain@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

docs: mention when increasing http.postBuffer is valuable

Users in a wide variety of situations find themselves with HTTP push
problems.  Oftentimes these issues are due to antivirus software,
filtering proxies, or other man-in-the-middle situations; other times,
they are due to simple unreliability of the network.

However, a common solution to HTTP push problems found online is to
increase http.postBuffer.  This works for none of the aforementioned
situations and is only useful in a small, highly restricted number of
cases: essentially, when the connection does not properly support
HTTP/1.1.

Document when raising this value is appropriate and what it actually
does, and discourage people from using it as a general solution for push
problems, since it is not effective there.

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

doc: dissuade users from trying to ignore tracked files

It is quite common for users to want to ignore the changes to a file
that Git tracks.  Common scenarios for this case are IDE settings and
configuration files, which should generally not be tracked and possibly
generated from tracked files using a templating mechanism.

However, users learn about the assume-unchanged and skip-worktree bits
and try to use them to do this anyway.  This is problematic, because
when these bits are set, many operations behave as the user expects, but
they usually do not help when git checkout needs to replace a file.

There is no sensible behavior in this case, because sometimes the data
is precious, such as certain configuration files, and sometimes it is
irrelevant data that the user would be happy to discard.

Since this is not a supported configuration and users are prone to
misuse the existing features for unintended purposes, causing general
sadness and confusion, let's document the existing behavior and the
pitfalls in the documentation for git update-index so that users know
they should explore alternate solutions.

In addition, let's provide a recommended solution to dealing with the
common case of configuration files, since there are well-known
approaches used successfully in many environments.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

doc: provide guidance on user.name format

It's a frequent misconception that the user.name variable controls
authentication in some way, and as a result, beginning users frequently
attempt to change it when they're having authentication troubles.
Document that the convention is that this variable represents some form
of a human's personal name, although that is not required.  In addition,
address concerns about whether Unicode is supported.

Use the term "personal name" as this is likely to draw the intended
contrast, be applicable across cultures which may have different naming
conventions, and be easily understandable to people who do not speak
English as their first language.  Indicate that "some form" is
conventionally used, as people may use a nickname or preferred name
instead of a full legal name.

Point users who may be confused about authentication to an appropriate
configuration option instead.  Provide a shortened form of this
information in the configuration option description.

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

docs: expand on possible and recommended user config options

In the section on setting author and committer information, we omit the
author.* and committer.* variables, so mention them for completeness.
In addition, guide users to the typical case: simply setting user.name
and user.email, which are recommended if one does not need complex
configuration.

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

doc: move author and committer information to git-commit(1)

While at one time it made perfect sense to store information about
configuring author and committer information in the documentation for
git commit-tree, in modern Git that operation is seldom used.  Most
users will use git commit and expect to find comprehensive documentation
about its use in the manual page for that command.

Considering that there is significant confusion about how one is to use
the user.name and user.email variables, let's put as much documentation
as possible into an obvious place where users will be more likely to
find it.

In addition, expand the environment variables section to describe their
use more fully.  Even though we now describe all of the options there
and in the configuration settings documentation, preserve the existing
text in git-commit.txt so that people can easily reason about the
ordering of the various options they can use.  Explain the use of the
author.* and committer.* options as well.

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

docs: use "currently" for the present time

In many languages, the adverb with the root "actual" means "at the
present time."  However, this usage is considered dated or even archaic
in English, and for referring to events occurring at the present time,
we usually prefer "currently" or "presently".  "Actually" is commonly
used in modern English only for the meaning of "in fact" or to express a
contrast with what is expected.

Since the documentation refers to the available options at the present
time (that is, at the time of writing) instead of drawing a contrast,
let's switch to "currently," which both is commonly used and sounds less
formal than "presently."

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

fetch: document and test --refmap=""

To prevent long blocking time during a 'git fetch' call, a user
may want to set up a schedule for background 'git fetch' processes.
However, these runs will update the refs/remotes branches due to
the default refspec set in the config when Git adds a remote.
Hence the user will not notice when remote refs are updated during
their foreground fetches. In fact, they may _want_ those refs to
stay put so they can work with the refs from their last foreground
fetch call.

This can be accomplished by overriding the configured refspec using
'--refmap=' along with a custom refspec:

  git fetch --refmap='' <remote> +refs/heads/*:refs/hidden/<remote>/*

to populate a custom ref space and download a pack of the new
reachable objects. This kind of call allows a few things to happen:

1. We download a new pack if refs have updated.
2. Since the refs/hidden branches exist, GC will not remove the
   newly-downloaded data.
3. With fetch.writeCommitGraph enabled, the refs/hidden refs are
   used to update the commit-graph file.

To avoid the refs/hidden directory from filling without bound, the
--prune option can be included. When providing a refspec like this,
the --prune option does not delete remote refs and instead only
deletes refs in the target refspace.

Update the documentation to clarify how '--refmap=""' works and
create tests to guarantee this behavior remains in the future.

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

Sync with maint

* maint:
  msvc: accommodate for vcpkg's upgrade to OpenSSL v1.1.x

t7800: don't rely on reuse_worktree_file()

A test in t7800 tries to make sure that when git-difftool runs an
external tool that fails, it stops looking at files. Our fake failing
tool prints the file name it was asked to diff before exiting non-zero,
and then we confirm the output contains only that file.

However, this subtly relies on our internal reuse_worktree_file().
Because we're diffing between branches, the command run by difftool
might see:

  - the git-stored filename (e.g., "file"), if we decided that the
    working tree contents were up-to-date with the object in the index
    and HEAD, and we could reuse them

  - a temporary filename (e.g. "/tmp/abc123_file") if we had to dump the
    contents from the object database

If the latter case happens, then the test fails, because it's expecting
the string "file". I discovered this when debugging something unrelated
with reuse_worktree_file(). I _thought_ it should be able to be
triggered by a racy-git situation, but running:

  ./t7800-difftool.sh --stress --run=2,13

never seems to fail. However, by my reading of reuse_worktree_file(),
this would probably always fail under Cygwin, because it sets
NO_FAST_WORKING_DIRECTORY. At any rate, since reuse_worktree_file()
is meant to be an optimization that may or may not trigger, our test
should be robust either way.

Instead of checking the filename, let's just make sure we got a single
line of output (which would not be true if we continued after the first
failure).

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

t4018: drop "debugging" cat from hunk-header tests

We run a series of hunk-header tests in a loop, and each one does this:

  test_when_finished 'cat actual' &&      # for debugging only

This is pretty pointless. When the test succeeds, we waste time running
a useless cat process. If you're debugging a failure with "-i", then we
won't run the when-finished part at all. So it helps only if you're
running with something like "--verbose-log".

Since we expect the tests to succeed most of the time, a better way to
do this would be a helper that checks the output and dumps "actual" only
when it fails. But it's probably not even worth the effort, as anyone
debugging a failure could just run with "-i" and investigate the
"actual" file themselves.

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

Makefile: use compat regex with SANITIZE=address

Recent versions of the gcc and clang Address Sanitizer produce test
failures related to regexec(). This triggers with gcc-10 and clang-8
(but not gcc-9 nor clang-7). Running:

  make CC=gcc-10 SANITIZE=address test

results in failures in t4018, t3206, and t4062.

The cause seems to be that when built with ASan, we use a different
version of regexec() than normal. And this version doesn't understand
the REG_STARTEND flag. Here's my evidence supporting that.

The failure in t4062 is an ASan warning:

  expecting success of 4062.2 '-G matches':
   git diff --name-only -G "^(0{64}){64}$" HEAD^ >out &&
   test 4096-zeroes.txt = "$(cat out)"

  =================================================================
  ==672994==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x7fa76f672000 at pc 0x7fa7726f75b6 bp 0x7ffe41bdda70 sp 0x7ffe41bdd220
  READ of size 4097 at 0x7fa76f672000 thread T0
      #0 0x7fa7726f75b5  (/lib/x86_64-linux-gnu/libasan.so.6+0x4f5b5)
      #1 0x562ae0c9c40e in regexec_buf /home/peff/compile/git/git-compat-util.h:1117
      #2 0x562ae0c9c40e in diff_grep /home/peff/compile/git/diffcore-pickaxe.c:52
      #3 0x562ae0c9cc28 in pickaxe_match /home/peff/compile/git/diffcore-pickaxe.c:166
      [...]

In this case we're looking in a buffer which was mmap'd via
reuse_worktree_file(), and whose size is 4096 bytes. But libasan's
regex tries to look at byte 4097 anyway! If we tweak Git like this:

  diff --git a/diff.c b/diff.c
  index 8e2914c031..cfae60c120 100644
  --- a/diff.c
  +++ b/diff.c
  @@ -3880,7 +3880,7 @@ static int reuse_worktree_file(struct index_state *istate,
           */
          if (ce_uptodate(ce) ||
              (!lstat(name, &st) && !ie_match_stat(istate, ce, &st, 0)))
  -               return 1;
  +               return 0;

          return 0;
   }

to use a regular buffer (with a trailing NUL) instead of an mmap, then
the complaint goes away.

The other failures are actually diff output with an incorrect funcname
header. If I instrument xdiff to show the funcname matching like so:

  diff --git a/xdiff-interface.c b/xdiff-interface.c
  index 8509f9ea22..f6c3dc1986 100644
  --- a/xdiff-interface.c
  +++ b/xdiff-interface.c
  @@ -197,6 +197,7 @@ struct ff_regs {
    struct ff_reg {
    regex_t re;
    int negate;
  + char *printable;
    } *array;
   };

  @@ -218,7 +219,12 @@ static long ff_regexp(const char *line, long len,

    for (i = 0; i < regs->nr; i++) {
    struct ff_reg *reg = regs->array + i;
  - if (!regexec_buf(&reg->re, line, len, 2, pmatch, 0)) {
  + int ret = regexec_buf(&reg->re, line, len, 2, pmatch, 0);
  + warning("regexec %s:\n  regex: %s\n  buf: %.*s",
  + ret == 0 ? "matched" : "did not match",
  + reg->printable,
  + (int)len, line);
  + if (!ret) {
    if (reg->negate)
    return -1;
    break;
  @@ -264,6 +270,7 @@ void xdiff_set_find_func(xdemitconf_t *xecfg, const char *value, int cflags)
    expression = value;
    if (regcomp(&reg->re, expression, cflags))
    die("Invalid regexp to look for hunk header: %s", expression);
  + reg->printable = xstrdup(expression);
    free(buffer);
    value = ep + 1;
    }

then when compiling with ASan and gcc-10, running the diff from t4018.66
produces this:

  $ git diff -U1 cpp-skip-access-specifiers
  warning: regexec did not match:
    regex: ^[     ]*[A-Za-z_][A-Za-z_0-9]*:[[:space:]]*($|/[/*])
    buf: private:
  warning: regexec matched:
    regex: ^((::[[:space:]]*)?[A-Za-z_].*)$
    buf: private:
  diff --git a/cpp-skip-access-specifiers b/cpp-skip-access-specifiers
  index 4d4a9db..ebd6f42 100644
  --- a/cpp-skip-access-specifiers
  +++ b/cpp-skip-access-specifiers
  @@ -6,3 +6,3 @@ private:
          void DoSomething();
          int ChangeMe;
  };
          void DoSomething();
  -       int ChangeMe;
  +       int IWasChanged;
   };

That first regex should match (and is negated, so it should be telling
us _not_ to match "private:"). But it wouldn't if regexec() is looking
at the whole buffer, and not just the length-limited line we've fed to
regexec_buf(). So this is consistent again with REG_STARTEND being
ignored.

The correct output (compiling without ASan, or gcc-9 with Asan) looks
like this:

  warning: regexec matched:
    regex: ^[     ]*[A-Za-z_][A-Za-z_0-9]*:[[:space:]]*($|/[/*])
    buf: private:
  [...more lines that we end up not using...]
  warning: regexec matched:
    regex: ^((::[[:space:]]*)?[A-Za-z_].*)$
    buf: class RIGHT : public Baseclass
  diff --git a/cpp-skip-access-specifiers b/cpp-skip-access-specifiers
  index 4d4a9db..ebd6f42 100644
  --- a/cpp-skip-access-specifiers
  +++ b/cpp-skip-access-specifiers
  @@ -6,3 +6,3 @@ class RIGHT : public Baseclass
          void DoSomething();
  -       int ChangeMe;
  +       int IWasChanged;
   };

So it really does seem like libasan's regex engine is ignoring
REG_STARTEND. We should be able to work around it by compiling with
NO_REGEX, which would use our local regexec(). But to make matters even
more interesting, this isn't enough by itself.

Because ASan has support from the compiler, it doesn't seem to intercept
our call to regexec() at the dynamic library level. It actually
recognizes when we are compiling a call to regexec() and replaces it
with ASan-specific code at that point. And unlike most of our other
compat code, where we might have git_mmap() or similar, the actual
symbol name in the compiled compat/regex code is regexec(). So just
compiling with NO_REGEX isn't enough; we still end up in libasan!

We can work around that by having the preprocessor replace regexec with
git_regexec (both in the callers and in the actual implementation), and
we truly end up with a call to our custom regex code, even when
compiling with ASan. That's probably a good thing to do anyway, as it
means anybody looking at the symbols later (e.g., in a debugger) would
have a better indication of which function is which. So we'll do the
same for the other common regex functions (even though just regexec() is
enough to fix this ASan problem).

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

built-in add -i: accept open-ended ranges again

The interactive `add` command allows selecting multiple files for some
of its sub-commands, via unique prefixes, indices or index ranges.

When re-implementing `git add -i` in C, we even added a code comment
talking about ranges with a missing end index, such as `2-`, but the
code did not actually accept those, as pointed out in
https://github.com/git-for-windows/git/issues/2466#issuecomment-574142760.

Let's fix this, and add a test case to verify that this stays fixed
forever.

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

built-in add -i: do not try to `patch`/`diff` an empty list of files

When the user does not select any files to `patch` or `diff`, there is
no need to call `run_add_p()` on them.

Even worse: we _have_ to avoid calling `parse_pathspec()` with an empty
list because that would trigger this error:

BUG: pathspec.c:557: PATHSPEC_PREFER_CWD requires arguments

So let's avoid doing any work on a list of files that is empty anyway.

This fixes https://github.com/git-for-windows/git/issues/2466.

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

submodule.c: mark more strings for translation

Signed-off-by: Ralf Thielow <ralf.thielow@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

dir: point treat_leading_path() warning to the right place

Commit 777b420347 (dir: synchronize treat_leading_path() and
read_directory_recursive(), 2019-12-19) tried to add two warning
comments in those functions, pointing at each other. But the one in
treat_leading_path() just points at itself.

Let's fix that. Since the comment also redirects the reader for more
details to "the commit that added this warning", and since we're now
modifying the warning (creating a new commit without those details),
let's mention the actual commit id.

Signed-off-by: Jeff King <peff@peff.net>
Reviewed-by: Elijah Newren <newren@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>