| Message ID | pull.1850.git.1736432663587.gitgitgadget@gmail.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | GIT-VERSION-GEN: allow it to be run in parallel | expand |
On Thu, 9 Jan 2025 at 15:24, Johannes Schindelin via GitGitGadget <gitgitgadget@gmail.com> 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/pull.216.git.gitgitgadget@gmail.com/ > - "$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
Martin Ågren <martin.agren@gmail.com> writes: >> ... attempts to remedy this in git/git were unsuccessful, see e.g. >> https://lore.kernel.org/git/pull.216.git.gitgitgadget@gmail.com/ > ... > 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.
diff --git a/GIT-VERSION-GEN b/GIT-VERSION-GEN index 6d1cb66d69a..5b49e2d72fb 100755 --- a/GIT-VERSION-GEN +++ b/GIT-VERSION-GEN @@ -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