test-ref-store: tweaks to for-each-reflog-ent format
We have some tests that read from files in .git/logs/ hierarchy
when checking if correct reflog entries are created, but that is
too specific to the files backend. Other backends like reftable
may not store its reflog entries in such a "one line per entry"
format.
Update for-each-reflog-ent test helper to produce output that
is identical to lines in a reflog file files backend uses.
That way, (1) the current tests can be updated to use the test
helper to read the reflog entries instead of (parts of) reflog
files, and perform the same inspection for correctness, and (2)
when the ref backend is swapped to another backend, the updated
test can be used as-is to check the correctness.
Adapt t1400 to use the for-each-reflog-ent test helper.
Signed-off-by: Han-Wen Nienhuys <hanwen@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Before, --reflog option would look for '\t' in the reflog message. As refs.c
already parses the reflog line, the '\t' was never found, and show-branch
--reflog would always say "(none)" as reflog message
Add test.
Signed-off-by: Han-Wen Nienhuys <hanwen@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Jeff King [Thu, 2 Dec 2021 05:37:53 +0000 (00:37 -0500)]
log: load decorations with --simplify-by-decoration
It's possible to specify --simplify-by-decoration but not --decorate. In
this case we do respect the simplification, but we don't actually show
any decorations. However, it works by lazy-loading the decorations when
needed; this is discussed in more detail in 0cc7380d88 (log-tree: call
load_ref_decorations() in get_name_decoration(), 2019-09-08).
This works for basic cases, but will fail to respect any --decorate-refs
option (or its variants). Those are handled only when cmd_log_init()
loads the ref decorations up front, which is only when --decorate is
specified explicitly (or as of the previous commit, when the userformat
asks for %d or similar).
We can solve this by making sure to load the decorations if we're going
to simplify using them but they're not otherwise going to be displayed.
The new test shows a simple case that fails without this patch. Note
that we expect two commits in the output: the one we asked for by
--decorate-refs, and the initial commit. The latter is just a quirk of
how --simplify-by-decoration works. Arguably it may be a bug, but it's
unrelated to this patch (which is just about the loading of the
decorations; you get the same behavior before this patch with an
explicit --decorate).
Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Jeff King [Thu, 2 Dec 2021 05:35:43 +0000 (00:35 -0500)]
log: handle --decorate-refs with userformat "%d"
In order to show ref decorations, we first have to load them. If you
run:
git log --decorate
then git-log will recognize the option and load them up front via
cmd_log_init(). Likewise if log.decorate is set.
If you don't say --decorate explicitly, but do mention "%d" or "%D" in
the output format, like so:
git log --format=%d
then this also works, because we lazy-load the ref decorations. This has
been true since 3b3d443feb (add '%d' pretty format specifier to show
decoration, 2008-09-04), though the lazy-load was later moved into
log-tree.c.
But there's one problem: that lazy-load just uses the defaults; it
doesn't take into account any --decorate-refs options (or its exclude
variant, or their config). So this does not work:
git log --decorate-refs=whatever --format=%d
It will decorate using all refs, not just the specified ones. This has
been true since --decorate-refs was added in 65516f586b (log: add option
to choose which refs to decorate, 2017-11-21). Adding further confusion
is that it _may_ work because of the auto-decoration feature. If that's
in use (and it often is, as it's the default), then if the output is
going to stdout, we do enable decorations early (and so load them up
front, respecting the extra options). But otherwise we do not. So:
would typically behave differently than it does when the output goes to
the pager or terminal!
The solution is simple: we should recognize in cmd_log_init() that we're
going to show decorations, and make sure we load them there. We already
check userformat_find_requirements(), so we can couple this with our
existing code there.
There are two new tests. The first shows off the actual fix. The second
makes sure that our fix doesn't cause us to stomp on an existing
--decorate option (see the new comment in the code, as well).
Reported-by: Josh Rampersad <josh.rampersad@voiceflow.com> Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Anders Kaseorg [Wed, 1 Dec 2021 22:15:46 +0000 (14:15 -0800)]
receive-pack: protect current branch for bare repository worktree
A bare repository won’t have a working tree at "..", but it may still
have separate working trees created with git worktree. We should protect
the current branch of such working trees from being updated or deleted,
according to receive.denyCurrentBranch.
Signed-off-by: Anders Kaseorg <andersk@mit.edu> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Anders Kaseorg [Wed, 1 Dec 2021 22:15:45 +0000 (14:15 -0800)]
receive-pack: clean dead code from update_worktree()
update_worktree() can only be called with a non-NULL worktree parameter,
because that’s the only case where we set do_update_worktree = 1.
worktree->path is always initialized to non-NULL.
Signed-off-by: Anders Kaseorg <andersk@mit.edu> Signed-off-by: Junio C Hamano <gitster@pobox.com>
As a side effect of using find_shared_symref, we’ll also refuse the
fetch when we’re on a detached HEAD because we’re rebasing or bisecting
on the branch in question. This seems like a sensible change.
Signed-off-by: Anders Kaseorg <andersk@mit.edu> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Anders Kaseorg [Wed, 1 Dec 2021 22:15:43 +0000 (14:15 -0800)]
worktree: simplify find_shared_symref() memory ownership model
Storing the worktrees list in a static variable meant that
find_shared_symref() had to rebuild the list on each call (which is
inefficient when the call site is in a loop), and also that each call
invalidated the pointer returned by the previous call (which is
confusing).
Instead, make it the caller’s responsibility to pass in the worktrees
list and manage its lifetime.
Signed-off-by: Anders Kaseorg <andersk@mit.edu> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Anders Kaseorg [Wed, 1 Dec 2021 22:15:42 +0000 (14:15 -0800)]
branch: lowercase error messages
Documentation/CodingGuidelines says “do not end error messages with a
full stop” and “do not capitalize the first word”. Clean up existing
messages, some of which we will be touching in later steps in the
series, that deviate from these rules in this file, as a preparation for
the main part of the topic.
Signed-off-by: Anders Kaseorg <andersk@mit.edu> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Anders Kaseorg [Wed, 1 Dec 2021 22:15:41 +0000 (14:15 -0800)]
receive-pack: lowercase error messages
Documentation/CodingGuidelines says “do not end error messages with a
full stop” and “do not capitalize the first word”. Clean up existing
messages, some of which we will be touching in later steps in the
series, that deviate from these rules in this file, as a preparation for
the main part of the topic.
Signed-off-by: Anders Kaseorg <andersk@mit.edu> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Anders Kaseorg [Wed, 1 Dec 2021 22:15:40 +0000 (14:15 -0800)]
fetch: lowercase error messages
Documentation/CodingGuidelines says “do not end error messages with a
full stop” and “do not capitalize the first word”. Clean up existing
messages, some of which we will be touching in later steps in the
series, that deviate from these rules in this file, as a preparation for
the main part of the topic.
Signed-off-by: Anders Kaseorg <andersk@mit.edu> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Fabian Stelzer [Wed, 1 Dec 2021 08:53:15 +0000 (09:53 +0100)]
test-lib: make BAIL_OUT() work in tests and prereq
BAIL_OUT() is meant to abort the whole test run and print a message with
a standard prefix that can be parsed to stdout. Since for every test the
normal fd`s are redirected in test_eval_ this output would not be seen
when used within the context of a test or prereq like we do in
test_have_prereq(). To make this function work in these contexts we move
the setup of the fd aliases a few lines up before the first use of
BAIL_OUT() and then have this function always print to the alias.
Signed-off-by: Fabian Stelzer <fs@gigacodes.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
git-compat-util: add a test balloon for C99 support
The C99 standard was released in January 1999, now 22 years ago. It
provides a variety of useful features, including variadic arguments for
macros, declarations after statements, designated initializers, and a
wide variety of other useful features, many of which we already use.
We'd like to take advantage of these features, but we want to be
cautious. As far as we know, all major compilers now support C99 or a
later C standard, such as C11 or C17. POSIX has required C99 support as
a requirement for the 2001 revision, so we can safely assume any POSIX
system which we are interested in supporting has C99.
Even MSVC, long a holdout against modern C, now supports both C11 and
C17 with an appropriate update. Moreover, even if people are using an
older version of MSVC on these systems, they will generally need some
implementation of the standard Unix utilities for the testsuite, and GNU
coreutils, the most common option, has required C99 since 2009.
Therefore, we can safely assume that a suitable version of GCC or clang
is available to users even if their version of MSVC is not sufficiently
capable.
Let's add a test balloon to git-compat-util.h to see if anyone is using
an older compiler. We'll add a comment telling people how to enable
this functionality on GCC and Clang, even though modern versions of both
will automatically do the right thing, and ask people still experiencing
a problem to report that to us on the list.
Note that C89 compilers don't provide the __STDC_VERSION__ macro, so we
use a well-known hack of using "- 0". On compilers with this macro, it
doesn't change the value, and on C89 compilers, the macro will be
replaced with nothing, and our value will be 0.
For sparse, we explicitly request the gnu99 style because we've
traditionally taken advantage of some GCC- and clang-specific extensions
when available and we'd like to retain the ability to do that. sparse
also defaults to C89 without it, so things will fail for us if we don't.
Update the cmake configuration to require C11 for MSVC. We do this
because this will make MSVC to use C11, since it does not explicitly
support C99. We do this with a compiler options because setting the
C_STANDARD option does not work in our CI on MSVC and at the moment, we
don't want to require C11 for Unix compilers.
In the Makefile, don't set any compiler flags for the compiler itself,
since on some systems, such as FreeBSD, we actually need C11, and asking
for C99 causes things to fail to compile. The error message should make
it obvious what's going wrong and allow a user to set the appropriate
option when building in the event they're using a Unix compiler that
doesn't support it by default.
Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Phillip Wood [Wed, 1 Dec 2021 00:05:06 +0000 (00:05 +0000)]
xdiff: implement a zealous diff3, or "zdiff3"
"zdiff3" is identical to ordinary diff3 except that it allows compaction
of common lines on the two sides of history at the beginning or end of
the conflict hunk. For example, the following diff3 conflict:
1
2
3
4
<<<<<<
A
B
C
D
E
||||||
5
6
======
A
X
C
Y
E
>>>>>>
7
8
9
has common lines 'A', 'C', and 'E' on the two sides. With zdiff3, one
would instead get the following conflict:
1
2
3
4
A
<<<<<<
B
C
D
||||||
5
6
======
X
C
Y
>>>>>>
E
7
8
9
Note that the common lines, 'A', and 'E' were moved outside the
conflict. Unlike with the two-way conflicts from the 'merge'
conflictStyle, the zdiff3 conflict is NOT split into multiple conflict
regions to allow the common 'C' lines to be shown outside a conflict,
because zdiff3 shows the base version too and the base version cannot be
reasonably split.
Note also that the removing of lines common to the two sides might make
the remaining text inside the conflict region match the base text inside
the conflict region (for example, if the diff3 conflict had '5 6 E' on
the right side of the conflict, then the common line 'E' would be moved
outside and both the base and right side's remaining conflict text would
be the lines '5' and '6'). This has the potential to surprise users and
make them think there should not have been a conflict, but there
definitely was a conflict and it should remain.
In 9a5315edfdf (Merge branch 'js/patch-mode-in-others-in-c',
2020-02-05), Git acquired a built-in implementation of `git add`'s
interactive mode that could be turned on via the config option
`add.interactive.useBuiltin`.
The first official Git version to support this knob was v2.26.0.
In 2df2d81ddd0 (add -i: use the built-in version when
feature.experimental is set, 2020-09-08), this built-in implementation
was also enabled via `feature.experimental`. The first version with this
change was v2.29.0.
More than a year (and very few bug reports) later, it is time to declare
the built-in implementation mature and to turn it on by default.
We specifically leave the `add.interactive.useBuiltin` configuration in
place, to give users an "escape hatch" in the unexpected case should
they encounter a previously undetected bug in that implementation.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
t2016: require the PERL prereq only when necessary
The scripted version of the interactive mode of `git add` still requires
Perl, but the built-in version does not. Let's only require the PERL
prereq if testing the scripted version.
This addresses a long-standing NEEDSWORK added in 35166b1fb54 (t2016:
add a NEEDSWORK about the PERL prerequisite, 2020-10-07).
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Elijah Newren [Tue, 30 Nov 2021 03:58:39 +0000 (03:58 +0000)]
sequencer: avoid adding exec commands for non-commit creating commands
The `--exec <cmd>` is documented as
Append "exec <cmd>" after each line creating a commit in the final
history.
...
If --autosquash is used, "exec" lines will not be appended for the
intermediate commits, and will only appear at the end of each
squash/fixup series.
Unfortunately, it would also add exec commands after non-pick
operations, such as 'no-op', which could be seen for example with
git rebase -i --exec true HEAD
todo_list_add_exec_commands() intent was to insert exec commands after
each logical pick, while trying to consider a chains of fixup and squash
commits to be part of the pick before it. So it would keep an 'insert'
boolean tracking if it had seen a pick or merge, but not write the exec
command until it saw the next non-fixup/squash command. Since that
would make it miss the final exec command, it had some code that would
check whether it still needed to insert one at the end, but instead of a
simple
if (insert)
it had a
if (insert || <condition that is always true>)
That's buggy; as per the docs, we should only add exec commands for
lines that create commits, i.e. only if insert is true. Fix the
conditional.
There was one testcase in the testsuite that we tweak for this change;
it was introduced in 54fd3243da ("rebase -i: reread the todo list if
`exec` touched it", 2017-04-26), and was merely testing that after an
exec had fired that the todo list would be re-read. The test at the
time would have worked given any revision at all, though it would only
work with 'HEAD' as a side-effect of this bug. Since we're fixing this
bug, choose something other than 'HEAD' for that test.
Finally, add a testcase that verifies when we have no commits to pick,
that we get no exec lines in the generated todo list.
Reported-by: Nikita Bobko <nikitabobko@gmail.com> Signed-off-by: Elijah Newren <newren@gmail.com> Acked-by: Taylor Blau <me@ttaylorr.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Junio C Hamano [Mon, 29 Nov 2021 23:41:51 +0000 (15:41 -0800)]
Merge branch 'mc/clean-smudge-with-llp64'
The clean/smudge conversion code path has been prepared to better
work on platforms where ulong is narrower than size_t.
* mc/clean-smudge-with-llp64:
clean/smudge: allow clean filters to process extremely large files
odb: guard against data loss checking out a huge file
git-compat-util: introduce more size_t helpers
odb: teach read_blob_entry to use size_t
t1051: introduce a smudge filter test for extremely large files
test-lib: add prerequisite for 64-bit platforms
test-tool genzeros: generate large amounts of data more efficiently
test-genzeros: allow more than 2G zeros in Windows
Junio C Hamano [Mon, 29 Nov 2021 23:41:48 +0000 (15:41 -0800)]
Merge branch 'jc/unsetenv-returns-an-int'
The compatibility implementation for unsetenv(3) were written to
mimic ancient, non-POSIX, variant seen in an old glibc; it has been
changed to return an integer to match the more modern era.
* jc/unsetenv-returns-an-int:
unsetenv(3) returns int, not void
Junio C Hamano [Mon, 29 Nov 2021 23:41:47 +0000 (15:41 -0800)]
Merge branch 'jc/fix-ref-sorting-parse'
Things like "git -c branch.sort=bogus branch new HEAD", i.e. the
operation modes of the "git branch" command that do not need the
sort key information, no longer errors out by seeing a bogus sort
key.
* jc/fix-ref-sorting-parse:
for-each-ref: delay parsing of --sort=<atom> options
Junio C Hamano [Mon, 29 Nov 2021 23:41:45 +0000 (15:41 -0800)]
Merge branch 'ab/refs-errno-cleanup'
The "remainder" of hn/refs-errno-cleanup topic.
* ab/refs-errno-cleanup: (21 commits)
refs API: post-migration API renaming [2/2]
refs API: post-migration API renaming [1/2]
refs API: don't expose "errno" in run_transaction_hook()
refs API: make expand_ref() & repo_dwim_log() not set errno
refs API: make resolve_ref_unsafe() not set errno
refs API: make refs_ref_exists() not set errno
refs API: make refs_resolve_refdup() not set errno
refs tests: ignore ignore errno in test-ref-store helper
refs API: ignore errno in worktree.c's find_shared_symref()
refs API: ignore errno in worktree.c's add_head_info()
refs API: make files_copy_or_rename_ref() et al not set errno
refs API: make loose_fill_ref_dir() not set errno
refs API: make resolve_gitlink_ref() not set errno
refs API: remove refs_read_ref_full() wrapper
refs/files: remove "name exist?" check in lock_ref_oid_basic()
reflog tests: add --updateref tests
refs API: make refs_rename_ref_available() static
refs API: make parse_loose_ref_contents() not set errno
refs API: make refs_read_raw_ref() not set errno
refs API: add a version of refs_resolve_ref_unsafe() with "errno"
...
Allow "git status --porcelain=v2" to show the number of stash
entries with --show-stash like the normal output does.
* ow/stash-count-in-status-porcelain-output:
status: print stash info with --porcelain=v2 --show-stash
status: count stash entries in separate function
Han-Wen Nienhuys [Mon, 29 Nov 2021 18:20:22 +0000 (18:20 +0000)]
t1404: mark directory/file conflict tests with REFFILES
The files backend uses file system locking on individual refs, which means a
directory/file conflict can prevent locks being taken. For example, in a repo
with just the ref "foo", an update
(DELETE "foo") + (ADD "foo/bar")
cannot be executed in the files backend, as one cannot take a lock on foo/bar.
The current reftable proof-of-concept integration supports these tranactions, as
the result is a repo with just "foo/bar", which has no directory/file conflict.
Signed-off-by: Han-Wen Nienhuys <hanwen@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Junio C Hamano [Mon, 29 Nov 2021 20:53:56 +0000 (12:53 -0800)]
Merge branch 'vd/sparse-reset' into ld/sparse-diff-blame
* vd/sparse-reset:
unpack-trees: improve performance of next_cache_entry
reset: make --mixed sparse-aware
reset: make sparse-aware (except --mixed)
reset: integrate with sparse index
reset: expand test coverage for sparse checkouts
sparse-index: update command for expand/collapse test
reset: preserve skip-worktree bit in mixed reset
reset: rename is_missing to !is_in_reset_tree
Victoria Dye [Mon, 29 Nov 2021 15:52:43 +0000 (15:52 +0000)]
unpack-trees: improve performance of next_cache_entry
To find the first non-unpacked cache entry, `next_cache_entry` iterates
through index, starting at `cache_bottom`. The performance of this in full
indexes is helped by `cache_bottom` advancing with each invocation of
`mark_ce_used` (called by `unpack_index_entry`). However, the presence of
sparse directories can prevent the `cache_bottom` from advancing in a sparse
index case, effectively forcing `next_cache_entry` to search from the
beginning of the index each time it is called.
The `cache_bottom` must be preserved for the sparse index (see 17a1bb570b
(unpack-trees: preserve cache_bottom, 2021-07-14)). Therefore, to retain the
benefit `cache_bottom` provides in non-sparse index cases, a separate `hint`
position indicates the first position `next_cache_entry` should search,
updated each execution with a new position.
Signed-off-by: Victoria Dye <vdye@github.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Victoria Dye [Mon, 29 Nov 2021 15:52:42 +0000 (15:52 +0000)]
reset: make --mixed sparse-aware
Remove the `ensure_full_index` guard on `read_from_tree` and update `git
reset --mixed` to ensure it can use sparse directory index entries wherever
possible. Sparse directory entries are reset using `diff_tree_oid`, which
requires `change` and `add_remove` functions to process the internal
contents of the sparse directory. The `recursive` diff option handles cases
in which `reset --mixed` must diff/merge files that are nested multiple
levels deep in a sparse directory.
The use of pathspecs with `git reset --mixed` introduces scenarios in which
internal contents of sparse directories may be matched by the pathspec. In
order to reset *all* files in the repo that may match the pathspec, the
following conditions on the pathspec require index expansion before
performing the reset:
* "magic" pathspecs
* wildcard pathspecs that do not match only in-cone files or entire sparse
directories
* literal pathspecs matching something outside the sparse checkout
definition
Helped-by: Elijah Newren <newren@gmail.com> Signed-off-by: Victoria Dye <vdye@github.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Victoria Dye [Mon, 29 Nov 2021 15:52:41 +0000 (15:52 +0000)]
reset: make sparse-aware (except --mixed)
Remove `ensure_full_index` guard on `prime_cache_tree` and update
`prime_cache_tree_rec` to correctly reconstruct sparse directory entries in
the cache tree. While processing a tree's entries, `prime_cache_tree_rec`
must determine whether a directory entry is sparse or not by searching for
it in the index (*without* expanding the index). If a matching sparse
directory index entry is found, no subtrees are added to the cache tree
entry and the entry count is set to 1 (representing the sparse directory
itself). Otherwise, the tree is assumed to not be sparse and its subtrees
are recursively added to the cache tree.
Helped-by: Elijah Newren <newren@gmail.com> Signed-off-by: Victoria Dye <vdye@github.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Victoria Dye [Mon, 29 Nov 2021 15:52:40 +0000 (15:52 +0000)]
reset: integrate with sparse index
Disable `command_requires_full_index` repo setting and add
`ensure_full_index` guards around code paths that cannot yet use sparse
directory index entries. `reset --soft` does not modify the index, so no
compatibility changes are needed for it to function without expanding the
index. For all other reset modes (`--mixed`, `--hard`, `--keep`, `--merge`),
the full index is expanded to prevent cache tree corruption and invalid
variable accesses.
Additionally, the `read_cache()` check verifying an uncorrupted index is
moved after argument parsing and preparing the repo settings. The index is
not used by the preceding argument handling, but `read_cache()` must be run
*after* enabling sparse index for the command (so that the index is not
expanded unnecessarily) and *before* using the index for reset (so that it
is verified as uncorrupted).
Signed-off-by: Victoria Dye <vdye@github.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Victoria Dye [Mon, 29 Nov 2021 15:52:39 +0000 (15:52 +0000)]
reset: expand test coverage for sparse checkouts
Add new tests for `--merge` and `--keep` modes, as well as mixed reset with
pathspecs. New performance test cases exercise various execution paths for
`reset`.
Co-authored-by: Derrick Stolee <dstolee@microsoft.com> Signed-off-by: Derrick Stolee <dstolee@microsoft.com> Signed-off-by: Victoria Dye <vdye@github.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Derrick Stolee [Mon, 29 Nov 2021 13:47:46 +0000 (13:47 +0000)]
t/t*: remove custom GIT_TRACE2_EVENT_NESTING
The previous change modified GIT_TRACE2_EVENT_NESTING by default within
test-lib.sh. These custom assignments throughout the test suite are no
longer necessary.
Signed-off-by: Derrick Stolee <dstolee@microsoft.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Derrick Stolee [Mon, 29 Nov 2021 13:47:45 +0000 (13:47 +0000)]
test-lib.sh: set GIT_TRACE2_EVENT_NESTING
The GIT_TRACE2_EVENT feed has a limited nesting depth to avoid
overloading the feed when recursing into deep paths while adding more
nested regions.
Some tests use the GIT_TRACE2_EVENT feed to look for internal events,
ensuring that intended behavior is happening.
One such example is in t4216-log-bloom.sh which looks for a statistic
given as a trace2_data_intmax() call. This test started failing under
'-x' with 2ca245f8be5 (csum-file.h: increase hashfile buffer size,
2021-05-18) because the change in stderr triggered the progress API to
create an extra trace2 region, ejecting the statistic.
This change increases the value of GIT_TRACE2_EVENT_NESTING across the
entire test suite to avoid errors like this. Future changes will remove
custom assignments of GIT_TRACE2_EVENT_NESTING from some test scripts
that were aware of this limitation.
Reported-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Helped-by: Jeff King <peff@peff.net> Signed-off-by: Derrick Stolee <dstolee@microsoft.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
add -p: avoid use of undefined $key when ReadKey -> EOF
b5cc003253 (add -i: ignore terminal escape sequences, 2011-05-17)
add an additional check to the original code to better handle keys
for escape sequences, but failed to account for the possibility
the first ReadKey call returned undef (ex: stdin closes) and that
was being handled fine by the original code in ca6ac7f135 (add -p:
prompt for single characters, 2009-02-05)
Add a test for undefined and encapsulate the loop and the original
print that relied on it within it.
After this, the following command (in a suitable repository state)
wouldn't print any error:
$ git -c interactive.singleKey add -p </dev/null
Signed-off-by: Carlo Marcelo Arenas Belón <carenas@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Junio C Hamano [Sun, 28 Nov 2021 19:25:35 +0000 (11:25 -0800)]
refs: document callback for reflog-ent iterators
refs_for_each_reflog_ent() and refs_for_each_reflog_ent_reverse()
functions take a callback function that gets called with the details
of each reflog entry. Its parameters were not documented beyond
their names. Elaborate a bit on each of them.
Helped-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
mingw-w64's pthread_unistd.h had a bug that mistakenly (because there is
no support for the *lockfile() functions required[1]) defined
_POSIX_THREAD_SAFE_FUNCTIONS and that was being worked around since 3ecd153a3b (compat/mingw: support MSys2-based MinGW build, 2016-01-14).
The bug was fixed in winphtreads, but as a side effect, leaves the
reentrant functions from time.h no longer visible and therefore breaks
the build.
Since the intention all along was to avoid using the fallback functions,
formalize the use of POSIX by setting the corresponding feature flag and
compile out the implementation for the fallback functions.
[1] https://unix.org/whitepapers/reentrant.html
Signed-off-by: Carlo Marcelo Arenas Belón <carenas@gmail.com> Acked-by: Johannes Schindelin <Johannes.Schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
run-command API: remove "env" member, always use "env_array"
Remove the "env" member from "struct child_process" in favor of always
using the "env_array". As with the preceding removal of "argv" in
favor of "args" this gets rid of current and future oddities around
memory management at the API boundary (see the amended API docs).
For some of the conversions we can replace patterns like:
child.env = env->v;
With:
strvec_pushv(&child.env_array, env->v);
But for others we need to guard the strvec_pushv() with a NULL check,
since we're not passing in the "v" member of a "struct strvec",
e.g. in the case of tmp_objdir_env()'s return value.
Ideally we'd rename the "env_array" member to simply "env" as a
follow-up, since it and "args" are now inconsistent in not having an
"_array" suffix, and seemingly without any good reason, unless we look
at the history of how they came to be.
But as we've currently got 122 in-tree hits for a "git grep env_array"
let's leave that for now (and possibly forever). Doing that rename
would be too disruptive.
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
difftool: use "env_array" to simplify memory management
Amend code added in 03831ef7b50 (difftool: implement the functionality
in the builtin, 2017-01-19) to use the "env_array" in the
run_command.[ch] API. Now we no longer need to manage our own
"index_env" buffer.
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
run-command API: remove "argv" member, always use "args"
Remove the "argv" member from the run-command API, ever since "args"
was added in c460c0ecdca (run-command: store an optional argv_array,
2014-05-15) being able to provide either "argv" or "args" has led to
some confusion and bugs.
If we hadn't gone in that direction and only had an "argv" our
problems wouldn't have been solved either, as noted in [1] (and in the
documentation amended here) it comes with inherent memory management
issues: The caller would have to hang on to the "argv" until the
run-command API was finished. If the "argv" was an argument to main()
this wasn't an issue, but if it it was manually constructed using the
API might be painful.
We also have a recent report[2] of a user of the API segfaulting,
which is a direct result of it being complex to use. This commit
addresses the root cause of that bug.
This change is larger than I'd like, but there's no easy way to avoid
it that wouldn't involve even more verbose intermediate steps. We use
the "argv" as the source of truth over the "args", so we need to
change all parts of run-command.[ch] itself, as well as the trace2
logging at the same time.
The resulting Windows-specific code in start_command() is a bit nasty,
as we're now assigning to a strvec's "v" member, instead of to our own
"argv". There was a suggestion of some alternate approaches in reply
to an earlier version of this commit[3], but let's leave larger a
larger and needless refactoring of this code for now.
run-command API users: use strvec_push(), not argv construction
Change a pattern of hardcoding an "argv" array size, populating it and
assigning to the "argv" member of "struct child_process" to instead
use "strvec_push()" to add data to the "args" member.
As noted in the preceding commit this moves us further towards being
able to remove the "argv" member in a subsequent commit
These callers could have used strvec_pushl(), but moving to
strvec_push() makes the diff easier to read, and keeps the arguments
aligned as before.
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
run-command API users: use strvec_pushl(), not argv construction
Change a pattern of hardcoding an "argv" array size, populating it and
assigning to the "argv" member of "struct child_process" to instead
use "strvec_pushl()" to add data to the "args" member.
This implements the same behavior as before in fewer lines of code,
and moves us further towards being able to remove the "argv" member in
a subsequent commit.
Since we've entirely removed the "argv" variable(s) we can be sure
that no potential logic errors of the type discussed in a preceding
commit are being introduced here, i.e. ones where the local "argv" was
being modified after the assignment to "struct child_process"'s
"argv".
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
run-command tests: use strvec_pushv(), not argv assignment
As in the preceding commit change this API user to use strvec_pushv()
instead of assigning to the "argv" member directly. This leaves us
without test coverage of how the "argv" assignment in this API works,
but we'll be removing it in a subsequent commit.
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
run-command API users: use strvec_pushv(), not argv assignment
Migrate those run-command API users that assign directly to the "argv"
member to use a strvec_pushv() of "args" instead.
In these cases it did not make sense to further refactor these
callers, e.g. daemon.c could be made to construct the arguments closer
to handle(), but that would require moving the construction from its
cmd_main() and pass "argv" through two intermediate functions.
It would be possible for a change like this to introduce a regression
if we were doing:
cp.argv = argv;
argv[1] = "foo";
And changed the code, as is being done here, to:
strvec_pushv(&cp.args, argv);
argv[1] = "foo";
But as viewing this change with the "-W" flag reveals none of these
functions modify variable that's being pushed afterwards in a way that
would introduce such a logic error.
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
upload-archive: use regular "struct child_process" pattern
This pattern added [1] in seems to have been intentional, but since
[2] and [3] we've wanted do initialization of what's now the "struct
strvec" "args" and "env_array" members. Let's not trample on that
initialization here.
1. 1bc01efed17 (upload-archive: use start_command instead of fork,
2011-11-19)
2. c460c0ecdca (run-command: store an optional argv_array, 2014-05-15)
3. 9a583dc39e (run-command: add env_array, an optional argv_array for
env, 2014-10-19)
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Eric Sunshine [Thu, 25 Nov 2021 22:52:16 +0000 (23:52 +0100)]
worktree: stop being overly intimate with run_command() internals
add_worktree() reuses a `child_process` for three run_command()
invocations, but to do so, it has overly-intimate knowledge of
run-command.c internals. In particular, it knows that it must reset
child_process::argv to NULL for each subsequent invocation[*] in order
for start_command() to latch the newly-populated child_process::args for
each invocation, even though this behavior is not a part of the
documented API. Beyond having overly-intimate knowledge of run-command.c
internals, the reuse of one `child_process` for three run_command()
invocations smells like an unnecessary micro-optimization. Therefore,
stop sharing one `child_process` and instead use a new one for each
run_command() call.
[*] If child_process::argv is not reset to NULL, then subsequent
run_command() invocations will instead incorrectly access a dangling
pointer to freed memory which had been allocated by child_process::args
on the previous run. This is due to the following code in
start_command():
if (!cmd->argv)
cmd->argv = cmd->args.v;
Signed-off-by: Eric Sunshine <sunshine@sunshineco.com> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Yoichi Nakayama <yoichi.nakayama@gmail.com> Acked-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Victoria Dye [Tue, 23 Nov 2021 00:20:33 +0000 (00:20 +0000)]
sparse-index: update do_read_index to ensure correct sparsity
Unless `command_requires_full_index` forces index expansion, ensure in-core
index sparsity matches config settings on read by calling
`ensure_correct_sparsity`. This makes the behavior of the in-core index more
consistent between different methods of updating sparsity: manually changing
the `index.sparse` config setting vs. executing
`git sparse-checkout --[no-]sparse-index init`
Although index sparsity is normally updated with `git sparse-checkout init`,
ensuring correct sparsity after a manual `index.sparse` change has some
practical benefits:
1. It allows for command-by-command sparsity toggling with
`-c index.sparse=<true|false>`, e.g. when troubleshooting issues with the
sparse index.
2. It prevents users from experiencing abnormal slowness after setting
`index.sparse` to `true` due to use of a full index in all commands until
the on-disk index is updated.
Helped-by: Junio C Hamano <gitster@pobox.com> Co-authored-by: Derrick Stolee <dstolee@microsoft.com> Signed-off-by: Victoria Dye <vdye@github.com> Reviewed-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Victoria Dye [Tue, 23 Nov 2021 00:20:32 +0000 (00:20 +0000)]
sparse-index: add ensure_correct_sparsity function
The `ensure_correct_sparsity` function is intended to provide a means of
aligning the in-core index with the sparsity required by the repository
settings and other properties of the index. The function first checks
whether a sparse index is allowed (per repository & sparse checkout pattern
settings). If the sparse index may be used, the index is converted to
sparse; otherwise, it is explicitly expanded with `ensure_full_index`.
Helped-by: Junio C Hamano <gitster@pobox.com> Signed-off-by: Victoria Dye <vdye@github.com> Reviewed-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Victoria Dye [Tue, 23 Nov 2021 00:20:31 +0000 (00:20 +0000)]
sparse-index: avoid unnecessary cache tree clearing
When converting a full index to sparse, clear and recreate the cache tree
only if the cache tree is not fully valid. The convert_to_sparse operation
should exit silently if a cache tree update cannot be successfully completed
(e.g., due to a conflicted entry state). However, because this failure
scenario only occurs when at least a portion of the cache tree is invalid,
we can save ourselves the cost of clearing and recreating the cache tree by
skipping the check when the cache tree is fully valid.
Helped-by: Derrick Stolee <dstolee@microsoft.com> Co-authored-by: Junio C Hamano <gitster@pobox.com> Signed-off-by: Victoria Dye <vdye@github.com> Reviewed-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Victoria Dye [Tue, 23 Nov 2021 00:20:30 +0000 (00:20 +0000)]
test-read-cache.c: prepare_repo_settings after config init
Move `prepare_repo_settings` after the git directory has been set up in
`test-read-cache.c`. The git directory settings must be initialized to
properly assign repo settings using the worktree-level git config.
Signed-off-by: Victoria Dye <vdye@github.com> Reviewed-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Enzo Matsumiya [Thu, 25 Nov 2021 00:02:39 +0000 (21:02 -0300)]
pager: fix crash when pager program doesn't exist
When prepare_cmd() fails for, e.g., pager process setup,
child_process_clear() frees the memory in pager_process.args, but .argv
was pointed to pager_process.args.v earlier in start_command(), so it's
now a dangling pointer.
setup_pager() is then called a second time, from cmd_log_init_finish()
in this case, and any further operations using its .argv, e.g. strvec_*,
will use the dangling pointer and eventually crash. According to trivial
tests, setup_pager() is not called twice if the first call is
successful.
This patch makes sure that pager_process is properly initialized on
setup_pager(). Drop CHILD_PROCESS_INIT from its declaration since it's
no longer really necessary.
Add a test to catch possible regressions.
Reproducer:
$ git config pager.show INVALID_PAGER
$ git show $VALID_COMMIT
error: cannot run INVALID_PAGER: No such file or directory
[1] 3619 segmentation fault (core dumped) git show $VALID_COMMIT
Signed-off-by: Enzo Matsumiya <ematsumiya@suse.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
The "linux-clang" and "linux-gcc" jobs both run "make test" twice, but
with different environment variables. Running these in sequence seems
to have been done to work around some constraint on Travis, see ae59a4e44f3 (travis: run tests with GIT_TEST_SPLIT_INDEX, 2018-01-07).
By having these run in parallel we'll get jobs that finish much sooner
than they otherwise would have.
We can also simplify the control flow in "ci/run-build-and-tests.sh"
as a result, since we won't run "make test" twice we don't need to run
"make" twice at all, let's default to "make all test" after setting
the variables, and then override it to just "all" for the compile-only
tests.
Add a comment to clarify that new "test" targets should adjust
$MAKE_TARGETS rather than being added after the "case/esac". This
should avoid future confusion where e.g. the compilation-only
"pedantic" target will unexpectedly start running tests. See [1] and
[2].
CI: use "$runs_on_pool", not "$jobname" to select packages & config
Change the setup hooks for the CI to use "$runs_on_pool" for the
"$regular" job. Now we won't need as much boilerplate when adding new
jobs to the "regular" matrix, see 956d2e4639b (tests: add a test mode
for SANITIZE=leak, run it in CI, 2021-09-23) for the last such commit.
I.e. now instead of needing to enumerate each jobname when we select
packages we can install things depending on the pool we're running
in.
That we didn't do this dates back to the now gone dependency on Travis
CI, but even if we add a new CI target in the future this'll be easier
to port over, since we can probably treat "ubuntu-latest" as a
stand-in for some recent Linux that can run "apt" commands.
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
CI: rename the "Linux32" job to lower-case "linux32"
As a follow-up to the preceding commit's shortening of CI job names,
rename the only job that starts with an upper-case letter to be
consistent with the rest. It was added in 88dedd5e72c (Travis: also
test on 32-bit Linux, 2017-03-05).
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Change the names used for the GitHub CI workflows to be short enough
to (mostly) fit in the pop-up tool-tips that GitHub shows in the
commit view. I.e. when mouse-clicking on the passing or failing
check-mark next to the commit subject.
These names are seemingly truncated to 17-20 characters followed by
three dots ("..."). Since a "CI/PR / " prefix is added to them the job
names looked like this before (windows-test and vs-test jobs omitted):
By omitting the "/PR" from the top-level name, and pushing the
$jobname to the front we'll now instead get:
CI / config (push)
CI / win build (push...
CI / win+VS build (...
CI / linux-clang (ub...
CI / linux-gcc (ubun...
CI / osx-clang (osx)...
CI / osx-gcc (osx) (...
CI / linux-gcc-defau...
CI / linux-leaks (ub...
CI / linux-musl (alp...
CI / Linux32 (daald/...
CI / pedantic (fedor...
CI / static-analysis...
CI / sparse (push)...
CI / documentation
We then have no truncation in the expanded view. See [1] for how it
looked before, [2] for a currently visible CI run using this commit,
and [3] for the GitHub workflow syntax involved being changed here.
Let's also use the existing "pool" field as before. It's occasionally
useful to know we're running on say ubuntu v.s. fedora. The "-latest"
suffix is useful to some[4], and since it's now at the end it doesn't
hurt readability in the short view compared to saying "ubuntu" or
"macos".
Remove support for running the CI in travis. The last builds in it are
from 5 months ago[1] (as of 2021-11-19), and our documentation has
referred to GitHub CI instead since f003a91f5c5 (SubmittingPatches:
replace discussion of Travis with GitHub Actions, 2021-07-22).
We'll now run the "t9810 t9816" and tests on OSX. We didn't before, as
we'd carried the Travis exclusion of them forward from 522354d70f4 (Add Travis CI support, 2015-11-27). Let's hope whatever
issue there was with them was either Travis specific, or fixed since
then (I'm not sure).
The "apt-add-repository" invocation (which we were doing in GitHub CI)
isn't needed, it was another Travis-only case that was carried forward
into more general code. See 0f0c51181df (travis-ci: install packages
in 'ci/install-dependencies.sh', 2018-11-01).
Remove the "linux-gcc-4.8" job added in fb9d7431cf4 (travis-ci: build
with GCC 4.8 as well, 2019-07-18), it only ran in Travis CI.
1. https://travis-ci.org/github/git/git/builds
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Junio C Hamano [Tue, 23 Nov 2021 22:48:00 +0000 (14:48 -0800)]
Merge branch 'hm/paint-hits-in-log-grep' into maint
"git grep" looking in a blob that has non-UTF8 payload was
completely broken when linked with versions of PCREv2 library older
than 10.34 in the latest release.
* hm/paint-hits-in-log-grep:
Revert "grep/pcre2: fix an edge case concerning ascii patterns and UTF-8 data"
Jeff King [Sun, 21 Nov 2021 23:10:56 +0000 (18:10 -0500)]
t7006: simplify exit-code checks for sigpipe tests
Some tests in t7006 check for a SIGPIPE result by recording $? and
comparing it with test_match_signal. Before the previous commit, the
command was on the left-hand side of a pipe, and so we had to do some
subshell trickery to extract it.
But now that this is no longer the case, we can do things much more
simply: just run the command directly, using braces to avoid wrecking
the &&-chain, and then record $?. We could almost use test_expect_code
here, but it doesn't know about test_match_signal.
Likewise, for tests which expect success (i.e., not SIGPIPE), we can
just put them in the &&-chain as usual. That even lets us get rid of the
!MINGW check, since the expectation is the same on both sides.
Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Jeff King [Sun, 21 Nov 2021 22:54:39 +0000 (17:54 -0500)]
t7006: clean up SIGPIPE handling in trace2 tests
Comit c24b7f6736 (pager: test for exit code with and without SIGPIPE,
2021-02-02) introduced some tests that don't reliably generate SIGPIPE
where we expect it (i.e., when our pager doesn't read all of the output
from git-log).
There are two problems that somewhat cancel each other out.
First is that the output of git-log isn't very large (only around 800
bytes). So even if the pager doesn't read all of our output, it's racy
whether or not we'll actually get a SIGPIPE (we won't if we write all of
the output into the pipe buffer before the pager exits).
But we wrap git-log with test_terminal, which is supposed to propagate
the exit status of git-log. However, it doesn't always do so;
test_terminal will copy to stdout any lines that it got from our fake
pager, and it pipes to an empty command. So most of the time we are
seeing a SIGPIPE from test_terminal itself (though this is likewise
racy).
Let's try to make this more robust in two ways:
1. We'll put a commit with a huge message at the tip of history. Since
this is over a megabyte, it should fill the OS pipe buffer
completely, causing git-log to keep trying to write even after the
pager has exited.
2. We'll redirect the output of test_terminal to /dev/null. That means
it can never get SIGPIPE itself, and will always be giving us the
exit code from git-log.
These two changes reveal that one of the tests was looking for the wrong
behavior. If we try to start a pager that does not exist (according to
execve()), then the error propagates from start_command() back to the
pager code as an error, and we avoid redirecting git-log's stdout to the
broken pager entirely. Instead, it goes straight to the original stdout
(test_terminal's pty in this case), and we do not see a SIGPIPE at all.
So the test "git attempts to page to nonexisting pager command, gets
SIGPIPE" is checking the wrong outcome; it should be looking for a
successful exit (and was only confused by test_terminal's SIGPIPE).
There's a related test, "git discards nonexisting pager without
SIGPIPE", which sets the pager to a shell command which will read all
input and _then_ run a non-existing command. But that doesn't trigger
the same execve() behavior. We really do run the shell there and
redirect git-log's stdout to it. And the fact that the shell then exits
127 is not interesting. It is not different at that point than the
earlier test to check for "exit 1". So we can drop that test entirely.
Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Jeff King [Mon, 22 Nov 2021 21:28:13 +0000 (16:28 -0500)]
run-command: unify signal and regular logic for wait_or_whine()
Since 507d7804c0 (pager: don't use unsafe functions in signal handlers,
2015-09-04), we have a separate code path in wait_or_whine() for the
case that we're in a signal handler. But that code path misses some of
the cases handled by the main logic.
This was improved in be8fc53e36 (pager: properly log pager exit code
when signalled, 2021-02-02), but that covered only case: actually
returning the correct error code. But there are some other cases:
- if waitpid() returns failure, we wouldn't notice and would look at
uninitialized garbage in the status variable; it's not clear if it's
possible to trigger this or not
- if the process exited by signal, then we would still report "-1"
rather than the correct signal code
This latter case even had a test added in be8fc53e36, but it doesn't
work reliably. It sets the pager command to:
>pager-used; test-tool sigchain
The latter command will die by signal, but because there are multiple
commands, there will be a shell in between. And it's the shell whose
waitpid() call will see the signal death, and it will then exit with
code 143, which is what Git will see.
To make matters even more confusing, some shells (such as bash) will
realize that there's nothing for the shell to do after test-tool
finishes, and will turn it into an exec. So the test was only checking
what it thought when /bin/sh points to a shell like bash (we're relying
on the shell used internally by Git to spawn sub-commands here, so even
running the test under bash would not be enough).
This patch adjusts the tests to explicitly call "exec" in the pager
command, which produces a consistent outcome regardless of shell. Note
that without the code change in this patch it _should_ fail reliably,
but doesn't. That test, like its siblings, tries to trigger SIGPIPE in
the git-log process writing to the pager, but only do so racily. That
will be fixed in a follow-on patch.
For the code change here, we have two options:
- we can teach the in_signal code to handle WIFSIGNALED()
- we can stop returning early when in_signal is set, and instead
annotate individual calls that we need to skip in this case
The former is a simpler patch, but means we're essentially duplicating
all of the logic. So instead I went with the latter. The result is a
bigger patch, and we do run the risk of new code being added but
forgetting to handle in_signal. But in the long run it seems more
maintainable.
I've skipped any non-trivial calls for the in_signal case, like calling
error(). We'll also skip the call to clear_child_for_cleanup(), as we
were before. This is arguably the wrong thing to do, since we wouldn't
want to try to clean it up again. But:
- we can't call it as-is, because it calls free(), which we must avoid
in a signal handler (we'd have to pass in_signal so it can skip the
free() call)
- we'll only go through the list of children to clean once, since our
cleanup_children_on_signal() handler pops itself after running (and
then re-raises, so eventually we'd just exit). So this cleanup only
matters if a process is on the cleanup list _and_ it has a separate
handler to clean itself up. Which is questionable in the first place
(and AFAIK we do not do).
- double-cleanup isn't actually that bad anyway. waitpid() will just
return an error, which we won't even report because of in_signal.
Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
The change in f652672 (dir: select directories correctly, 2021-09-24)
caused a regression in directory-based matches with non-cone-mode
patterns, especially for .gitignore patterns. A test is included to
prevent this regression in the future.
The commit ed495847 (dir: fix pattern matching on dirs, 2021-09-24) was
reverted in 5ceb663 (dir: fix directory-matching bug, 2021-11-02) for
similar reasons. Neither commit changed tests, and tests added later in
the series continue to pass when these commits are reverted.
Reported-by: Danial Alihosseini <danial.alihosseini@gmail.com> Signed-off-by: Derrick Stolee <dstolee@microsoft.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Jonathan Tan [Mon, 22 Nov 2021 18:41:14 +0000 (10:41 -0800)]
Doc: no midx and partial clone relation
The multi-pack index treats promisor packfiles (that is, packfiles that
have an accompanying .promisor file) the same as other packfiles. Remove
a section in the documentation that seems to indicate otherwise.
Signed-off-by: Jonathan Tan <jonathantanmy@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Han-Wen Nienhuys [Mon, 22 Nov 2021 14:19:08 +0000 (14:19 +0000)]
refs: drop force_create argument of create_reflog API
There is only one caller, builtin/checkout.c, and it hardcodes
force_create=1.
This argument was introduced in abd0cd3a301 (refs: new public ref function:
safe_create_reflog, 2015-07-21), which promised to immediately use it in a
follow-on commit, but that never happened.
Signed-off-by: Han-Wen Nienhuys <hanwen@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Junio C Hamano [Mon, 22 Nov 2021 05:57:03 +0000 (21:57 -0800)]
Merge branch 'hm/paint-hits-in-log-grep'
"git grep" looking in a blob that has non-UTF8 payload was
completely broken when linked with certain versions of PCREv2
library in the latest release.
* hm/paint-hits-in-log-grep:
Revert "grep/pcre2: fix an edge case concerning ascii patterns and UTF-8 data"
Fabian Stelzer [Sat, 20 Nov 2021 15:04:00 +0000 (16:04 +0100)]
test-lib: introduce required prereq for test runs
In certain environments or for specific test scenarios we might expect a
specific prerequisite check to succeed. Therefore we would like to abort
running our tests if this is not the case.
To remedy this we add the environment variable GIT_TEST_REQUIRE_PREREQ
which can be set to a space separated list of prereqs. If one of these
prereq tests fail then the whole test run will abort.
Signed-off-by: Fabian Stelzer <fs@gigacodes.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Fabian Stelzer [Sat, 20 Nov 2021 15:03:59 +0000 (16:03 +0100)]
test-lib: show missing prereq summary
When running the full test suite many tests can be skipped because of
missing prerequisites. It not easy right now to get an overview of which
ones are missing.
When switching to a new machine or environment some libraries and tools
might be missing or maybe a dependency broke completely. In this case
the tests would indicate nothing since all dependant tests are simply
skipped. This could hide broken behaviour or missing features in the
build. Therefore this patch summarizes the missing prereqs at the end of
the test run making it easier to spot such cases.
- Add failed prereqs to the test results.
- Aggregate and then show them with the totals.
Signed-off-by: Fabian Stelzer <fs@gigacodes.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
ci(check-whitespace): update stale file top comments
Earlier a066a90d (ci(check-whitespace): restrict to the intended
commits, 2021-07-14) changed the check-whitespace task to stop using a
shallow clone, and cc003621 (ci(check-whitespace): stop requiring a
read/write token, 2021-07-14) changed the way how the errors the task
discovered is signaled back to the user.
They however forgot to update the comment that outlines what is done in
the task. Correct them.
Signed-off-by: Hans Krentel (hakre) <hanskrentel@yahoo.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Jeff King [Fri, 19 Nov 2021 20:58:55 +0000 (15:58 -0500)]
fetch-pack: ignore SIGPIPE when writing to index-pack
When fetching, we send the incoming pack to index-pack (or
unpack-objects) via the sideband demuxer. If index-pack hits an error
(e.g., because an object fails fsck), then it will die immediately. This
may cause us to get SIGPIPE on the fetch, as we're still trying to write
pack contents from the sideband demuxer (which is typically a thread,
and thus takes down the whole fetch process).
You can see this in action with:
./t5702-protocol-v2.sh --stress --run=59
which ends with (wrapped for readability):
test_must_fail: died by signal 13: git -c protocol.version=2 \
-c transfer.fsckobjects=1 -c fetch.uriprotocols=http,https \
clone http://127.0.0.1:5708/smart/http_parent http_child
not ok 59 - packfile-uri with transfer.fsckobjects fails on bad object
This is mostly cosmetic. The actual error of interest (in this case, the
object that failed the fsck check) comes from index-pack straight to
stderr, so the user still sees it. They _might_ even see fetch-pack
complaining about index-pack failing, because the main thread is racing
with the sideband-demuxer. But they'll definitely see the signal death
in the exit code, which is what the test is complaining about.
We can make this more predictable by just ignoring SIGPIPE. The sideband
demuxer uses write_or_die(), so it will notice and stop (gracefully,
because we hook die_routine() to exit just the thread). And during this
section we're not writing anywhere else where we'd be concerned about
SIGPIPE preventing us from wasting effort writing to nowhere.
Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Jeff King [Fri, 19 Nov 2021 21:28:30 +0000 (16:28 -0500)]
refs: work around gcc-11 warning with REF_HAVE_NEW
Using gcc-11 (or 12) to compile refs.o with -O3 results in:
In file included from hashmap.h:4,
from cache.h:6,
from refs.c:5:
In function ‘oidcpy’,
inlined from ‘ref_transaction_add_update’ at refs.c:1065:3,
inlined from ‘ref_transaction_update’ at refs.c:1094:2,
inlined from ‘ref_transaction_verify’ at refs.c:1132:9:
hash.h:262:9: warning: argument 2 null where non-null expected [-Wnonnull]
262 | memcpy(dst->hash, src->hash, GIT_MAX_RAWSZ);
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
In file included from git-compat-util.h:177,
from cache.h:4,
from refs.c:5:
refs.c: In function ‘ref_transaction_verify’:
/usr/include/string.h:43:14: note: in a call to function ‘memcpy’ declared ‘nonnull’
43 | extern void *memcpy (void *__restrict __dest, const void *__restrict __src,
| ^~~~~~
That call to memcpy() is in a conditional block that requires
REF_HAVE_NEW to be set. But in ref_transaction_update(), we make sure it
isn't set coming in:
if (flags & ~REF_TRANSACTION_UPDATE_ALLOWED_FLAGS)
BUG("illegal flags 0x%x passed to ref_transaction_update()", flags);
So it should be impossible to reach that memcpy() with a NULL oid. But
for whatever reason, gcc doesn't accept that hitting the BUG() means we
won't go any further, even though it's marked with the noreturn
attribute. And the conditional is correct; ALLOWED_FLAGS doesn't contain
HAVE_NEW or HAVE_OLD, and you can even simplify it to check for those
flags explicitly and the compiler still complains.
We can work around this by just clearing the disallowed flags
explicitly. This should be a noop because of the BUG() check, but it
makes the compiler happy.
Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Mugdha Pattnaik [Fri, 19 Nov 2021 10:56:27 +0000 (10:56 +0000)]
submodule: absorb git dir instead of dying on deinit
Currently, running 'git submodule deinit' on repos where the
submodule's '.git' is a directory, aborts with a message that is not
exactly user friendly.
Let's change this to instead warn the user that the .git/ directory
has been absorbed into the superproject.
The rest of the deinit function can operate as it already does with
new-style submodules.
In one test, we used to require "git submodule deinit" to fail even
with the "--force" option when the submodule's .git/ directory is not
absorbed. Adjust it to expect the operation to pass.
Suggested-by: Atharva Raykar <raykar.ath@gmail.com> Signed-off-by: Mugdha Pattnaik <mugdhapattnaik@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Alex Henrie [Thu, 18 Nov 2021 15:43:17 +0000 (08:43 -0700)]
pull: don't say that merge is "the default strategy"
Git no longer has a default strategy for reconciling divergent branches,
because there's no way for Git to know which strategy is appropriate in
any particular situation.
The initially proposed version in [*], that eventually became 031e2f7a (pull: abort by default when fast-forwarding is not
possible, 2021-07-22), dropped this phrase from the message, but
it was left in the final version by accident.
Junio C Hamano [Fri, 19 Nov 2021 17:06:36 +0000 (09:06 -0800)]
Revert "grep/pcre2: fix an edge case concerning ascii patterns and UTF-8 data"
This reverts commit ae39ba431ab861548eb60b4bd2e1d8b8813db76f, as it
breaks "grep" when looking for a string in non UTF-8 haystack, when
linked with certain versions of PCREv2 library.
Fabian Stelzer [Fri, 19 Nov 2021 15:07:07 +0000 (16:07 +0100)]
ssh signing: make sign/amend test more resilient
The test `amending already signed commit` is using git checkout to
select a specific commit to amend. In case an earlier test fails and
leaves behind a dirty index/worktree this test would fail as well.
Using `checkout -f` will avoid interference by most other tests.
Signed-off-by: Fabian Stelzer <fs@gigacodes.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Fabian Stelzer [Fri, 19 Nov 2021 15:07:06 +0000 (16:07 +0100)]
ssh signing: support non ssh-* keytypes
The user.signingKey config for ssh signing supports either a path to a
file containing the key or for the sake of convenience a literal string
with the ssh public key. To differentiate between those two cases we
check if the first few characters contain "ssh-" which is unlikely to be
the start of a path. ssh supports other key types which are not prefixed
with "ssh-" and will currently be treated as a file path and therefore
fail to load. To remedy this we move the prefix check into its own
function and introduce the prefix `key::` for literal ssh keys. This way
we don't need to add new key types when they become available. The
existing `ssh-` prefix is retained for compatibility with current user
configs but removed from the official documentation to discourage its
use.
Signed-off-by: Fabian Stelzer <fs@gigacodes.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
projects / thirdparty / git.git / log
