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

boot/activation: move /run/booted-system creation into actiovation sc… #265866

Closed
wants to merge 1 commit into from

Conversation

SuperSandro2000
Copy link
Member

@SuperSandro2000 SuperSandro2000 commented Nov 6, 2023

…ript

to be easily accessible for different boot style systems like NixOS-WSL. Since the creation is earlier as before this should be save with no regressions.

Description of changes

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.

…ript

to be easily accessible for different boot style systems like NixOS-WSL.
Since the creation is earlier as before this should be save with no
regressions.
Copy link
Member

@roberth roberth left a comment

Choose a reason for hiding this comment

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

Easier perhaps, but less precise. How bad is the alternative?

@dasJ dasJ requested a review from nikstur November 6, 2023 18:23
@SuperSandro2000
Copy link
Member Author

I don't have an idea for alternatives tbh.

@roberth
Copy link
Member

roberth commented Dec 10, 2023

The best alternative is to keep it as is.
Setting the symlink is a responsibility of stage 2, and not the activation script.
WSL is a stage 2, among other things, so it must implement this.
I suppose you could refactor stage 2 into a library + program?

@SuperSandro2000
Copy link
Member Author

Setting the symlink is a responsibility of stage 2, and not the activation script.

Why though? Shouldn't we do as much as late as possible to have the full environment already?


Yeah, having some library around those things would allow us to activate what we need without needing to copy paste things.

@roberth
Copy link
Member

roberth commented Dec 10, 2023

Setting the symlink is a responsibility of stage 2, and not the activation script.

Why though? Shouldn't we do as much as late as possible to have the full environment already?

Partly Chesterton's fence, but also shouldn't /run/booted-system be set as early as possible, ie in stage 2? This step is part of setting up the environment rather than consuming it, so doing it asap seems best.

@wegank wegank added the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Jul 4, 2024
@SuperSandro2000 SuperSandro2000 deleted the booted-system branch July 9, 2024 14:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants