Skip to content

Commit

Permalink
GIT-VERSION-GEN: allow it to be run in parallel
Browse files Browse the repository at this point in the history
"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, a38edab (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/12663456654
https://github.com/git-for-windows/git-sdk-32/actions/runs/12683174970
https://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/[email protected]/

This commit fixes one of the issues that are currently delaying Git for
Windows v2.48.0-rc2.

Signed-off-by: Johannes Schindelin <[email protected]>
  • Loading branch information
dscho authored and Git for Windows Build Agent committed Jan 9, 2025
1 parent 3397477 commit 87fab0d
Showing 1 changed file with 4 additions and 4 deletions.
8 changes: 4 additions & 4 deletions GIT-VERSION-GEN
Original file line number Diff line number Diff line change
Expand Up @@ -86,11 +86,11 @@ sed -e "s|@GIT_VERSION@|$GIT_VERSION|" \
-e "s|@GIT_BUILT_FROM_COMMIT@|$GIT_BUILT_FROM_COMMIT|" \
-e "s|@GIT_USER_AGENT@|$GIT_USER_AGENT|" \
-e "s|@GIT_DATE@|$GIT_DATE|" \
"$INPUT" >"$OUTPUT"+
"$INPUT" >"$OUTPUT".$$

if ! test -f "$OUTPUT" || ! cmp "$OUTPUT"+ "$OUTPUT" >/dev/null
if ! test -f "$OUTPUT" || ! cmp "$OUTPUT".$$ "$OUTPUT" >/dev/null
then
mv "$OUTPUT"+ "$OUTPUT"
mv "$OUTPUT".$$ "$OUTPUT"
else
rm "$OUTPUT"+
rm "$OUTPUT".$$
fi

0 comments on commit 87fab0d

Please sign in to comment.