Skip to content

Conversation

@vtjnash
Copy link
Member

@vtjnash vtjnash commented Nov 7, 2025

Some mktemp only added -p argument recently for BSD vs. gnu consistency with other arguments, so use abspath instead to emulate it. Use it to make various file builds atomic (particularly those shared between release and debug).]

(Coded by Claude)

@vtjnash vtjnash added the building Build system, or building Julia or its dependencies label Nov 7, 2025
Comment on lines +156 to +166
@TMPFILE=$$(mktemp $(abspath $@.XXXXXX)); \
sed <'$<' >$$TMPFILE -e 's/@JULIA_SHLIB_SYMBOL_VERSION@/JL_LIBJULIA_$(SOMAJOR)/'; \
mv $$TMPFILE $@
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about we add a new helper script like this (created via ChatGPT, just to keep it interesting ;-)), stored as, say, atomic_write_if_different.sh somewhere we can easily reach it:

#!/bin/sh
# Atomic write-if-different helper
# Usage: atomic_write_if_different.sh TARGET

set -eu

target=$1
tmp=$(mktemp "${target}.XXXXXX") || exit 1

# Ensure temporary file gets cleaned up on exit or interrupt
cleanup() {
    rm -f "$tmp"
}
trap cleanup EXIT INT TERM HUP

# Read stdin to the temporary file
cat > "$tmp"

# If target does not exist, move new file into place
if [ ! -f "$target" ]; then
    mv "$tmp" "$target"
    trap - EXIT
    exit 0
fi

# Compare old and new; only replace if different
if cmp -s "$tmp" "$target"; then
    # identical
    rm -f "$tmp"
else
    mv "$tmp" "$target"
fi

trap - EXIT
exit 0

Then here we can write

Suggested change
@TMPFILE=$$(mktemp $(abspath $@.XXXXXX)); \
sed <'$<' >$$TMPFILE -e 's/@JULIA_SHLIB_SYMBOL_VERSION@/JL_LIBJULIA_$(SOMAJOR)/'; \
mv $$TMPFILE $@
sed <'$<' -e 's/@JULIA_SHLIB_SYMBOL_VERSION@/JL_LIBJULIA_$(SOMAJOR)/' \
| PATH/TO/atomic_write_if_different.sh $@

The result has more comprehensive functionality and is arguably easier to read and maintain -- I think this is more visible in some of the other places.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I do like that. We need to be careful though, since half of the places we use this, the "if different" policy should not be applied, while the other half it should be (and a few don't care)

fingolfin added a commit that referenced this pull request Nov 10, 2025
It is not supported in macOS 13 userland which causes CI failures. For
background info [see
here](#59980 (comment)).

This is a subset of PR #60080 but unlike that, it only solves the
specific issue causing trouble in CI right now, and does not attempt to
resolve a bunch of other things -- which may very well all be fine and
nice and desirable, but it's quite a bit more to review.
Use mktemp more to make various file builds atomic (particularly those
shared between release and debug).
@vtjnash vtjnash force-pushed the jn/makefile-mktemp branch from 6cf075d to 55d45cc Compare December 2, 2025 20:13
@vtjnash vtjnash added the merge me PR is reviewed. Merge when all tests are passing label Dec 2, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

building Build system, or building Julia or its dependencies merge me PR is reviewed. Merge when all tests are passing

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants