-
-
Notifications
You must be signed in to change notification settings - Fork 5.7k
make: use older mktemp, more #60080
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
make: use older mktemp, more #60080
Conversation
| @TMPFILE=$$(mktemp $(abspath $@.XXXXXX)); \ | ||
| sed <'$<' >$$TMPFILE -e 's/@JULIA_SHLIB_SYMBOL_VERSION@/JL_LIBJULIA_$(SOMAJOR)/'; \ | ||
| mv $$TMPFILE $@ |
There was a problem hiding this comment.
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 0Then here we can write
| @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.
There was a problem hiding this comment.
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)
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).
6cf075d to
55d45cc
Compare
Some mktemp only added -p argument recently for BSD vs. gnu consistency with other arguments, so use
abspathinstead to emulate it. Use it to make various file builds atomic (particularly those shared between release and debug).](Coded by Claude)