On Windows, there are several categories of absolute paths. One such
category starts with a backslash and is implicitly relative to the
drive associated with the current working directory. Example:
c:
git clone https://github.com/git-for-windows/git \G4W
should clone into C:\G4W.
There is currently a problem with that, in that mingw_mktemp() does not
expect the _wmktemp() function to prefix the absolute path with the
drive prefix, and as a consequence, the resulting path does not fit into
the originally-passed string buffer. The symptom is a "Result too large"
error.
Reported by Juan Carlos Arevalo Baeza.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Last-minute fix for a regression in "git blame --abbrev=<length>"
when insane <length> is specified; we used to correctly cap it to
the hash output length but broke it during the cycle.
* ps/build-sign-compare:
builtin/blame: fix out-of-bounds write with blank boundary commits
builtin/blame: fix out-of-bounds read with excessive `--abbrev`
"Why would one want to run it in parallel?" I hear you ask. I am glad
you are curious, because a curious story is what it is, indeed.
The `GIT-VERSION-GEN` script is quite a pillar of Git's source code,
with most lines being unchanged for the past 15 years. Until the v2.48.0
release candidate cycle.
Its original purpose was to generate the version string and store it in
the `GIT-VERSION-FILE`.
This paradigm changed quite dramatically when support for building with
Meson was introduced. Most crucially, a38edab7c8 (Makefile: generate
doc versions via GIT-VERSION-GEN, 2024-12-06) changed the way the
documentation is built by using the `GIT-VERSION-GEN` file to write out
the `asciidocor-extensions.rb` and `asciidoc.conf` files with now
hard-coded version strings.
Crucially, the Makefile rule to generate those files needs to be run in
every build because `GIT_VERSION` could have been specified in the
`make` command-line, which would require these files to be modified.
This introduced a surprising race condition!
And this is how that race surfaces: When calling `make -j2 html man`
from the top-level directory (a variant of which is invoked in Git for
Windows' release process), two sub-processes are spawned, a `make -C
Documentation html` one and a `make -C Documentation man` one. Both run
the rule to (re-)generate `asciidoctor-extensions.rb` or
`asciidoc.conf`, invoking `GIT-VERSION-GEN` to do so. That script first
generates a temporary file (appending the `+` character to the
filename), then looks whether it contains something different than the
already existing file (if it exists, that is), and either replaces it if
needed, or removes the temporary file. If one of the two parallel
invocations removes that temporary file before the other can compare it,
or even worse: if one tries to replace the target file just after the
other _started_ writing the temporary file (but did not finish writing
it yet), that race condition now causes bad builds.
This may sound highly theoretical, but due to the design of Git's build
process, Git for Windows is forced to use a (slow) POSIX emulation layer
to run that script and in the blink of an eye it becomes very much not
theoretical at all. See Exhibit A: These GitHub workflow runs failed
because one of the two competing `make` processes tried to remove the
temporary file when the other process had already done so:
https://github.com/git-for-windows/git-sdk-32/actions/runs/12663456654https://github.com/git-for-windows/git-sdk-32/actions/runs/12683174970https://github.com/git-for-windows/git-sdk-64/actions/runs/12649348496
While it is undesirable to run this script over and over again,
certainly when this involves above-mentioned slow POSIX emulation layer,
the stage of the release cycle in which we are presently finding
ourselves does not lend itself to a re-design where this script could be
run once, and once only, but instead dictates that a quick and reliable
work-around be implemented that prevents the race condition without
changing the overall architecture of the build process.
This patch does that: By using a filename suffix for the temporary file
which is based on the currently-executing script's process ID, We
guarantee that the two competing invocations cannot overwrite or remove
each others' temporary files.
The filename suffix still ends in `+` to ensure that the temporary
artifacts are matched by the `*+` pattern in `.gitignore` that was added
in f9bbaa384e (Add intermediate build products to .gitignore,
2009-11-08).
Helped-by: Martin Ågren <martin.agren@gmail.com>
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
When passing the `-b` flag to git-blame(1), then any blamed boundary
commits which were marked as uninteresting will not get their actual
commit ID printed, but will instead be replaced by a couple of spaces.
The flag can lead to an out-of-bounds write as though when combined with
`--abbrev=` when the abbreviation length is longer than `GIT_MAX_HEXSZ`
as we simply use memset(3p) on that array with the user-provided length
directly. The result is most likely that we segfault.
An obvious fix would be to cull `length` to `GIT_MAX_HEXSZ` many bytes.
But when the underlying object ID is SHA1, and if the abbreviated length
exceeds the SHA1 length, it would cause us to print more bytes than
desired, and the result would be misaligned.
Instead, fix the bug by computing the length via strlen(3p). This makes
us write as many bytes as the formatted object ID requires and thus
effectively limits the length of what we may end up printing to the
length of its hash. If `--abbrev=` asks us to abbreviate to something
shorter than the full length of the underlying hash function it would be
handled by the call to printf(3p) correctly.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
In 6411a0a896 (builtin/blame: fix type of `length` variable when
emitting object ID, 2024-12-06) we have fixed the type of the `length`
variable. In order to avoid a cast from `size_t` to `int` in the call to
printf(3p) with the "%.*s" formatter we have converted the code to
instead use fwrite(3p), which accepts the length as a `size_t`.
It was reported though that this makes us read over the end of the OID
array when the provided `--abbrev=` length exceeds the length of the
object ID. This is because fwrite(3p) of course doesn't stop when it
sees a NUL byte, whereas printf(3p) does.
Fix the bug by reverting back to printf(3p) and culling the provided
length to `GIT_MAX_HEXSZ` to keep it from overflowing when cast to an
`int`.
Reported-by: Johannes Schindelin <Johannes.Schindelin@gmx.de>
Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
"Why would one want to run it in parallel?" I hear you ask. I am glad
you are curious, because a curious story is what it is, indeed.
The `GIT-VERSION-GEN` script is quite a pillar of Git's source code,
with most lines being unchanged for the past 15 years. Until the v2.48.0
release candidate cycle.
Its original purpose was to generate the version string and store it in
the `GIT-VERSION-FILE`.
This paradigm changed quite dramatically when support for building with
Meson was introduced. Most crucially, a38edab7c8 (Makefile: generate
doc versions via GIT-VERSION-GEN, 2024-12-06) changed the way the
documentation is built by using the `GIT-VERSION-GEN` file to write out
the `asciidocor-extensions.rb` and `asciidoc.conf` files with now
hard-coded version strings.
Crucially, the Makefile rule to generate those files need to be run
in every build because `GIT_VERSION` could have been specified, which
would require these files to be changed.
This introduced a surprising race condition!
And this is how that race surfaces: When calling `make -j2 html man`
from the top-level directory (a variant of which is invoked in Git for
Windows' release process), two sub-processes are spawned, a `make -C
Documentation html` one and a `make -C Documentation man` one. Both run
the rule to (re-)generate `asciidoctor-extensions.rb` or
`asciidoc.conf`, invoking `GIT-VERSION-GEN` to do so. That script first
generates a temporary file (appending the `+` character to the
filename), then looks whether it contains something different than the
already existing file (if it exists, that is), and either replaces it if
needed, or removes the temporary file. If one of the two parallel
invocations removes that temporary file before the other can compare it,
or even worse: if one tries to replace the target file just after the
other _started_ writing the temporary file (but did not finish), that
race condition now causes bad builds.
This may sound highly theoretical, but due to Git's choices, Git for
Windows is forced to use a (slow) POSIX emulation layer to run that
script and in the blink of an eye it becomes very much not theoretical
at all. See these failed GitHub workflow runs as Exhibit A:
https://github.com/git-for-windows/git-sdk-32/actions/runs/12663456654https://github.com/git-for-windows/git-sdk-32/actions/runs/12683174970https://github.com/git-for-windows/git-sdk-64/actions/runs/12649348496
While it is undesirable to run this script over and over again,
certainly when this involves above-mentioned slow POSIX emulation layer,
the stage of the release cycle in which we are presently finding
ourselves dictates that a quick and reliable work-around be implemented
that works around the race condition without changing the overall
architecture of the build process.
This patch does that: By using a filename suffix for the temporary file
that includes the currently-executing script's process ID, We guarantee
that the two competing invocations cannot overwrite or remove each
others' temporary files.
Incidentally, this also fixes something else: The `+` character is
not even a valid filename character on Windows. The only reason why Git
for Windows did not need this is that above-mentioned POSIX emulation
layer also plays a couple of tricks with filenames (tricks that are not
interoperable with regular Windows programs, though), and previous
attempts to remedy this in git/git were unsuccessful, see e.g.
https://lore.kernel.org/git/pull.216.git.gitgitgadget@gmail.com/
This commit fixes one of the issues that are currently delaying Git for
Windows v2.48.0-rc2.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
In 6411a0a896 (builtin/blame: fix type of `length` variable when
emitting object ID, 2024-12-06) we have fixed the type of the `length`
variable. In order to avoid a cast from `size_t` to `int` in the call to
printf(3p) with the "%.*s" formatter we have converted the code to
instead use fwrite(3p), which accepts the length as a `size_t`.
It was reported though that this makes us read over the end of the OID
array when the provided `--abbrev=` length exceeds the length of the
object ID. This is because fwrite(3p) of course doesn't stop when it
sees a NUL byte, where as printf(3p) does.
Fix the bug by reverting back to printf(3p) and culling the provided
length to keep it from overflowing when cast to an `int`.
Note that when calling `git blame --abbrev=<N>` with an `N` that is
larger than the maximal OID hex size, there is a subtle side effect that
makes it behave _differently_ than specifying said maximal hex size:
When the command outputs boundary, unblamable or ignored commits' OIDs,
those outputs are prefixed with characters indicating this, and the
`abbrev` value is used to align the information that comes after the
OID, clipping it as needed. Specifying a "too large" abbrev value here
tells Git that yes, we want the full OIDs and don't you worry about
alignment.
Thanks to SHA-256 being _larger_ than the default SHA-1-based OIDs, and
thanks to clipping at `GIT_MAX_HEXSZ`, this change of behavior can only
be observed when running the test in SHA-256 mode.
Nevertheless, this means that we cannot cull at `GIT_MAX_HEXSZ` but at a
slightly larger threshold value.
This patch is a slightly modified version of the one sent as
https://lore.kernel.org/git/20250109-b4-pks-blame-truncate-hash-length-v1-1-9ad4bb09e059@pks.im
to the Git mailing list.
Reported-by: Johannes Schindelin <Johannes.Schindelin@gmx.de>
Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
The build procedure in "meson" for the "perl/" hierarchy lacked
necessary dependencies, which has been corrected.
* sj/meson-perl-build-fix:
meson: fix perl dependencies
As indicated by the `#undef malloc` line in `reftable/basics.h`, it is
quite common to use allocators other than the default one by defining
`malloc` constants and friends.
This pattern is used e.g. in Git for Windows, which uses the powerful
and performant `mimalloc` allocator.
Furthermore, in `reftable/basics.c` this `#undef malloc` is
_specifically_ disabled by virtue of defining the
`REFTABLE_ALLOW_BANNED_ALLOCATORS` constant before including
`reftable/basic.h`, to ensure that such a custom allocator is also used
in the reftable code.
However, in 8db127d43f (reftable: avoid leaks on realloc error,
2024-12-28) and in 2cca185e85 (reftable: fix allocation count on
realloc error, 2024-12-28), `reftable_set_alloc()` function calls were
introduced that pass `malloc`, `realloc` and `free` function pointers as
parameters _after_ `reftable/basics.h` ensured that they were no longer
`#define`d. This would override the custom allocator and re-set it to
the default allocator provided by, say, libc or MSVCRT.
This causes problems because those calls happen after the initial
allocator has already been used to initialize an array, which is
subsequently resized using the overridden default `realloc()` allocator.
You cannot mix and match allocators like that, which leads to a
`STATUS_HEAP_CORRUPTION` (C0000374) on Windows, and when running this
unit test through shell and/or `prove` (which only support 7-bit status
codes), it surfaces as exit code 127.
It is actually unnecessary to use those function pointers to
`malloc`/`realloc`/`free`, though: The `reftable` code goes out of its
way to fall back to the initial allocator when passing `NULL` parameters
instead. So let's do that instead of causing heap corruptions.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Acked-by: René Scharfe <l.s.r@web.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
`generate_perl_command` needs `depends: [git_version_file]` and the uses
in top-level meson.build were fine, but the ones in perl/ weren't, causing
parallel build failures in some cases as GIT-BUILD-OPTIONS wasn't yet
available.
Signed-off-by: Sam James <sam@gentoo.org>
Acked-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Correct verb tense, add missing words, avoid double blank lines,
and rephrase things that don’t read well to me like “Turn this linkage
to relative paths”.
Signed-off-by: Kristoffer Haugsbakk <code@khaugsbakk.name>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Commit 2406bf5 (Win32: detect unix socket support at runtime,
2024-04-03) introduced a runtime detection for whether the operating
system supports unix sockets for Windows, but a mistake snuck into the
tests. When building and testing Git without NO_UNIX_SOCKETS we
currently skip t0301-credential-cache on Windows if unix sockets are
supported and run the tests if they aren't.
Flip that logic to actually work the way it was intended.
Signed-off-by: Matthias Aßhauer <mha1993@live.de>
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
The part about keeping the original error number hasn't been accurate since
commit c11f75c (mingw: make sure errno is set correctly when socket
operations fail, 2019-11-25) and the part about strerror() not knowing
about these errors is untrue since the previous commit.
Signed-off-by: Matthias Aßhauer <mha1993@live.de>
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
We map WSAGetLastError() errors to errno errors in winsock_error_to_errno(),
but the MSVC strerror() implementation only produces "Unknown error" for
most of them. Produce some more meaningful error messages in these
cases.
Our builds for ARM64 link against the newer UCRT strerror() that does know
these errors, so we won't change the strerror() used there.
The wording of the messages is copied from glibc strerror() messages.
Reported-by: M Hickford <mirth.hickford@gmail.com>
Signed-off-by: Matthias Aßhauer <mha1993@live.de>
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Originally introduced as `core.useBuiltinFSMonitor` in Git for Windows
and developed, improved and stabilized there, the built-in FSMonitor
only made it into upstream Git (after unnecessarily long hemming and
hawing and throwing overly perfectionist style review sticks into the
spokes) as `core.fsmonitor = true`.
In Git for Windows, with this topic branch, we re-introduce the
now-obsolete config setting, with warnings suggesting to existing users
how to switch to the new config setting, with the intention to
ultimately drop the patch at some stage.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
This topic branch re-adds the deprecated --stdin/-z options to `git
reset`. Those patches were overridden by a different set of options in
the upstream Git project before we could propose `--stdin`.
We offered this in MinGit to applications that wanted a safer way to
pass lots of pathspecs to Git, and these applications will need to be
adjusted.
Instead of `--stdin`, `--pathspec-from-file=-` should be used, and
instead of `-z`, `--pathspec-file-nul`.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
A fix for calling `vim` in Windows Terminal caused a regression and was
reverted. We partially un-revert this, to get the fix again.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
This patch introduces support to set special NTFS attributes that are
interpreted by the Windows Subsystem for Linux as file mode bits, UID
and GID.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
With this patch, Git for Windows works as intended on mounted APFS
volumes (where renaming read-only files would fail).
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
This is the recommended way on GitHub to describe policies revolving around
security issues and about supported versions.
Helped-by: Sven Strickroth <email@cs-ware.de>
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Git for Windows accepts pull requests; Core Git does not. Therefore we
need to adjust the template (because it only matches core Git's
project management style, not ours).
Also: direct Git for Windows enhancements to their contributions page,
space out the text for easy reading, and clarify that the mailing list
is plain text, not HTML.
Signed-off-by: Philip Oakley <philipoakley@iee.org>
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
These are Git for Windows' Git GUI and gitk patches. We will have to
decide at some point what to do about them, but that's a little lower
priority (as Git GUI seems to be unmaintained for the time being, and
the gitk maintainer keeps a very low profile on the Git mailing list,
too).
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Getting started contributing to Git can be difficult on a Windows
machine. CONTRIBUTING.md contains a guide to getting started, including
detailed steps for setting up build tools, running tests, and
submitting patches to upstream.
[includes an example by Pratik Karki how to submit v2, v3, v4, etc.]
Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
The Git project followed Git for Windows' lead and added their Code of
Conduct, based on the Contributor Covenant v1.4, later updated to v2.0.
We adapt it slightly to Git for Windows.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
The Git for Windows project has grown quite complex over the years,
certainly much more complex than during the first years where the
`msysgit.git` repository was abusing Git for package management purposes
and the `git/git` fork was called `4msysgit.git`.
Let's describe the status quo in a thorough way.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Reintroduce the 'core.useBuiltinFSMonitor' config setting (originally added
in 0a756b2a25 (fsmonitor: config settings are repository-specific,
2021-03-05)) after its removal from the upstream version of FSMonitor.
Upstream, the 'core.useBuiltinFSMonitor' setting was rendered obsolete by
"overloading" the 'core.fsmonitor' setting to take a boolean value. However,
several applications (e.g., 'scalar') utilize the original config setting,
so it should be preserved for a deprecation period before complete removal:
* if 'core.fsmonitor' is a boolean, the user is correctly using the new
config syntax; do not use 'core.useBuiltinFSMonitor'.
* if 'core.fsmonitor' is unspecified, use 'core.useBuiltinFSMonitor'.
* if 'core.fsmonitor' is a path, override and use the builtin FSMonitor if
'core.useBuiltinFSMonitor' is 'true'; otherwise, use the FSMonitor hook
indicated by the path.
Additionally, for this deprecation period, advise users to switch to using
'core.fsmonitor' to specify their use of the builtin FSMonitor.
Signed-off-by: Victoria Dye <vdye@github.com>
The `--stdin` option was a well-established paradigm in other commands,
therefore we implemented it in `git reset` for use by Visual Studio.
Unfortunately, upstream Git decided that it is time to introduce
`--pathspec-from-file` instead.
To keep backwards-compatibility for some grace period, we therefore
reinstate the `--stdin` option on top of the `--pathspec-from-file`
option, but mark it firmly as deprecated.
Helped-by: Victoria Dye <vdye@github.com>
Helped-by: Matthew John Cheetham <mjcheetham@outlook.com>
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
In e3f7e01b50 (Revert "editor: save and reset terminal after calling
EDITOR", 2021-11-22), we reverted the commit wholesale where the
terminal state would be saved and restored before/after calling an
editor.
The reverted commit was intended to fix a problem with Windows Terminal
where simply calling `vi` would cause problems afterwards.
To fix the problem addressed by the revert, but _still_ keep the problem
with Windows Terminal fixed, let's revert the revert, with a twist: we
restrict the save/restore _specifically_ to the case where `vi` (or
`vim`) is called, and do not do the same for any other editor.
This should still catch the majority of the cases, and will bridge the
time until the original patch is re-done in a way that addresses all
concerns.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
