doc: replace LKML link with lore.kernel.org

Since we're now recommending lore.kernel.org, replace LKML link
with lore.kernel.org.

Although LKML has been around for a long time, nothing lasts forever
(see Gmane). Since LKML uses opaque message identifiers, switching to
lore.kernel.org should be a strict improvement since, even if
lore.kernel.org goes down, the Message-ID will allow future readers to
look up the referenced messages on any other archive.

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

RelNotes: replace Gmane with real Message-IDs

The only references to Gmane that remain are in RelNotes. Although these
are definitely not in active use, they might be of historical interest
for future readers so let's ensure that mail references are more robust.

Replace links to Gmane with links to lore.kernel.org (which is our new
preferred mailing list archive and has the Message-ID in the URL) and
bare Gmane ID references with Message-IDs.

The Message-IDs were found by searching for "gmane:<id>" on
https://public-inbox.org/git/ and taking the resulting message.

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

doc: replace MARC links with lore.kernel.org

Since we're now recommending lore.kernel.org, replace marc.info links
with lore.kernel.org.

Although MARC has been around for a long time, nothing lasts forever
(see Gmane). Since MARC uses opaque message identifiers, switching to
lore.kernel.org should be a strict improvement since, even if
lore.kernel.org goes down, the Message-ID will allow future readers to
look up the referenced messages on any other archive.

We leave behind one reference to MARC in the README.md since it's a
perfectly fine mail archive for personal reading, just not for linking
messages for the future.

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

Merge branch 'prevent-name-squatting-on-windows'

This patch series fixes an issue where Git could formerly have been
tricked into creating a `.git` file with an unexpected (and therefore
unprotected) NTFS short name.

Incidentally, it also fixes an issue where a tree entry containing a
backslash could be tricked into following a symbolic link, i.e. Git
could be tricked into writing files outside the worktree.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>

Merge branch 'jk/fast-import-unsafe'

The `--export-marks` option of `git fast-import` is exposed also via the
in-stream command `feature export-marks=...` and it allows overwriting
arbitrary paths.

This topic branch prevents the in-stream version, to prevent arbitrary
file accesses by `git fast-import` streams coming from untrusted sources
(e.g. in remote helpers that are based on `git fast-import`).

This fixes CVE-2019-1348.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>

path.c: document the purpose of `is_ntfs_dotgit()`

Previously, this function was completely undocumented. It is worth,
though, to explain what is going on, as it is not really obvious at all.

Suggested-by: Garima Singh <garima.singh@microsoft.com>
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>

mingw: disallow backslash characters in tree objects' file names

The backslash character is not a valid part of a file name on Windows.
Hence it is dangerous to allow writing files that were unpacked from
tree objects, when the stored file name contains a backslash character:
it will be misinterpreted as directory separator.

This not only causes ambiguity when a tree contains a blob `a\b` and a
tree `a` that contains a blob `b`, but it also can be used as part of an
attack vector to side-step the careful protections against writing into
the `.git/` directory during a clone of a maliciously-crafted
repository.

Let's prevent that, addressing CVE-2019-1354.

Note: we guard against backslash characters in tree objects' file names
_only_ on Windows (because on other platforms, even on those where NTFS
volumes can be mounted, the backslash character is _not_ a directory
separator), and _only_ when `core.protectNTFS = true` (because users
might need to generate tree objects for other platforms, of course
without touching the worktree, e.g. using `git update-index
--cacheinfo`).

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>

clone --recurse-submodules: prevent name squatting on Windows

In addition to preventing `.git` from being tracked by Git, on Windows
we also have to prevent `git~1` from being tracked, as the default NTFS
short name (also known as the "8.3 filename") for the file name `.git`
is `git~1`, otherwise it would be possible for malicious repositories to
write directly into the `.git/` directory, e.g. a `post-checkout` hook
that would then be executed _during_ a recursive clone.

When we implemented appropriate protections in 2b4c6efc821 (read-cache:
optionally disallow NTFS .git variants, 2014-12-16), we had analyzed
carefully that the `.git` directory or file would be guaranteed to be
the first directory entry to be written. Otherwise it would be possible
e.g. for a file named `..git` to be assigned the short name `git~1` and
subsequently, the short name generated for `.git` would be `git~2`. Or
`git~3`. Or even `~9999999` (for a detailed explanation of the lengths
we have to go to protect `.gitmodules`, see the commit message of
e7cb0b4455c (is_ntfs_dotgit: match other .git files, 2018-05-11)).

However, by exploiting two issues (that will be addressed in a related
patch series close by), it is currently possible to clone a submodule
into a non-empty directory:

- On Windows, file names cannot end in a space or a period (for
  historical reasons: the period separating the base name from the file
  extension was not actually written to disk, and the base name/file
  extension was space-padded to the full 8/3 characters, respectively).
  Helpfully, when creating a directory under the name, say, `sub.`, that
  trailing period is trimmed automatically and the actual name on disk
  is `sub`.

  This means that while Git thinks that the submodule names `sub` and
  `sub.` are different, they both access `.git/modules/sub/`.

- While the backslash character is a valid file name character on Linux,
  it is not so on Windows. As Git tries to be cross-platform, it
  therefore allows backslash characters in the file names stored in tree
  objects.

  Which means that it is totally possible that a submodule `c` sits next
  to a file `c\..git`, and on Windows, during recursive clone a file
  called `..git` will be written into `c/`, of course _before_ the
  submodule is cloned.

Note that the actual exploit is not quite as simple as having a
submodule `c` next to a file `c\..git`, as we have to make sure that the
directory `.git/modules/b` already exists when the submodule is checked
out, otherwise a different code path is taken in `module_clone()` that
does _not_ allow a non-empty submodule directory to exist already.

Even if we will address both issues nearby (the next commit will
disallow backslash characters in tree entries' file names on Windows,
and another patch will disallow creating directories/files with trailing
spaces or periods), it is a wise idea to defend in depth against this
sort of attack vector: when submodules are cloned recursively, we now
_require_ the directory to be empty, addressing CVE-2019-1349.

Note: the code path we patch is shared with the code path of `git
submodule update --init`, which must not expect, in general, that the
directory is empty. Hence we have to introduce the new option
`--force-init` and hand it all the way down from `git submodule` to the
actual `git submodule--helper` process that performs the initial clone.

Reported-by: Nicolas Joly <Nicolas.Joly@microsoft.com>
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>

fast-import: disallow "feature import-marks" by default

As with export-marks in the previous commit, import-marks can access the
filesystem. This is significantly less dangerous than export-marks
because it only involves reading from arbitrary paths, rather than
writing them. However, it could still be surprising and have security
implications (e.g., exfiltrating data from a service that accepts
fast-import streams).

Let's lump it (and its "if-exists" counterpart) in with export-marks,
and enable the in-stream version only if --allow-unsafe-features is set.

Signed-off-by: Jeff King <peff@peff.net>

fast-import: disallow "feature export-marks" by default

The fast-import stream command "feature export-marks=<path>" lets the
stream write marks to an arbitrary path. This may be surprising if you
are running fast-import against an untrusted input (which otherwise
cannot do anything except update Git objects and refs).

Let's disallow the use of this feature by default, and provide a
command-line option to re-enable it (you can always just use the
command-line --export-marks as well, but the in-stream version provides
an easy way for exporters to control the process).

This is a backwards-incompatible change, since the default is flipping
to the new, safer behavior. However, since the main users of the
in-stream versions would be import/export-based remote helpers, and
since we trust remote helpers already (which are already running
arbitrary code), we'll pass the new option by default when reading a
remote helper's stream. This should minimize the impact.

Note that the implementation isn't totally simple, as we have to work
around the fact that fast-import doesn't parse its command-line options
until after it has read any "feature" lines from the stream. This is how
it lets command-line options override in-stream. But in our case, it's
important to parse the new --allow-unsafe-features first.

There are three options for resolving this:

  1. Do a separate "early" pass over the options. This is easy for us to
     do because there are no command-line options that allow the
     "unstuck" form (so there's no chance of us mistaking an argument
     for an option), though it does introduce a risk of incorrect
     parsing later (e.g,. if we convert to parse-options).

  2. Move the option parsing phase back to the start of the program, but
     teach the stream-reading code never to override an existing value.
     This is tricky, because stream "feature" lines override each other
     (meaning we'd have to start tracking the source for every option).

  3. Accept that we might parse a "feature export-marks" line that is
     forbidden, as long we don't _act_ on it until after we've parsed
     the command line options.

     This would, in fact, work with the current code, but only because
     the previous patch fixed the export-marks parser to avoid touching
     the filesystem.

     So while it works, it does carry risk of somebody getting it wrong
     in the future in a rather subtle and unsafe way.

I've gone with option (1) here as simple, safe, and unlikely to cause
regressions.

This fixes CVE-2019-1348.

Signed-off-by: Jeff King <peff@peff.net>

fast-import: delay creating leading directories for export-marks

When we parse the --export-marks option, we don't immediately open the
file, but we do create any leading directories. This can be especially
confusing when a command-line option overrides an in-stream one, in
which case we'd create the leading directory for the in-stream file,
even though we never actually write the file.

Let's instead create the directories just before opening the file, which
means we'll create only useful directories. Note that this could change
the handling of relative paths if we chdir() in between, but we don't
actually do so; the only permanent chdir is from setup_git_directory()
which runs before either code path (potentially we should take the
pre-setup dir into account to avoid surprising the user, but that's an
orthogonal change).

The test just adapts the existing "override" test to use paths with
leading directories. This checks both that the correct directory is
created (which worked before but was not tested), and that the
overridden one is not (our new fix here).

While we're here, let's also check the error result of
safe_create_leading_directories(). We'd presumably notice any failure
immediately after when we try to open the file itself, but we can give a
more specific error message in this case.

Signed-off-by: Jeff King <peff@peff.net>

fast-import: stop creating leading directories for import-marks

When asked to import marks from "subdir/file.marks", we create the
leading directory "subdir" if it doesn't exist. This makes no sense for
importing marks, where we only ever open the path for reading.

Most of the time this would be a noop, since if the marks file exists,
then the leading directories exist, too. But if it doesn't (e.g.,
because --import-marks-if-exists was used), then we'd create the useless
directory.

This dates back to 580d5f83e7 (fast-import: always create marks_file
directories, 2010-03-29). Even then it was useless, so it seems to have
been added in error alongside the --export-marks case (which _is_
helpful).

Signed-off-by: Jeff King <peff@peff.net>

fast-import: tighten parsing of boolean command line options

We parse options like "--max-pack-size=" using skip_prefix(), which
makes sense to get at the bytes after the "=". However, we also parse
"--quiet" and "--stats" with skip_prefix(), which allows things like
"--quiet-nonsense" to behave like "--quiet".

This was a mistaken conversion in 0f6927c229 (fast-import: put option
parsing code in separate functions, 2009-12-04). Let's tighten this to
an exact match, which was the original intent.

Signed-off-by: Jeff King <peff@peff.net>

t9300: create marks files for double-import-marks test

Our tests confirm that providing two "import-marks" options in a
fast-import stream is an error. However, the invoked command would fail
even without covering this case, because the marks files themselves do
not actually exist.  Let's create the files to make sure we fail for the
right reason (we actually do, because the option parsing happens before
we open anything, but this future-proofs our test).

Signed-off-by: Jeff King <peff@peff.net>

t9300: drop some useless uses of cat

These waste a process, and make the line longer than it needs to be.

Signed-off-by: Jeff King <peff@peff.net>

pkt-line: fix a typo

This typo was introduced in 2153d478b74 (pkt-line: introduce
packet_read_with_status, 2018-03-14).

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

The second batch

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

Merge branch 'jk/gitweb-anti-xss'

Some codepaths in "gitweb" that forgot to escape URLs generated
based on end-user input have been corrected.

* jk/gitweb-anti-xss:
  gitweb: escape URLs generated by href()
  t/gitweb-lib.sh: set $REQUEST_URI
  t/gitweb-lib.sh: drop confusing quotes
  t9502: pass along all arguments in xss helper

Merge branch 'ar/install-doc-update-cmds-needing-the-shell'

Doc update.

* ar/install-doc-update-cmds-needing-the-shell:
  INSTALL: use existing shell scripts as example

Merge branch 'ma/t7004'

Test fix.

* ma/t7004:
  t7004: check existence of correct tag

Merge branch 'js/complete-svn-recursive'

The completion script (in contrib/) has been taught that "git svn"
supports the "--recursive" option.

* js/complete-svn-recursive:
  completion: tab-complete "git svn --recursive"

Merge branch 'jk/send-pack-remote-failure'

Error handling after "git push" finishes sending the packdata and
waits for the response to the remote side has been improved.

* jk/send-pack-remote-failure:
  send-pack: check remote ref status on pack-objects failure

Merge branch 'jc/fsmonitor-sanity-fix'

Remove an incorrect BUG() that should not trigger.

* jc/fsmonitor-sanity-fix:
  fsmonitor: do not compare bitmap size with size of split index

Merge branch 'sg/skip-skipped-prereq'

Test update to avoid wasted cycles.

* sg/skip-skipped-prereq:
  test-lib: don't check prereqs of test cases that won't be run anyway

Merge branch 'ds/test-read-graph'

Dev support for commit-graph feature.

* ds/test-read-graph:
  test-tool: use 'read-graph' helper

Merge branch 'rs/use-copy-array-in-mingw-shell-command-preparation'

Code cleanup.

* rs/use-copy-array-in-mingw-shell-command-preparation:
  mingw: use COPY_ARRAY for copying array

Merge branch 'rs/parse-options-dup-null-fix'

Code cleanup.

* rs/parse-options-dup-null-fix:
  parse-options: avoid arithmetic on pointer that's potentially NULL

Merge branch 'jt/fetch-remove-lazy-fetch-plugging'

"git fetch" codepath had a big "do not lazily fetch missing objects
when I ask if something exists" switch.  This has been corrected by
marking the "does this thing exist?" calls with "if not please do not
lazily fetch it" flag.

* jt/fetch-remove-lazy-fetch-plugging:
  promisor-remote: remove fetch_if_missing=0
  clone: remove fetch_if_missing=0
  fetch: remove fetch_if_missing=0

Merge branch 'jk/optim-in-pack-idx-conversion'

Code clean-up.

* jk/optim-in-pack-idx-conversion:
  pack-objects: avoid pointless oe_map_new_pack() calls

Merge branch 'dl/complete-rebase-onto'

The completion script (in contrib/) learned that the "--onto"
option of "git rebase" can take its argument as the value of the
option.

* dl/complete-rebase-onto:
  completion: learn to complete `git rebase --onto=`

Merge branch 'tg/stash-refresh-index'

Recent update to "git stash pop" made the command empty the index
when run with the "--quiet" option, which has been corrected.

* tg/stash-refresh-index:
  stash: make sure we have a valid index before writing it

Merge branch 'nn/doc-rebase-merges'

Doc update.

* nn/doc-rebase-merges:
  doc: improve readability of --rebase-merges in git-rebase

Merge branch 'dd/sequencer-utf8'

Handling of commit objects that use non UTF-8 encoding during
"rebase -i" has been improved.

* dd/sequencer-utf8:
  sequencer: reencode commit message for am/rebase --show-current-patch
  sequencer: reencode old merge-commit message
  sequencer: reencode squashing commit's message
  sequencer: reencode revert/cherry-pick's todo list
  sequencer: reencode to utf-8 before arrange rebase's todo list
  t3900: demonstrate git-rebase problem with multi encoding
  configure.ac: define ICONV_OMITS_BOM if necessary
  t0028: eliminate non-standard usage of printf

Merge branch 'jk/remove-sha1-to-hex'

Code clean-up.

* jk/remove-sha1-to-hex:
  hex: drop sha1_to_hex()
  hex: drop sha1_to_hex_r()

Merge branch 'dj/typofix-merge-strat'

Typofix.

* dj/typofix-merge-strat:
  merge-strategies: fix typo "reflected to" to "reflected in"

Merge branch 'rj/bundle-ui-updates'

"git bundle" has been taught to use the parse options API.  "git
bundle verify" learned "--quiet" and "git bundle create" learned
options to control the progress output.

* rj/bundle-ui-updates:
  bundle-verify: add --quiet
  bundle-create: progress output control
  bundle: framework for options before bundle file

Merge branch 'rs/skip-iprefix'

Code simplification.

* rs/skip-iprefix:
  convert: use skip_iprefix() in validate_encoding()
  utf8: use skip_iprefix() in same_utf_encoding()

Merge branch 'ln/userdiff-elixir'

The patterns to detect function boundary for Elixir language has
been added.

* ln/userdiff-elixir:
  userdiff: add Elixir to supported userdiff languages

Merge branch 'py/shortlog-list-options-for-log'

Documentation pages for "git shortlog" now lists commit limiting
options explicitly.

* py/shortlog-list-options-for-log:
  git-shortlog.txt: include commit limiting options

Merge branch 'en/doc-typofix'

Docfix.

* en/doc-typofix:
  Fix spelling errors in no-longer-updated-from-upstream modules
  multimail: fix a few simple spelling errors
  sha1dc: fix trivial comment spelling error
  Fix spelling errors in test commands
  Fix spelling errors in messages shown to users
  Fix spelling errors in names of tests
  Fix spelling errors in comments of testcases
  Fix spelling errors in code comments
  Fix spelling errors in documentation outside of Documentation/
  Documentation: fix a bunch of typos, both old and new

Merge branch 'ns/test-desc-typofix'

Typofix.

* ns/test-desc-typofix:
  t: fix typo in test descriptions

Merge branch 'en/t6024-style'

Test updates.

* en/t6024-style:
  t6024: modernize style

Merge branch 'en/misc-doc-fixes'

Misc doc fixes.

* en/misc-doc-fixes:
  name-hash.c: remove duplicate word in comment
  hashmap: fix documentation misuses of -> versus .
  git-filter-branch.txt: correct argument name typo

Merge branch 'js/fetch-multi-lockfix'

Fetching from multiple remotes into the same repository in parallel
had a bad interaction with the recent change to (optionally) update
the commit-graph after a fetch job finishes, as these parallel
fetches compete with each other.  Which has been corrected.

* js/fetch-multi-lockfix:
  fetch: avoid locking issues between fetch.jobs/fetch.writeCommitGraph
  fetch: add the command-line option `--write-commit-graph`

Merge branch 'rs/trace2-dots'

Code cleanup.

* rs/trace2-dots:
  trace2: add dots directly to strbuf in perf_fmt_prepare()

Merge branch 'kw/fsmonitor-watchman-fix'

The watchman integration for fsmonitor was racy, which has been
corrected to be more conservative.

* kw/fsmonitor-watchman-fix:
  fsmonitor: fix watchman integration

Merge branch 'cb/curl-use-xmalloc'

HTTP transport had possible allocator/deallocator mismatch, which
has been corrected.

* cb/curl-use-xmalloc:
  remote-curl: unbreak http.extraHeader with custom allocators

Merge branch 'rt/fetch-message-fix'

A small message update.

* rt/fetch-message-fix:
  fetch.c: fix typo in a warning message

Merge branch 'es/myfirstcontrib-updates'

Doc updates.

* es/myfirstcontrib-updates:
  myfirstcontrib: hint to find gitgitgadget allower
  myfirstcontrib: add dependency installation step
  myfirstcontrib: add 'psuh' to command-list.txt

Merge branch 'hw/config-doc-in-header'

Follow recent push to move API docs from Documentation/ to header
files and update config.h

* hw/config-doc-in-header:
  config: move documentation to config.h

Merge branch 'dl/doc-diff-no-index-implies-exit-code'

Doc update.

* dl/doc-diff-no-index-implies-exit-code:
  git-diff.txt: document return code of `--no-index`

Merge branch 'js/vreportf-wo-buffering'

Messages from die() etc. can be mixed up from multiple processes
without even line buffering on Windows, which has been worked
around.

* js/vreportf-wo-buffering:
  vreportf(): avoid relying on stdio buffering

Merge branch 'pb/no-recursive-reset-hard-in-worktree-add'

"git worktree add" internally calls "reset --hard" that should not
descend into submodules, even when submodule.recurse configuration
is set, but it was affected.  This has been corrected.

* pb/no-recursive-reset-hard-in-worktree-add:
  worktree: teach "add" to ignore submodule.recurse config

Merge branch 'pb/help-list-gitsubmodules-among-guides'

Help update.

* pb/help-list-gitsubmodules-among-guides:
  help: add gitsubmodules to the list of guides

Merge branch 'sg/blame-indent-heuristics-is-now-the-default'

Message update.

* sg/blame-indent-heuristics-is-now-the-default:
  builtin/blame.c: remove '--indent-heuristic' from usage string

Merge branch 'mr/clone-dir-exists-to-path-exists'

Code cleanup.

* mr/clone-dir-exists-to-path-exists:
  clone: rename static function `dir_exists()`.

Merge branch 'ma/bisect-doc-sample-update'

"git merge --no-commit" needs "--no-ff" if you do not want to move
HEAD, which has been corrected in the manual page for "git bisect".

* ma/bisect-doc-sample-update:
  Documentation/git-bisect.txt: add --no-ff to merge command

Merge branch 'js/git-path-head-dot-lock-fix'

"git rev-parse --git-path HEAD.lock" did not give the right path
when run in a secondary worktree.

* js/git-path-head-dot-lock-fix:
  git_path(): handle `.lock` files correctly
  t1400: wrap setup code in test case

Merge branch 'jc/log-graph-simplify'

The implementation of "git log --graph" got refactored and then its
output got simplified.

* jc/log-graph-simplify:
  t4215: use helper function to check output
  graph: fix coloring of octopus dashes
  graph: flatten edges that fuse with their right neighbor
  graph: smooth appearance of collapsing edges on commit lines
  graph: rename `new_mapping` to `old_mapping`
  graph: commit and post-merge lines for left-skewed merges
  graph: tidy up display of left-skewed merges
  graph: example of graph output that can be simplified
  graph: extract logic for moving to GRAPH_PRE_COMMIT state
  graph: remove `mapping_idx` and `graph_update_width()`
  graph: reduce duplication in `graph_insert_into_new_columns()`
  graph: reuse `find_new_column_by_commit()`
  graph: handle line padding in `graph_next_line()`
  graph: automatically track display width of graph lines

Merge branch 'jk/cleanup-object-parsing-and-fsck'

Crufty code and logic accumulated over time around the object
parsing and low-level object access used in "git fsck" have been
cleaned up.

* jk/cleanup-object-parsing-and-fsck: (23 commits)
  fsck: accept an oid instead of a "struct tree" for fsck_tree()
  fsck: accept an oid instead of a "struct commit" for fsck_commit()
  fsck: accept an oid instead of a "struct tag" for fsck_tag()
  fsck: rename vague "oid" local variables
  fsck: don't require an object struct in verify_headers()
  fsck: don't require an object struct for fsck_ident()
  fsck: drop blob struct from fsck_finish()
  fsck: accept an oid instead of a "struct blob" for fsck_blob()
  fsck: don't require an object struct for report()
  fsck: only require an oid for skiplist functions
  fsck: only provide oid/type in fsck_error callback
  fsck: don't require object structs for display functions
  fsck: use oids rather than objects for object_name API
  fsck_describe_object(): build on our get_object_name() primitive
  fsck: unify object-name code
  fsck: require an actual buffer for non-blobs
  fsck: stop checking tag->tagged
  fsck: stop checking commit->parent counts
  fsck: stop checking commit->tree value
  commit, tag: don't set parsed bit for parse failures
  ...

grep: don't return an expression from pcre2_free()

Previously, the void pcre2_free() function in grep.c returned free().
While free() itself is void, afaict it's still an expression as per
section A.2.3, subsection 6.8.6 (jump-statement) in both C99 [1] and C11
[2]:

> return expression

Section 6.8.6.4 in C99 [1] and C11 [2] says that:

> A return statement with an expression shall not appear in a function
> whose return type is void.

The consequence of the old behavior was that developer builds with
pedantic errors enabled broke Git if PCRE2 was enabled and a
smart-enough compiler to detect these errors was used.  This commit
fixes pedantic builds of Git that enables --with-libpcre.

[1] http://www.open-std.org/jtc1/sc22/wg14/www/docs/n1256.pdf
[2] http://www.open-std.org/jtc1/sc22/wg14/www/docs/n1548.pdf

Signed-off-by: Hans Jerry Illikainen <hji@dyntopia.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

t7811: don't create unused file

The file "empty" became unused with 1c5e94f459 (tests: use
'test_must_be_empty' instead of 'test_cmp <empty> <out>', 2018-08-19);
get rid of it.

Signed-off-by: René Scharfe <l.s.r@web.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

t9300: don't create unused file

The file "frontend" became unused with 4de0bbd898 (t9300: use perl
"head -c" clone in place of "dd bs=1 count=16000" kluge, 2010-12-13);
get rid of it.

Signed-off-by: René Scharfe <l.s.r@web.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

doc: replace public-inbox links with lore.

Since we're now recommending lore.kernel.org (and because the
public-inbox.org domain might eventually go away), let's update our
internal references to use it, too. That future-proofs our references,
and sets the example we want people to follow.

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

doc: recommend lore. over public-inbox.org

Since lore.kernel.org now has the same archive as public-inbox.org and
may have more longevity going forward[1], let's recommend people use it
for finding or referencing messages.

[1] https://public-inbox.org/git/20191120195556.GA25189@dcvr/
      or if you like:
    https://lore.kernel.org/git/20191120195556.GA25189@dcvr/

Signed-off-by: Jeff King <peff@peff.net>
Acked-by: Eric Wong <e@80x24.org>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

send-pack: use OBJECT_INFO_QUICK to check negative objects

When pushing, we feed pack-objects a list of both positive and negative
objects. The positive objects are what we want to send, and the negative
objects are what the other side told us they have, which we can use to
limit the size of the push.

Before passing along a negative object, send_pack() will make sure we
actually have it (since we only know about it because the remote
mentioned it, not because it's one of our refs). So it's expected that
some of these objects will be missing on the local side. But looking for
a missing object is more expensive than one that we have: it triggers
reprepare_packed_git() to handle a racy repack, plus it has to explore
every alternate's loose object tree (which can be slow if you have a lot
of them, or have a high-latency filesystem).

This isn't usually a big problem, since repositories you're pushing to
don't generally have a large number of refs that are unrelated to what
the client has. But there's no reason such a setup is wrong, and it
currently performs poorly.

We can fix this by using OBJECT_INFO_QUICK, which tells the lookup
code that we expect objects to be missing. Notably, it will not re-scan
the packs, and it will use the loose cache from 61c7711cfe (sha1-file:
use loose object cache for quick existence check, 2018-11-12).

The downside is that in the rare case that we race with a local repack,
we might fail to feed some objects to pack-objects, making the resulting
push larger. But we'd never produce an invalid or incorrect push, just a
less optimal one. That seems like a reasonable tradeoff, and we already
do similar things on the fetch side (e.g., when marking COMPLETE
commits).

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

ci: build Git with GCC 9 in the 'osx-gcc' build job

Our 'osx-gcc' build job on Travis CI relied on GCC 8 being installed
(but not linked) in the image we use [1].  Alas, since the last update
of this image a few days ago this is not the case anymore, and now it
contains GCC 9 (installed and linked) instead of GCC 8.  The results
are failed 'osx-gcc' jobs, because they can't find the 'gcc-8' command
[2].

Let's move on to use GCC 9, with hopefully better error reporting and
improved -Wfoo flags and what not.  On Travis CI this has the benefit
that we can spare a few seconds while installing dependencies, because
it already comes pre-installed, at least for now.  The Azure Pipelines
OSX image doesn't include GCC, so we have to install it ourselves
anyway, and then we might as well install the newer version.

In a vain attempt I tried to future-proof this a bit:

  - Install 'gcc@9' specifically, so we'll still get what we want even
    after GCC 10 comes out, and the "plain" 'gcc' package starts to
    refer to 'gcc@10'.

  - Run both 'brew install gcc@9' and 'brew link gcc@9'.  If 'gcc@9'
    is already installed and linked, then both commands are noop and
    exit with success.  But as we saw in the past, sometimes the image
    contains the expected GCC package installed but not linked, so
    maybe it will happen again in the future as well.  In that case
    'brew install' is still a noop, and instructs the user to run
    'brew link' instead, so that's what we'll do.  And if 'gcc@9' is
    not installed, then 'brew install' will install it, and the
    subsequent 'brew link' becomes a noop.

An additional benefit of this patch is that from now on we won't
unnecessarily install GCC and its dependencies in the 'osx-clang' jobs
on Azure Pipelines.

[1] 7d4733c501 (ci: fix GCC install in the Travis CI GCC OSX job,
    2019-10-24)
[2] https://travis-ci.org/git/git/jobs/615442297#L333

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

test: use test_must_be_empty F instead of test_cmp empty F

Use test_must_be_empty instead of comparing it to an empty file.  That's
more efficient, as the function only needs built-in meta-data only check
in the usual case, and provides nicer debug output otherwise.

Helped-by: Denton Liu <liu.denton@gmail.com>
Signed-off-by: René Scharfe <l.s.r@web.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

test: use test_must_be_empty F instead of test -z $(cat F)

Use test_must_be_empty instead of reading the file and comparing its
contents to an empty string.  That's more efficient, as the function
only needs built-in meta-data only check in the usual case, and provides
nicer debug output otherwise.

Signed-off-by: René Scharfe <l.s.r@web.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

t1400: use test_must_be_empty

Use test_must_be_empty instead of reading the file and comparing its
contents to an empty string.  That's more efficient, as the function
only needs built-in meta-data only check in the usual case, and provides
nicer debug output otherwise.

Signed-off-by: René Scharfe <l.s.r@web.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

t1410: use test_line_count

Use test_line_count to check if the number of lines matches
expectations, for improved consistency and nicer debug output.

Signed-off-by: René Scharfe <l.s.r@web.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

t1512: use test_line_count

Use test_line_count to check if the number of lines matches
expectations, for improved consistency and nicer debug output.

Signed-off-by: René Scharfe <l.s.r@web.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

run-command: use prepare_git_cmd() in prepare_cmd()

Call prepare_git_cmd() instead of open-coding it.

Signed-off-by: René Scharfe <l.s.r@web.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

name-rev: use skip_prefix() instead of starts_with()

Let skip_prefix() advance refname to get rid of two magic numbers.

Signed-off-by: René Scharfe <l.s.r@web.de>
Acked-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

push: use skip_prefix() instead of starts_with()

Get rid of a magic number by using skip_prefix().

Signed-off-by: René Scharfe <l.s.r@web.de>
Acked-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

shell: use skip_prefix() instead of starts_with()

Get rid of a magic number by using skip_prefix() instead of
starts_with().

Signed-off-by: René Scharfe <l.s.r@web.de>
Acked-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

fmt-merge-msg: use skip_prefix() instead of starts_with()

Get rid of two magic numbers by using skip_prefix().

Signed-off-by: René Scharfe <l.s.r@web.de>
Acked-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

fetch: use skip_prefix() instead of starts_with()

Get rid of magic numbers by letting skip_prefix() set the pointer
"what".

Signed-off-by: René Scharfe <l.s.r@web.de>
Acked-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

perf-lib: use a single filename for all measurement types

The perf tests write files recording the results of tests.  These
results are later aggregated by 'aggregate.perl'.  If the tests are run
multiple times, those results are overwritten by the new results.  This
works just fine as long as there are only perf tests measuring the
times, whose results are stored in "$base".times files.

However 22bec79d1a ("t/perf: add infrastructure for measuring sizes",
2018-08-17) introduced a new type of test for measuring the size of
something.  The results of this are written to "$base".size files.

"$base" is essentially made up of the basename of the script plus the
test number.  So if test numbers shift because a new test was
introduced earlier in the script we might end up with both a ".times"
and a ".size" file for the same test.  In the aggregation script the
".times" file is preferred over the ".size" file, so some size tests
might end with performance numbers from a previous run of the test.

This is mainly relevant when writing perf tests that check both
performance and sizes, and can get quite confusing during
developement.

We could fix this by doing a more thorough job of cleaning out old
".times" and ".size" files before running each test. However, an even
easier solution is to just use the same filename for both types of
measurement, meaning we'll always overwrite the previous result. We
don't even need to change the file format to distinguish the two;
aggregate.perl already decides which is which based on a regex of the
content (this may become ambiguous if we add new types in the future,
but we could easily add a header field to the file at that point).

Based on an initial patch from Thomas Gummerer, who discovered the
problem and did all of the analysis (which I stole for the commit
message above):

  https://public-inbox.org/git/20191119185047.8550-1-t.gummerer@gmail.com/

Helped-by: Thomas Gummerer <t.gummerer@gmail.com>
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

submodule: fix 'submodule status' when called from a subdirectory

When calling `git submodule status` while in a subdirectory, we are
incorrectly not detecting modified submodules and
thus reporting that all of the submodules are unchanged.

This is because the submodule helper is calling `diff-index` with the
submodule path assuming the path is relative to the current prefix
directory, however the submodule path used is actually relative to the root.

Always pass NULL as the `prefix` when running diff-files on the
submodule, to make sure the submodule's path is interpreted as relative
to the superproject's repository root.

Signed-off-by: Manish Goregaokar <manishsmail@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

revision: free topo_walk_info before creating a new one in init_topo_walk

init_topo_walk doesn't reuse an existing topo_walk_info, and currently
leaks the one that might exist on the current rev_info if it was already
used for a topo walk beforehand.

Signed-off-by: Mike Hommey <mh@glandium.org>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

revision: clear the topo-walk flags in reset_revision_walk

Not doing so can lead to wrong topo-walks when using the revision walk API
consecutively.

Signed-off-by: Mike Hommey <mh@glandium.org>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

git-compat-util.h: drop the `PRIuMAX` and other fallback definitions

Git's code base already seems to be using `PRIdMAX` without any such
fallback definition for quite a while (75459410edd (json_writer: new
routines to create JSON data, 2018-07-13), to be precise, and the
first Git version to include that commit was v2.19.0).  Having a
fallback definition only for `PRIuMAX` is a bit inconsistent.

We do sometimes get portability reports more than a year after the
problem was introduced.  This one should be fairly safe.  PRIuMAX is
in C99 (for that matter, SCNuMAX, PRIu32 and others also are), and
we've been picking up other C99-isms without complaint.

The PRIuMAX fallback definition was originally added in 3efb1f343a
(Check for PRIuMAX rather than NO_C99_FORMAT in fast-import.c.,
2007-02-20). But it was replacing a construct that was introduced in
an even earlier commit, 579d1fbfaf (Add NO_C99_FORMAT to support
older compilers., 2006-07-30), which talks about gcc 2.95.

That's pretty ancient at this point.

Signed-off-by: Hariom Verma <hariom18599@gmail.com>
Helped-by: Jeff King <peff@peff.net>
[jc: tweaked both message and code, taking what peff wrote]
Signed-off-by: Junio C Hamano <gitster@pobox.com>

reset: parse rev as tree-ish in patch mode

Since 2f328c3d ("reset $sha1 $pathspec: require $sha1 only to be
treeish", 2013-01-14), we allowed "git reset $object -- $path" to reset
individual paths that match the pathspec to take the blob from a tree
object, not necessarily a commit, while the form to reset the tip of the
current branch to some other commit still must be given a commit.

Like resetting with paths, "git reset --patch" does not update HEAD, and
need not require a commit. The path-filtered form, "git reset --patch
$object -- $pathspec", has accepted a tree-ish since 2f328c3d.

"git reset --patch" is documented as accepting a <tree-ish> since
bf44142f ("reset: update documentation to require only tree-ish with
paths", 2013-01-16). Documentation changes are not required.

Loosen the restriction that requires a commit for the unfiltered "git
reset --patch $object".

Signed-off-by: Nika Layzell <nika@thelayzells.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

doc: mention that 'git submodule update' fetches missing commits

'git submodule update' will fetch new commits from the submodule remote
if the SHA-1 recorded in the superproject is not found. This was not
mentioned in the documentation.

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

doc: document 'git submodule status --cached'

'git submodule status --cached' reports the SHAs recorded in the
index of the superproject, instead of the SHAs that are checked out
in the submodule.

Signed-off-by: Manish Goregaokar <manishsmail@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

sequencer: don't re-read todo for revert and cherry-pick

When 'git revert' or 'git cherry-pick --edit' is invoked with multiple
commits, then after editing the first commit message is finished both
these commands should continue with processing the second commit and
launch another editor for its commit message, assuming there are
no conflicts, of course.

Alas, this inadvertently changed with commit a47ba3c777 (rebase -i:
check for updated todo after squash and reword, 2019-08-19): after
editing the first commit message is finished, both 'git revert' and
'git cherry-pick --edit' exit with error, claiming that "nothing to
commit, working tree clean".

The reason for the changed behaviour is twofold:

  - Prior to a47ba3c777 the up-to-dateness of the todo list file was
    only checked after 'exec' instructions, and that commit moved
    those checks to the common code path.  The intention was that this
    check should be performed after instructions spawning an editor
    ('squash' and 'reword') as well, so the ongoing 'rebase -i'
    notices when the user runs a 'git rebase --edit-todo' while
    squashing/rewording a commit message.

    However, as it happened that check is now performed even after
    'revert' and 'pick' instructions when they involved editing the
    commit message.  And 'revert' by default while 'pick' optionally
    (with 'git cherry-pick --edit') involves editing the commit
    message.

  - When invoking 'git revert' or 'git cherry-pick --edit' with
    multiple commits they don't read a todo list file but assemble the
    todo list in memory, thus the associated stat data used to check
    whether the file has been updated is all zeroed out initially.

    Then the sequencer writes all instructions (including the very
    first) to the todo file, executes the first 'revert/pick'
    instruction, and after the user finished editing the commit
    message the changes of a47ba3c777 kick in, and it checks whether
    the todo file has been modified.  The initial all-zero stat data
    obviously differs from the todo file's current stat data, so the
    sequencer concludes that the file has been modified.  Technically
    it is not wrong, of course, because the file just has been written
    indeed by the sequencer itself, though the file's contents still
    match what the sequencer was invoked with in the beginning.
    Consequently, after re-reading the todo file the sequencer
    executes the same first instruction _again_, thus ending up in
    that "nothing to commit" situation.

The todo list was never meant to be edited during multi-commit 'git
revert' or 'cherry-pick' operations, so perform that "has the todo
file been modified" check only when the sequencer was invoked as part
of an interactive rebase.

Reported-by: Brian Norris <briannorris@chromium.org>
Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

t5608-clone-2gb.sh: turn GIT_TEST_CLONE_2GB into a bool

The GIT_TEST_CLONE_2GB environment variable is only ever checked with
'test -z' in 't5608-clone-2gb.sh', so any non-empty value is
interpreted as "yes, run these expensive tests", even
'GIT_TEST_CLONE_2GB=NoThanks'.

Similar GIT_TEST_* environment variables have already been turned into
bools in 3b072c577b (tests: replace test_tristate with "git
env--helper", 2019-06-21), so let's turn GIT_TEST_CLONE_2GB into a
bool as well, to follow suit.

Our CI builds set GIT_TEST_CLONE_2GB=YesPlease, so adjust them
accordingly, thus removing the last 'YesPlease' from our CI scripts.

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

tests: add 'test_bool_env' to catch non-bool GIT_TEST_* values

Since 3b072c577b (tests: replace test_tristate with "git env--helper",
2019-06-21) we get the normalized bool values of various GIT_TEST_*
environment variables via 'git env--helper'.  Now, while the 'git
env--helper' command itself does catch invalid values in the
environment variable or in the given --default and exits with error
(exit code 128 or 129, respectively), it's invoked in conditions like
'if ! git env--helper ...', which means that all invalid bool values
are interpreted the same as the ordinary 'false' (exit code 1).  This
has led to inadvertently skipped httpd tests in our CI builds for a
couple of weeks, see 3960290675 (ci: restore running httpd tests,
2019-09-06).

Let's be more careful about what the test suite accepts as bool values
in GIT_TEST_* environment variables, and error out loud and clear on
invalid values instead of simply skipping tests.  Add the
'test_bool_env' helper function to encapsulate the invocation of 'git
env--helper' and the verification of its exit code, and replace all
invocations of that command in our test framework and test suite with
a call to this new helper (except in 't0017-env-helper.sh', of
course).

  $ GIT_TEST_GIT_DAEMON=YesPlease ./t5570-git-daemon.sh
  fatal: bad numeric config value 'YesPlease' for 'GIT_TEST_GIT_DAEMON': invalid unit
  error: test_bool_env requires bool values both for $GIT_TEST_GIT_DAEMON and for the default fallback

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

sequencer: fix empty commit check when amending

This fixes a regression introduced in 356ee4659b ("sequencer: try to
commit without forking 'git commit'", 2017-11-24). When amending a
commit try_to_commit() was using the wrong parent when checking if the
commit would be empty. When amending we need to check against HEAD^ not
HEAD.

t3403 may not seem like the natural home for the new tests but a further
patch series will improve the advice printed by `git commit`. That
series will mutate these tests to check that the advice includes
suggesting `rebase --skip` to skip the fixup that would empty the
commit.

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

git svn: stop using `rebase --preserve-merges`

We deprecated `--preserve-merges` in favor of `--rebase-merges`; Let's
reflect that in `git svn`.

Note: Even when the user asks for `--preserve-merges`, we now silently
pass `--rebase-merges` to `git rebase` instead. Technically, this is a
change of behavior. But practically, `git svn` only ever asks for a
non-interactive rebase, and `--preserve-merges` and `--rebase-merges`
are on par with regard to that.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Acked-by: Eric Wong <e@80x24.org>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

gpg-interface: limit search for primary key fingerprint

The VALIDSIG status line from GnuPG with --status-fd is documented to
have 9 required and 1 optional fields [1].  The final, and optional,
field is used to specify the fingerprint of the primary key that made
the signature in case it was made by a subkey.  However, this field is
only available for OpenPGP signatures; not for CMS/X.509.

If the VALIDSIG status line does not have the optional 10th field, the
current code will continue reading onto the next status line.  And this
is the case for non-OpenPGP signatures [1].

The consequence is that a subsequent status line may be considered as
the "primary key" for signatures that does not have an actual primary
key.

Limit the search of these 9 or 10 fields to the single line to avoid
this problem.  If the 10th field is missing, report that there is no
primary key fingerprint.

[Reference]

[1] GnuPG Details, General status codes
https://git.gnupg.org/cgi-bin/gitweb.cgi?p=gnupg.git;a=blob;f=doc/DETAILS;h=6ce340e8c04794add995e84308bb3091450bd28f;hb=HEAD#l483

The documentation says:

    VALIDSIG <args>

    The args are:

    - <fingerprint_in_hex>
    - <sig_creation_date>
    - <sig-timestamp>
    - <expire-timestamp>
    - <sig-version>
    - <reserved>
    - <pubkey-algo>
    - <hash-algo>
    - <sig-class>
    - [ <primary-key-fpr> ]

    This status indicates that the signature is cryptographically
    valid. [...] PRIMARY-KEY-FPR is the fingerprint of the primary key
    or identical to the first argument.

    The primary-key-fpr parameter is used for OpenPGP and not available
    for CMS signatures.  [...]

Signed-off-by: Hans Jerry Illikainen <hji@dyntopia.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

gpg-interface: refactor the free-and-xmemdupz pattern

Introduce a static replace_cstring() function to simplify repeated
pattern of free-and-xmemdupz() for GPG status line parsing.

This also helps us avoid potential memleaks if parsing of new status
lines are introduced in the future.

Signed-off-by: Hans Jerry Illikainen <hji@dyntopia.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

unpack-trees: skip stat on fsmonitor-valid files

The index might be aware that a file hasn't modified via fsmonitor, but
unpack-trees did not pay attention to it and checked via ie_match_stat
which can be inefficient on certain filesystems. This significantly slows
down commands that run oneway_merge, like checkout and reset --hard.

This patch makes oneway_merge check whether a file is considered
unchanged through fsmonitor and skips ie_match_stat on it. unpack-trees
also now correctly copies over fsmonitor validity state from the source
index. Finally, for correctness, we force a refresh of fsmonitor state in
tweak_fsmonitor.

After this change, commands like stash (that use reset --hard
internally) go from 8s or more to ~2s on a 250k file repository on a
mac.

Helped-by: Junio C Hamano <gitster@pobox.com>
Helped-by: Kevin Willford <Kevin.Willford@microsoft.com>
Signed-off-by: Utsav Shah <utsav@dropbox.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

t5520: replace `! git` with `test_must_fail git`

Currently, if a git command fails in an unexpected way, such as a
segfault, it will be masked and ignored. Replace the ! with
test_must_fail so that only expected failures pass.

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

t5520: remove redundant lines in test cases

In the previous patches, the mechanical application of changes left some
duplicate statements in the test case which were not strictly incorrect
but were redundant and possibly misleading. Remove these duplicate
statements so that it is clear that the intent behind the tests are that
the content of the file stays the same throughout the whole test case.

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

t5520: replace $(cat ...) comparison with test_cmp

We currently have many instances of `test <line> = $(cat <file>)` and
`test $(cat <file>) = <line>`.  In the case where this fails, it will be
difficult for a developer to debug since the output will be masked.
Replace these instances with invocations of test_cmp().

This change was done with the following GNU sed expressions:

s/\(\s*\)test \([^=]*\)= "$(cat \([^)]*\))"/\1echo \2>expect \&\&\n\1test_cmp expect \3/
s/\(\s*\)test "$(cat \([^)]*\))" = \([^&]*\)\( &&\)\?$/\1echo \3 >expect \&\&\n\1test_cmp expect \2\4/

A future patch will clean up situations where we have multiple duplicate
statements within a test case. This is done to keep this patch purely
mechanical.

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

t5520: don't put git in upstream of pipe

Before, if the invocation of git failed, it would be masked by the pipe
since only the return code of the last element of a pipe is used.
Rewrite the test to put the git command on its own line so its return
code is not masked.

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

t5520: test single-line files by git with test_cmp

In case an invocation of a git command fails within the command
substitution, the failure will be masked. Replace the command
substitution with a file-redirection and a call to test_cmp.

This change was done with the following GNU sed expressions:

s/\(\s*\)test \([^ ]*\) = "$(\(git [^)]*\))"/\1echo \2 >expect \&\&\n\1\3 >actual \&\&\n\1test_cmp expect actual/
s/\(\s*\)test "$(\(git [^)]*\))" = \([^ ]*\)/\1echo \3 >expect \&\&\n\1\2 >actual \&\&\n\1test_cmp expect actual/

A future patch will clean up situations where we have multiple duplicate
statements within a test case. This is done to keep this patch purely
mechanical.

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

t5520: use test_cmp_rev where possible

In case an invocation of `git rev-list` fails within the command
substitution, the failure will be masked. Remove the command
substitution and use test_cmp_rev() so that failures can be discovered.

This change was done with the following sed expressions:

s/test "$(git rev-parse.* \([^)]*\))" = "$(git rev-parse \([^)]*\))"/test_cmp_rev \1 \2/
s/test \([^ ]*\) = "$(git rev-parse.* \([^)]*\))"/test_cmp_rev \1 \2/
s/test "$(git rev-parse.* \([^)]*\))" != "$(git rev-parse.* \([^)]*\))"/test_cmp_rev ! \1 \2/
s/test \([^ ]*\) != "$(git rev-parse.* \([^)]*\))"/test_cmp_rev ! \1 \2/

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