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

detect-hostname-change: improve installer detection #601

Merged
merged 1 commit into from
Jan 21, 2025

Conversation

Enzime
Copy link
Member

@Enzime Enzime commented Jan 19, 2025

Currently the detection fails if we're trying to do an installation from a NixOS system or installer, the only time I can see that check working is if someone was trying to run nixos-install directly from a non-NixOS Linux distro which would also require they have Nix installed which would be super uncommon

@zowoq
Copy link
Contributor

zowoq commented Jan 19, 2025

The disko installTest uses machine as the initial hostname, should that be added here or add NIXOS_NO_CHECK in disko?

# Ignore if the system is getting installed
if [[ ! -e /run/current-system ]]; then
# https://github.com/nix-community/nixos-images/blob/2fc023e024c0a5e8e98ae94363dbf2962da10886/nix/installer.nix#L12-L13
if [[ ! -e /run/current-system || "$actual" == "nixos" || "$actual" == "nixos-installer" ]]; then
Copy link
Member

Choose a reason for hiding this comment

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

Why are we including nixos here?

Copy link
Member Author

Choose a reason for hiding this comment

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

By default the NixOS installer ISOs use nixos as the hostname

Copy link
Member

Choose a reason for hiding this comment

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

That's not specific to the installer, but just the default for networking.hostName if unset.

So if your goal here is to check for installers, it's probably preferable to check /etc/os-release for VARIANT_ID as nixos-anywhere does. Or an other method which isn't as likely to yield false matches.

If it's even needed, the check is easy to disable during install via NIXOS_NO_CHECK

I proposed that for nixos-anywhere in nix-community/nixos-anywhere#447

Copy link
Member Author

Choose a reason for hiding this comment

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

We can't check /etc/os-release because this runs as part of activation which runs inside the nixos-enter chroot

Instead after some testing I realized that /run/booted-system won't exist when installing as it gets created by stage 2 normally

I've also removed the nixos hostname check

@zowoq zowoq mentioned this pull request Jan 19, 2025
@Enzime Enzime force-pushed the improve-installer-detection branch 2 times, most recently from 8141ef1 to 291a502 Compare January 21, 2025 03:14
@Enzime Enzime force-pushed the improve-installer-detection branch from 291a502 to 3351e1b Compare January 21, 2025 04:34
@phaer phaer merged commit a3bf377 into nix-community:main Jan 21, 2025
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants