| Commit message (Collapse) | Author | Files | Lines |
|---|
|
The purpose of chainlint.sed is to detect &&-chain breakage only within
subshells (one level deep); it doesn't bother checking for top-level
&&-chain breakage since the &&-chain checker built into t/test-lib.sh
should detect broken &&-chains outside of subshells by making them
magically exit with code 117.
Unfortunately, one of the chainlint.sed self-tests has overly intimate
knowledge of this particular division of responsibilities and only cares
about what chainlint.sed itself will produce, while ignoring the fact
that a more all-encompassing linter would complain about a broken
&&-chain outside the subshell. This makes it difficult to re-use the
test with a more capable chainlint implementation should one ever be
developed. Therefore, adjust the test and its "expected" output to
avoid being specific to the tunnel-vision of this one implementation.
Signed-off-by: Eric Sunshine <sunshine@sunshineco.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
The purpose of chainlint.sed is to detect &&-chain breakage only within
subshells (one level deep); it doesn't bother checking for top-level
&&-chain breakage since the &&-chain checker built into t/test-lib.sh
should detect broken &&-chains outside of subshells by making them
magically exit with code 117. However, this division of labor may not
always be the case if a more capable chainlint implementation is ever
developed. Beyond that, due to being sed-based and due to its use of
heuristics, chainlint.sed has several limitations (such as being unable
to detect &&-chain breakage in subshells more than one level deep since
it only manually emulates recursion into a subshell).
Some of the comments in the chainlint self-tests unnecessarily reflect
the limitations of chainlint.sed even though those limitations are not
what is being tested. Therefore, simplify and generalize the comments to
explain only what is being tested, thus ensuring that they won't become
outdated if a more capable chainlint is ever developed.
Signed-off-by: Eric Sunshine <sunshine@sunshineco.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
The chainlint self-test code snippets are supposed to represent the body
of a test_expect_success() or test_expect_failure(), yet the contents of
a few tests would have caused the shell to report syntax errors had they
been real test bodies due to the mix of single- and double-quotes.
Although chainlint.sed, with its simplistic heuristics, is blind to this
problem, a future more robust chainlint implementation might not have
such a limitation. Therefore, stop mixing quote types haphazardly in
those tests and unify quoting throughout. While at it, drop chunks of
tests which merely repeat what is already tested elsewhere but with
alternative quotes.
Signed-off-by: Eric Sunshine <sunshine@sunshineco.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
The chainlint self-test code snippets are supposed to represent the body
of a test_expect_success() or test_expect_failure(), yet the contents of
these tests would have caused the shell to report syntax errors had they
been real test bodies. Although chainlint.sed, with its simplistic
heuristics, is blind to these syntactic problems, a future more robust
chainlint implementation might not have such a limitation, so make these
snippets syntactically valid.
Signed-off-by: Eric Sunshine <sunshine@sunshineco.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
|
|
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>
|
|
Visual Studio reports C4334 "was 64-bit shift intended" warning because
of size miss-match.
Promote unity to the matching type to fit with the assignment.
Signed-off-by: Philip Oakley <philipoakley@iee.email>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
Visual Studio reports C4334 "was 64-bit shift intended" warning
because of size miss-match.
Promote unity to the matching type to fit with its subsequent operation.
Signed-off-by: Philip Oakley <philipoakley@iee.email>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
Visual Studio reports C4334 "was 64-bit shift intended" warning
because of size mismatch.
Promote unity to the matching type to fit with the `&` operator.
Signed-off-by: Philip Oakley <philipoakley@iee.email>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
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>
|
|
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
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>
|
|
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>
|
|
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>
|
|
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>
|
|
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>
|
|
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>
|
|
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>
|
|
human was introduced in acdd37769de8b0fe37a74bfc0475b63bdc55e9dc
auto:* was introduced in 2fd7c22992d37469db957e9a4d3884a6c0a4d182
Those formats were missing when other values were added to completion
at 5a59a2301f6ec9bcf1b101edb9ca33beb465842f
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>
|
|
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>
|
|
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>
|
|
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>
|
|
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>
|
|
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>
|
|
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
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>
|
|
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>
|
|
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>
|
|
This reverts commit 3d411afabc9a96f41d47c07d6af6edda3d29ec92,
blindly opening /dev/tty and calling tcsetattr() seems to be causing
problems.
cf. https://bugs.eclipse.org/bugs/show_bug.cgi?id=577358
cf. https://lore.kernel.org/git/04ab7301-ea34-476c-eae4-4044fef74b91@gmail.com/
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
This reverts commit f6526728f950cacfd5b5e42bcc65f2c47f3da654.
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>
|
|
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>
|
|
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>
|
|
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
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>
|
|
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>
|
|
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);
and then only set it if the variable isn't NULL:
flags |= (new_oid ? REF_HAVE_NEW : 0) | (old_oid ? REF_HAVE_OLD : 0);
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>
|
|
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>
|
|
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.
* https://lore.kernel.org/git/20210627000855.530985-1-alexhenrie24@gmail.com/
Signed-off-by: Alex Henrie <alexhenrie24@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
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.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
If writing a trace2 message fails, we optionally warn the user of this
fact. However, in 0ee10fd (usage: add trace2 entry upon warning(),
2020-11-23), we added a trace entry to the warning() function. This
means that we can enter an infinite loop of failing trace2 writes and
warnings. Fix this by disabling the failing trace2 destination before
issuing the warning.
Additionally, trace2 sets a default SIGPIPE handler
(tr2main_signal_handler) when it is initialized. This handler generates
its own trace2 messages when a signal is received. If a trace2 write
fails due to a broken pipe, this handler will run and then cause another
failed write. Fix this by temporarily ignoring SIGPIPE while writing
trace2 messages. This is safe because the write will still fail, and we
will disable the failing destination.
Signed-off-by: Josh Steadmon <steadmon@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
In a subsequent commit, we would like external-facing functions to be
able to accept "struct repository" and "struct branch" as a pair. This
is useful for functions like pushremote_for_branch(), which need to take
values from the remote_state and branch, even if branch == NULL.
However, a caller may supply an unrelated repository and branch, which
is not supported behavior.
To prevent misuse, add a die_on_missing_branch() helper function that
dies if a given branch is not from a given repository. Speed up the
existence check by replacing the branches list with a branches_hash
hashmap.
Like read_config(), die_on_missing_branch() is only called from
non-static functions; static functions are less prone to misuse because
they have strong conventions for keeping remote_state and branch in
sync.
Signed-off-by: Glen Choo <chooglen@google.com>
Reviewed-by: Jonathan Tan <jonathantanmy@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
Replace all remaining references of the_repository->remote_state in
static functions with a struct remote_state parameter.
To do so, move read_config() calls to non-static functions and create a
family of static functions, "remotes_*", that behave like "repo_*", but
accept struct remote_state instead of struct repository. In the case
where a static function calls a non-static function, replace the
non-static function with its "remotes_*" equivalent.
Signed-off-by: Glen Choo <chooglen@google.com>
Reviewed-by: Jonathan Tan <jonathantanmy@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
Without changing external-facing functions, replace
the_repository->remote_state internally by adding a struct remote_state
parameter.
As a result, external-facing functions are still tied to the_repository,
but most static functions no longer reference
the_repository->remote_state. The exceptions are those that are used in
a way that depends on external-facing functions e.g. the callbacks to
remote_get_1().
Signed-off-by: Glen Choo <chooglen@google.com>
Reviewed-by: Jonathan Tan <jonathantanmy@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
remote.c does not works with non-the_repository because it stores its
state as static variables. To support non-the_repository, we can use a
per-repository struct for the remotes subsystem.
Prepare for this change by defining a struct remote_state that holds
the remotes subsystem state and move the static variables of remote.c
into the_repository->remote_state.
This introduces no behavioral or API changes.
Signed-off-by: Glen Choo <chooglen@google.com>
Reviewed-by: Jonathan Tan <jonathantanmy@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
"git push remote-name" (that is, with no refspec given on the command
line) should push the refspecs in remote.remote-name.push. There is no
test case that checks this behavior in detached HEAD, so add one.
Signed-off-by: Glen Choo <chooglen@google.com>
Reviewed-by: Jonathan Tan <jonathantanmy@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
The already-up-to-date pull bug was fixed for --ff-only but it did not
include the case where --ff or --ff-only are not specified. This updates
the --ff-only fix to include the case where --ff or --ff-only are not
specified in command line flags or config.
Signed-off-by: Erwin Villejo <erwin.villejo@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
One of the tests in t5319 corrupts the checksum of the midx file by
writing a single 0xff over the final byte, and then confirms that we
detect the problem. This usually works fine, but would break if the
actual checksum ended with that same byte already.
It seems like this should happen in 1 out of 256 test runs, but it turns
out to be less often in practice. The contents of the midx are mostly
deterministic because it's based on the objects, and we remove most
sources of randomness by setting GIT_COMMITTER_DATE, etc. However,
there's still some randomness: some objects are duplicated between
packs, and the midx must decide which to use, which can be based on
timing.
So very occasionally we can end up with a real 0xff byte, and the test
fails. The most robust fix would be to read out the final byte and then
change it to something else (e.g., adding 1 mod 256). But that's awkward
to do in shell. Let's just blindly corrupt 10 bytes instead of 1, which
reduces our chances of an accidental noop to 1 in 2^80.
Reported-by: SZEDER Gábor <szeder.dev@gmail.com>
Signed-off-by: Jeff King <peff@peff.net>
Reviewed-by: Taylor Blau <me@ttaylorr.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
The "checkout" command is one of the main sources of leaks in the test
suite, let's fix the common ones by not leaking from the "struct
branch_info".
Doing this is rather straightforward, albeit verbose, we need to
xstrdup() constant strings going into the struct, and free() the ones
we clobber as we go along.
This also means that we can delete previous partial leak fixes in this
area, i.e. the "path_to_free" accounting added by 96ec7b1e708 (Convert
resolve_ref+xstrdup to new resolve_refdup function, 2011-12-13).
There was some discussion about whether "we should retain the "const
char *" here and cast at free() time, or have it be a "char *". Since
this is not a public API with any sort of API boundary let's use
"char *", as is already being done for the "refname" member of the
same struct.
The tests to mark as passing were found with:
rm .prove; GIT_SKIP_TESTS=t0027 prove -j8 --state=save t[0-9]*.sh :: --immediate
# apply & compile this change
prove -j8 --state=failed :: --immediate
I.e. the ones that were newly passing when the --state=failed command
was run. I left out "t3040-subprojects-basic.sh" and
"t4131-apply-fake-ancestor.sh" to to optimization-level related
differences similar to the ones noted in[1], except that these would
be something the current 'linux-leaks' job would run into.
1. https://lore.kernel.org/git/cover-v3-0.6-00000000000-20211022T175227Z-avarab@gmail.com/
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
Use size_t to match n when building the bitmask for checking whether a
rank is occupied, instead of the default signed int.
Signed-off-by: René Scharfe <l.s.r@web.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
