setup-hooks/strip: uniqify files by inode number before stripping#314175
setup-hooks/strip: uniqify files by inode number before stripping#314175lheckemann merged 1 commit intoNixOS:stagingfrom
Conversation
|
Why not just move the symlinkification to before the stripping, like @puckipedia suggested? |
|
Mesa changes are going to need to go through staging anyway, and this does fix the root cause as also suggested. However, fixing Mesa might be easier to review. I'll investigate, we should quite possibly do both anyway. |
|
The way I understand this, is that possibly other store paths might also be affected by this, not just mesa, if they use hardlinks and stripping strips in parallell from two places. |
|
cc @trofi |
|
I built all the way up to Mesa. I did some instrumenting and there are several other packages with multiple hardlinks to strippable binaries that could break like Mesa:
I had store optimization disabled so these derivations are doing this themselves. Also did figure out that this patch as it is is wrong, force push coming once I do some more revision and testing. Didn't want to ping until after it was ready but guess that's already done. |
tpwrules
force-pushed
the
strip-unique-inodes
branch
from
60e3040 to
282bc36
Compare
NixOS#246164 but for hardlinks. Mesa, among other packages, has binaries that are linked together and can end up corrupted when the same binary is stripped through two different names. To resolve this, print out the device and inode number before each file name, sort/uniq based on that, then cut it back out before stripping. The symlink resolution logic is removed as the same file accessed through two different links in `$paths` will necessarily have the same numbers. File/directory within the paths listed in `$paths` are correctly not (and were never) processed due to the `-type f` predicate and (implied) `-P` option to `find`.
tpwrules
force-pushed
the
strip-unique-inodes
branch
from
282bc36 to
4d6d293
Compare
ofborg
bot
added
10.rebuild-darwin-stdenv
lheckemann
left a comment
lheckemann
left a comment
There was a problem hiding this comment.
I haven't built anything, but the diff looks good.
|
Successfully created backport PR for |
|
I wonder if it's worth sending this to 23.11 too, since we have at least a few more staging cycles to go in it. |
|
That would fix the issue in all stable releases which seems like a good thing. I would be happy to at least build such a patch on my machine. |
|
Successfully created backport PR for |
|
Backport failed for Please cherry-pick the changes locally and resolve any conflicts. git fetch origin staging-24.05
git worktree add -d .worktree/backport-314175-to-staging-24.05 origin/staging-24.05
cd .worktree/backport-314175-to-staging-24.05
git switch --create backport-314175-to-staging-24.05
git cherry-pick -x 4d6d293fad1a84b953e6decb32959425fb9d2043 |
8 participants
Description of changes
Mesa, among other packages, has binaries that are linked together and
can end up corrupted when the same binary is stripped through two
different names.
To resolve this, print out the device and inode number before each file
name, sort/uniq based on that, then cut it back out before stripping.
The symlink resolution logic is removed as the same file accessed
through two different links in
$pathswill necessarily have the samenumbers. File/directory within the paths listed in
$pathsarecorrectly not (and were never) processed due to the
-type fpredicateand (implied)
-Poption tofind.Things done
nix.conf? (See Nix manual)sandbox = relaxedsandbox = truenix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage./result/bin/)Add a 👍 reaction to pull requests you find important.