-
Notifications
You must be signed in to change notification settings - Fork 143
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
Skip nix upgrade-nix
when Nix is installed in a nix profile
#622
Conversation
Also check that Nix can be upgraded before running `nix upgrade-nix` to work around a bug. See: <NixOS/nix#5473>
// Should we attempt to upgrade Nix with `nix upgrade-nix`? | ||
#[allow(unused_mut)] | ||
let mut should_self_upgrade = cfg!(target_os = "macos"); | ||
|
||
#[cfg(target_os = "linux")] | ||
{ | ||
// We can't use `nix upgrade-nix` on NixOS. | ||
if let Ok(Distribution::NixOS) = Distribution::detect() { | ||
should_self_upgrade = false; | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
// Should we attempt to upgrade Nix with `nix upgrade-nix`? | |
#[allow(unused_mut)] | |
let mut should_self_upgrade = cfg!(target_os = "macos"); | |
#[cfg(target_os = "linux")] | |
{ | |
// We can't use `nix upgrade-nix` on NixOS. | |
if let Ok(Distribution::NixOS) = Distribution::detect() { | |
should_self_upgrade = false; | |
} | |
} | |
// Should we attempt to upgrade Nix with `nix upgrade-nix`? | |
let mut should_self_upgrade = if cfg!(target_os = "macos") { | |
true | |
} else if cfg!(target_os = "linux") { | |
!matches!(Distribution::detect(), Distribution::NixOS) | |
} else { | |
unreachable!() | |
}; |
Maybe we can do this here, it has better readability, though the generated code looks bad:
// On Linux
let should = if false {
true
} else if true {
match Distribution::detect() {
Distribution::NixOS => true,
_ => false,
}
} else {
::core::panicking::panic("internal error: entered unreachable code")
};
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, I am sorry, I didn't realize that this Distribution
type is only defined on Linux, and since cfg!(target_os = "macos")
will be simply translated to a bool value, it requires that this type is also available on macOS, so it won't compile:(
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ahh, I'll revert.
28424c5
to
01087f6
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks
nix upgrade-nix
errors ifnix
is installed in anix profile
:Upstream issue: NixOS/nix#5473
This patch duplicates the relevant Nix logic and skips running
nix upgrade-nix
if this error would occur: https://github.com/NixOS/nix/blob/f0180487a0e4c0091b46cb1469c44144f5400240/src/nix/upgrade-nix.cc#L102-L139I've also split the
nix upgrade-nix
call into a separate step so we can use theSkipStep
error messages appropriately.Standards checklist:
CONTRIBUTING.md
cargo build
)cargo fmt
)cargo clippy
)cargo test
)For new steps
--dry-run
option works with this step--yes
option works with this step if it is supported bythe underlying command
If you developed a feature or a bug fix for someone else and you do not have the
means to test it, please tag this person here.