patchShebangs: preserve times, resolves #33084 #33281
Conversation
| @@ -54,7 +54,11 @@ patchShebangs() { | |||
| echo "$f: interpreter directive changed from \"$oldInterpreterLine\" to \"$newInterpreterLine\"" | |||
| # escape the escape chars so that sed doesn't interpret them | |||
| escapedInterpreterLine=$(echo "$newInterpreterLine" | sed 's|\\|\\\\|g') | |||
| # Preserve times, see: https://github.com/NixOS/nixpkgs/pull/33084 | |||
There was a problem hiding this comment.
Golfing: mtime=$(stat -c '%Y' "$f"); whatever; touch -d @$mtime "$f" (I think that does the same thing, works with GNU, at least ...)
There was a problem hiding this comment.
It won't, it doesn't preserve nanoseconds.
There was a problem hiding this comment.
That's a good idea Just to local mtime=... so that it doesn't pollute the global scope.
edit oh wrote that before the nanoseconds. Bummer.
There was a problem hiding this comment.
It seems error-prone to convert %y format into format accepted by touch. If local suffix is problematic, mktemp could be used in beginning of patchShebangs to reserve a unique file name in /tmp and then that one file can be used as a buffer.
There was a problem hiding this comment.
On my end, touch -d "$(stat -c '%y' ...)" ... works as I expect, in that the mtimes are identical, to the nanosecond, without the need for conversion. Same for touch -d @$(stat -c '%.Y'). Perhaps these are GNU-isms, not to be relied on?
There was a problem hiding this comment.
Do we have these options in the bootstrapping phase (in case this setup-hook is used there)?
There was a problem hiding this comment.
touch -d @$(stat -c '%Y' README.md) default.nix
in the nixpkgs repo also works with busybox. There is no posix version of the stat command so there is no standard set of flags we would have to rely on.
There was a problem hiding this comment.
We're not restricted to POSIX, since stdenv supplies coreutils.
|
I found |
| @@ -54,7 +54,11 @@ patchShebangs() { | |||
| echo "$f: interpreter directive changed from \"$oldInterpreterLine\" to \"$newInterpreterLine\"" | |||
| # escape the escape chars so that sed doesn't interpret them | |||
| escapedInterpreterLine=$(echo "$newInterpreterLine" | sed 's|\\|\\\\|g') | |||
| # Preserve times, see: https://github.com/NixOS/nixpkgs/pull/33084 | |||
| cp -p "$f" "$f.tmp" | |||
There was a problem hiding this comment.
Making a full copy of the file, just to store its mtime, seems very inefficient to me.
Can't you do touch -r "$f" "$f.tmp"?
There was a problem hiding this comment.
That file does not exists before cp -> sed edit $f inplace.
Close NixOS#33281. Edits by vcunat: - use Eelco's idea: empty file instead of full copy - use longer name suffix to decrease the likelihood of collision
|
Staged 4b1b6ee . |
Motivation for this change
See #33084 (comment). Please test: I haven't.
/cc @johbo
Things done
build-use-sandboxinnix.confon non-NixOS)nix-shell -p nox --run "nox-review wip"./result/bin/)