feat: enhance Nix build and switch commands to support named and auto-detected hosts - #715
Conversation
|
Caution Review failedThe pull request is closed. 📝 WalkthroughSummary by CodeRabbit
WalkthroughThe pull request modifies the Makefile to add host-aware routing logic for nixosConfigurations builds and switches, conditionally selecting targets based on HOST or DETECTED_HOST variables. Additionally, the dotagents submodule pointer is updated to a newer commit. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Possibly related PRs
Poem
✨ Finishing touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Summary of ChangesHello @shunkakinoki, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly improves the flexibility and clarity of managing NixOS configurations by introducing advanced host selection capabilities for both building and activating systems. It allows users to specify a target host, leverage auto-detection, or default to the system's configured host, ensuring a more streamlined and transparent workflow for multi-host Nix environments. Highlights
Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
Mesa DescriptionTL;DREnhanced Nix build and switch commands to support named and auto-detected hosts. What changed?
Description generated by Mesa. Update settings |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request enhances the nix-build and nix-switch Makefile targets to support named and auto-detected hosts, which is a great improvement for flexibility. The implementation, however, introduces significant code duplication in both targets. My review includes suggestions to refactor this logic by using a shell variable to store the target host, which simplifies the code and improves maintainability. The same refactoring pattern is applicable to both nix-build and nix-switch targets.
| if [ -n "$(HOST)" ]; then \ | ||
| echo "Building named host: $(HOST)"; \ | ||
| sudo $(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 $(NIX_ALLOW_UNFREE) $(NIX_EXEC) run $(NIX_FLAGS) nixpkgs#nixos-rebuild -- build --flake .#$(DETECTED_HOST) --impure; \ | ||
| else \ | ||
| sudo $(NIX_ALLOW_UNFREE) $(NIX_EXEC) run $(NIX_FLAGS) nixpkgs#nixos-rebuild -- build --flake .#$(NIX_SYSTEM) --impure; \ | ||
| fi; \ |
There was a problem hiding this comment.
This if/elif/else block contains duplicated code. The sudo nixos-rebuild command is repeated in each branch with only a different host. To improve maintainability and reduce redundancy, you can determine the host to use in a shell variable first, and then execute the command once with that variable.
_host_to_build="$(NIX_SYSTEM)"; \
if [ -n "$(HOST)" ]; then \
echo "Building named host: $(HOST)"; \
_host_to_build="$(HOST)"; \
elif [ -n "$(DETECTED_HOST)" ]; then \
echo "Auto-detected host: $(DETECTED_HOST)"; \
_host_to_build="$(DETECTED_HOST)"; \
fi; \
sudo $(NIX_ALLOW_UNFREE) $(NIX_EXEC) run $(NIX_FLAGS) nixpkgs#nixos-rebuild -- build --flake .#$$_host_to_build --impure; \
| if [ -n "$(HOST)" ]; then \ | ||
| echo "Switching named host: $(HOST)"; \ | ||
| sudo $(NIX_ALLOW_UNFREE) $(NIX_EXEC) run $(NIX_FLAGS) --impure nixpkgs#nixos-rebuild -- switch --flake .#$(HOST); \ | ||
| elif [ -n "$(DETECTED_HOST)" ]; then \ | ||
| echo "Auto-detected host: $(DETECTED_HOST)"; \ | ||
| sudo $(NIX_ALLOW_UNFREE) $(NIX_EXEC) run $(NIX_FLAGS) --impure nixpkgs#nixos-rebuild -- switch --flake .#$(DETECTED_HOST); \ | ||
| else \ | ||
| sudo $(NIX_ALLOW_UNFREE) $(NIX_EXEC) run $(NIX_FLAGS) --impure nixpkgs#nixos-rebuild -- switch --flake .#$(NIX_SYSTEM); \ | ||
| fi; \ |
There was a problem hiding this comment.
Similar to the nix-build target, this block for nix-switch also has significant code duplication. The sudo nixos-rebuild command is repeated across branches. Refactoring this to use a shell variable for the host will make the code cleaner and easier to maintain.
_host_to_switch="$(NIX_SYSTEM)"; \
if [ -n "$(HOST)" ]; then \
echo "Switching named host: $(HOST)"; \
_host_to_switch="$(HOST)"; \
elif [ -n "$(DETECTED_HOST)" ]; then \
echo "Auto-detected host: $(DETECTED_HOST)"; \
_host_to_switch="$(DETECTED_HOST)"; \
fi; \
sudo $(NIX_ALLOW_UNFREE) $(NIX_EXEC) run $(NIX_FLAGS) --impure nixpkgs#nixos-rebuild -- switch --flake .#$$_host_to_switch; \
![mesa-dot-dev[bot]](https://avatars.githubusercontent.com/in/1050077?s=60&v=4){kind=small}
There was a problem hiding this comment.
Performed full review of 0c79f76...bee485f
Analysis
-
Inconsistent implementation patterns between
homeConfigurationsandnixosConfigurationswith different approaches for host selection could create maintenance confusion -
Lack of error handling and validation for host configurations - no checks if specified HOST exists or contains valid identifiers, leading to potential cryptic failures rather than clear error messages
-
Informational echo statements may interfere with CI/CD pipelines, scripts, or automated tools that parse stdout without proper stderr redirection
-
Missing documentation for the HOST variable - its purpose, expected values, relationship to other variables, and usage examples are not explained in the code
-
Duplicated host resolution logic could be extracted into a dedicated variable to improve maintainability
Tip
Help
Slash Commands:
/review- Request a full code review/review latest- Review only changes since the last review/describe- Generate PR description. This will update the PR body or issue comment depending on your configuration/help- Get help with Mesa commands and configuration options
2 files reviewed | 1 comments | Edit Agent Settings • Read Docs
| elif [ "$(NIX_CONFIG_TYPE)" = "nixosConfigurations" ]; then \ | ||
| sudo $(NIX_ALLOW_UNFREE) $(NIX_EXEC) run $(NIX_FLAGS) nixpkgs#nixos-rebuild -- build --flake .#$(NIX_SYSTEM) --impure; \ | ||
| if [ -n "$(HOST)" ]; then \ | ||
| echo "Building named host: $(HOST)"; \ |
There was a problem hiding this comment.
Consider adding validation to check if the specified HOST configuration actually exists in the flake. Currently, if a user sets HOST=nonexistent, they'll get a late-stage Nix error rather than a clear validation message. Example:
if ! nix flake show --json 2>/dev/null | grep -q "nixosConfigurations.$(HOST)"; then \
echo "Error: Host configuration '$(HOST)' not found in flake"; \
exit 1; \
fi; \
Prompt for Agent
Task: Address review feedback left on GitHub.
Repository: shunkakinoki/dotfiles#715
File: Makefile#L298
Action: Open this file location in your editor, inspect the highlighted code, and resolve the issue described below.
Feedback:
Consider adding validation to check if the specified HOST configuration actually exists in the flake. Currently, if a user sets `HOST=nonexistent`, they'll get a late-stage Nix error rather than a clear validation message. Example:
```make
if ! nix flake show --json 2>/dev/null | grep -q "nixosConfigurations.$(HOST)"; then \
echo "Error: Host configuration '$(HOST)' not found in flake"; \
exit 1; \
fi; \
</details>
There was a problem hiding this comment.
Pull request overview
Enhances the Makefile’s Nix build/switch workflows to support explicitly named hosts and auto-detected hosts, improving flexibility and transparency when working with flake-based NixOS configurations.
Changes:
- Added host resolution priority for
HOST→DETECTED_HOST→NIX_SYSTEMinnix-buildandnix-switch(NixOS configs). - Added informational output indicating which host is being built/switched.
- Updated the
dotagentssubmodule pointer.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
Makefile |
Adds host selection logic and log output for nix-build/nix-switch NixOS paths. |
dotagents |
Bumps submodule commit to a newer revision. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| sudo $(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 $(NIX_ALLOW_UNFREE) $(NIX_EXEC) run $(NIX_FLAGS) nixpkgs#nixos-rebuild -- build --flake .#$(DETECTED_HOST) --impure; \ | ||
| else \ | ||
| sudo $(NIX_ALLOW_UNFREE) $(NIX_EXEC) run $(NIX_FLAGS) nixpkgs#nixos-rebuild -- build --flake .#$(NIX_SYSTEM) --impure; \ |
There was a problem hiding this comment.
$(HOST) / $(DETECTED_HOST) are interpolated unquoted into the shell command line. If either contains whitespace or shell metacharacters, the command can break or be abused (e.g., argument splitting / shell injection). Wrap the --flake argument in shell quotes so it’s passed as a single argument (and consider similarly quoting any other interpolations that become standalone args).
| sudo $(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 $(NIX_ALLOW_UNFREE) $(NIX_EXEC) run $(NIX_FLAGS) nixpkgs#nixos-rebuild -- build --flake .#$(DETECTED_HOST) --impure; \ | |
| else \ | |
| sudo $(NIX_ALLOW_UNFREE) $(NIX_EXEC) run $(NIX_FLAGS) nixpkgs#nixos-rebuild -- build --flake .#$(NIX_SYSTEM) --impure; \ | |
| sudo $(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 $(NIX_ALLOW_UNFREE) $(NIX_EXEC) run $(NIX_FLAGS) nixpkgs#nixos-rebuild -- build --flake '.#$(DETECTED_HOST)' --impure; \ | |
| else \ | |
| sudo $(NIX_ALLOW_UNFREE) $(NIX_EXEC) run $(NIX_FLAGS) nixpkgs#nixos-rebuild -- build --flake '.#$(NIX_SYSTEM)' --impure; \ |
| sudo $(NIX_ALLOW_UNFREE) $(NIX_EXEC) run $(NIX_FLAGS) --impure nixpkgs#nixos-rebuild -- switch --flake .#$(HOST); \ | ||
| elif [ -n "$(DETECTED_HOST)" ]; then \ | ||
| echo "Auto-detected host: $(DETECTED_HOST)"; \ | ||
| sudo $(NIX_ALLOW_UNFREE) $(NIX_EXEC) run $(NIX_FLAGS) --impure nixpkgs#nixos-rebuild -- switch --flake .#$(DETECTED_HOST); \ | ||
| else \ | ||
| sudo $(NIX_ALLOW_UNFREE) $(NIX_EXEC) run $(NIX_FLAGS) --impure nixpkgs#nixos-rebuild -- switch --flake .#$(NIX_SYSTEM); \ |
There was a problem hiding this comment.
nix-build passes --impure to nixos-rebuild (after --), but nix-switch currently passes --impure only to nix run (before --). If nixos-rebuild needs --impure for consistent flake evaluation/behavior, nix-switch may not behave the same as nix-build. Consider passing --impure to nixos-rebuild in the switch path as well (or consistently choose one approach in both targets).
| sudo $(NIX_ALLOW_UNFREE) $(NIX_EXEC) run $(NIX_FLAGS) --impure nixpkgs#nixos-rebuild -- switch --flake .#$(HOST); \ | |
| elif [ -n "$(DETECTED_HOST)" ]; then \ | |
| echo "Auto-detected host: $(DETECTED_HOST)"; \ | |
| sudo $(NIX_ALLOW_UNFREE) $(NIX_EXEC) run $(NIX_FLAGS) --impure nixpkgs#nixos-rebuild -- switch --flake .#$(DETECTED_HOST); \ | |
| else \ | |
| sudo $(NIX_ALLOW_UNFREE) $(NIX_EXEC) run $(NIX_FLAGS) --impure nixpkgs#nixos-rebuild -- switch --flake .#$(NIX_SYSTEM); \ | |
| sudo $(NIX_ALLOW_UNFREE) $(NIX_EXEC) run $(NIX_FLAGS) --impure nixpkgs#nixos-rebuild -- switch --flake .#$(HOST) --impure; \ | |
| elif [ -n "$(DETECTED_HOST)" ]; then \ | |
| echo "Auto-detected host: $(DETECTED_HOST)"; \ | |
| sudo $(NIX_ALLOW_UNFREE) $(NIX_EXEC) run $(NIX_FLAGS) --impure nixpkgs#nixos-rebuild -- switch --flake .#$(DETECTED_HOST) --impure; \ | |
| else \ | |
| sudo $(NIX_ALLOW_UNFREE) $(NIX_EXEC) run $(NIX_FLAGS) --impure nixpkgs#nixos-rebuild -- switch --flake .#$(NIX_SYSTEM) --impure; \ |
| if [ -n "$(HOST)" ]; then \ | ||
| echo "Building named host: $(HOST)"; \ | ||
| sudo $(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 $(NIX_ALLOW_UNFREE) $(NIX_EXEC) run $(NIX_FLAGS) nixpkgs#nixos-rebuild -- build --flake .#$(DETECTED_HOST) --impure; \ | ||
| else \ | ||
| sudo $(NIX_ALLOW_UNFREE) $(NIX_EXEC) run $(NIX_FLAGS) nixpkgs#nixos-rebuild -- build --flake .#$(NIX_SYSTEM) --impure; \ | ||
| fi; \ |
There was a problem hiding this comment.
The same host-resolution decision tree (HOST → DETECTED_HOST → NIX_SYSTEM, plus logging) now exists in multiple targets (nix-build and nix-switch). To reduce drift and simplify future updates, consider centralizing this into a single Make variable (e.g., a resolved host name) or a small shared Make macro used by both targets.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Changes
nix-buildandnix-switchMakefile targets to support named host configuration viaHOSTvariableDETECTED_HOSTvariableNIX_SYSTEMwhen neitherHOSTnorDETECTED_HOSTis setTechnical Details
The changes improve flexibility in building and switching Nix configurations by:
HOST) for targeted buildsDETECTED_HOST) for convenienceNIX_SYSTEMusageBoth
nix-buildandnix-switchtargets now follow the same host resolution logic, ensuring consistency between build and activation workflows.Testing
HOSTvariableDETECTED_HOSTvariableNIX_SYSTEMwhen both variables are unsetGenerated with Claude Code by glm-4.7
Summary by cubic
Add host selection to nix-build and nix-switch using HOST and DETECTED_HOST, with fallback to NIX_SYSTEM and clearer logs. This makes targeted builds/switches easier and keeps behavior consistent.
New Features
Dependencies
Written for commit bee485f. Summary will update on new commits.