Skip to content

nixos/activation-script: Fix dependencies for dry activation#137508

Merged
ajs124 merged 1 commit intoNixOS:masterfrom
helsinki-systems:fix/dry-activation
Sep 14, 2021
Merged

nixos/activation-script: Fix dependencies for dry activation#137508
ajs124 merged 1 commit intoNixOS:masterfrom
helsinki-systems:fix/dry-activation

Conversation

@dasJ
Copy link
Member

@dasJ dasJ commented Sep 12, 2021

Motivation for this change

#136605 broke tools like agenix.
The comment in the code should pretty much explain why this fixes it.
For the simple test, this results in:

$ grep '#### Activation script snippet' dry-activate
#### Activation script snippet specialfs does not support dry activation.
#### Activation script snippet binfmt does not support dry activation.
#### Activation script snippet stdio does not support dry activation.
#### Activation script snippet binsh does not support dry activation.
#### Activation script snippet domain does not support dry activation.
#### Activation script snippet users:
#### Activation script snippet groups does not support dry activation.
#### Activation script snippet etc does not support dry activation.
#### Activation script snippet hostname does not support dry activation.
#### Activation script snippet modprobe does not support dry activation.
#### Activation script snippet nix does not support dry activation.
#### Activation script snippet udevd does not support dry activation.
#### Activation script snippet usrbinenv does not support dry activation.
#### Activation script snippet var does not support dry activation.
#### Activation script snippet wrappers does not support dry activation.
Things done
  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandbox = true set in nix.conf? (See Nix manual)
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • 21.11 Release Notes (or backporting 21.05 Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
    • (Module updates) Added a release notes entry if the change is significant
    • (Module addition) Added a release notes entry if adding a new NixOS module
  • Fits CONTRIBUTING.md.

@github-actions github-actions bot added 6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 8.has: module (update) This PR changes an existing module in `nixos/` labels Sep 12, 2021
@dasJ dasJ requested review from Mic92 and ryantm September 12, 2021 09:37
@ofborg ofborg bot added 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin. 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux. labels Sep 12, 2021
Copy link
Member

@ryantm ryantm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is a good solution, @dasJ! The only possible issue I see is if some dry activation script has a dependency that doesn't support dry activation, but the dry-activation script depends critically on the dependency running, which seems unlikely to me.

@dasJ
Copy link
Member Author

dasJ commented Sep 12, 2021

I don't think this is a relevant problem because when you get to dry-running anything you have already booted your system, hence all Activation Scripts were already run in non-dry mode. This means all things that I can think of (like setting the hostname, loading kernel modules, …) should have already been run at the point you get get the chance to do a dry activation

@dasJ dasJ requested review from Lassulus and ajs124 September 13, 2021 11:43
@ajs124 ajs124 merged commit 9c56624 into NixOS:master Sep 14, 2021
@ajs124 ajs124 deleted the fix/dry-activation branch September 14, 2021 13:55
@dasJ
Copy link
Member Author

dasJ commented Sep 16, 2021

@ryantm You should be able to remove your workaround: https://nixpk.gs/pr-tracker.html?pr=137508

ryantm added a commit to ryantm/agenix that referenced this pull request Sep 16, 2021
NixOS/nixpkgs#137508 should remove the need
for this.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 8.has: module (update) This PR changes an existing module in `nixos/` 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin. 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants