git.git
5 years agoconfig: add --fixed-value option, un-implemented
Derrick Stolee [Wed, 25 Nov 2020 22:12:53 +0000 (22:12 +0000)]
config: add --fixed-value option, un-implemented

The 'git config' builtin takes a 'value-pattern' parameter for several
actions. This can cause confusion when expecting exact value matches
instead of regex matches, especially when the input string contains
metacharacters. While callers can escape the patterns themselves, it
would be more friendly to allow an argument to disable the pattern
matching in favor of an exact string match.

Add a new '--fixed-value' option that does not currently change the
behavior. The implementation will be filled in by later changes for
each appropriate action. For now, check and test that --fixed-value
will abort the command when included with an incompatible action or
without a 'value-pattern' argument.

The name '--fixed-value' was chosen over something simpler like
'--fixed' because some commands allow regular expressions on the
key in addition to the value.

Signed-off-by: Derrick Stolee <redacted>
Signed-off-by: Junio C Hamano <redacted>
5 years agot1300: add test for --replace-all with value-pattern
Derrick Stolee [Wed, 25 Nov 2020 22:12:52 +0000 (22:12 +0000)]
t1300: add test for --replace-all with value-pattern

The --replace-all option was added in 4ddba79d (git-config-set: add more
options) but was not tested along with the 'value-pattern' parameter.
Since we will be updating this option to optionally treat 'value-pattern'
as a fixed string, let's add a test here that documents the current
behavior.

Signed-off-by: Derrick Stolee <redacted>
Signed-off-by: Junio C Hamano <redacted>
5 years agot1300: test "set all" mode with value-pattern
Derrick Stolee [Wed, 25 Nov 2020 22:12:51 +0000 (22:12 +0000)]
t1300: test "set all" mode with value-pattern

Without additional modifiers, 'git config <key> <value>' attempts
to set a single value in the .git/config file. When the
value-pattern parameter is supplied, this command behaves in a
non-trivial manner.

Consider 'git config <key> <value> <value-pattern>'. The expected
behavior is as follows:

1. If there are multiple existing values that match 'value-pattern',
   then the command fails. Users should use --replace-all instead.

2. If there is no existing values match 'value-pattern', then the
   'key=value' pair is appended, making this 'key' a multi-valued
   config setting.

3. If there is one existing value that matches 'value-pattern', then
   the new config has one entry where 'key=value'.

Add a test that demonstrates these options. Break from the existing
pattern in t1300-config.sh to use 'git config --file=<file>' instead of
modifying .git/config directly to prevent possibly incompatible repo
states. Also use 'git config --file=<file> --list' for config state
comparison instead of the config file format. This makes the tests
more readable.

Signed-off-by: Derrick Stolee <redacted>
Signed-off-by: Junio C Hamano <redacted>
5 years agoconfig: replace 'value_regex' with 'value_pattern'
Derrick Stolee [Wed, 25 Nov 2020 22:12:50 +0000 (22:12 +0000)]
config: replace 'value_regex' with 'value_pattern'

The 'value_regex' argument in the 'git config' builtin is poorly named,
especially related to an upcoming change that allows exact string
matches instead of ERE pattern matches.

Perform a mostly mechanical change of every instance of 'value_regex' to
'value_pattern' in the codebase. This is only critical for documentation
and error messages, but it is best to be consistent inside the codebase,
too.

For documentation, use 'value-pattern' which is better punctuation. This
affects Documentation/git-config.txt and the usage in builtin/config.c,
which was already mixed between 'value_regex' and 'value-regex'.

I gave some thought to leaving the value_regex variables inside config.c
that are regex_t pointers. However, it is probably best to keep the name
consistent with the rest of the variables.

This does not update the translations inside the po/ directory, as that
creates conflicts with ongoing work. The input strings should
automatically update through automation, and a few of the output strings
currently use "[value_regex]" directly.

Signed-off-by: Derrick Stolee <redacted>
Signed-off-by: Junio C Hamano <redacted>
5 years agoconfig: convert multi_replace to flags
Derrick Stolee [Wed, 25 Nov 2020 22:12:49 +0000 (22:12 +0000)]
config: convert multi_replace to flags

We will extend the flexibility of the config API. Before doing so, let's
take an existing 'int multi_replace' parameter and replace it with a new
'unsigned flags' parameter that can take multiple options as a bit field.

Update all callers that specified multi_replace to now specify the
CONFIG_FLAGS_MULTI_REPLACE flag. To add more clarity, extend the
documentation of git_config_set_multivar_in_file() including a clear
labeling of its arguments. Other config API methods in config.h require
only a change of the final parameter from 'int' to 'unsigned'.

Signed-off-by: Derrick Stolee <redacted>
Signed-off-by: Junio C Hamano <redacted>
5 years agomidx.c: protect against disappearing packs
Taylor Blau [Wed, 25 Nov 2020 17:17:33 +0000 (12:17 -0500)]
midx.c: protect against disappearing packs

When a packed object is stored in a multi-pack index, but that pack has
racily gone away, the MIDX code simply calls die(), when it could be
returning an error to the caller, which would in turn lead to
re-scanning the pack directory.

A pack can racily disappear, for example, due to a simultaneous 'git
repack -ad',

You can also reproduce this with two terminals, where one is running:

    git init
    while true; do
      git commit -q --allow-empty -m foo
      git repack -ad
      git multi-pack-index write
    done

(in effect, constantly writing new MIDXs), and the other is running:

    obj=$(git rev-parse HEAD)
    while true; do
      echo $obj | git cat-file --batch-check='%(objectsize:disk)' || break
    done

That will sometimes hit the error preparing packfile from
multi-pack-index message, which this patch fixes.

Right now, that path to discovering a missing pack looks something like
'find_pack_entry()' calling 'fill_midx_entry()' and eventually making
its way to call 'nth_midxed_pack_entry()'.

'nth_midxed_pack_entry()' already checks 'is_pack_valid()' and
propagates an error if the pack is invalid. So, this works if the pack
has gone away between calling 'prepare_midx_pack()' and before calling
'is_pack_valid()', but not if it disappears before then.

Catch the case where the pack has already disappeared before
'prepare_midx_pack()' by returning an error in that case, too.

Co-authored-by: Jeff King <redacted>
Signed-off-by: Taylor Blau <redacted>
Signed-off-by: Junio C Hamano <redacted>
5 years agopackfile.c: protect against disappearing indexes
Taylor Blau [Wed, 25 Nov 2020 17:17:28 +0000 (12:17 -0500)]
packfile.c: protect against disappearing indexes

In 17c35c8969 (packfile: skip loading index if in multi-pack-index,
2018-07-12) we stopped loading the .idx file for packs that are
contained within a multi-pack index.

This saves us the effort of loading an .idx and doing some lightweight
validity checks by way of 'packfile.c:load_idx()', but introduces a race
between processes that need to load the index (e.g., to generate a
reverse index) and processes that can delete the index.

For example, running the following in your shell:

    $ git init repo && cd repo
    $ git commit --allow-empty -m 'base'
    $ git repack -ad && git multi-pack-index write

followed by:

    $ rm -f .git/objects/pack/pack-*.idx
    $ git rev-parse HEAD | git cat-file --batch-check='%(objectsize:disk)'

will result in a segfault prior to this patch. What's happening here is
that we notice that the pack is in the multi-pack index, and so don't
check that it still has a .idx. When we then try and load that index to
generate a reverse index, we don't have it, so the call to
'find_pack_revindex()' in 'packfile.c:packed_object_info()' returns
NULL, and then dereferencing it causes a segfault.

Of course, we don't ever expect someone to remove the index file by
hand, or to be in a state where we never wrote it to begin with (yet
find that pack in the multi-pack-index). But, this can happen in a
timing race with 'git repack -ad', which removes all existing packs
after writing a new pack containing all of their objects.

Avoid this by reverting the hunk of 17c35c8969 which stops loading the
index when the pack is contained in a MIDX. This makes the latter half
of 17c35c8969 useless, since we'll always have a non-NULL
'p->index_data', in which case that if statement isn't guarding
anything.

These two together effectively revert 17c35c8969, and avoid the race
explained above.

Co-authored-by: Jeff King <redacted>
Signed-off-by: Taylor Blau <redacted>
Signed-off-by: Junio C Hamano <redacted>
5 years agohelp.c: help.autocorrect=never means "do not compute suggestions"
Drew DeVault [Wed, 25 Nov 2020 21:01:45 +0000 (13:01 -0800)]
help.c: help.autocorrect=never means "do not compute suggestions"

While help.autocorrect can be set to 0 to decline auto-execution of
possibly mistyped commands, it still spends cycles to compute the
suggestions, and it wastes screen real estate.

Update help.autocorrect to accept the string "never" to just exit
with error upon mistyped commands to help users who prefer to never
see suggested corrections at all.

While at it, introduce "immediate" as a more readable way to
immediately execute the auto-corrected command, which can be done
with negative value.

Signed-off-by: Drew DeVault <redacted>
Signed-off-by: Junio C Hamano <redacted>
5 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 <redacted>
Signed-off-by: Junio C Hamano <redacted>
5 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 <redacted>
Signed-off-by: Junio C Hamano <redacted>
5 years agousage: add trace2 entry upon warning()
Jonathan Tan [Mon, 23 Nov 2020 20:45:22 +0000 (12:45 -0800)]
usage: add trace2 entry upon warning()

Emit a trace2 error event whenever warning() is called, just like when
die(), error(), or usage() is called.

This helps debugging issues that would trigger warnings but not errors.
In particular, this might have helped debugging an issue I encountered
with commit graphs at $DAYJOB [1].

There is a tradeoff between including potentially relevant messages and
cluttering up the trace output produced. I think that warning() messages
should be included in traces, because by its nature, Git is used over
multiple invocations of the Git tool, and a failure (currently traced)
in a Git invocation might be caused by an unexpected interaction in a
previous Git invocation that only has a warning (currently untraced) as
a symptom - as is the case in [1].

[1] https://lore.kernel.org/git/20200629220744.1054093-1-jonathantanmy@google.com/

Signed-off-by: Jonathan Tan <redacted>
Signed-off-by: Junio C Hamano <redacted>
5 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 <redacted>
Signed-off-by: Junio C Hamano <redacted>
5 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 <redacted>
Signed-off-by: Junio C Hamano <redacted>
5 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 <redacted>
Signed-off-by: Junio C Hamano <redacted>
5 years agomaintenance: include 'cron' details in docs
Derrick Stolee [Tue, 24 Nov 2020 04:16:43 +0000 (04:16 +0000)]
maintenance: include 'cron' details in docs

Advanced and expert users may want to know how 'git maintenance start'
schedules background maintenance in order to customize their own
schedules beyond what the maintenance.* config values allow. Start a new
set of sections in git-maintenance.txt that describe how 'cron' is used
to run these tasks.

This is particularly valuable for users who want to inspect what Git is
doing or for users who want to customize the schedule further. Having a
baseline can provide a way forward for users who have never worked with
cron schedules.

Helped-by: Eric Sunshine <redacted>
Signed-off-by: Derrick Stolee <redacted>
Signed-off-by: Junio C Hamano <redacted>
5 years agomaintenance: extract platform-specific scheduling
Derrick Stolee [Tue, 24 Nov 2020 04:16:42 +0000 (04:16 +0000)]
maintenance: extract platform-specific scheduling

The existing schedule mechanism using 'cron' is supported by POSIX
platforms, but not Windows. It also works slightly differently on
macOS to significant detriment of the user experience. To allow for
new implementations on these platforms, extract a method that
performs the platform-specific scheduling mechanism. This will be
swapped at compile time with new implementations on specialized
platforms.

As we add this generality, rename GIT_TEST_CRONTAB to
GIT_TEST_MAINT_SCHEDULER. Further, this variable is now parsed as
"<scheduler>:<command>" so we can test platform-specific scheduling
logic even when not on the correct platform. By specifying the
<scheduler> in this string, we will be able to test all three sets of
Git logic from a Linux machine.

Co-authored-by: Eric Sunshine <redacted>
Signed-off-by: Eric Sunshine <redacted>
Signed-off-by: Derrick Stolee <redacted>
Signed-off-by: Junio C Hamano <redacted>
5 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 <redacted>
Signed-off-by: Junio C Hamano <redacted>
5 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 <redacted>
Signed-off-by: Junio C Hamano <redacted>
5 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 <redacted>
Signed-off-by: Junio C Hamano <redacted>
5 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 <redacted>
5 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

5 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

5 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]()

5 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()

5 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

5 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

5 years agogrep: use designated initializers for `grep_defaults`
Martin Ågren [Sat, 21 Nov 2020 18:31:08 +0000 (19:31 +0100)]
grep: use designated initializers for `grep_defaults`

In 15fabd1bbd ("builtin/grep.c: make configuration callback more
reusable", 2012-10-09), we learned to fill a `static struct grep_opt
grep_defaults` which we can use as a blueprint for other such structs.

At the time, we didn't consider designated initializers to be widely
useable, but these days, we do. (See, e.g., cbc0f81d96 ("strbuf: use
designated initializers in STRBUF_INIT", 2017-07-10).)

Use designated initializers to let the compiler set up the struct and so
that we don't need to remember to call `init_grep_defaults()`.

Signed-off-by: Martin Ågren <redacted>
Signed-off-by: Junio C Hamano <redacted>
5 years agogrep: don't set up a "default" repo for grep
Martin Ågren [Sat, 21 Nov 2020 18:31:07 +0000 (19:31 +0100)]
grep: don't set up a "default" repo for grep

`init_grep_defaults()` fills a `static struct grep_opt grep_defaults`.
This struct is then used by `grep_init()` as a blueprint for other such
structs. Notably, `grep_init()` takes a `struct repo *` and assigns it
into the target struct.

As a result, it is unnecessary for us to take a `struct repo *` in
`init_grep_defaults()` as well. We assign it into the default struct and
never look at it again. And in light of how we return early if we have
already set up the default struct, it's not just unnecessary, but is
also a bit confusing: If we are called twice and with different repos,
is it a bug or a feature that we ignore the second repo?

Drop the repo parameter for `init_grep_defaults()`.

Signed-off-by: Martin Ågren <redacted>
Signed-off-by: Junio C Hamano <redacted>
5 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 <redacted>
Signed-off-by: Junio C Hamano <redacted>
5 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 <redacted>
Reviewed-by: Eric Sunshine <redacted>
Signed-off-by: Junio C Hamano <redacted>
5 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 <redacted>
Signed-off-by: Junio C Hamano <redacted>
5 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 <redacted>
Signed-off-by: Ævar Arnfjörð Bjarmason <redacted>
Signed-off-by: Junio C Hamano <redacted>
5 years agotests: drop prereq `PREPARE_FOR_MAIN_BRANCH` where no longer needed
Johannes Schindelin [Wed, 18 Nov 2020 23:44:46 +0000 (23:44 +0000)]
tests: drop prereq `PREPARE_FOR_MAIN_BRANCH` where no longer needed

We introduced the `PREPARE_FOR_MAIN_BRANCH` prereq for the sole purpose
of allowing us to perform the non-trivial adjustments regarding the
`master` -> `main` rename before the automatable ones.

Now that the transition is almost complete, we can stop using it in most
instances. The only two exceptions are t5526 and t9902: at the time of
writing, there are other patches in flight that touch these test
scripts, therefore their transition to `main` is postponed to a later
date.

This patch is the result of this command:

sed -i 's/PREPARE_FOR_MAIN_BRANCH[ ,]//' t/t[0-9]*.sh &&
git checkout HEAD -- t/t5526\* t/t9902\*

Signed-off-by: Johannes Schindelin <redacted>
Signed-off-by: Junio C Hamano <redacted>
5 years agot99*: adjust the references to the default branch name "main"
Johannes Schindelin [Wed, 18 Nov 2020 23:44:45 +0000 (23:44 +0000)]
t99*: adjust the references to the default branch name "main"

Carefully excluding t9902, which sees independent development elsewhere
at the time of writing, we use `main` as the default branch name in
t9903. This trick was performed via

$ (cd t &&
   sed -i -e 's/master/main/g' -e 's/MASTER/MAIN/g' \
-e 's/Master/Main/g' -- t99*.sh lib-cvs.sh &&
   git checkout HEAD -- t9902\*)

This allows us to define `GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main`
for all tests (except the ones we specifically excluded for now).

Signed-off-by: Johannes Schindelin <redacted>
Signed-off-by: Junio C Hamano <redacted>
5 years agotests(git-p4): transition to the default branch name `main`
Johannes Schindelin [Wed, 18 Nov 2020 23:44:44 +0000 (23:44 +0000)]
tests(git-p4): transition to the default branch name `main`

In the previous commits, we adjusted the test suite to use the branch
name `main` for initial branches.

The `git p4`-related tests are a bit harder to adjust because `git p4`
uses the ref `refs/heads/p4/master` to track the remote branches, and
for now, we do not want to change that (this might be the subject of a
future patch series). We only need to adjust for the actual initial
branch name to be changed to `main`.

This allows us to define `GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main`
for those tests.

Signed-off-by: Johannes Schindelin <redacted>
Signed-off-by: Junio C Hamano <redacted>
5 years agot9[5-7]*: adjust the references to the default branch name "main"
Johannes Schindelin [Wed, 18 Nov 2020 23:44:43 +0000 (23:44 +0000)]
t9[5-7]*: adjust the references to the default branch name "main"

This trick was performed via

$ (cd t &&
   sed -i -e 's/master/main/g' -e 's/MASTER/MAIN/g' \
-e 's/Master/Main/g' -- t9[5-7]*.sh lib-cvs.sh)

This allows us to define `GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main`
for those tests.

Signed-off-by: Johannes Schindelin <redacted>
Signed-off-by: Junio C Hamano <redacted>
5 years agot9[0-4]*: adjust the references to the default branch name "main"
Johannes Schindelin [Wed, 18 Nov 2020 23:44:42 +0000 (23:44 +0000)]
t9[0-4]*: adjust the references to the default branch name "main"

This trick was performed via

$ (cd t &&
   sed -i -e 's/master/main/g' -e 's/MASTER/MAIN/g' \
-e 's/Master/Main/g' -- t9[0-4]*.sh)

This allows us to define `GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main`
for those tests.

Signed-off-by: Johannes Schindelin <redacted>
Signed-off-by: Junio C Hamano <redacted>
5 years agot8*: adjust the references to the default branch name "main"
Johannes Schindelin [Wed, 18 Nov 2020 23:44:41 +0000 (23:44 +0000)]
t8*: adjust the references to the default branch name "main"

This trick was performed via

$ (cd t &&
   sed -i -e 's/master/main/g' -e 's/MASTER/MAIN/g' \
-e 's/Master/Main/g' -- t8*.sh annotate*.sh)

This allows us to define `GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main`
for those tests.

Signed-off-by: Johannes Schindelin <redacted>
Signed-off-by: Junio C Hamano <redacted>
5 years agot7[5-9]*: adjust the references to the default branch name "main"
Johannes Schindelin [Wed, 18 Nov 2020 23:44:40 +0000 (23:44 +0000)]
t7[5-9]*: adjust the references to the default branch name "main"

Excluding t7817, which is added in an unrelated patch series at the time
of writing, this adjusts t7[5-9]*. This trick was performed via

$ (cd t &&
   sed -i -e 's/master/main/g' -e 's/MASTER/MAIN/g' \
-e 's/Master/Main/g' -- t7[5-9]*.sh)

This allows us to define `GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main`
for those tests.

Signed-off-by: Johannes Schindelin <redacted>
Signed-off-by: Junio C Hamano <redacted>
5 years agot7[0-4]*: adjust the references to the default branch name "main"
Johannes Schindelin [Wed, 18 Nov 2020 23:44:39 +0000 (23:44 +0000)]
t7[0-4]*: adjust the references to the default branch name "main"

Carefully excluding t7064, which sees independent development elsewhere
at the time of writing, we use `main` as the default branch name in
t7[0-4]*. This trick was performed via

$ (cd t &&
   sed -i -e 's/master/main/g' -e 's/MASTER/MAIN/g' \
-e 's/Master/Main/g' -- t7[0-4]*.sh &&
   git checkout HEAD -- t7064\*)

This allows us to define `GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main`
for those tests.

Signed-off-by: Johannes Schindelin <redacted>
Signed-off-by: Junio C Hamano <redacted>
5 years agot6[4-9]*: adjust the references to the default branch name "main"
Johannes Schindelin [Wed, 18 Nov 2020 23:44:38 +0000 (23:44 +0000)]
t6[4-9]*: adjust the references to the default branch name "main"

This trick was performed via

$ (cd t &&
   sed -i -e 's/master/main/g' -e 's/MASTER/MAIN/g' \
-e 's/Master/Main/g' -- t6[4-9]*.sh)

This allows us to define `GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main`
for those tests.

Signed-off-by: Johannes Schindelin <redacted>
Signed-off-by: Junio C Hamano <redacted>
5 years agot64*: preemptively adjust alignment to prepare for `master` -> `main`
Johannes Schindelin [Wed, 18 Nov 2020 23:44:37 +0000 (23:44 +0000)]
t64*: preemptively adjust alignment to prepare for `master` -> `main`

We are in the process of renaming the default branch name to `main`,
which is two characters shorter than `master`. Therefore, some lines
need to be adjusted in t6416, t6422 and t6427 that want to align text
involving the default branch name.

Signed-off-by: Johannes Schindelin <redacted>
Signed-off-by: Junio C Hamano <redacted>
5 years agot6[0-3]*: adjust the references to the default branch name "main"
Johannes Schindelin [Wed, 18 Nov 2020 23:44:36 +0000 (23:44 +0000)]
t6[0-3]*: adjust the references to the default branch name "main"

Carefully excluding t6300, which sees independent development elsewhere
at the time of writing, we use `main` as the default branch name in
t6[0-3]*. This trick was performed via

$ (cd t &&
   sed -i -e 's/master/main/g' -e 's/MASTER/MAIN/g' \
-e 's/Master/Main/g' -- t6[0-3]*.sh &&
   git checkout HEAD -- t6300\*)

This allows us to define `GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main`
for those tests.

Signed-off-by: Johannes Schindelin <redacted>
Signed-off-by: Junio C Hamano <redacted>
5 years agot5[6-9]*: adjust the references to the default branch name "main"
Johannes Schindelin [Wed, 18 Nov 2020 23:44:35 +0000 (23:44 +0000)]
t5[6-9]*: adjust the references to the default branch name "main"

This trick was performed via

$ (cd t &&
   sed -i -e 's/master/main/g' -e 's/MASTER/MAIN/g' \
-e 's/Master/Main/g' -- t5[6-9]*.sh)

This allows us to define `GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main`
for those tests.

Signed-off-by: Johannes Schindelin <redacted>
Signed-off-by: Junio C Hamano <redacted>
5 years agot55[4-9]*: adjust the references to the default branch name "main"
Johannes Schindelin [Wed, 18 Nov 2020 23:44:34 +0000 (23:44 +0000)]
t55[4-9]*: adjust the references to the default branch name "main"

This trick was performed via

$ (cd t &&
   sed -i -e 's/master/main/g' -e 's/MASTER/MAIN/g' \
-e 's/Master/Main/g' -e 's/retsam/niam/g' \
-- t55[4-9]*.sh t556x*)

This allows us to define `GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main`
for those tests.

Note that t5541 uses the reversed `master` name: `retsam`. We replace it
by the equivalent for `main`: `niam`.

Signed-off-by: Johannes Schindelin <redacted>
Signed-off-by: Junio C Hamano <redacted>
5 years agot55[23]*: adjust the references to the default branch name "main"
Johannes Schindelin [Wed, 18 Nov 2020 23:44:33 +0000 (23:44 +0000)]
t55[23]*: adjust the references to the default branch name "main"

Carefully excluding t5526, which sees independent development elsewhere
at the time of writing, we use `main` as the default branch name in
t55[23]*. This trick was performed via

$ (cd t &&
   sed -i -e 's/master/main/g' -e 's/MASTER/MAIN/g' \
-e 's/Master/Main/g' -e 's/naster/nain/g' -- \
t55[23]*.sh &&
   git checkout HEAD -- t5526\*)

Note that t5533 contains a variation of the name `master` (`naster`)
that we rename here, too.

This commit allows us to define
`GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main` for that range of tests.

Signed-off-by: Johannes Schindelin <redacted>
Signed-off-by: Junio C Hamano <redacted>
5 years agot551*: adjust the references to the default branch name "main"
Johannes Schindelin [Wed, 18 Nov 2020 23:44:32 +0000 (23:44 +0000)]
t551*: adjust the references to the default branch name "main"

This trick was performed via

$ (cd t &&
   sed -i -e 's/master/main/g' -e 's/MASTER/MAIN/g' \
-e 's/Master/Main/g' -- t551*.sh)

This allows us to define `GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main`
for those tests.

Signed-off-by: Johannes Schindelin <redacted>
Signed-off-by: Junio C Hamano <redacted>
5 years agot550*: adjust the references to the default branch name "main"
Johannes Schindelin [Wed, 18 Nov 2020 23:44:31 +0000 (23:44 +0000)]
t550*: adjust the references to the default branch name "main"

This trick was performed via

$ (cd t &&
   sed -i -e 's/master/main/g' -e 's/MASTER/MAIN/g' \
-e 's/Master/Main/g' -- t550*.sh)

This allows us to define `GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main`
for those tests.

Signed-off-by: Johannes Schindelin <redacted>
Signed-off-by: Junio C Hamano <redacted>
5 years agot5503: prepare aligned comment for replacing `master` with `main`
Johannes Schindelin [Wed, 18 Nov 2020 23:44:30 +0000 (23:44 +0000)]
t5503: prepare aligned comment for replacing `master` with `main`

In an upcoming commit, we will use `main` as the default branch name in
t5503 instead of `master`. This will require extra padding in ASCII-art
commit graphs, which we hereby add preemptively.

By doing this preemptively rather than after the commit applying the
search-and-replace, it is more obvious that we caught all aligned
comments that are affected by the latter commit.

Signed-off-by: Johannes Schindelin <redacted>
Signed-off-by: Junio C Hamano <redacted>
5 years agot5[0-4]*: adjust the references to the default branch name "main"
Johannes Schindelin [Wed, 18 Nov 2020 23:44:29 +0000 (23:44 +0000)]
t5[0-4]*: adjust the references to the default branch name "main"

Carefully excluding t5310, which is developed independently of the
current patch series at the time of writing, we now use `main` as
default branch in t5[0-4]*. This trick was performed via

$ (cd t &&
   sed -i -e 's/master/main/g' -e 's/MASTER/MAIN/g' \
-e 's/Master/Main/g' -- t5[0-4]*.sh &&
   git checkout HEAD -- t5310\*)

This allows us to define `GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main`
for those tests.

Signed-off-by: Johannes Schindelin <redacted>
Signed-off-by: Junio C Hamano <redacted>
5 years agot5323: prepare centered comment for `master` -> `main`
Johannes Schindelin [Wed, 18 Nov 2020 23:44:28 +0000 (23:44 +0000)]
t5323: prepare centered comment for `master` -> `main`

We are about to search-and-replace all mentions of `master` in t5323 by
`main`, which is two characters shorter. To prepare for that, let's add
padding to centered lines that will make them briefly uncentered, but
will be re-centered in the commit that performs that rename.

Doing it this way (instead of padding after replacing) makes it easier
to verify the validity of the patch that replaces `master` by `main`.

Signed-off-by: Johannes Schindelin <redacted>
Signed-off-by: Junio C Hamano <redacted>
5 years agot4*: adjust the references to the default branch name "main"
Johannes Schindelin [Wed, 18 Nov 2020 23:44:27 +0000 (23:44 +0000)]
t4*: adjust the references to the default branch name "main"

Carefully excluding t4013 and t4015, which see independent development
elsewhere at the time of writing, we use `main` as the default branch
name in t4*. This trick was performed via

$ (cd t &&
   sed -i -e 's/master/main/g' -e 's/MASTER/MAIN/g' \
-e 's/Master/Main/g' -- t4*.sh t4211/*.export &&
   git checkout HEAD -- t4013\*)

This allows us to define `GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main`
for those tests.

Signed-off-by: Johannes Schindelin <redacted>
Signed-off-by: Junio C Hamano <redacted>
5 years agot3[5-9]*: adjust the references to the default branch name "main"
Johannes Schindelin [Wed, 18 Nov 2020 23:44:26 +0000 (23:44 +0000)]
t3[5-9]*: adjust the references to the default branch name "main"

This trick was performed via

$ (cd t &&
   sed -i -e 's/master/main/g' -e 's/MASTER/MAIN/g' \
-e 's/Master/Main/g' -- t3[5-9]*.sh)

This allows us to define `GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main`
for those tests.

Signed-off-by: Johannes Schindelin <redacted>
Signed-off-by: Junio C Hamano <redacted>
5 years agot34*: adjust the references to the default branch name "main"
Johannes Schindelin [Wed, 18 Nov 2020 23:44:25 +0000 (23:44 +0000)]
t34*: adjust the references to the default branch name "main"

Carefully excluding t3404, which sees independent development elsewhere
at the time of writing, we use `main` as the default branch name in
t34*. This trick was performed via

$ (cd t &&
   sed -i -e 's/master/main/g' -e 's/MASTER/MAIN/g' \
-e 's/Master/Main/g' -- t34*.sh &&
   git checkout HEAD -- t34\*)

This allows us to define `GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main`
for those tests.

Signed-off-by: Johannes Schindelin <redacted>
Signed-off-by: Junio C Hamano <redacted>
5 years agot3416: preemptively adjust alignment in a comment
Johannes Schindelin [Wed, 18 Nov 2020 23:44:24 +0000 (23:44 +0000)]
t3416: preemptively adjust alignment in a comment

We are about to adjust t3416 for the new default branch name `main`.
This name is two characters shorter and therefore needs two spaces more
padding to align correctly.

Adjusting the alignment before the big search-and-replace makes it
easier to verify that the final result does not leave any misaligned
lines behind.

Signed-off-by: Johannes Schindelin <redacted>
Signed-off-by: Junio C Hamano <redacted>
5 years agot3[0-3]*: adjust the references to the default branch name "main"
Johannes Schindelin [Wed, 18 Nov 2020 23:44:23 +0000 (23:44 +0000)]
t3[0-3]*: adjust the references to the default branch name "main"

Carefully excluding t3040, which sees independent development elsewhere
at the time of writing, we transition above-mentioned tests to the
default branch name `main`. This trick was performed via

$ (cd t &&
   sed -i -e 's/master/main/g' -e 's/MASTER/MAIN/g' \
-e 's/Master/Main/g' -- t3[0-3]*.sh t3206/* &&
   git checkout HEAD -- t3040\*)

This allows us to define `GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main`
for those tests.

Signed-off-by: Johannes Schindelin <redacted>
Signed-off-by: Junio C Hamano <redacted>
5 years agot2*: adjust the references to the default branch name "main"
Johannes Schindelin [Wed, 18 Nov 2020 23:44:22 +0000 (23:44 +0000)]
t2*: adjust the references to the default branch name "main"

Carefully excluding t2106, which sees independent development elsewhere
at the time of writing, we transition above-mentioned tests to the
default branch name `main`. This trick was performed via

$ (cd t &&
   sed -i -e 's/master/main/g' -e 's/MASTER/MAIN/g' \
-e 's/Master/Main/g' -- t2*.sh &&
   git checkout HEAD -- t2106\*)

This allows us to define `GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main`
for those tests.

Signed-off-by: Johannes Schindelin <redacted>
Signed-off-by: Junio C Hamano <redacted>
5 years agot[01]*: adjust the references to the default branch name "main"
Johannes Schindelin [Wed, 18 Nov 2020 23:44:21 +0000 (23:44 +0000)]
t[01]*: adjust the references to the default branch name "main"

Carefully excluding t1309, which sees independent development elsewhere
at the time of writing, we transition above-mentioned tests to the
default branch name `main`. This trick was performed via

$ (cd t &&
   sed -i -e 's/master/main/g' -e 's/MASTER/MAIN/g' \
-e 's/Master/Main/g' -e 's/naster/nain/g' -- t[01]*.sh &&
   git checkout HEAD -- t1309\*)

Note that t5533 contains a variation of the name `master` (`naster`)
that we rename here, too.

This allows us to define `GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main`
for those tests.

Helped-by: Ævar Arnfjörð Bjarmason <redacted>
Signed-off-by: Johannes Schindelin <redacted>
Signed-off-by: Junio C Hamano <redacted>
5 years agot0060: preemptively adjust alignment
Johannes Schindelin [Wed, 18 Nov 2020 23:44:20 +0000 (23:44 +0000)]
t0060: preemptively adjust alignment

We are about to adjust t0060 for the new default branch name `main`.
This name is two characters shorter and therefore needs two spaces more
padding to align correctly.

Adjusting the alignment before the big search-and-replace makes it
easier to verify that the final result does not leave any misaligned
lines behind.

Signed-off-by: Johannes Schindelin <redacted>
Signed-off-by: Junio C Hamano <redacted>
5 years agotests: mark tests relying on the current default for `init.defaultBranch`
Johannes Schindelin [Wed, 18 Nov 2020 23:44:19 +0000 (23:44 +0000)]
tests: mark tests relying on the current default for `init.defaultBranch`

In addition to the manual adjustment to let the `linux-gcc` CI job run
the test suite with `master` and then with `main`, this patch makes sure
that GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME is set in all test scripts
that currently rely on the initial branch name being `master by default.

To determine which test scripts to mark up, the first step was to
force-set the default branch name to `master` in

- all test scripts that contain the keyword `master`,

- t4211, which expects `t/t4211/history.export` with a hard-coded ref to
  initialize the default branch,

- t5560 because it sources `t/t556x_common` which uses `master`,

- t8002 and t8012 because both source `t/annotate-tests.sh` which also
  uses `master`)

This trick was performed by this command:

$ sed -i '/^ *\. \.\/\(test-lib\|lib-\(bash\|cvs\|git-svn\)\|gitweb-lib\)\.sh$/i\
GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=master\
export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME\
' $(git grep -l master t/t[0-9]*.sh) \
t/t4211*.sh t/t5560*.sh t/t8002*.sh t/t8012*.sh

After that, careful, manual inspection revealed that some of the test
scripts containing the needle `master` do not actually rely on a
specific default branch name: either they mention `master` only in a
comment, or they initialize that branch specificially, or they do not
actually refer to the current default branch. Therefore, the
aforementioned modification was undone in those test scripts thusly:

$ git checkout HEAD -- \
t/t0027-auto-crlf.sh t/t0060-path-utils.sh \
t/t1011-read-tree-sparse-checkout.sh \
t/t1305-config-include.sh t/t1309-early-config.sh \
t/t1402-check-ref-format.sh t/t1450-fsck.sh \
t/t2024-checkout-dwim.sh \
t/t2106-update-index-assume-unchanged.sh \
t/t3040-subprojects-basic.sh t/t3301-notes.sh \
t/t3308-notes-merge.sh t/t3423-rebase-reword.sh \
t/t3436-rebase-more-options.sh \
t/t4015-diff-whitespace.sh t/t4257-am-interactive.sh \
t/t5323-pack-redundant.sh t/t5401-update-hooks.sh \
t/t5511-refspec.sh t/t5526-fetch-submodules.sh \
t/t5529-push-errors.sh t/t5530-upload-pack-error.sh \
t/t5548-push-porcelain.sh \
t/t5552-skipping-fetch-negotiator.sh \
t/t5572-pull-submodule.sh t/t5608-clone-2gb.sh \
t/t5614-clone-submodules-shallow.sh \
t/t7508-status.sh t/t7606-merge-custom.sh \
t/t9302-fast-import-unpack-limit.sh

We excluded one set of test scripts in these commands, though: the range
of `git p4` tests. The reason? `git p4` stores the (foreign) remote
branch in the branch called `p4/master`, which is obviously not the
default branch. Manual analysis revealed that only five of these tests
actually require a specific default branch name to pass; They were
modified thusly:

$ sed -i '/^ *\. \.\/lib-git-p4\.sh$/i\
GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=master\
export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME\
' t/t980[0167]*.sh t/t9811*.sh

Signed-off-by: Johannes Schindelin <redacted>
Signed-off-by: Junio C Hamano <redacted>
5 years agoMerge 'jk/diff-release-filespec-fix' into js/default-branch-name-tests-final-stretch
Junio C Hamano [Thu, 19 Nov 2020 23:27:59 +0000 (15:27 -0800)]
Merge 'jk/diff-release-filespec-fix' into js/default-branch-name-tests-final-stretch

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

5 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 <redacted>
Signed-off-by: Johannes Schindelin <redacted>
Signed-off-by: Junio C Hamano <redacted>
5 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 <redacted>
Signed-off-by: Junio C Hamano <redacted>
5 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 <redacted>
Signed-off-by: Junio C Hamano <redacted>
5 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 <redacted>
Signed-off-by: Junio C Hamano <redacted>
5 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 <redacted>
5 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

5 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

5 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'

5 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

5 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

5 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

5 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

5 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

5 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

5 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

5 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

5 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

5 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

5 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)

5 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

5 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 <redacted>
Signed-off-by: Junio C Hamano <redacted>
5 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 <redacted>
Signed-off-by: Junio C Hamano <redacted>
5 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 <redacted>
Signed-off-by: Junio C Hamano <redacted>
5 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 <redacted>
Signed-off-by: Junio C Hamano <redacted>
5 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 <redacted>
Signed-off-by: Junio C Hamano <redacted>
5 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 <redacted>
Signed-off-by: Junio C Hamano <redacted>
5 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 <redacted>
Signed-off-by: Taylor Blau <redacted>
Signed-off-by: Junio C Hamano <redacted>
5 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 <redacted>
Signed-off-by: Junio C Hamano <redacted>
5 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 <redacted>
Signed-off-by: Felipe Contreras <redacted>
Signed-off-by: Junio C Hamano <redacted>
5 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 <redacted>
Signed-off-by: Philippe Blain <redacted>
Signed-off-by: Junio C Hamano <redacted>
5 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 <redacted>
Signed-off-by: Junio C Hamano <redacted>
5 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 <redacted>
Signed-off-by: Junio C Hamano <redacted>
5 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 <redacted>
Signed-off-by: Junio C Hamano <redacted>
5 years agoadd -i: verify in the tests that colors can be overridden
Johannes Schindelin [Mon, 16 Nov 2020 16:08:32 +0000 (16:08 +0000)]
add -i: verify in the tests that colors can be overridden

Now that the Perl version produces the same output as the built-in
version (mostly fixing bugs in the latter), let's add a regression test
to verify that it stays this way.

Note that we only `grep` for the colored error message instead of
verifying that the entire `stderr` consists of just this one line: when
running the test script using the `-x` option to trace the
commands, the sub-shell in `force_color` causes those commands to be
traced into `err.raw` (unless running in Bash where we set the
`BASH_XTRACEFD` variable to avoid that).

Also note that the color reset in the `<BLUE>+<RESET><BLUE>new<RESET>`
line might look funny and unnecessary, as the corresponding `old` line
does not reset the color after the diff marker only to turn the color
back on right away.

However, this is a (necessary) side effect of the white-space check: in
`emit_line_ws_markup()`, we first emit the diff marker via
`emit_line_0()` and then the rest of the line via `ws_check_emit()`. To
leave them somewhat decoupled, the color has to be reset after the diff
marker to allow for the rest of the line to start with another color (or
inverted, in case of white-space issues).

Finally, we have to simulate hunk editing: the `git add -p` command
cannot rely on the internal diff machinery for coloring after letting
the user edit a hunk; It has to "re-color" the edited hunk. This is the
primary reason why that command is interested in the exact values of the
`color.diff.*` settings in the first place. To test this re-coloring, we
therefore have to pretend to edit a hunk and then show that hunk in the
regression test.

Co-authored-by: Jeff King <redacted>
Signed-off-by: Johannes Schindelin <redacted>
Signed-off-by: Junio C Hamano <redacted>
5 years agoadd -p: prefer color.diff.context over color.diff.plain
Johannes Schindelin [Mon, 16 Nov 2020 16:08:31 +0000 (16:08 +0000)]
add -p: prefer color.diff.context over color.diff.plain

Git's diff machinery allows users to override the colors to use in
diffs, even the plain-colored context lines. As of 8dbf3eb6850 (diff.h:
rename DIFF_PLAIN color slot to DIFF_CONTEXT, 2015-05-27), the preferred
name of the config setting is `color.diff.context`, although Git still
allows `color.diff.plain`.

In the context of `git add -p`, this logic is a bit hard to replicate:
`git_diff_basic_config()` reads all config values sequentially and if it
sees _any_ `color.diff.context` or `color.diff.plain`, it accepts the
new color. The Perl version of `git add -p` needs to go through `git
config --get-color`, though, which allows only one key to be specified.
The same goes for the built-in version of `git add -p`, which has to go
through `repo_config_get_value()`.

The best we can do here is to look for `.context` and if none is found,
fall back to looking for `.plain`, and if still not found, fall back to
the hard-coded default (which in this case is simply the empty string,
as context lines are typically rendered without colored).

This still leads to inconsistencies when both config names are used: the
initial diff will be colored by the diff machinery. Once edited by a
user, a hunk has to be re-colored by `git add -p`, though, which would
then use the other setting to color the context lines.

In practice, this is not _all_ that bad. The `git config` manual says
this in the `color.diff.<slot>`:

`context` (context text - `plain` is a historical synonym)

We should therefore assume that users use either one or the other, but
not both names. Besides, it is relatively uncommon to look at a hunk
after editing it because it is immediately staged by default.

Signed-off-by: Johannes Schindelin <redacted>
Signed-off-by: Junio C Hamano <redacted>
5 years agoadd -i (Perl version): color header to match the C version
Johannes Schindelin [Mon, 16 Nov 2020 16:08:30 +0000 (16:08 +0000)]
add -i (Perl version): color header to match the C version

Both versions of `add -i` indent non-flat lists by five spaces. However
when using color the C version prints these spaces after the ANSI color
codes whereas the Perl version prints them before the color codes.
Change the Perl version to match the C version to allow for introducing
a test that verifies that both versions produce the exact same output.

Signed-off-by: Johannes Schindelin <redacted>
Signed-off-by: Junio C Hamano <redacted>
5 years agoadd -i (built-in): use the same indentation as the Perl version
Johannes Schindelin [Mon, 16 Nov 2020 16:08:29 +0000 (16:08 +0000)]
add -i (built-in): use the same indentation as the Perl version

When copying the spaces used to indent non-flat lists in `git add -i`,
one space was appended by mistake. This makes the output of the built-in
version of `git add -i` inconsistent with the Perl version. Let's adjust
the built-in version to produce the same output as the Perl version.

Signed-off-by: Johannes Schindelin <redacted>
Signed-off-by: Junio C Hamano <redacted>
5 years agoadd -p (built-in): do not color the progress indicator separately
Johannes Schindelin [Mon, 16 Nov 2020 16:08:28 +0000 (16:08 +0000)]
add -p (built-in): do not color the progress indicator separately

The Perl version of this command colors the progress indicator and the
prompt message in one go.

Let's do the same in the built-in version so that the same upcoming test
(which will compare the output of `git add -p` against a known-good
version) will pass both for the Perl version as well as for the built-in
version.

Signed-off-by: Johannes Schindelin <redacted>
Signed-off-by: Junio C Hamano <redacted>
5 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 <redacted>
Signed-off-by: Jeff King <redacted>
Signed-off-by: Taylor Blau <redacted>
Signed-off-by: Junio C Hamano <redacted>
git clone https://git.99rst.org/PROJECT