Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

nixos: replace activationScripts 2/x #267983

Merged
merged 13 commits into from
Dec 29, 2023

Conversation

nikstur
Copy link
Contributor

@nikstur nikstur commented Nov 16, 2023

Description of changes

Replace some more relatively easy to replace activationScripts. This is the second PR of the series started here: #263203

It's part of the broader "Perlless Activation" endeavour: #267982

Things done

  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandboxing enabled in nix.conf? (See Nix manual)
    • sandbox = relaxed
    • sandbox = true
  • Tested, as applicable:
  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
  • Tested basic functionality of all binary files (usually in ./result/bin/)
  • 23.11 Release Notes (or backporting 23.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.

@nikstur nikstur requested a review from dasJ as a code owner November 16, 2023 23:34
@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 Nov 16, 2023
philiptaron

This comment was marked as off-topic.

@philiptaron
Copy link
Contributor

I misclicked on a review. I'm going to actually review this...

(Side note: I hate that I can't review individual commits.)

@nikstur nikstur mentioned this pull request Nov 28, 2023
13 tasks
Copy link
Contributor

@philiptaron philiptaron left a comment

Choose a reason for hiding this comment

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

Some curiosities, but this looks good to me.

nixos/modules/config/ldap.nix Show resolved Hide resolved
nixos/modules/config/ldap.nix Show resolved Hide resolved
nixos/modules/config/nix-channel.nix Show resolved Hide resolved
nixos/modules/security/ipa.nix Show resolved Hide resolved
nixos/modules/security/ipa.nix Show resolved Hide resolved
nixos/modules/services/networking/yggdrasil.nix Outdated Show resolved Hide resolved
nixos/modules/tasks/trackpoint.nix Outdated Show resolved Hide resolved
nixos/modules/virtualisation/vmware-host.nix Outdated Show resolved Hide resolved
nixos/tests/nextcloud/basic.nix Outdated Show resolved Hide resolved
@nikstur nikstur force-pushed the replace-simple-activation-2 branch 2 times, most recently from 266bb93 to 7de0090 Compare November 30, 2023 00:35
Copy link
Member

@SuperSandro2000 SuperSandro2000 left a comment

Choose a reason for hiding this comment

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

would have love to give some deeper feedback but the changes are a bit tricky for me :/

nixos/modules/hardware/video/amdgpu-pro.nix Outdated Show resolved Hide resolved
nixos/tests/nextcloud/basic.nix Outdated Show resolved Hide resolved
Replace with a dedicated system servie ordered before the other VMWare
services.
Replace with a dedicated systemd service.
Replace with a separate systemd service ordered before sysinit.target
Replace with separate service because it cannot be moved into the
preStart of the yggdrasil service.
Replace with a seprate systemd service
Replaced with a dedicated systemd service.
@nikstur
Copy link
Contributor Author

nikstur commented Dec 29, 2023

@ofborg test yggdrasil nextcloud.basic26 wrappers borgbackup incron

@nikstur
Copy link
Contributor Author

nikstur commented Dec 29, 2023

I added shutdown.target dependencies to the sysinit services to be aligned with #271326 and converted the nextcloud service to tmpfiles.

@nikstur nikstur merged commit 5a9c0b7 into NixOS:master Dec 29, 2023
22 of 23 checks passed
Copy link
Contributor

@philiptaron philiptaron left a comment

Choose a reason for hiding this comment

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

So glad to see this land.

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: 1-10 10.rebuild-linux: 1-10
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants