Skip to content
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 19 additions & 3 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -544,7 +544,15 @@ nix-build: nix-connect nix-trust ## Build Nix configuration.
HOST=runner HOSTNAME=runner $(NIX_ALLOW_UNFREE) $(NIX_EXEC) build .#$(NIX_CONFIG_TYPE).runner.system $(NIX_FLAGS) --impure --no-update-lock-file --show-trace; \
fi; \
elif [ "$(NIX_CONFIG_TYPE)" = "nixosConfigurations" ]; then \
HOST=runner HOSTNAME=runner $(NIX_ALLOW_UNFREE) $(NIX_EXEC) build .#nixosConfigurations.runner.config.system.build.toplevel $(NIX_FLAGS) --impure --no-update-lock-file --show-trace; \
if [ -n "$(HOST)" ]; then \
echo "Building named NixOS host: $(HOST)"; \
$(SUDO) env HOST=$(HOST) HOSTNAME=$(HOST) $(NIX_ALLOW_UNFREE) $(NIX_EXEC) run $(NIX_FLAGS) nixpkgs#nixos-rebuild -- build --flake .#$(HOST) --impure; \
elif [ -n "$(DETECTED_HOST)" ]; then \
echo "Auto-detected host: $(DETECTED_HOST)"; \
$(SUDO) env HOST=$(DETECTED_HOST) HOSTNAME=$(DETECTED_HOST) $(NIX_ALLOW_UNFREE) $(NIX_EXEC) run $(NIX_FLAGS) nixpkgs#nixos-rebuild -- build --flake .#$(DETECTED_HOST) --impure; \
else \
HOST=runner HOSTNAME=runner $(NIX_ALLOW_UNFREE) $(NIX_EXEC) build .#nixosConfigurations.runner.config.system.build.toplevel $(NIX_FLAGS) --impure --no-update-lock-file --show-trace; \
fi; \
Comment on lines +547 to +555

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

Inconsistency in CI NixOS Build Commands

There is a significant inconsistency in how NixOS configurations are built in CI:

  1. The Darwin CI branch (lines 528-545) and the Linux CI fallback branch (line 554) both use nix build .#nixosConfigurations.<host>.config.system.build.toplevel directly without sudo or nixos-rebuild.
  2. However, the newly added named and auto-detected host branches on Linux CI (lines 549 and 552) use $(SUDO) ... nixos-rebuild -- build via nix run nixpkgs#nixos-rebuild.

Using sudo and nixos-rebuild in CI for building (not switching) is problematic because:

  • Unnecessary Privileges: Building a Nix derivation does not require root privileges. Running build commands with sudo can cause permission issues in the workspace or build cache (e.g., root-owned files).
  • Overhead: Running nix run nixpkgs#nixos-rebuild downloads and evaluates nixos-rebuild from nixpkgs, adding unnecessary network dependency and overhead in CI.

Recommendation

Use nix build directly, matching the fallback and Darwin branches.

Note: Since the spec test spec/make_build_host_resolution_spec.sh (not in this diff) asserts on --flake .#matic, you will need to update it to expect .#nixosConfigurations.matic.config.system.build.toplevel instead.

			if [ -n "$(HOST)" ]; then \
				echo "Building named NixOS host: $(HOST)"; \
				HOST=$(HOST) HOSTNAME=$(HOST) $(NIX_ALLOW_UNFREE) $(NIX_EXEC) build .#nixosConfigurations.$(HOST).config.system.build.toplevel $(NIX_FLAGS) --impure --no-update-lock-file --show-trace; \
			elif [ -n "$(DETECTED_HOST)" ]; then \
				echo "Auto-detected host: $(DETECTED_HOST)"; \
				HOST=$(DETECTED_HOST) HOSTNAME=$(DETECTED_HOST) $(NIX_ALLOW_UNFREE) $(NIX_EXEC) build .#nixosConfigurations.$(DETECTED_HOST).config.system.build.toplevel $(NIX_FLAGS) --impure --no-update-lock-file --show-trace; \
			else \
				HOST=runner HOSTNAME=runner $(NIX_ALLOW_UNFREE) $(NIX_EXEC) build .#nixosConfigurations.runner.config.system.build.toplevel $(NIX_FLAGS) --impure --no-update-lock-file --show-trace; \
			fi; \

elif [ "$(NIX_CONFIG_TYPE)" = "homeConfigurations" ]; then \
if [ -n "$(HOST)" ]; then \
echo "Building named home config: $(HOST)"; \
Expand Down Expand Up @@ -689,8 +697,16 @@ nix-switch: ## Activate Nix configuration.
if [ "$(OS)" = "Darwin" ]; then \
$(SUDO) env CI="$$CI" IN_DOCKER="$$IN_DOCKER" HOST=runner HOSTNAME=runner $(NIX_ALLOW_UNFREE) $(DARWIN_REBUILD) switch --flake .#runner --impure --no-update-lock-file; \
elif [ "$(NIX_CONFIG_TYPE)" = "nixosConfigurations" ]; then \
echo "⏭️ NixOS switch skipped in CI as the runner is not a NixOS system"; \
$(SUDO) env HOST=runner HOSTNAME=runner $(NIX_ALLOW_UNFREE) $(NIX_EXEC) run $(NIX_FLAGS) --impure nixpkgs#nixos-rebuild -- switch --flake .#runner --no-update-lock-file || exit 0; \
if [ -n "$(HOST)" ]; then \
echo "Switching named host: $(HOST)"; \
$(SUDO) env HOST=$(HOST) HOSTNAME=$(HOST) $(NIX_ALLOW_UNFREE) $(NIX_EXEC) run $(NIX_FLAGS) --impure nixpkgs#nixos-rebuild -- switch --flake .#$(HOST) --no-update-lock-file || exit 0; \
elif [ -n "$(DETECTED_HOST)" ]; then \
echo "Auto-detected host: $(DETECTED_HOST)"; \
$(SUDO) env HOST=$(DETECTED_HOST) HOSTNAME=$(DETECTED_HOST) $(NIX_ALLOW_UNFREE) $(NIX_EXEC) run $(NIX_FLAGS) --impure nixpkgs#nixos-rebuild -- switch --flake .#$(DETECTED_HOST) --no-update-lock-file || exit 0; \

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Silent failure masking on real hosts in CI switch: The || exit 0 here is copied from the runner fallback (which needs it because CI runners aren't NixOS), but applying it to the HOST and DETECTED_HOST arms means a genuine nixos-rebuild switch failure on a named host will be swallowed and the Make recipe will still succeed. The non-CI equivalents at Makefile:743-748 deliberately don't do this. Not reachable today because ubuntu-latest can't switch NixOS anyway, but it becomes a real problem the moment a self-hosted NixOS runner (e.g., the Framework 13 matic machine this PR is enabling) runs make nix-switch. Consider dropping || exit 0 from the two HOST/DETECTED_HOST arms and keeping it only on the runner fallback.

else \
echo "⏭️ NixOS switch skipped in CI as the runner is not a NixOS system"; \
$(SUDO) env HOST=runner HOSTNAME=runner $(NIX_ALLOW_UNFREE) $(NIX_EXEC) run $(NIX_FLAGS) --impure nixpkgs#nixos-rebuild -- switch --flake .#runner --no-update-lock-file || exit 0; \
fi; \
elif [ "$(NIX_CONFIG_TYPE)" = "homeConfigurations" ]; then \
if [ "$$SKIP_HOME_MANAGER_SWITCH" = "true" ]; then \
echo "⏭️ Home-manager switch skipped (SKIP_HOME_MANAGER_SWITCH=true)"; \
Expand Down
Loading