GIT-VERSION-GEN: allow it to be run in parallel

[dscho]

Changes since v1:

• Appended + again, to get the benefit of the .gitignore pattern that prevents the temporary files from being committed.

Cc: Patrick Steinhardt [email protected]
cc: Martin Ågren [email protected]

[dscho]

/submit

[gitgitgadget]

Submitted as [email protected]

To fetch this version into FETCH_HEAD:

git fetch https://github.com/gitgitgadget/git/ pr-1850/dscho/asciidoctor-extensions-gen-race-work-around-v1

To fetch this version to local tag pr-1850/dscho/asciidoctor-extensions-gen-race-work-around-v1:

git fetch --no-tags https://github.com/gitgitgadget/git/ tag pr-1850/dscho/asciidoctor-extensions-gen-race-work-around-v1

[gitgitgadget]

On the Git mailing list, Martin Ågren wrote (reply to this):

On Thu, 9 Jan 2025 at 15:24, Johannes Schindelin via GitGitGadget
<[email protected]> wrote:
> 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.

Nicely described. Indeed, there's a reason recursive make is considered
harmful. This is of course not the time or place for addressing that.

> 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]/

> -       "$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

Our `.gitignore` contains an entry "*+" to ignore this sort of temporary
files. Yes, they're supposed to disappear within a second or so, but
according to f9bbaa384e (Add intermediate build products to .gitignore,
2009-11-08), they can linger after interrupted builds. Maybe separate
tooling built around git could pick up these as untracked files for a
second, causing them to come and go in whatever GUI.

You could use "$OUTPUT"."$$"+ to restore this. That of course
invalidates your remark about "Incidentally, ..." above, but might give
this fix a tiny bit less chance of regressing something somewhere?

Martin

[gitgitgadget]

User Martin Ågren <[email protected]> has been added to the cc: list.

[gitgitgadget]

On the Git mailing list, Junio C Hamano wrote (reply to this):

Martin Ågren <[email protected]> writes:

>> ... attempts to remedy this in git/git were unsuccessful, see e.g.
>> https://lore.kernel.org/git/[email protected]/
> ...
> You could use "$OUTPUT"."$$"+ to restore this. That of course
> invalidates your remark about "Incidentally, ..." above, but might give
> this fix a tiny bit less chance of regressing something somewhere?

Thanks for being careful.

My reading of that old thread cited there tells me that the reason
that previous one failed was mostly because it wasn't being self
consistent and only touched the use of "+" in the Documentation
directory but not what the top-level Makefile did, and also because
it did not adjust .gitignore patterns, so it is good that somebody
actually read the cited thread and made sure this time we do better.

Again, I was not opposed to moving from "+" to something else that
is equally short-and-sweet, and I still am not ("~" is a fine suffix
for this kind of thing, for example).  But if we are aiming for a
short-term fix, I think your ".$$+" may make the most sense.

Thanks.

[gitgitgadget]

This patch series was integrated into seen via git@a883025.

[gitgitgadget]

This patch series was integrated into seen via git@631d441.