Skip to content

Conversation

amozeo
Copy link
Contributor

@amozeo amozeo commented Mar 13, 2025

previously flake and buildingFile variables were used to deduce which method to use this originally came to that, because flake variable was used to check if script is building flake or module

after introduction of buildingFile variable, some parts got messy,
checking if building flake was with -z $flake, checking if building module was with -n $flake, and checking if building by attrset was with -n $buildingFile

using single variable to check which build method to use between two is fine, but when more methods are added, in my opinion it's better to use associative array

resolves issues pointed in #333788

Previously, nixos-rebuild -A foo would build the default.nix in the current directory, while nixos-rebuild without -A would not use file-based building at all.

This difference compared to what nix-build would do is confusing. Furthermore, using default.nix is not great, because there's many of those files all around, and they're almost never for NixOS.

issue for by-attrset build method quoted above is resolved in the following way:

  • use nixos-system entry in nix path
  • deprecate default.nix and warn about it
  • use system.nix

supercedes #333788

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/)
  • 25.05 Release Notes (or backporting 24.11 and 25.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.

Add a 👍 reaction to pull requests you find important.

@github-actions github-actions bot added the 6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS label Mar 13, 2025
@github-actions github-actions bot added 10.rebuild-darwin: 1-10 This PR causes between 1 and 10 packages to rebuild on Darwin. 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux. labels Mar 13, 2025
@nixos-discourse
Copy link

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/pinning-nixos-with-npins/63721/3

@nyabinary nyabinary requested a review from infinisil May 1, 2025 14:37
infinisil and others added 6 commits May 1, 2025 17:06
Otherwise we'd run into trouble when a filename contained e.g. a space
previously flake and buildingFile variables were used
to deduce which method to use
this originally came to that because flake variable
was used to check if script is building flake or module

after introduction of buildingFile variable, some parts got messy,
checking if building flake was with -z $flake,
checking if building module was with -n $flake,
and checking if building by attrset was with -n $buildingFile
don't forget about $noFlake in that mess

using single variable to check which build method to use
between two is fine, but when more methods are added,
in my opinion it's better to use associative array

using it also allows us easily check if user didn't use
arguments from different build methods at the same time
nixos-rebuild now detects by-attrset configurations differently.
The configuration file of by-attrset format is now detected
by system.nix filename instead of default.nix,
but manual didn't reflect that.
Use similar mechanism to what nixos-rebuild uses
to evaluate which configuration file, and format, to build
nixos-install now detects by-attrset configurations differently,
and uses system.nix filename instead of default.nix
@amozeo amozeo force-pushed the pkgs/nixos-rebuild/rework-logic branch from e8f4dba to 53a47a1 Compare May 1, 2025 15:09
@amozeo
Copy link
Contributor Author

amozeo commented May 1, 2025

I rebased onto current master, last time I forgot to remove old check that still used old $buildingAttribute variable

@github-actions github-actions bot added the 10.rebuild-darwin: 1 This PR causes 1 package to rebuild on Darwin. label May 1, 2025
@rnhmjoj
Copy link
Contributor

rnhmjoj commented May 6, 2025

It's probably too late for 25.05, but can we get things going so this PR can get into the next release?
I think that given the breaking nature of this change it needs a release note. Also the system.nix file should also be mentioned by NixOS manual. An example for how to pin NixOS instead of using a channel would be good too.

@nixpkgs-ci nixpkgs-ci bot added 2.status: merge conflict This PR has merge conflicts with the target branch and removed 2.status: merge conflict This PR has merge conflicts with the target branch labels Aug 13, 2025
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 10.rebuild-darwin: 1-10 This PR causes between 1 and 10 packages to rebuild on Darwin. 10.rebuild-darwin: 1 This PR causes 1 package 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.

4 participants