nixos/activation: allow quoted values in /etc/os-release #163320
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
github-actions
bot
added
6.topic: nixos
Member
Author
|
Looping in everyone who was involved in the referenced PR: @aanderse @davidak @andrevmatos |
dasJ
approved these changes
Mar 8, 2022
Member
dasJ
left a comment
dasJ
left a comment
There was a problem hiding this comment.
If there's no strong opinions against merging this I'll do so today or tomorrow since the fix feels important
Member
Author
|
It's not super urgent, as a NixOS installation would have /etc/NIXOS so this check would only be hit in that if missing.
|
ofborg
bot
added
10.rebuild-darwin: 0
andrevmatos
reviewed
Mar 8, 2022
| # /etc/os-release. | ||
| die "This is not a NixOS installation!\n" unless | ||
| -f "/etc/NIXOS" || (read_file("/etc/os-release", err_mode => 'quiet') // "") =~ /ID=nixos/s; | ||
| -f "/etc/NIXOS" || (read_file("/etc/os-release", err_mode => 'quiet') // "") =~ /ID="?nixos"?/s; |
Member
There was a problem hiding this comment.
Maybe we could broaden the NIXOS check?
Suggested change
| -f "/etc/NIXOS" || (read_file("/etc/os-release", err_mode => 'quiet') // "") =~ /ID="?nixos"?/s; | |
| -e "/etc/NIXOS" || (read_file("/etc/os-release", err_mode => 'quiet') // "") =~ /ID="?nixos"?/s; |
Member
Author
|
I understand you have it symlinked to /dev/null and it therefore fails the -f check (presumable -d would work in your case), but what is the actual *reason* for supporting something that points to either a !file or a missing file?
|
peterhoeg
changed the title
Member
|
No strong reasons, just thought that a verification check could always be widened if it doesn't break anything, but no problem if it's not changed, already updated my installation to |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description of changes
After #162168 was merged, the value of the ID key in /etc/os-release is now quoted, so change our activation check to also allow quoted values.
Things done
sandbox = trueset innix.conf? (See Nix manual)nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage./result/bin/)nixos/doc/manual/md-to-db.shto update generated release notes