Test updates.
* hn/ref-api-tests-update:
t7004: use "test-tool ref-store" for reflog inspection
t7004: create separate tags for different tests
t5550: require REFFILES
t5540: require REFFILES
Perf tests were run with end-user's shell, but it has been
corrected to use the shell specified by $TEST_SHELL_PATH.
* ja/perf-use-specified-shell:
t/perf: do not run tests in user's $SHELL
Use of certain "git rev-list" options with "git fast-export"
created nonsense results (the worst two of which being "--reverse"
and "--invert-grep --grep=<foo>"). The use of "--first-parent" is
made to behave a bit more sensible than before.
* ws/fast-export-with-revision-options:
fast-export: fix surprising behavior with --first-parent
The way "git p4" shows file sizes in its output has been updated to
use human-readable units.
* jh/p4-human-unit-numbers:
git-p4: show progress as an integer
git-p4: print size values in appropriate units
Certain sparse-checkout patterns that are valid in non-cone mode
led to segfault in cone mode, which has been corrected.
* ds/sparse-checkout-malformed-pattern-fix:
sparse-checkout: refuse to add to bad patterns
sparse-checkout: fix OOM error with mixed patterns
sparse-checkout: fix segfault on malformed patterns
There are two identical declarations of `ensure_full_index()` in
cache.h.
Commit 3964fc2aae ("sparse-index: add guard to ensure full index",
2021-03-30) provided an empty implementation of `ensure_full_index()`,
declaring it in a new file sparse-index.h. When commit 4300f8442a
("sparse-index: implement ensure_full_index()", 2021-03-30) fleshed out
the implementation, it added an identical declaration to cache.h.
Then 118a2e8bde ("cache: move ensure_full_index() to cache.h",
2021-04-01) favored having the declaration in cache.h. Because of the
double declaration, at that point we could have just dropped the one in
sparse-index.h, but instead it got moved to cache.h.
As a result, cache.h contains the exact same function declaration twice.
Drop the one under "/* Name hashing */", in favor of the one under
"/* Initialize and use the cache information */".
Signed-off-by: Martin Ågren <martin.agren@gmail.com>
Acked-by: Victoria Dye <vdye@github.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Christoph Reiter reported on the Git for Windows issue tracker[1], that
mingw_strftime() imports strftime() from ucrtbase.dll with the wrong
calling convention. It should be __cdecl instead of WINAPI, which we
always use in DECLARE_PROC_ADDR().
The MSYS2 project encountered cmake sefaults on x86 Windows caused by
the same issue in the cmake source. [2] There are no known git crashes
that where caused by this, yet, but we should try to prevent them.
We import two other non-WINAPI functions via DECLARE_PROC_ADDR(), too.
* NtSetSystemInformation() (NTAPI)
* GetUserNameExW() (SEC_ENTRY)
NTAPI, SEC_ENTRY and WINAPI are all ususally defined as __stdcall,
but there are circumstances where they're defined differently.
Teach DECLARE_PROC_ADDR() about calling conventions and be explicit
about when we want to use which calling convention.
Import winnt.h for the definition of NTAPI and sspi.h for SEC_ENTRY
near their respective only users.
[1] https://github.com/git-for-windows/git/issues/3560
[2] https://github.com/msys2/MINGW-packages/issues/10152
Reported-By: Christoph Reiter <reiter.christoph@gmail.com>
Signed-off-by: Matthias Aßhauer <mha1993@live.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Like in the previous patch for compat/qsort_s.c, remove the optimization
of using an on-stack buffer to avoid small allocations. This ensures
maximum alignment for the array elements and simplifies the code a bit.
The performance impact for the current callers is unlikely to be
noticeable:
* compat/mingw.c::make_environment_block() uses ALLOC_ARRAY and
ALLOC_GROW several times already, so another allocation of up to 1KB
should not matter much.
* diffcore-rename.c::diffcore_rename_extended() is called once per diff
or twice per merge, and those require allocations for each object and
more already.
* merge-ort.c::detect_and_process_renames() is called once per merge.
It's responsible for the two per-merge diffcore_rename_extended()
calls mentioned above as well, though. So this is possibly the most
impacted caller. Per-object allocations are likely to dwarf the
additional small allocations in git_stable_qsort(), though.
Signed-off-by: René Scharfe <l.s.r@web.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The compatibility definition for qsort_s() uses "char buffer[1024]"
on the stack to avoid making malloc() calls for small temporary
space, which essentially hand-rolls alloca().
But the elements of the array being sorted may have alignment needs
more strict than what an array of bytes may have. &buf[0] may be
word aligned, but using the address as if it stores the first
element of an array of a struct, whose first member may need to be
aligned on double-word boundary, would be a no-no.
We could use xalloca() from git-compat-util.h, or alloca() directly
on platforms with HAVE_ALLOCA_H, but let's try using unconditionally
xmalloc() before we know the performance characteristics of the
callers.
It may not make much of an argument to inspect the current callers
and say "it shouldn't matter to any of them", but anyway:
* The one in object-name.c is used to sort potential matches to a
given ambiguous object name prefix in the error path;
* The one in pack-write.c is done once per a pack .idx file being
written to create the reverse index, so (1) the cost of malloc()
overhead is dwarfed by the cost of the packing operation, and (2)
the number of entries being sorted is the number of objects in a
pack;
* The one in ref-filter.c is used by "branch --list", "tag --list",
and "for-each-ref", only once per operation. We sort an array of
pointers with entries, each corresponding to a ref that is shown.
* The one in string-list.c is used by sort_string_list(), which is
way too generic to assume any access patterns, so it may or may
not matter, but I do not care too much ;-)
Signed-off-by: Junio C Hamano <gitster@pobox.com>
When fetching packfiles, we write a bunch of lockfiles for the packfiles
we're writing into the repository. In order to not leave behind any
cruft in case we exit or receive a signal, we register both an exit
handler as well as signal handlers for common signals like SIGINT. These
handlers will then unlink the locks and free the data structure tracking
them. We have observed a deadlock in this logic though:
(gdb) bt
#0 __lll_lock_wait_private () at ../sysdeps/unix/sysv/linux/x86_64/lowlevellock.S:95
#1 0x00007f4932bea2cd in _int_free (av=0x7f4932f2eb20 <main_arena>, p=0x3e3e4200, have_lock=0) at malloc.c:3969
#2 0x00007f4932bee58c in __GI___libc_free (mem=<optimized out>) at malloc.c:2975
#3 0x0000000000662ab1 in string_list_clear ()
#4 0x000000000044f5bc in unlock_pack_on_signal ()
#5 <signal handler called>
#6 _int_free (av=0x7f4932f2eb20 <main_arena>, p=<optimized out>, have_lock=0) at malloc.c:4024
#7 0x00007f4932bee58c in __GI___libc_free (mem=<optimized out>) at malloc.c:2975
#8 0x000000000065afd5 in strbuf_release ()
#9 0x000000000066ddb9 in delete_tempfile ()
#10 0x0000000000610d0b in files_transaction_cleanup.isra ()
#11 0x0000000000611718 in files_transaction_abort ()
#12 0x000000000060d2ef in ref_transaction_abort ()
#13 0x000000000060d441 in ref_transaction_prepare ()
#14 0x000000000060e0b5 in ref_transaction_commit ()
#15 0x00000000004511c2 in fetch_and_consume_refs ()
#16 0x000000000045279a in cmd_fetch ()
#17 0x0000000000407c48 in handle_builtin ()
#18 0x0000000000408df2 in cmd_main ()
#19 0x00000000004078b5 in main ()
The process was killed with a signal, which caused the signal handler to
kick in and try free the data structures after we have unlinked the
locks. It then deadlocks while calling free(3P).
The root cause of this is that it is not allowed to call certain
functions in async-signal handlers, as specified by signal-safety(7).
Next to most I/O functions, this list of disallowed functions also
includes memory-handling functions like malloc(3P) and free(3P) because
they may not be reentrant. As a result, if we execute such functions in
the signal handler, then they may operate on inconistent state and fail
in unexpected ways.
Fix this bug by not calling non-async-signal-safe functions when running
in the signal handler. We're about to re-raise the signal anyway and
will thus exit, so it's not much of a problem to keep the string list of
lockfiles untouched. Note that it's fine though to call unlink(2), so
we'll still clean up the lockfiles correctly.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
Reviewed-by: brian m. carlson <sandals@crustytoothpaste.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
'git update-index --refresh' and '--really-refresh' should force writing
of the index file if racy timestamps have been encountered, as
'git status' already does [1].
Note that calling 'git update-index --refresh' still does not guarantee
that there will be no more racy timestamps afterwards (the same holds
true for 'git status'):
- calling 'git update-index --refresh' immediately after touching and
adding a file may still leave racy timestamps if all three operations
occur within the racy-tolerance (usually 1 second unless USE_NSEC has
been defined)
- calling 'git update-index --refresh' for timestamps which are set into
the future will leave them racy
To guarantee that such racy timestamps will be resolved would require to
wait until the system clock has passed beyond these timestamps and only
then write the index file. Especially for future timestamps, this does
not seem feasible because of possibly long delays/hangs.
[1] https://lore.kernel.org/git/d3dd805c-7c1d-30a9-6574-a7bfcb7fc013@syntevo.com/
Signed-off-by: Marc Strapetz <marc.strapetz@syntevo.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
"git status" fixes racy timestamps regardless of the worktree being
dirty or not. The new test cases capture this behavior.
Signed-off-by: Marc Strapetz <marc.strapetz@syntevo.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The current `grep`-approach in "--no-optional-locks prevents index
update" may fail e.g. for `out` file contents "1234567890999" [1].
Fix this by using test-lib's new mtime-verification API.
[1] https://lore.kernel.org/git/xmqqczl5hpaq.fsf@gitster.g/T/#u
Signed-off-by: Marc Strapetz <marc.strapetz@syntevo.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Add functions `test_set_magic_mtime` and `test_is_magic_mtime` which can
be used to (re)set the mtime of a file to a predefined ("magic")
timestamp, then perform some operations and finally check for mtime
changes of the file.
The core implementation follows the suggestion from the
mailing list [1].
[1] https://lore.kernel.org/git/xmqqczl5hpaq.fsf@gitster.g/T/#u
Signed-off-by: Marc Strapetz <marc.strapetz@syntevo.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Symlink changes are tracked in a string_list, with the util pointer
value indicating whether a symlink is kept or removed. Using fake
pointer values requires awkward casts. Use one strset for each type of
change instead to simplify and shorten the code.
Original-patch-by: Jessica Clarke <jrtc27@jrtc27.com>
Signed-off-by: René Scharfe <l.s.r@web.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
CalledProcessError is an exception class from the subprocess namespace.
When raising this exception, git-p4 would instantiate CalledProcessError
objects without properly referencing the subprocess namespace causing
the script to fail.
Resolves the issue by replacing CalledProcessError with
subprocess.CalledProcessError.
Signed-off-by: Joel Holdsworth <jholdsworth@nvidia.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Previously the git-p4 script would log commands as stringified
representations of the command parameter, leading to output such as
this:
Reading pipe: ['git', 'config', '--bool', 'git-p4.useclientspec']
Now that all commands are list objects, this patch instead joins the
elements of the list into a single string so the output now looks more
readable:
Reading pipe: git config --bool git-p4.useclientspec
Signed-off-by: Joel Holdsworth <jholdsworth@nvidia.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
In the majority of the subprocess calls where shell=True was used, it
was only needed to parse command arguments by spaces. In each of these
cases, the commands are now being passed in as lists instead of strings.
This change aids the comprehensibility of the code. Constucting commands
and arguments using strings risks bugs from unsanitized inputs, and the
attendant complexity of properly quoting and escaping command arguments.
Signed-off-by: Joel Holdsworth <jholdsworth@nvidia.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Previously, the script would invoke subprocess functions setting the
shell argument True if the command argument was a string, setting it
False otherwise.
This patch replaces this implicit type-driven behaviour with explicit
shell arguments specified by the caller.
The apparent motive for the implict behaviour is that the subprocess
functions do not divide command strings into args. Invoking
subprocess.call("echo hello") will attempt to execute a program by the
name "echo hello". With subprocess.call("echo hello", shell=True), sh
-c "echo hello" will be executed instead, which will cause the command
and args to be divided by spaces.
Eventually, all usage of shell=True, that is not necessary for some
purpose beyond parsing command strings, should be removed. For now,
this patch makes the usage of shells explicit.
Signed-off-by: Joel Holdsworth <jholdsworth@nvidia.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
In a similar spirit as a previous commit, use the new `grep_and_expr()`
to construct the AND node in `compile_pattern_and()`.
Unlike the aforementioned previous commit, this is not about code
duplication, since this is the only spot in grep.c where an AND node is
constructed. Rather, this is about visual consistency with the other
`compile_pattern_xyz()` functions.
Signed-off-by: Taylor Blau <me@ttaylorr.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
There are two functions that have very similar logic of finding a header
value. find_commit_header, and find_header. We can conslidate the logic
by introducing a new function find_header_mem, which is equivalent to
find_commit_header except it takes a len parameter that determines how
many bytes will be read. find_commit_header and find_header can then both
call find_header_mem.
This reduces duplicate logic, as the logic for finding header values
can now all live in one place.
Signed-off-by: John Cai <johncai86@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
When constructing an OR node, the grep.c code uses `grep_or_expr()` to
make a node, assign its kind, and set its left and right children. The
same is not done for AND nodes.
Prepare to introduce a new `grep_and_expr()` function which will share
code with the existing implementation of `grep_or_expr()` by introducing
a new function which compiles either kind of binary expression, and
reimplement `grep_or_expr()` in terms of it.
Signed-off-by: Taylor Blau <me@ttaylorr.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Move the definition of grep_not_expr() up and use this function in
compile_pattern_not() to simplify the code and reduce duplication.
Signed-off-by: René Scharfe <l.s.r@web.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Move the definition of grep_or_expr() up and use this function in
compile_pattern_or() to reduce code duplication.
Signed-off-by: René Scharfe <l.s.r@web.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
"git grep --perl-regexp" failed to match UTF-8 characters with
wildcard when the pattern consists only of ASCII letters, which has
been corrected.
* rs/pcre2-utf:
grep/pcre2: factor out literal variable
grep/pcre2: use PCRE2_UTF even with ASCII patterns
"git log --invert-grep --author=<name>" used to exclude commits
written by the given author, but now "--invert-grep" only affects
the matches made by the "--grep=<pattern>" option.
* rs/log-invert-grep-with-headers:
log: let --invert-grep only invert --grep
The default merge message prepared by "git merge" records the name
of the current branch; the name can be overridden with a new option
to allow users to pretend a merge is made on a different branch.
* jc/merge-detached-head-name:
merge: allow to pretend a merge is made into a different branch
Among some code paths that ask an yes/no question, only one place
gave a prompt that looked different from the others, which has been
updated to match what the others create.
* km/help-prompt-fix:
help: make auto-correction prompt more consistent
"git upload-pack" (the other side of "git fetch") used a 8kB buffer
but most of its payload came on 64kB "packets". The buffer size
has been enlarged so that such a packet fits.
* jv/use-larger-buffer-in-upload-pack:
upload-pack.c: increase output buffer size
Correctness and performance update to "diff --color-moved" feature.
* pw/diff-color-moved-fix:
diff --color-moved: intern strings
diff: use designated initializers for emitted_diff_symbol
diff --color-moved-ws=allow-indentation-change: improve hash lookups
diff --color-moved: stop clearing potential moved blocks
diff --color-moved: shrink potential moved blocks as we go
diff --color-moved: unify moved block growth functions
diff --color-moved: call comparison function directly
diff --color-moved-ws=allow-indentation-change: simplify and optimize
diff: simplify allow-indentation-change delta calculation
diff --color-moved: avoid false short line matches and bad zebra coloring
diff --color-moved=zebra: fix alternate coloring
diff --color-moved: rewind when discarding pmb
diff --color-moved: factor out function
diff --color-moved: clear all flags on blocks that are too short
diff --color-moved: add perf tests
"git am" learns "--empty=(stop|drop|keep)" option to tweak what is
done to a piece of e-mail without a patch in it.
* xw/am-empty:
am: support --allow-empty to record specific empty patches
am: support --empty=<option> to handle empty patches
doc: git-format-patch: describe the option --always
Many git commands that deal with working tree files try to remove a
directory that becomes empty (i.e. "git switch" from a branch that
has the directory to another branch that does not would attempt
remove all files in the directory and the directory itself). This
drops users into an unfamiliar situation if the command was run in
a subdirectory that becomes subject to removal due to the command.
The commands have been taught to keep an empty directory if it is
the directory they were started in to avoid surprising users.
* en/keep-cwd:
t2501: simplify the tests since we can now assume desired behavior
dir: new flag to remove_dir_recurse() to spare the original_cwd
dir: avoid incidentally removing the original_cwd in remove_path()
stash: do not attempt to remove startup_info->original_cwd
rebase: do not attempt to remove startup_info->original_cwd
clean: do not attempt to remove startup_info->original_cwd
symlinks: do not include startup_info->original_cwd in dir removal
unpack-trees: add special cwd handling
unpack-trees: refuse to remove startup_info->original_cwd
setup: introduce startup_info->original_cwd
t2501: add various tests for removing the current working directory
The conditions to choose different definitions of the FLEX_ARRAY
macro for vendor compilers has been simplified to make it easier to
maintain.
* jc/flex-array-definition:
flex-array: simplify compiler-specific workaround
The RCS keyword substitution in "git p4" used to be done assuming
that the contents are UTF-8 text, which can trigger decoding
errors. We now treat the contents as a bytestring for robustness
and correctness.
* jh/p4-rcs-expansion-in-bytestring:
git-p4: resolve RCS keywords in bytes not utf-8
git-p4: open temporary patch file for write only
git-p4: add raw option to read_pipelines
git-p4: pre-compile RCS keyword regexes
git-p4: use with statements to close files after use in patchRCSKeywords
Even if some of these messages are not subject to gettext i18n, this
helps bring a single style of message for a given error type.
Signed-off-by: Jean-Noël Avila <jn.avila@free.fr>
Reviewed-by: Johannes Sixt <j6t@kdbg.org>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
They are all replaced by "the option '%s' requires '%s'", which is a
new string but replaces 17 previous unique strings.
Signed-off-by: Jean-Noël Avila <jn.avila@free.fr>
Reviewed-by: Johannes Sixt <j6t@kdbg.org>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
