git
3 years agopretty format %(trailers): add a "key_value_separator"
Ævar Arnfjörð Bjarmason [Wed, 9 Dec 2020 15:52:08 +0000 (16:52 +0100)] 
pretty format %(trailers): add a "key_value_separator"

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
3 years agopretty format %(trailers) test: split a long line
Ævar Arnfjörð Bjarmason [Sun, 6 Dec 2020 00:24:45 +0000 (01:24 +0100)] 
pretty format %(trailers) test: split a long line

Split a very long line in a test introduced in 0b691d86851 (pretty:
add support for separator option in %(trailers), 2019-01-28). This
makes it easier to read, especially as follow-up commits will copy
this test as a template.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
3 years agoTenth batch
Junio C Hamano [Thu, 3 Dec 2020 07:44:25 +0000 (23:44 -0800)] 
Tenth batch

Signed-off-by: Junio C Hamano <gitster@pobox.com>
3 years agoMerge branch 'pk/subsub-fetch-fix'
Junio C Hamano [Thu, 3 Dec 2020 08:18:07 +0000 (00:18 -0800)] 
Merge branch 'pk/subsub-fetch-fix'

An earlier attempt to fix "git fetch --recurse-submodules" broke
another use case; revert it until a better fix is found.

* pk/subsub-fetch-fix:
  Revert "submodules: fix of regression on fetching of non-init subsub-repo"

3 years agoMerge branch 'jk/stop-pack-objects-when-fetch-is-killed'
Junio C Hamano [Thu, 3 Dec 2020 08:18:07 +0000 (00:18 -0800)] 
Merge branch 'jk/stop-pack-objects-when-fetch-is-killed'

"git fetch" that is killed may leave a pack-objects process behind,
still computing to find a good compression, wasting cycles.  This
has been corrected.

* jk/stop-pack-objects-when-fetch-is-killed:
  upload-pack: kill pack-objects helper on signal or exit

3 years agoMerge branch 'jk/stop-pack-objects-when-push-is-killed'
Junio C Hamano [Thu, 3 Dec 2020 08:18:06 +0000 (00:18 -0800)] 
Merge branch 'jk/stop-pack-objects-when-push-is-killed'

"git push" that is killed may leave a pack-objects process behind,
still computing to find a good compression, wasting cycles.  This
has been corrected.

* jk/stop-pack-objects-when-push-is-killed:
  send-pack: kill pack-objects helper on signal or exit

3 years agoMerge branch 'tb/repack-simplify'
Junio C Hamano [Thu, 3 Dec 2020 08:18:06 +0000 (00:18 -0800)] 
Merge branch 'tb/repack-simplify'

Simplify the logic to deal with a repack operation that ended up
creating the same packfile.

* tb/repack-simplify:
  builtin/repack.c: don't move existing packs out of the way
  builtin/repack.c: keep track of what pack-objects wrote
  repack: make "exts" array available outside cmd_repack()

3 years agoMerge branch 'pb/pull-rebase-recurse-submodules'
Junio C Hamano [Thu, 3 Dec 2020 08:18:06 +0000 (00:18 -0800)] 
Merge branch 'pb/pull-rebase-recurse-submodules'

"git pull --rebase --recurse-submodules" checked for local changes
in a wrong range and failed to run correctly when it should.

* pb/pull-rebase-recurse-submodules:
  pull: check for local submodule modifications with the right range
  t5572: describe '--rebase' tests a little more
  t5572: add notes on a peculiar test
  pull --rebase: compute rebase arguments in separate function

3 years agoMerge branch 'ab/retire-parse-remote'
Junio C Hamano [Thu, 3 Dec 2020 08:18:06 +0000 (00:18 -0800)] 
Merge branch 'ab/retire-parse-remote'

"git-parse-remote" shell script library outlived its usefulness.

* ab/retire-parse-remote:
  submodule: fix fetch_in_submodule logic
  parse-remote: remove this now-unused library
  submodule: remove sh function in favor of helper
  submodule: use "fetch" logic instead of custom remote discovery

3 years agoRevert "submodules: fix of regression on fetching of non-init subsub-repo"
Junio C Hamano [Wed, 2 Dec 2020 23:07:14 +0000 (15:07 -0800)] 
Revert "submodules: fix of regression on fetching of non-init subsub-repo"

This reverts commit 1b7ac4e6d4d490b224f5206af7418ed74e490608; in
<CAN0XMOLiS_8JZKF_wW70BvRRxkDHyUoa=Z3ODtB_Bd6f5Y=7JQ@mail.gmail.com>,
Ralf Thielow reports that "git fetch" with submodule.recurse set can
result in a bogus and infinitely recursive fetching of the same
submodule.

3 years agoupload-pack: kill pack-objects helper on signal or exit
Jeff King [Tue, 1 Dec 2020 12:15:13 +0000 (07:15 -0500)] 
upload-pack: kill pack-objects helper on signal or exit

We spawn an external pack-objects process to actually send objects to
the remote side. If we are killed by a signal during this process, then
pack-objects may continue to run. As soon as it starts producing output
for the pack, it will see a failure writing to upload-pack and exit
itself. But before then, it may do significant work traversing the
object graph, compressing deltas, etc, which will all be pointless. So
let's make sure to kill as soon as we know that the caller will not read
the result.

There's no test here, since it's inherently racy, but here's an easy
reproduction is on a large-ish repo like linux.git:

  - make sure you don't have pack bitmaps (since they make the enumerating
    phase go quickly). For linux.git it takes ~30s or so to walk the
    whole graph on my machine.

  - run "git clone --no-local -q . dst"; the "-q" is important because
    if pack-objects is writing progress to upload-pack (to get
    multiplexed over the sideband to the client), then it will notice
    pretty quickly the failure to write to stderr

  - kill the client-side clone process in another terminal (don't use
    ^C, as that will send SIGINT to all of the processes)

  - run "ps au | grep git" or similar to observe upload-pack dying
    within 5 seconds (it will send a keepalive that will notice the
    client has gone away)

  - but you'll still see pack-objects consuming 100% CPU (and 1GB+ of
    RAM) during the traversal and delta compression phases. It will exit
    as soon as it starts to write the pack (when it will notice that
    upload-pack went away).

With this patch, pack-objects exits as soon as upload-pack does.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
3 years agoNinth batch
Junio C Hamano [Mon, 30 Nov 2020 22:49:16 +0000 (14:49 -0800)] 
Ninth batch

Signed-off-by: Junio C Hamano <gitster@pobox.com>
3 years agoMerge branch 'sa/credential-store-timeout'
Junio C Hamano [Mon, 30 Nov 2020 22:49:45 +0000 (14:49 -0800)] 
Merge branch 'sa/credential-store-timeout'

Multiple "credential-store" backends can race to lock the same
file, causing everybody else but one to fail---reattempt locking
with some timeout to reduce the rate of the failure.

* sa/credential-store-timeout:
  crendential-store: use timeout when locking file

3 years agoMerge branch 'km/stash-error-message-fix'
Junio C Hamano [Mon, 30 Nov 2020 22:49:45 +0000 (14:49 -0800)] 
Merge branch 'km/stash-error-message-fix'

Error message fix.

* km/stash-error-message-fix:
  stash: add missing space to an error message

3 years agoMerge branch 'hn/sleep-millisec-decl'
Junio C Hamano [Mon, 30 Nov 2020 22:49:44 +0000 (14:49 -0800)] 
Merge branch 'hn/sleep-millisec-decl'

Move a definition of compatibility wrapper from cache.h to
git-compat-util.h

* hn/sleep-millisec-decl:
  move sleep_millisec to git-compat-util.h

3 years agoMerge branch 'js/t3404-master-to-primary'
Junio C Hamano [Mon, 30 Nov 2020 22:49:44 +0000 (14:49 -0800)] 
Merge branch 'js/t3404-master-to-primary'

A test script got cleaned up and then made not to depend on the
value of init.defaultBranch.

* js/t3404-master-to-primary:
  t3404: do not depend on any specific default branch name

3 years agoMerge branch 'na/notes-displayref-is-not-boolean'
Junio C Hamano [Mon, 30 Nov 2020 22:49:44 +0000 (14:49 -0800)] 
Merge branch 'na/notes-displayref-is-not-boolean'

Config parser fix for "git notes".

* na/notes-displayref-is-not-boolean:
  t3301: test proper exit response to no-value notes.displayRef.
  notes.c: fix a segfault in notes_display_config()

3 years agoMerge branch 'jc/do-not-just-explain-but-update-your-patch'
Junio C Hamano [Mon, 30 Nov 2020 22:49:43 +0000 (14:49 -0800)] 
Merge branch 'jc/do-not-just-explain-but-update-your-patch'

Expectation for the original contributor after responding to a
review comment to use the explanation in a patch update has been
described.

* jc/do-not-just-explain-but-update-your-patch:
  MyFirstContribition: answering questions is not the end of the story

3 years agoMerge branch 'mt/worktree-error-message-fix'
Junio C Hamano [Mon, 30 Nov 2020 22:49:43 +0000 (14:49 -0800)] 
Merge branch 'mt/worktree-error-message-fix'

Fix formulation of an error message with two placeholders in "git
worktree add" subcommand.

* mt/worktree-error-message-fix:
  worktree: fix order of arguments in error message

3 years agoMerge branch 'ab/gc-keep-base-option'
Junio C Hamano [Mon, 30 Nov 2020 22:49:42 +0000 (14:49 -0800)] 
Merge branch 'ab/gc-keep-base-option'

Fix an option name in "gc" documentation.

* ab/gc-keep-base-option:
  gc: rename keep_base_pack variable for --keep-largest-pack
  gc docs: change --keep-base-pack to --keep-largest-pack

3 years agoMerge branch 'js/t1309-master-to-topic'
Junio C Hamano [Mon, 30 Nov 2020 22:49:42 +0000 (14:49 -0800)] 
Merge branch 'js/t1309-master-to-topic'

Test preparation.

* js/t1309-master-to-topic:
  t1309: use a neutral branch name in the `onbranch` test cases

3 years agoMerge branch 'js/pull-rebase-use-advise'
Junio C Hamano [Mon, 30 Nov 2020 22:49:42 +0000 (14:49 -0800)] 
Merge branch 'js/pull-rebase-use-advise'

UI improvement.

* js/pull-rebase-use-advise:
  pull: colorize the hint about setting `pull.rebase`

3 years agoMerge branch 'js/t4015-wo-master'
Junio C Hamano [Mon, 30 Nov 2020 22:49:41 +0000 (14:49 -0800)] 
Merge branch 'js/t4015-wo-master'

A test script got cleaned up not to depend on the value of
init.defaultBranch.

* js/t4015-wo-master:
  t4015: let the test pass with any default branch name

3 years agoMerge branch 'js/t3040-cleanup'
Junio C Hamano [Mon, 30 Nov 2020 22:49:41 +0000 (14:49 -0800)] 
Merge branch 'js/t3040-cleanup'

Cleanup.

* js/t3040-cleanup:
  t3040: remove stale note

3 years agoMerge branch 'js/t2106-cleanup'
Junio C Hamano [Mon, 30 Nov 2020 22:49:41 +0000 (14:49 -0800)] 
Merge branch 'js/t2106-cleanup'

A test script got cleaned up and then made not to depend on the
value of init.defaultBranch.

* js/t2106-cleanup:
  t2106: ensure that the checkout fails for the expected reason
  t2106: make test independent of the current main branch name
  t2106: adjust style to the current conventions

3 years agoEighth batch
Junio C Hamano [Wed, 25 Nov 2020 22:32:32 +0000 (14:32 -0800)] 
Eighth batch

Signed-off-by: Junio C Hamano <gitster@pobox.com>
3 years agoMerge branch 'sg/tests-prereq'
Junio C Hamano [Wed, 25 Nov 2020 23:24:54 +0000 (15:24 -0800)] 
Merge branch 'sg/tests-prereq'

A lazily defined test prerequisite can now be defined in terms of
another lazily defined test prerequisite.

* sg/tests-prereq:
  tests: fix description of 'test_set_prereq'
  tests: make sure nested lazy prereqs work reliably

3 years agoMerge branch 'rs/plug-diff-cache-leak'
Junio C Hamano [Wed, 25 Nov 2020 23:24:53 +0000 (15:24 -0800)] 
Merge branch 'rs/plug-diff-cache-leak'

Memleak fix.

* rs/plug-diff-cache-leak:
  diff-lib: plug minor memory leaks in do_diff_cache()

3 years agoMerge branch 'rs/gc-sort-func-cast-fix'
Junio C Hamano [Wed, 25 Nov 2020 23:24:53 +0000 (15:24 -0800)] 
Merge branch 'rs/gc-sort-func-cast-fix'

Fix broken sorting of maintenance tasks.

* rs/gc-sort-func-cast-fix:
  gc: fix cast in compare_tasks_by_selection()

3 years agoMerge branch 'jc/ci-github-set-env'
Junio C Hamano [Wed, 25 Nov 2020 23:24:53 +0000 (15:24 -0800)] 
Merge branch 'jc/ci-github-set-env'

Another CI adjustment.

* jc/ci-github-set-env:
  ci: avoid `set-env` construct in print-test-failures.sh

3 years agoMerge branch 'sg/t5310-jgit-wants-sha1'
Junio C Hamano [Wed, 25 Nov 2020 23:24:53 +0000 (15:24 -0800)] 
Merge branch 'sg/t5310-jgit-wants-sha1'

Since jgit does not yet work with SHA-256 repositories, mark the
tests that uses it not to run unless we are testing with ShA-1
repositories.

* sg/t5310-jgit-wants-sha1:
  t5310-pack-bitmaps: skip JGit tests with SHA256

3 years agoMerge branch 'rs/archive-plug-leak-refname'
Junio C Hamano [Wed, 25 Nov 2020 23:24:53 +0000 (15:24 -0800)] 
Merge branch 'rs/archive-plug-leak-refname'

Memleak fix.

* rs/archive-plug-leak-refname:
  archive: release refname after use

3 years agoMerge branch 'ma/list-object-filter-opt-msgfix'
Junio C Hamano [Wed, 25 Nov 2020 23:24:52 +0000 (15:24 -0800)] 
Merge branch 'ma/list-object-filter-opt-msgfix'

Error message fix.

* ma/list-object-filter-opt-msgfix:
  list-objects-filter-options: fix function name in BUG

3 years agoMerge branch 'pk/subsub-fetch-fix'
Junio C Hamano [Wed, 25 Nov 2020 23:24:52 +0000 (15:24 -0800)] 
Merge branch 'pk/subsub-fetch-fix'

"git fetch" did not work correctly with nested submodules where the
innermost submodule that is not of interest got updated in the
upstream, which has been corrected.

* pk/subsub-fetch-fix:
  submodules: fix of regression on fetching of non-init subsub-repo

3 years agoMerge branch 'jk/4gb-idx'
Junio C Hamano [Wed, 25 Nov 2020 23:24:52 +0000 (15:24 -0800)] 
Merge branch 'jk/4gb-idx'

The code was not prepared to deal with pack .idx file that is
larger than 4GB.

* jk/4gb-idx:
  packfile: detect overflow in .idx file size checks
  block-sha1: take a size_t length parameter
  fsck: correctly compute checksums on idx files larger than 4GB
  use size_t to store pack .idx byte offsets
  compute pack .idx byte offsets using size_t

3 years agoMerge branch 'jx/t5411-flake-fix'
Junio C Hamano [Wed, 25 Nov 2020 23:24:52 +0000 (15:24 -0800)] 
Merge branch 'jx/t5411-flake-fix'

The exchange between receive-pack and proc-receive hook did not
carefully check for errors.

* jx/t5411-flake-fix:
  receive-pack: use default version 0 for proc-receive
  receive-pack: gently write messages to proc-receive
  t5411: new helper filter_out_user_friendly_and_stable_output

3 years agoMerge branch 'rs/hashwrite-be64'
Junio C Hamano [Wed, 25 Nov 2020 23:24:52 +0000 (15:24 -0800)] 
Merge branch 'rs/hashwrite-be64'

Code simplification.

* rs/hashwrite-be64:
  pack-write: use hashwrite_be64()
  midx: use hashwrite_be64()
  csum-file: add hashwrite_be64()

3 years agoMerge branch 'sg/bisect-approximately-halfway'
Junio C Hamano [Wed, 25 Nov 2020 23:24:52 +0000 (15:24 -0800)] 
Merge branch 'sg/bisect-approximately-halfway'

"git bisect start/next" in a large span of history spends a lot of
time trying to come up with exactly the half-way point; this can be
optimized by stopping when we see a commit that is close enough to
the half-way point.

* sg/bisect-approximately-halfway:
  bisect: loosen halfway() check for a large number of commits

3 years agoMerge branch 'fc/bash-completion-alias-of-alias'
Junio C Hamano [Wed, 25 Nov 2020 23:24:51 +0000 (15:24 -0800)] 
Merge branch 'fc/bash-completion-alias-of-alias'

The command line completion script (in contrib/) learned to expand
commands that are alias of alias.

* fc/bash-completion-alias-of-alias:
  completion: bash: improve alias loop detection
  completion: bash: check for alias loop
  completion: bash: support recursive aliases

3 years agocrendential-store: use timeout when locking file
Simão Afonso [Wed, 25 Nov 2020 18:31:23 +0000 (18:31 +0000)] 
crendential-store: use timeout when locking file

When holding the lock for rewriting the credential file, use a timeout
to avoid race conditions when the credentials file needs to be updated
in parallel.

An example would be doing `fetch --all` on a repository with several
remotes that need credentials, using parallel fetching.

The timeout can be configured using "credentialStore.lockTimeoutMS",
defaulting to 1 second.

Signed-off-by: Simão Afonso <simao.afonso@powertools-tech.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
3 years agomove sleep_millisec to git-compat-util.h
Han-Wen Nienhuys [Tue, 24 Nov 2020 19:10:11 +0000 (19:10 +0000)] 
move sleep_millisec to git-compat-util.h

The sleep function is defined in wrapper.c, so it makes more sense to be a in
system compatibility header.

Signed-off-by: Han-Wen Nienhuys <hanwen@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
3 years agoMyFirstContribition: answering questions is not the end of the story
Junio C Hamano [Fri, 20 Nov 2020 17:52:26 +0000 (09:52 -0800)] 
MyFirstContribition: answering questions is not the end of the story

A review exchange may begin with a reviewer asking "what did you
mean by this phrase in your log message (or here in the doc)?", the
author answering what was meant, and then the reviewer saying "ah,
that is what you meant---then the flow of the logic makes sense".

But that is not the happy end of the story.  New contributors often
forget that the material that has been reviewed in the above exchange
is still unclear in the same way to the next person who reads it,
until it gets updated.

While we are in the vicinity, rephrase the verb "request" used to
refer to comments by reviewers to "suggest"---this matches the
contrast between "original" and "suggested" that appears later in
the same paragraph, and more importantly makes it clearer that it is
not like authors are to please reviewers' wishes but rather
reviewers are merely helping authors to polish their commits.

Reviewed-by: Emily Shaffer <emilyshaffer@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
3 years agot3404: do not depend on any specific default branch name
Johannes Schindelin [Tue, 24 Nov 2020 10:15:49 +0000 (10:15 +0000)] 
t3404: do not depend on any specific default branch name

Now that we can override the default branch name in the tests via
`GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME`, we should avoid expecting a
particular hard-coded name.

So let's rename the initial branch immediately to `primary` and work
with that.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
3 years agosubmodule: fix fetch_in_submodule logic
Jeff King [Tue, 24 Nov 2020 09:06:05 +0000 (04:06 -0500)] 
submodule: fix fetch_in_submodule logic

Commit 1c1518071c (submodule: use "fetch" logic instead of custom remote
discovery, 2020-11-14) rewrote the logic in fetch_in_submodule to do:

  elif test "$2" -ne ""

But this is nonsense in shell: -ne is for numeric comparisons. This
should be "=" or more idiomatically:

  elif test -n "$2"

But once we fix that, many tests start failing. Because that commit
introduced another problem. The caller that passes 3 arguments looks
like this:

    fetch_in_submodule "$sm_path" $depth "$sha1"

Note the unquoted $depth parameter. When it isn't set, the function will
see only 2 arguments, and the function has no idea if what it sees in $2
is an option to go on the command line, or a refspec to pass on stdin.
In the old code before that commit:

   fetch_in_submodule () (
        sanitize_submodule_env &&
        cd "$1" &&
  -     case "$2" in
  -     '')
  -             git fetch ;;
  -     *)
  -             shift
  -             git fetch $(get_default_remote) "$@" ;;
  -     esac

we treated those the same, so it didn't matter. But in the new logic
(with my fix above):

  +     if test $# -eq 3
  +     then
  +             echo "$3" | git fetch --stdin "$2"
  +     elif test -n "$n"
  +     then
  +             git fetch "$2"
  +     else
  +             git fetch
  +     fi

we use the number of parameters to distinguish the two. Let's insist
that the caller pass an empty string for positional parameter two if
they want to have a third parameter after it.

But that still leaves one problem. In the --stdin block, we
unconditionally pass "$2" to git-fetch, even if it's the empty string.
Rather than add another conditional, we can use :+ parameter expansion
to include it only if it's non-empty. In fact, we can do the same for
the elif, too, simplifying it further. Technically this is overkill,
since we know the --depth parameter will not have whitespace (and
indeed, most callers do not bother quoting it), but it doesn't hurt for
the function to be careful.

It's somewhat amazing that no tests were failing. I think what happened
is that:

  - the 3-arg form rarely triggered; any call with a non-empty $depth
    and a $sha1 would work, but one with an empty $depth would only have
    2 arguments

  - because of the wrong arguments to "test", the shell would complain
    and exit non-zero. So we never ran the middle conditional at all

  - that left every call running "git fetch" with no arguments. A
    well-written test could have detected the distinction here, but in
    practice omitting --depth just means fetching more commits, and
    fetching everything (rather than a single sha1) works as long as the
    commit in question is reachable

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
3 years agostash: add missing space to an error message
Kyle Meyer [Tue, 24 Nov 2020 00:52:12 +0000 (19:52 -0500)] 
stash: add missing space to an error message

Restore a space that was lost in 8a0fc8d19d (stash: convert apply to
builtin, 2019-02-25).

Signed-off-by: Kyle Meyer <kyle@kyleam.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
3 years agot3301: test proper exit response to no-value notes.displayRef.
Nate Avers [Mon, 23 Nov 2020 03:23:42 +0000 (22:23 -0500)] 
t3301: test proper exit response to no-value notes.displayRef.

Signed-off-by: Nate Avers <nate@roosteregg.cc>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
3 years agonotes.c: fix a segfault in notes_display_config()
Nate Avers [Mon, 23 Nov 2020 03:23:41 +0000 (22:23 -0500)] 
notes.c: fix a segfault in notes_display_config()

If notes.displayRef is configured with no value[1], control should be
returned to the caller when notes.c:notes_display_config() checks if 'v'
is NULL. Otherwise, both git log --notes and git diff-tree --notes will
subsequently segfault when refs.h:has_glob_specials() calls strpbrk()
with a NULL first argument.

[1] Examples:
.git/config:
[notes]
displayRef
$ git -c notes.displayRef [...]

Signed-off-by: Nate Avers <nate@roosteregg.cc>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
3 years agoSeventh batch
Junio C Hamano [Sat, 21 Nov 2020 23:12:41 +0000 (15:12 -0800)] 
Seventh batch

Signed-off-by: Junio C Hamano <gitster@pobox.com>
3 years agoMerge branch 'pd/mergetool-nvimdiff'
Junio C Hamano [Sat, 21 Nov 2020 23:14:39 +0000 (15:14 -0800)] 
Merge branch 'pd/mergetool-nvimdiff'

Fix regression introduced when nvimdiff support in mergetool was added.

* pd/mergetool-nvimdiff:
  mergetool: avoid letting `list_tool_variants` break user-defined setups
  mergetools/bc: add `bc4` to the alias list for Beyond Compare

3 years agoMerge branch 'ab/config-mak-uname-simplify'
Junio C Hamano [Sat, 21 Nov 2020 23:14:38 +0000 (15:14 -0800)] 
Merge branch 'ab/config-mak-uname-simplify'

Build configuration cleanup.

* ab/config-mak-uname-simplify:
  config.mak.uname: remove unused NEEDS_SSL_WITH_CURL flag
  config.mak.uname: remove unused the NO_R_TO_GCC_LINKER flag

3 years agoMerge branch 'en/strmap'
Junio C Hamano [Sat, 21 Nov 2020 23:14:38 +0000 (15:14 -0800)] 
Merge branch 'en/strmap'

A specialization of hashmap that uses a string as key has been
introduced.  Hopefully it will see wider use over time.

* en/strmap:
  shortlog: use strset from strmap.h
  Use new HASHMAP_INIT macro to simplify hashmap initialization
  strmap: take advantage of FLEXPTR_ALLOC_STR when relevant
  strmap: enable allocations to come from a mem_pool
  strmap: add a strset sub-type
  strmap: split create_entry() out of strmap_put()
  strmap: add functions facilitating use as a string->int map
  strmap: enable faster clearing and reusing of strmaps
  strmap: add more utility functions
  strmap: new utility functions
  hashmap: provide deallocation function names
  hashmap: introduce a new hashmap_partial_clear()
  hashmap: allow re-use after hashmap_free()
  hashmap: adjust spacing to fix argument alignment
  hashmap: add usage documentation explaining hashmap_free[_entries]()

3 years agoMerge branch 'jk/diff-release-filespec-fix'
Junio C Hamano [Sat, 21 Nov 2020 23:14:38 +0000 (15:14 -0800)] 
Merge branch 'jk/diff-release-filespec-fix'

Running "git diff" while allowing external diff in a state with
unmerged paths used to segfault, which has been corrected.

* jk/diff-release-filespec-fix:
  t7800: simplify difftool test
  diff: allow passing NULL to diff_free_filespec_data()

3 years agoMerge branch 'jk/rev-parse-end-of-options'
Junio C Hamano [Sat, 21 Nov 2020 23:14:38 +0000 (15:14 -0800)] 
Merge branch 'jk/rev-parse-end-of-options'

"git rev-parse" learned the "--end-of-options" to help scripts to
safely take a parameter that is supposed to be a revision, e.g.
"git rev-parse --verify -q --end-of-options $rev".

* jk/rev-parse-end-of-options:
  rev-parse: handle --end-of-options
  rev-parse: put all options under the "-" check
  rev-parse: don't accept options after dashdash

3 years agoMerge branch 'jc/format-patch-name-max'
Junio C Hamano [Sat, 21 Nov 2020 23:14:38 +0000 (15:14 -0800)] 
Merge branch 'jc/format-patch-name-max'

The maximum length of output filenames "git format-patch" creates
has become configurable (used to be capped at 64).

* jc/format-patch-name-max:
  format-patch: make output filename configurable

3 years agosend-pack: kill pack-objects helper on signal or exit
Jeff King [Sat, 21 Nov 2020 00:29:21 +0000 (19:29 -0500)] 
send-pack: kill pack-objects helper on signal or exit

We spawn an external pack-objects process to actually send
objects to the remote side. If we are killed by a signal
during this process, the pack-objects will keep running and
complete the push, which may surprise the user. We should
take it down when we go down.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
3 years agoworktree: fix order of arguments in error message
Matheus Tavares [Fri, 20 Nov 2020 15:09:39 +0000 (12:09 -0300)] 
worktree: fix order of arguments in error message

`git worktree add` (without --force) errors out when given a path
that is already registered as a worktree and the path is missing on
disk. But the `cmd` and `path` strings are switched on the error
message. Let's fix that.

Signed-off-by: Matheus Tavares <matheus.bernardino@usp.br>
Reviewed-by: Eric Sunshine <sunshine@sunshineco.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
3 years agogc: rename keep_base_pack variable for --keep-largest-pack
Ævar Arnfjörð Bjarmason [Fri, 20 Nov 2020 11:55:22 +0000 (12:55 +0100)] 
gc: rename keep_base_pack variable for --keep-largest-pack

As noted in an earlier change the keep_base_pack variable name is a
relic from an earlier on-list version of ae4e89e549 ("gc: add
--keep-largest-pack option", 2018-04-15) before it was renamed to
--keep-largest-pack.

Let's change the variable name to avoid that confusion, it's easier to
read the code if there's a 1=1 mapping between the variable name and
option name.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
3 years agogc docs: change --keep-base-pack to --keep-largest-pack
Ævar Arnfjörð Bjarmason [Fri, 20 Nov 2020 11:55:21 +0000 (12:55 +0100)] 
gc docs: change --keep-base-pack to --keep-largest-pack

The --keep-base-pack option never existed in git.git. It was the name
for the --keep-largest-pack option in earlier revisions of that series
before it landed as ae4e89e549 ("gc: add --keep-largest-pack option",
2018-04-15).

The later patches in that series[1][2] weren't changed to also refer
to --keep-largest-pack, so we've had this reference to a nonexisting
option ever since the feature initially landed.

1. 55dfe13df9 ("gc: add gc.bigPackThreshold config", 2018-04-15)

2. 9806f5a7bf ("gc --auto: exclude base pack if not enough mem to
   "repack -ad"", 2018-04-15)

Reported-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
3 years agopull: colorize the hint about setting `pull.rebase`
Johannes Schindelin [Thu, 19 Nov 2020 10:22:29 +0000 (10:22 +0000)] 
pull: colorize the hint about setting `pull.rebase`

In d18c950a69f (pull: warn if the user didn't say whether to rebase or
to merge, 2020-03-09), a new hint was introduced to encourage users to
make a conscious decision about whether they want their pull to merge or
to rebase by configuring the `pull.rebase` setting.

This warning was clearly intended to advise users, but as pointed out in
https://lore.kernel.org/git/87ima2rdsm.fsf%40evledraar.gmail.com, it
uses `warning()` instead of `advise()`.

One consequence is that the advice is not colorized in the same manner
as other, similar messages. So let's use `advise()` instead.

Pointed-out-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
3 years agot4015: let the test pass with any default branch name
Johannes Schindelin [Wed, 18 Nov 2020 23:35:44 +0000 (23:35 +0000)] 
t4015: let the test pass with any default branch name

We do not need to hard-code the actual branch name, as we can use the
`test_commit` function to simplify the code and use the tag it
generates, thereby being a lot more precise in what we want.

Strangely enough, this test case would have succeeded even with an
overridden default branch name, obviously for the wrong reason. Let's
verify that it passes for the expected reason, by looking for a
tell-tale in Git's output.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
3 years agot1309: use a neutral branch name in the `onbranch` test cases
Johannes Schindelin [Thu, 19 Nov 2020 11:41:34 +0000 (11:41 +0000)] 
t1309: use a neutral branch name in the `onbranch` test cases

The `onbranch` test cases touched by this patch do not actually try to
include any other config. Their purpose is to avoid regressing on two
bugs in the `include.onbranch:<name>.path` code that we fixed in the
past, bugs that are actually unrelated to any concrete branch name.

The first bug was fixed in 85fe0e800ca (config: work around bug with
includeif:onbranch and early config, 2019-07-31). Essentially, when
reading early config, there would be a catch-22 trying to access the
refs, and therefore we simply cannot evaluate the condition at that
point. The test case ensures that we avoid emitting this bogus message:

BUG: refs.c:1851: attempting to get main_ref_store outside of repository

The second test case concerns the non-Git scenario, where we simply do
not have a current branch to begin with (because we don't have a
repository in the first place), and the test case was introduced in
22932d9169f (config: stop checking whether the_repository is NULL,
2019-08-06) to ensure that we don't cause a segmentation fault should
the code still incorrectly try to look at any ref.

In short, neither of these two test cases will ever look at a current
branch name, even in case of regressions. Therefore, the actual branch
name does not matter at all. We can therefore easily avoid
racially-charged branch names here, and that's what this patch does.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
3 years agogc: fix cast in compare_tasks_by_selection()
René Scharfe [Tue, 17 Nov 2020 21:59:49 +0000 (22:59 +0100)] 
gc: fix cast in compare_tasks_by_selection()

compare_tasks_by_selection() is used with QSORT and gets passed pointers
to the elements of "static struct maintenance_task tasks[]".  It casts
the *addresses* of these passed pointers to element pointers, though,
and thus effectively compares some unrelated values from the stack.  Fix
the casts to actually compare array elements.

Detected by USan (make SANITIZE=undefined test).

Signed-off-by: René Scharfe <l.s.r@web.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
3 years agoSixth batch
Junio C Hamano [Wed, 18 Nov 2020 21:33:25 +0000 (13:33 -0800)] 
Sixth batch

Signed-off-by: Junio C Hamano <gitster@pobox.com>
3 years agoMerge branch 'jc/blame-ignore-fix'
Junio C Hamano [Wed, 18 Nov 2020 21:32:54 +0000 (13:32 -0800)] 
Merge branch 'jc/blame-ignore-fix'

"git blame --ignore-revs-file=<file>" learned to ignore a
non-existent object name in the input, instead of complaining.

* jc/blame-ignore-fix:
  blame: silently ignore invalid ignore file objects

3 years agoMerge branch 'jc/sparse-error-for-developer-build'
Junio C Hamano [Wed, 18 Nov 2020 21:32:53 +0000 (13:32 -0800)] 
Merge branch 'jc/sparse-error-for-developer-build'

"make DEVELOPER=1 sparse" used to run sparse and let it emit
warnings; now such warnings will cause an error.

* jc/sparse-error-for-developer-build:
  Makefile: enable -Wsparse-error for DEVELOPER build

3 years agoMerge branch 'pb/blame-funcname-range-userdiff'
Junio C Hamano [Wed, 18 Nov 2020 21:32:53 +0000 (13:32 -0800)] 
Merge branch 'pb/blame-funcname-range-userdiff'

"git blame -L :funcname -- path" did not work well for a path for
which a userdiff driver is defined.

* pb/blame-funcname-range-userdiff:
  blame: simplify 'setup_blame_bloom_data' interface
  blame: simplify 'setup_scoreboard' interface
  blame: enable funcname blaming with userdiff driver
  line-log: mention both modes in 'blame' and 'log' short help
  doc: add more pointers to gitattributes(5) for userdiff
  blame-options.txt: also mention 'funcname' in '-L' description
  doc: line-range: improve formatting
  doc: log, gitk: move '-L' description to 'line-range-options.txt'

3 years agoMerge branch 'en/merge-ort-api-null-impl'
Junio C Hamano [Wed, 18 Nov 2020 21:32:53 +0000 (13:32 -0800)] 
Merge branch 'en/merge-ort-api-null-impl'

Preparation for a new merge strategy.

* en/merge-ort-api-null-impl:
  merge,rebase,revert: select ort or recursive by config or environment
  fast-rebase: demonstrate merge-ort's API via new test-tool command
  merge-ort-wrappers: new convience wrappers to mimic the old merge API
  merge-ort: barebones API of new merge strategy with empty implementation

3 years agoMerge branch 'ds/maintenance-part-3'
Junio C Hamano [Wed, 18 Nov 2020 21:32:53 +0000 (13:32 -0800)] 
Merge branch 'ds/maintenance-part-3'

Parts of "git maintenance" to ease writing crontab entries (and
other scheduling system configuration) for it.

* ds/maintenance-part-3:
  maintenance: add troubleshooting guide to docs
  maintenance: use 'incremental' strategy by default
  maintenance: create maintenance.strategy config
  maintenance: add start/stop subcommands
  maintenance: add [un]register subcommands
  for-each-repo: run subcommands on configured repos
  maintenance: add --schedule option and config
  maintenance: optionally skip --auto process

3 years agoMerge branch 'pw/rebase-i-orig-head'
Junio C Hamano [Wed, 18 Nov 2020 21:32:53 +0000 (13:32 -0800)] 
Merge branch 'pw/rebase-i-orig-head'

"git rebase -i" did not store ORIG_HEAD correctly.

* pw/rebase-i-orig-head:
  rebase -i: simplify get_revision_ranges()
  rebase -i: use struct object_id when writing state
  rebase -i: use struct object_id rather than looking up commit
  rebase -i: stop overwriting ORIG_HEAD buffer

3 years agoMerge branch 'rs/archive-high-compression'
Junio C Hamano [Wed, 18 Nov 2020 21:32:52 +0000 (13:32 -0800)] 
Merge branch 'rs/archive-high-compression'

"git archive" now allows compression level higher than "-9"
when generating tar.gz output.

* rs/archive-high-compression:
  archive: support compression levels beyond 9

3 years agoMerge branch 'dg/bswap-msvc'
Junio C Hamano [Wed, 18 Nov 2020 21:32:52 +0000 (13:32 -0800)] 
Merge branch 'dg/bswap-msvc'

Define ARM64 compiled with MSVC to be little-endian.

* dg/bswap-msvc:
  compat/bswap.h: don't assume MSVC is little-endian
  compat/bswap.h: simplify MSVC endianness detection

3 years agoMerge branch 'jk/format-patch-output'
Junio C Hamano [Wed, 18 Nov 2020 21:32:52 +0000 (13:32 -0800)] 
Merge branch 'jk/format-patch-output'

"git format-patch --output=there" did not work as expected and
instead crashed.  The option is now supported.

* jk/format-patch-output:
  format-patch: support --output option
  format-patch: tie file-opening logic to output_directory
  format-patch: refactor output selection

3 years agoMerge branch 'jc/line-log-takes-no-pathspec'
Junio C Hamano [Wed, 18 Nov 2020 21:32:52 +0000 (13:32 -0800)] 
Merge branch 'jc/line-log-takes-no-pathspec'

"git log -L<range>:<path>" is documented to take no pathspec, but
this was not enforced by the command line option parser, which has
been corrected.

* jc/line-log-takes-no-pathspec:
  log: diagnose -L used with pathspec as an error

3 years agoMerge branch 'rs/empty-reflog-check-fix'
Junio C Hamano [Wed, 18 Nov 2020 21:32:52 +0000 (13:32 -0800)] 
Merge branch 'rs/empty-reflog-check-fix'

The code to see if "git stash drop" can safely remove refs/stash
has been made more carerful.

* rs/empty-reflog-check-fix:
  stash: simplify reflog emptiness check

3 years agoMerge branch 'nk/perf-fsmonitor'
Junio C Hamano [Wed, 18 Nov 2020 21:32:52 +0000 (13:32 -0800)] 
Merge branch 'nk/perf-fsmonitor'

Add t/perf support for fsmonitor.

* nk/perf-fsmonitor:
  t/perf/fsmonitor: add benchmark for dirty status
  t/perf/fsmonitor: perf comparison of multiple fsmonitor integrations
  t/perf/fsmonitor: initialize test with git reset
  t/perf/fsmonitor: factor setup for fsmonitor into function
  t/perf/fsmonitor: silence initial git commit
  t/perf/fsmonitor: shorten DESC to basename
  t/perf/fsmonitor: factor description out for readability
  t/perf/fsmonitor: improve error message if typoing hook name
  t/perf/fsmonitor: move watchman setup to one-time-repo-setup
  t/perf/fsmonitor: separate one time repo initialization

3 years agoMerge branch 'en/merge-tests'
Junio C Hamano [Wed, 18 Nov 2020 21:32:52 +0000 (13:32 -0800)] 
Merge branch 'en/merge-tests'

Preparation for a new merge strategy.

* en/merge-tests:
  t6423: add more details about direct resolution of directories
  t6423: note improved ort handling with untracked files
  t6423, t6436: note improved ort handling with dirty files
  merge tests: expect slight differences in output for recursive vs. ort
  t6423: expect improved conflict markers labels in the ort backend
  t6404, t6423: expect improved rename/delete handling in ort backend
  t6416: correct expectation for rename/rename(1to2) + directory/file
  merge tests: expect improved directory/file conflict handling in ort
  t/: new helper for tests that pass with ort but fail with recursive

3 years agoMerge branch 'js/default-branch-name-adjust-t5515'
Junio C Hamano [Wed, 18 Nov 2020 21:32:51 +0000 (13:32 -0800)] 
Merge branch 'js/default-branch-name-adjust-t5515'

Prepare a test script to transition of the default branch name to
'main'.

* js/default-branch-name-adjust-t5515:
  t5515: use `main` as the name of the main branch for testing (conclusion)
  t5515: use `main` as the name of the main branch for testing (part 3)
  t5515: use `main` as the name of the main branch for testing (part 2)
  t5515: use `main` as the name of the main branch for testing (part 1)

3 years agoMerge branch 'dd/upload-pack-stateless-eof'
Junio C Hamano [Wed, 18 Nov 2020 21:32:51 +0000 (13:32 -0800)] 
Merge branch 'dd/upload-pack-stateless-eof'

"git fetch --depth=<n>" over the stateless RPC / smart HTTP
transport handled EOF from the client poorly at the server end.

* dd/upload-pack-stateless-eof:
  upload-pack: allow stateless client EOF just prior to haves

3 years agot3040: remove stale note
Johannes Schindelin [Wed, 18 Nov 2020 19:25:26 +0000 (19:25 +0000)] 
t3040: remove stale note

This comment was most likely a "note to self" during the development of
1c3e5c4ebc3 (Tests for core subproject support, 2007-04-19) and is
neither needed nor comprehensible at this point. Let's remove it.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
3 years agotests: fix description of 'test_set_prereq'
SZEDER Gábor [Wed, 18 Nov 2020 19:04:14 +0000 (20:04 +0100)] 
tests: fix description of 'test_set_prereq'

'test_set_prereq's description claims that prereqs can be specified to
'test_expect_code', but that is not the case (it is not meant to run a
test _case_, but a git command), so remove it.

OTOH that description doesn't mention 'test_external' and
'test_external_without_stderr' that do accept prereqs, so mention
them.

Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
3 years agotests: make sure nested lazy prereqs work reliably
SZEDER Gábor [Wed, 18 Nov 2020 19:04:13 +0000 (20:04 +0100)] 
tests: make sure nested lazy prereqs work reliably

Some test prereqs depend on other prereqs, so in a couple of cases we
have nested prereqs that look something like this:

  test_lazy_prereq FOO '
      test_have_prereq BAR &&
      check-foo
  '

This can be problematic, because lazy prereqs are evaluated in the
'$TRASH_DIRECTORY/prereq-test-dir' directory, which is the same for
every prereq, and which is automatically removed after the prereq has
been evaluated.  So if the inner prereq (BAR above) is a lazy prereq
that hasn't been evaluated yet, then after its evaluation the
'prereq-test-dir' shared with the outer prereq will be removed.
Consequently, 'check-foo' will find itself in a non-existing
directory, and won't be able to create/access any files in its cwd,
which could result in an unfulfilled outer prereq.

Luckily, this doesn't affect any of our current nested prereqs, either
because the inner prereq is not a lazy prereq (e.g. MINGW, CYGWIN or
PERL), or because the outer prereq happens to be checked without
touching any paths in its cwd (GPGSM and RFC1991 in 'lib-gpg.sh').

So to prevent nested prereqs from interfering with each other let's
evaluate each prereq in its own dedicated directory by appending the
prereq's name to the directory name, e.g. 'prereq-test-dir-SYMLINKS'.
In the test we check not only that the prereq test dir is still there,
but also that the inner prereq can't mess with the outer prereq's
files.

Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
3 years agot2106: ensure that the checkout fails for the expected reason
Johannes Schindelin [Wed, 18 Nov 2020 14:49:07 +0000 (14:49 +0000)] 
t2106: ensure that the checkout fails for the expected reason

During the transition of the test suite to a new default branch name, it
was noticed that this test case succeeded for the wrong reason when the
default branch name was overridden.

While we fixed that in the previous commit, let's make sure that we look
for a tell-tale in the error message that the `git checkout` failed for
the reason we wanted it to fail.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
3 years agot2106: make test independent of the current main branch name
Johannes Schindelin [Wed, 18 Nov 2020 14:49:06 +0000 (14:49 +0000)] 
t2106: make test independent of the current main branch name

We do have this wonderful shortcut `git checkout -` to go back to the
previous branch, thanks to the reflog.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
3 years agot2106: adjust style to the current conventions
Johannes Schindelin [Wed, 18 Nov 2020 14:49:05 +0000 (14:49 +0000)] 
t2106: adjust style to the current conventions

We settled on the style where the test cases' code starts by the opening
single quote being on the `test_expect_*` line, and the closing quote
being in its own line after the code.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
3 years agobuiltin/repack.c: don't move existing packs out of the way
Taylor Blau [Tue, 17 Nov 2020 20:15:16 +0000 (15:15 -0500)] 
builtin/repack.c: don't move existing packs out of the way

When 'git repack' creates a pack with the same name as any existing
pack, it moves the existing one to 'old-pack-xxx.{pack,idx,...}' and
then renames the new one into place.

Eventually, it would be nice to have 'git repack' allow for writing a
multi-pack index at the critical time (after the new packs have been
written / moved into place, but before the old ones have been deleted).
Guessing that this option might be called '--write-midx', this makes the
following situation (where repacks are issued back-to-back without any
new objects) impossible:

    $ git repack -adb
    $ git repack -adb --write-midx

In the second repack, the existing packs are overwritten verbatim with
the same rename-to-old sequence. At that point, the current MIDX is
invalidated, since it refers to now-missing packs. So that code wants to
be run after the MIDX is re-written. But (prior to this patch) the new
MIDX can't be written until the new packs are moved into place. So, we
have a circular dependency.

This is all hypothetical, since no code currently exists to write a MIDX
safely during a 'git repack' (the 'GIT_TEST_MULTI_PACK_INDEX' does so
unsafely). Putting hypothetical aside, though: why do we need to rename
existing packs to be prefixed with 'old-' anyway?

This behavior dates all the way back to 2ad47d6 (git-repack: Be
careful when updating the same pack as an existing one., 2006-06-25).
2ad47d6 is mainly concerned about a case where a newly written pack
would have a different structure than its index. This used to be
possible when the pack name was a hash of the set of objects. Under this
naming scheme, two packs that store the same set of objects could differ
in delta selection, object positioning, or both. If this happened, then
any such packs would be unreadable in the instant between copying the
new pack and new index (i.e., either the index or pack will be stale
depending on the order that they were copied).

But since 1190a1a (pack-objects: name pack files after trailer hash,
2013-12-05), this is no longer possible, since pack files are named not
after their logical contents (i.e., the set of objects), but by the
actual checksum of their contents. So, this old- behavior can safely go,
which allows us to avoid our circular dependency above.

In addition to avoiding the circular dependency, this patch also makes
'git repack' a lot simpler, since we don't have to deal with failures
encountered when renaming existing packs to be prefixed with 'old-'.

This patch is mostly limited to removing code paths that deal with the
'old' prefixing, with the exception of files that include the pack's
name in their own filename, like .idx, .bitmap, and related files. The
exception is that we want to continue to trust what pack-objects wrote.
That is, it is not the case that we pretend as if pack-objects didn't
write files identical to ones that already exist, but rather that we
respect what pack-objects wrote as the source of truth. That cuts two
ways:

  - If pack-objects produced an identical pack to one that already
    exists with a bitmap, but did not produce a bitmap, we remove the
    bitmap that already exists. (This behavior is codified in t7700.14).

  - If pack-objects produced an identical pack to one that already
    exists, we trust the just-written version of the coresponding .idx,
    .promisor, and other files over the ones that already exist. This
    ensures that we use the most up-to-date versions of this files,
    which is safe even in the face of format changes in, say, the .idx
    file (which would not be reflected in the .idx file's name).

Helped-by: Jeff King <peff@peff.net>
Signed-off-by: Taylor Blau <me@ttaylorr.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
3 years agoci: avoid `set-env` construct in print-test-failures.sh
Junio C Hamano [Tue, 17 Nov 2020 20:10:35 +0000 (12:10 -0800)] 
ci: avoid `set-env` construct in print-test-failures.sh

Imitating cac42e47 (ci: avoid using the deprecated `set-env`
construct, 2020-11-07), avoid deprecated ::set-env and use the
recommended alternative instead in print-test-failures.sh

Helped-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
3 years agocompletion: bash: improve alias loop detection
Felipe Contreras [Thu, 12 Nov 2020 22:34:52 +0000 (16:34 -0600)] 
completion: bash: improve alias loop detection

It is possible for the name of an alias to end with the name of another
alias, in which case the code will incorrectly detect a loop.

We can fix that by adding an extra space between words.

Suggested-by: SZEDER Gábor <szeder.dev@gmail.com>
Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
3 years agopull: check for local submodule modifications with the right range
Philippe Blain [Sat, 14 Nov 2020 00:34:45 +0000 (00:34 +0000)] 
pull: check for local submodule modifications with the right range

Ever since 'git pull' learned '--recurse-submodules' in a6d7eb2c7a
(pull: optionally rebase submodules (remote submodule changes only),
2017-06-23), we check if there are local submodule modifications by
checking the revision range 'curr_head --not rebase_fork_point'.

The goal of this check is to abort the pull if there are submodule
modifications in the local commits being rebased, since this scenario is
not supported.

However, the actual range of commits being rebased is not
'rebase_fork_point..curr_head', as the logic in
'get_rebase_newbase_and_upstream' reveals, it is 'upstream..curr_head'.

If the 'git merge-base --fork-point' invocation in
'get_rebase_fork_point' fails to find a fork point between the current
branch and the remote-tracking branch we are pulling from,
'rebase_fork_point' is null and since 4d36f88be7 (submodule: do not pass
null OID to setup_revisions, 2018-05-24), 'submodule_touches_in_range'
checks 'curr_head' and all its ancestors for submodule modifications.

Since it is highly likely that there are submodule modifications in this
range (which is in effect the whole history of the current branch), this
prevents 'git pull --rebase --recurse-submodules' from succeeding if no
fork point exists between the current branch and the remote-tracking
branch being pulled. This can happen, for example, when the current
branch was forked from a commit which was never recorded in the reflog
of the remote-tracking branch we are pulling, as the last two paragraphs
of the "Discussion on fork-point mode" section in git-merge-base(1)
explain.

Fix this bug by passing 'upstream' instead of 'rebase_fork_point' as the
'excl_oid' argument to 'submodule_touches_in_range'.

Reported-by: Brice Goglin <bgoglin@free.fr>
Signed-off-by: Philippe Blain <levraiphilippeblain@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
3 years agot5572: describe '--rebase' tests a little more
Philippe Blain [Sat, 14 Nov 2020 00:34:44 +0000 (00:34 +0000)] 
t5572: describe '--rebase' tests a little more

It can be hard at first glance to distinguish what is different between
the two tests 'recursive rebasing pull' and 'pull rebase recursing fails
with conflicts' in 't5572-pull-submodule.sh', and to understand how they
relate to the scenarios described in a6d7eb2c7a (pull: optionally rebase
submodules (remote submodule changes only), 2017-06-23), which
implemented '--recurse-submodules' for 'git pull' and added these tests.

Rename the tests to be more descriptive and add some bullet points
comments describing the different scenarios.

Signed-off-by: Philippe Blain <levraiphilippeblain@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
3 years agot5572: add notes on a peculiar test
Philippe Blain [Sat, 14 Nov 2020 00:34:43 +0000 (00:34 +0000)] 
t5572: add notes on a peculiar test

Test 5572.63 ("branch has no merge base with remote-tracking
counterpart") was introduced in 4d36f88be7 (submodule: do not pass null
OID to setup_revisions, 2018-05-24), as a regression test for the bug
this commit was fixing (preventing a 'fatal: bad object' error when the
current branch and the remote-tracking branch we are pulling have no
merge-base).

However, the commit message for 4d36f88be7 does not describe in which
real-life situation this bug was encountered. The brief discussion on the
mailing list [1] does not either.

The regression test is not really representative of a real-life
scenario: both the local repository and its upstream have only a single
commit, and the "no merge-base" scenario is simulated by recreating this
root commit in the local repository using 'git commit-tree' before
calling 'git pull --rebase --recurse-submodules'. The rebase succeeds
and results in the local branch being reset to the same root commit as
the upstream branch.

The fix in 4d36f88be7 modifies 'submodule.c::submodule_touches_in_range'
so that if 'excl_oid' is null, which is the case when the 'git merge-base
--fork-point' invocation in 'builtin/pull.c::get_rebase_fork_point'
errors (no fork-point), then instead of 'incl_oid --not excl_oid' being
passed to setup_revisions, only 'incl_oid' is passed, and
'submodule_touches_in_range' examines 'incl_oid' and all its ancestors
to verify that they do not touch the submodule.

In test 5572.63, the recreated lone root commit in the local repository is
thus the only commit being examined by 'submodule_touches_in_range', and
this commit *adds* the submodule. However, 'submodule_touches_in_range'
*succeeds* because 'combine-diff.c::diff_tree_combined' (see the
backtrace below) returns early since this commit is the root commit
and has no parents.

  #0  diff_tree_combined at combine-diff.c:1494
  #1  0x0000000100150cbe in diff_tree_combined_merge at combine-diff.c:1649
  #2  0x00000001002c7147 in collect_changed_submodules at submodule.c:869
  #3  0x00000001002c7d6f in submodule_touches_in_range at submodule.c:1268
  #4  0x00000001000ad58b in cmd_pull at builtin/pull.c:1040

In light of all this, add a note in t5572 documenting this peculiar
test.

[1] https://lore.kernel.org/git/20180524204729.19896-1-jonathantanmy@google.com/t/#u

Signed-off-by: Philippe Blain <levraiphilippeblain@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
3 years agopull --rebase: compute rebase arguments in separate function
Philippe Blain [Sat, 14 Nov 2020 00:34:42 +0000 (00:34 +0000)] 
pull --rebase: compute rebase arguments in separate function

The function 'run_rebase' is responsible for constructing the
command line to be passed to 'git rebase'. This includes both forwarding
pass-through options given to 'git pull' as well computing the <newbase>
and <upstream> arguments to 'git rebase'.

A following commit will need to access the <upstream> argument in
'cmd_pull' to fix a bug with 'git pull --rebase --recurse-submodules'.
In order to do so, refactor the code so that the <newbase> and
<upstream> commits are computed in a new, separate function,
'get_rebase_newbase_and_upstream'.

Signed-off-by: Philippe Blain <levraiphilippeblain@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
3 years agobuiltin/repack.c: keep track of what pack-objects wrote
Taylor Blau [Mon, 16 Nov 2020 18:41:17 +0000 (13:41 -0500)] 
builtin/repack.c: keep track of what pack-objects wrote

In the subsequent commit, it will become useful to keep track of which
metadata files were written by pack-objects. We already do this to an
extent with the 'exts' array, which only is used in the context of
existing packs.

Co-authored-by: Jeff King <peff@peff.net>
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Taylor Blau <me@ttaylorr.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
3 years agorepack: make "exts" array available outside cmd_repack()
Jeff King [Mon, 16 Nov 2020 18:41:12 +0000 (13:41 -0500)] 
repack: make "exts" array available outside cmd_repack()

We'll use it in a helper function soon.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Taylor Blau <me@ttaylorr.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
3 years agolist-objects-filter-options: fix function name in BUG
Martin Ågren [Sat, 14 Nov 2020 08:43:26 +0000 (09:43 +0100)] 
list-objects-filter-options: fix function name in BUG

Fix the function name we give in the BUG message. It's "config", not
"choice".

Signed-off-by: Martin Ågren <martin.agren@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
3 years agoarchive: release refname after use
René Scharfe [Sat, 14 Nov 2020 22:01:04 +0000 (23:01 +0100)] 
archive: release refname after use

parse_treeish_arg() uses dwim_ref() to set refname to a strdup'd string.
Release it after use.  Also remove the const qualifier from the refname
member to signify that ownership of the string is handed to the struct,
leaving cleanup duty with the caller of parse_treeish_arg(), thus
avoiding a cast.

Signed-off-by: René Scharfe <l.s.r@web.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
3 years agodiff-lib: plug minor memory leaks in do_diff_cache()
René Scharfe [Sat, 14 Nov 2020 18:37:03 +0000 (19:37 +0100)] 
diff-lib: plug minor memory leaks in do_diff_cache()

do_diff_cache() builds a struct rev_info to hand to diff_cache() from
scratch by initializing it using repo_init_revisions() and then
replacing its diffopt and prune_data members.

The diffopt member is initialized to a heap-allocated list of options,
though.  Release it using diff_setup_done() before overwriting it.

The initial value of the prune_data member doesn't need to be released,
but the copy created using copy_pathspec() does.  Clear it after use.

Signed-off-by: René Scharfe <l.s.r@web.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
3 years agopackfile: detect overflow in .idx file size checks
Jeff King [Fri, 13 Nov 2020 05:07:19 +0000 (00:07 -0500)] 
packfile: detect overflow in .idx file size checks

In load_idx(), we check that the .idx file is sized appropriately for
the number of objects it claims to have. We recently fixed the case
where the number of objects caused our expected size to overflow a
32-bit unsigned int, and we switched to size_t.

On a 64-bit system, this is fine; our size_t covers any expected size.
On a 32-bit system, though, it won't. The file may claim to have 2^31
objects, which will overflow even a size_t.

This doesn't hurt us at all for a well-formed idx file. A 32-bit system
would already have failed to mmap such a file, since it would be too
big. But an .idx file which _claims_ to have 2^31 objects but is
actually much smaller would fool our check.

This is a broken file, and for the most part we don't care that much
what happens. But:

  - it's a little friendlier to notice up front "woah, this file is
    broken" than it is to get nonsense results

  - later access of the data assumes that the loading function
    sanity-checked that we have at least enough bytes for the regular
    object-id table. A malformed .idx file could lead to an
    out-of-bounds read.

So let's use our overflow-checking functions to make sure that we're not
fooled by a malformed file.

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