installShellFiles: rework and add installBin function #332612
Conversation
AndersonTorres
force-pushed
the
rework-installShellFiles
branch
from
11dcb7a
to
4495dcd
Compare
|
@afh I have taken your bump to make a test here. Can you review it? |
AndersonTorres
force-pushed
the
rework-installShellFiles
branch
2 times, most recently
from
2ff4e55
to
e88662f
Compare
ofborg
bot
added
10.rebuild-darwin-stdenv
ofborg
bot
requested review from
amiloradovsky,
adevress,
prusnak,
gebner,
bjornfor and
konimex
AndersonTorres
force-pushed
the
rework-installShellFiles
branch
from
e88662f
to
b6bfc32
Compare
github-actions
bot
added
6.topic: python
6.topic: nixos
AndersonTorres
force-pushed
the
rework-installShellFiles
branch
4 times, most recently
from
4d20c7e
to
ab65482
Compare
AndersonTorres
force-pushed
the
rework-installShellFiles
branch
2 times, most recently
from
381286f
to
55d3c48
Compare
So that it can be migrated to by-name later.
Rationale: Since RFCs 140 and 146, the old category-based hierarchy is deprecated and obsolete, and a new approach took place: packages should be as self-contained as possible. This paradigm is reflected in many new strict checks that prohibit a package to refer to files outside its directory tree. Following this spirit, this commit essentially moves nixpkgs pkgs/test/default.nix to ./tests/default.nix. Further, to keep the top-level `tests` attribute, a green alias is kept in the place of older file.
It makes the maintenance and creation of tests more intuitive: just throw a Nix package under tests!
- Use FUNCNAME to track the name of function being called - use long options for install commands - use nix*Log functions for logging, instead of custom echoes - remove silent error construction `|| return`
AndersonTorres
force-pushed
the
rework-installShellFiles
branch
from
55d3c48
to
e7ffc66
Compare
AndersonTorres
force-pushed
the
rework-installShellFiles
branch
from
e7ffc66
to
1709b4d
Compare
Since Ericson2314 is the original codeowner of the files.
AndersonTorres
force-pushed
the
rework-installShellFiles
branch
from
1709b4d
to
618cffa
Compare
|
@philiptaron you commented about using @infinisil i want to ask your opinion about a specific decision. As I have written in f72e74d:
What do you think? Is this a good approach? |
That looks great to me, nice work! |
| passthru = { | ||
| tests = lib.packagesFromDirectoryRecursive { | ||
| inherit callPackage; | ||
| directory = ./tests; | ||
| }; |
There was a problem hiding this comment.
This is not worth addressing here, but note that this has a minor problem: Due to packagesFromDirectoryRecursive recursing for directories, a standard nix-build -A installShellFiles.tests wouldn't run any nested ones, because nix-build doesn't recurse more than one level. In my experience, recursive attribute sets give rise to various such oddities, which is why I recommend avoiding them. Cc @philiptaron
There was a problem hiding this comment.
Good to know! There is likely need then for a simple non-recursive variant for these flat use cases.
Description of changes
nawk for testing purposes
The rationales of this PR are better understood by reading the commit messages, but an executive resume is:
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.