symlinkJoin: ability to strip prefix from tree#345127
symlinkJoin: ability to strip prefix from tree#345127philiptaron merged 2 commits intoNixOS:masterfrom
Conversation
90deff2 to
ae2d4d1
Compare
philiptaron
left a comment
There was a problem hiding this comment.
Thanks for your contribution, Yaroslav (@CertainLach).
I'm requesting changes for three things.
- Documentation of the new behavior.
- Tests of the facility.
- Good errors from the new assert.
Of these, I care the most about the tests. 🙂
I appreciate the research you did to motivate adding this new functionality to symlinkJoin.
There was a problem hiding this comment.
Now that we're adding in something more than just a simple for loop, I think this needs a test.
See the file pkgs/build-support/trivial-builders/test/default.nix for the set of tests.
Run the tests using nix-build -A tests.trivial-builders.symlinkJoin.
Take a look at pkgs/test/replace-vars/default.nix for an example of how to write those tests. testers.testEqualContents in particular will be useful, and testBuildFailure for demonstrating what happens when the derivation ought to error.
That'll give you the ability to do demonstrate the following cases:
- Direct, normal use sans any path stripping (maybe with a nested list, per the recursive call).
- Build failure due to a missing path.
- Path stripping with missing paths
- Path stripping with failures due to other reasons that are skipped.
There was a problem hiding this comment.
Path stripping with failures due to other reasons that are skipped.
Error skipping behavior was unintentional, it was added to make command not error, i.e test -d nonexisting && lndir ... would fail because of test -d exit code, and test -d ... && lndir ... || true would fix that error code.
Changed to if test -d ...; then instead. This way the code is even more ugly, but more correct.
It will skip those paths, whose prefix references some file (This test was added too), but should not ignore other I/O errors.
ae2d4d1 to
28239f2
Compare
Allows it to use symlinkJoin for dropin paths like sysctl.d
28239f2 to
c213c2f
Compare
Currently, symlinkJoin always joins packages at its roots. It is possible to just suffix every package with wanted path to avoid that, but this causes an error due to missing path (If there is one
packagesoption, but multiple things that you want to extract from those packages)We already have uses for similar functionality in tree, but it it always duplicated, e.g
nixpkgs/nixos/modules/services/hardware/udev.nix
Lines 52 to 59 in 818089b
The patch is a little bit ugly to avoid causing mass-rebuilds, it would be nice to make it prettier during one.
Required for Qubes: #341215
Description of changes
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.