apply: don't use core.sharedRepository to create working tree files

core.sharedRepository defines which permissions Git should set when
creating files in $GIT_DIR, so that the repository may be shared with
other users. But (in its current form) the setting shouldn't affect how
files are created in the working tree. This is not respected by apply
and am (which uses apply), when creating leading directories:

$ cat d.patch
 diff --git a/d/f b/d/f
 new file mode 100644
 index 0000000..e69de29

Apply without the setting:
$ umask 0077
$ git apply d.patch
$ ls -ld d
 drwx------

Apply with the setting:
$ umask 0077
$ git -c core.sharedRepository=0770 apply d.patch
$ ls -ld d
 drwxrws---

Only the leading directories are affected. That's because they are
created with safe_create_leading_directories(), which calls
adjust_shared_perm() to set the directories' permissions based on
core.sharedRepository. To fix that, let's introduce a variant of this
function that ignores the setting, and use it in apply. Also add a
regression test and a note in the function documentation about the use
of each variant according to the destination (working tree or git
dir).

Signed-off-by: Matheus Tavares <matheus.bernardino@usp.br>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

banned.h: mark ctime_r() and asctime_r() as banned

The ctime_r() and asctime_r() functions are reentrant, but have
no check that the buffer we pass in is long enough (the manpage says it
"should have room for at least 26 bytes"). Since this is such an
easy-to-get-wrong interface, and since we have the much safer strftime()
as well as its more convenient strbuf_addftime() wrapper, let's ban both
of those.

Signed-off-by: Jeff King <peff@peff.net>
Reviewed-by: Taylor Blau <me@ttaylorr.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

gitignore: remove entry for git serve

b7ce24d095 (Turn `git serve` into a test helper, 2019-04-18) demoted git
serve from a builtin command to a test helper.  As a result the
git-serve binary is no longer built and thus doesn't have to be ignored
anymore.

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

gitignore: drop duplicate entry for git-sh-i18n

This was accidentally added by e00cf070a4 (git-sh-i18n.sh: add no-op
gettext() and eval_gettext() wrappers, 2011-05-14), even though an
earlier commit in the same series had already done so.

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

t7900: speed up expensive test

A test marked with EXPENSIVE creates two 2.5GB files and adds them to
the repository. This takes 194s to run on my machine, versus 2s when the
EXPENSIVE prereq isn't set. We can trim this down a bit by doing two
things:

  - use "git commit --quiet" to avoid spending time generating a diff
    summary (this actually only helps for the second commit, but I've
    added it here to both for consistency). This shaves off 8s.

  - set core.compression to 0. We know these files are full of random
    bytes, and so won't compress (that's the point of the test!).
    Spending cycles on zlib is pointless. This shaves off 122s.

After this, my total time to run the script is 64s. That won't help
normal runs without GIT_TEST_LONG set, of course, but it's easy enough
to do.

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

banned.h: mark non-reentrant gmtime, etc as banned

The traditional gmtime(), localtime(), ctime(), and asctime() functions
return pointers to shared storage. This means they're not thread-safe,
and they also run the risk of somebody holding onto the result across
multiple calls (where each call invalidates the previous result).

All callers should be using their reentrant counterparts.

Signed-off-by: Jeff King <peff@peff.net>
Reviewed-by: Taylor Blau <me@ttaylorr.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

config.txt: fix a typo (backslash != backquote)

Signed-off-by: Štěpán Němec <stepnem@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

builtin/bugreport.c: use thread-safe localtime_r()

To generate its filename, the 'git bugreport' builtin asks the system
for the current time with 'localtime()'. Since this uses a shared
buffer, it is not thread-safe.

Even though 'git bugreport' is not multi-threaded, using localtime() can
trigger some static analysis tools to complain, and a quick

    $ git grep -oh 'localtime\(_.\)\?' -- **/*.c | sort | uniq -c

shows that the only usage of the thread-unsafe 'localtime' is in a piece
of documentation.

So, convert this instance to use the thread-safe version for
consistency, and to appease some analysis tools.

Signed-off-by: Taylor Blau <me@ttaylorr.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

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>

Merge branch 'da/askpass-mask-checkbox'

Add a checkbox in the SSH askpass helper to optionally show the input
text which is often a password.

* da/askpass-mask-checkbox:
  git-gui: ssh-askpass: add a checkbox to show the input text

git-gui: ssh-askpass: add a checkbox to show the input text

Hide the input text by default since the field is
commonly used for sensative informations such as passwords.

Add a "Show input" checkbox to conditionally show the input.

Helped-by: Miguel Boekhold <miguel.boekhold@osudio.com>
Signed-off-by: Efimov Vasily <laer.18@gmail.com>
Signed-off-by: David Aguilar <davvid@gmail.com>
Signed-off-by: Pratyush Yadav <me@yadavpratyush.com>

Merge branch 'dr/russian-translation'

Update Russian translation.

* dr/russian-translation:
  git-gui: update Russian translation

git-gui: update Russian translation

Translation is done on Transifex: https://www.transifex.com/djm00n/git-po-ru/git-gui/
If you have any corrections please report them there.

Signed-off-by: Dimitriy Ryazantcev <dimitriy.ryazantcev@gmail.com>
Signed-off-by: Pratyush Yadav <me@yadavpratyush.com>

imap-send: parse default git config

git imap-send does not parse the default git config settings and thus ignore
core.askpass value.
Rewrite config parsing to support core settings.

Reported-by: Philippe Blain <levraiphilippeblain@gmail.com>
Signed-off-by: Nicolas Morey-Chaisemartin <nmoreychaisemartin@suse.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

Merge branch 'ms/commit-template'

Teach git-gui to read the commit message template and pre-populate it in
the commit message buffer.

* ms/commit-template:
  git-gui: use commit message template
  git-gui: Only touch GITGUI_MSG when needed

completion: zsh: fix file completion regression

Turns out we always need to set the ignored prefix (compset) to have
similar behavior as in default Bash.

The issue can be seen with:

  git show master:<tab>

Commit 94b2901cfe wrongly removed it.

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

tests: lib-functions: trivial style cleanups

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

test: completion: fix typos

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

.gitignore: remove dangling file

The library was removed 7 years ago on commit ae34ac126f. But not from
the .gitignore file.

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

refspec: trivial cleanup

We can remove one level of indentation and make the code clearer.

No functional changes.

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

Ninth batch

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

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

Merge branch 'km/stash-error-message-fix'

Error message fix.

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

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

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

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

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

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

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

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

Merge branch 'js/pull-rebase-use-advise'

UI improvement.

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

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

Merge branch 'js/t3040-cleanup'

Cleanup.

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

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

fetch-pack: disregard invalid pack lockfiles

9da69a6539 (fetch-pack: support more than one pack lockfile, 2020-06-10)
started to use a string_list for pack lockfile names instead of a single
string pointer.  It removed a NULL check from transport_unlock_pack() as
well, which is the function that eventually deletes these lockfiles and
releases their name strings.

index_pack_lockfile() can return NULL if it doesn't like the contents it
reads from the file descriptor passed to it.  unlink(2) is declared to
not accept NULL pointers (at least with glibc).  Undefined Behavior
Sanitizer together with Address Sanitizer detects a case where a NULL
lockfile name is passed to unlink(2) by transport_unlock_pack() in t1060
(make SANITIZE=address,undefined; cd t; ./t1060-object-corruption.sh).

Reinstate the NULL check to avoid undefined behavior, but put it right
at the source, so that the number of items in the string_list reflects
the number of valid lockfiles.

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

refspec: make @ a synonym of HEAD

Since commit 9ba89f484e git learned how to push to a remote branch using
the source @, for example:

  git push origin @:master

However, if the right-hand side is missing, the push fails:

  git push origin @

It is obvious what is the desired behavior, and allowing the push makes
things more consistent.

Additionally, @:master now has the same semantics as HEAD:master.

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

tests: push: trivial cleanup

No need to do two checkouts.

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

tests: push: improve cleanup of HEAD tests

So that we are not left in an inconsistent state between them.

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

MyFirstObjectWalk: drop `init_walken_defaults()`

In a recent commit, we stopped calling `init_grep_defaults()` from this
function. Thus, by the end of the tutorial, we still haven't added any
contents to this function. Let's remove it for simplicity.

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

grep: copy struct in one fell swoop

We have a `struct grep_opt` with our defaults which we then copy into
the caller's struct. Rather than zeroing the target struct and copying
each element one by one, just copy everything at once. This leaves the
code simpler and more maintainable.

We don't have any ownership issues with what we're copying now and can
just greedily copy the whole thing. If and when we do need to handle
such elements (`char *`?), we must and can handle it appropriately. Make
sure to leave a comment to our future selves.

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

perf/fsmonitor: use test_must_be_empty helper

Simplify test and make error messages more clear here.
Per feedback from Junio in
33226af42b (t/perf/fsmonitor: improve error message if typoing hook
name, 2020-10-26)

Signed-off-by: Nipunn Koorapati <nipunn@dropbox.com>
Acked-by: Derrick Stolee <dstolee@microsoft.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

maintenance: fix SEGFAULT when no repository

The "git maintenance run" and "git maintenance start/stop" commands
holds a file-based lock at the .git/maintenance.lock and
.git/schedule.lock respectively. These locks are used to ensure only
one maintenance process is executed at the time as both operations
involves writing data into the git repository.

The path to the lock file is built using
"the_repository->objects->odb->path" that results in SEGFAULT when we
have no repository available as "the_repository->objects->odb" is
set to NULL.

Let's teach maintenance command to use RUN_SETUP option that will
provide the validation and fail when running outside of a repository.
Hence fixing the SEGFAULT for all three operations and making the
behaviour consistent across all subcommands.

Setting the RUN_SETUP also provides the same protection for all
subcommands given that the "register" and "unregister" also requires to
be executed inside a repository.

Furthermore let's remove the local validation implemented by the
"register" and "unregister" as this will not be required anymore with
the new option.

Signed-off-by: Rafael Silva <rafaeloliveira.cs@gmail.com>
Reviewed-by: Derrick Stolee <dstolee@microsoft.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

git-gui: use commit message template

Use the file described by commit.template (if set) to show the commit message
template, just like other GUIs.

Signed-off-by: Martin Schön <Martin.Schoen@loewensteinmedical.de>
Signed-off-by: Pratyush Yadav <me@yadavpratyush.com>

git-gui: Only touch GITGUI_MSG when needed

In 4e55d19 (git-gui: Cleanup end-of-line whitespace in commit messages.,
2007-01-25), the logic to decide if GITGUI_MSG should be saved or
deleted was updated to not require the commit message buffer to be
modified. This fixes a situation where if the user quits and restarts
git-gui multiple times the commit message buffer was lost.

Unfortunately, the fix was not quite correct. The check for whether the
commit message buffer has been modified is useless. If the commit is
_not_ amend, then the check is never performed. If the commit is amend,
then saving the message does not matter anyway. Amend state is destroyed
on exit and the next time git-gui is opened it starts from scratch, but
with the older message retained in the buffer. If amend is selected,
the current message is over-written by the amend commit's message.

The correct fix would be to not touch GITGUI_MSG at all if the commit
message buffer is not modified. This way, the file is not deleted even
on multiple restarts. It has the added benefit of not writing the file
unnecessarily on every exit.

Signed-off-by: Pratyush Yadav <me@yadavpratyush.com>

Eighth batch

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

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

Merge branch 'rs/plug-diff-cache-leak'

Memleak fix.

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

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

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

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

Merge branch 'rs/archive-plug-leak-refname'

Memleak fix.

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

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

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

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

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

Merge branch 'rs/hashwrite-be64'

Code simplification.

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

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

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

maintenance: use 'git config --fixed-value'

When a repository's leading directories contain regex metacharacters,
the config calls for 'git maintenance register' and 'git maintenance
unregister' are not careful enough. Use the new --fixed-value option
to direct the config machinery to use exact string matches. This is a
more robust option than escaping these arguments in a piecemeal fashion.

For the test, require that we are not running on Windows since the '+'
and '*' characters are not allowed on that filesystem.

Reported-by: Emily Shaffer <emilyshaffer@google.com>
Reported-by: Jonathan Nieder <jrnieder@gmail.com>
Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

Merge branch 'ds/config-literal-value' into ds/maintenance-part-3

* ds/config-literal-value:
  config doc: value-pattern is not necessarily a regexp
  config: implement --fixed-value with --get*
  config: plumb --fixed-value into config API
  config: add --fixed-value option, un-implemented
  t1300: add test for --replace-all with value-pattern
  t1300: test "set all" mode with value-pattern
  config: replace 'value_regex' with 'value_pattern'
  config: convert multi_replace to flags

config doc: value-pattern is not necessarily a regexp

The introductory part of the "git config --help" mentions the
optional value-pattern argument, but give no hint that it can be
something other than a regular expression (worse, it just says
"POSIX regexp", which usually means BRE but the regexp the command
takes is ERE).  Also, it needs to be documented that the '!' prefix
to negate the match, which is only mentioned in this part of the
document, works only with regexp and not with the --fixed-value.

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

config: implement --fixed-value with --get*

The config builtin does its own regex matching of values for the --get,
--get-all, and --get-regexp modes. Plumb the existing 'flags' parameter
to the get_value() method so we can initialize the value-pattern argument
as a fixed string instead of a regex pattern.

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

config: plumb --fixed-value into config API

The git_config_set_multivar_in_file_gently() and related methods now
take a 'flags' bitfield, so add a new bit representing the --fixed-value
option from 'git config'. This alters the purpose of the value_pattern
parameter to be an exact string match. This requires some initialization
changes in git_config_set_multivar_in_file_gently() and a new strcmp()
call in the matches() method.

The new CONFIG_FLAGS_FIXED_VALUE flag is initialized in builtin/config.c
based on the --fixed-value option, and that needs to be updated in
several callers.

This patch only affects some of the modes of 'git config', and the rest
will be completed in the next change.

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

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 <dstolee@microsoft.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

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 <dstolee@microsoft.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

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 <dstolee@microsoft.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

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 <dstolee@microsoft.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

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 <dstolee@microsoft.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

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 <peff@peff.net>
Signed-off-by: Taylor Blau <me@ttaylorr.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

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 <peff@peff.net>
Signed-off-by: Taylor Blau <me@ttaylorr.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

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 <sir@cmpwn.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

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>

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>

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 <jonathantanmy@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

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>

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>

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>

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>

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>

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>

Seventh batch

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

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

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

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

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

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

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

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 <martin.agren@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

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 <martin.agren@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

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>

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>

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>

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>

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>

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>

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>

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>

Sixth batch

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

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

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