feat(apply): add image build option#127
Conversation
WalkthroughAdds image build support to the apply command, introduces a polymorphic BuildType interface with SystemBuild/VMBuild/ImageBuild, updates CLI options and apply flow to enumerate/validate image variants and resolve image paths, and adjusts activation/re-exec and dry-run behavior per build type. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant CLI as "nixos apply"
participant Config as "configuration"
participant Nix as "nix evaluator"
User->>CLI: run with --image <variant|_list>
activate CLI
CLI->>CLI: parse flags -> set BuildImage ("_list" or variant)
alt BuildImage == "_list"
CLI->>Config: getAvailableImageAttrs()
Config->>Nix: evaluate image attribute list
Nix-->>Config: list of variants
Config-->>CLI: variants
CLI-->>User: print variants and exit
else BuildImage == variant
CLI->>Config: BuildSystem(ImageBuild{Variant})
Config->>Nix: resolve image attribute / path
alt image resolved
Nix-->>Config: image path
Config-->>CLI: image path
CLI->>CLI: perform build/instantiate (apply rules, dry-run, activation)
CLI-->>User: print image path / result
else not found
Config->>Nix: getAvailableImageAttrs()
Nix-->>Config: list of variants
Config-->>CLI: variants
CLI-->>User: print variants and fail
end
end
deactivate CLI
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (4)
🧰 Additional context used🧬 Code graph analysis (3)internal/configuration/configuration.go (2)
internal/configuration/legacy.go (1)
cmd/apply/apply.go (6)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
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 |
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
internal/configuration/legacy.go (1)
187-205: Fix remote image builds for legacy configs.When
--build-hostis used with--image, we callnix-instantiate -A images.<variant>, but legacy images actually live underconfig.system.build.images.<variant>. That mismatch makes every remote image build fail before copying the drv.$ nix-instantiate … -A images.amazon error: attribute 'images' missingPlease mirror the local
buildLocalSystemlogic and prependconfig.system.build.for*ImageBuildhere.- instantiateArgv := []string{"nix-instantiate", "<nixpkgs/nixos>", "-A", buildType.BuildAttr()} + instantiateArgv := []string{"nix-instantiate", "<nixpkgs/nixos>", "-A"} + if _, ok := buildType.(*ImageBuild); ok { + instantiateArgv = append(instantiateArgv, fmt.Sprintf("config.system.build.%s", buildType.BuildAttr())) + } else { + instantiateArgv = append(instantiateArgv, buildType.BuildAttr()) + }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
cmd/apply/apply.go(9 hunks)cmd/install/install.go(1 hunks)doc/man/nixos-cli-apply.1.scd(1 hunks)internal/cmd/opts/opts.go(3 hunks)internal/configuration/configuration.go(2 hunks)internal/configuration/flake.go(3 hunks)internal/configuration/legacy.go(4 hunks)
🧰 Additional context used
🧬 Code graph analysis (5)
internal/configuration/flake.go (3)
internal/system/local.go (1)
LocalSystem(16-18)internal/configuration/configuration.go (2)
BuildType(71-73)SystemBuildOptions(13-26)internal/system/ssh.go (1)
SSHSystem(30-39)
cmd/install/install.go (1)
internal/configuration/configuration.go (1)
SystemBuild(75-77)
cmd/apply/apply.go (9)
internal/configuration/configuration.go (6)
BuildType(71-73)VMBuild(87-89)ImageBuild(99-101)SystemBuild(75-77)Configuration(28-32)AttributeEvaluationError(34-37)internal/utils/utils.go (1)
ExecAsRoot(15-26)internal/system/runner.go (2)
Command(16-23)NewCommand(25-34)internal/system/system.go (1)
System(9-14)internal/cmd/opts/opts.go (1)
ApplyNixOpts(48-76)internal/configuration/flake.go (1)
FlakeRef(17-23)internal/cmd/nixopts/convert.go (1)
NixOptionsToArgsListByCategory(91-132)internal/configuration/legacy.go (1)
LegacyConfiguration(16-22)internal/logger/logger.go (1)
Logger(12-27)
internal/configuration/configuration.go (2)
internal/build/build.go (1)
Flake(32-34)internal/build/vars/vars.go (1)
Flake(9-9)
internal/configuration/legacy.go (3)
internal/system/local.go (1)
LocalSystem(16-18)internal/configuration/configuration.go (4)
BuildType(71-73)SystemBuildOptions(13-26)ImageBuild(99-101)SystemBuild(75-77)internal/system/ssh.go (1)
SSHSystem(30-39)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Build/Test
1c89054 to
1e8a782
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
cmd/apply/apply.go (2)
310-332: Consider usingslices.Containsfor existence check.The validation logic is correct, but
slices.Containsis more idiomatic thanslices.Indexfor checking existence.Apply this diff:
- if slices.Index(images, imgBuild.Variant) < 0 { + if !slices.Contains(images, imgBuild.Variant) { err := fmt.Errorf("image type '%s' is not available", imgBuild.Variant) log.Error(err) log.Info("pass an empty string to `--image` to get a list of available images") return err }
334-339: Add build step message forImageBuild.The
ImageBuildcase lacks a corresponding build step message, which could be confusing for users.Apply this diff:
switch buildType.(type) { case *configuration.SystemBuild: log.Step("Building configuration...") case *configuration.VMBuild: log.Step("Building VM...") + case *configuration.ImageBuild: + log.Step("Building image...") }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
cmd/apply/apply.go(9 hunks)internal/cmd/opts/opts.go(3 hunks)internal/configuration/configuration.go(2 hunks)internal/configuration/legacy.go(5 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- internal/configuration/configuration.go
🧰 Additional context used
🧬 Code graph analysis (2)
cmd/apply/apply.go (8)
internal/configuration/configuration.go (6)
BuildType(71-73)VMBuild(87-89)ImageBuild(99-101)SystemBuild(75-77)Configuration(28-32)AttributeEvaluationError(34-37)internal/utils/utils.go (1)
ExecAsRoot(15-26)internal/system/runner.go (2)
Command(16-23)NewCommand(25-34)internal/system/system.go (1)
System(9-14)internal/cmd/opts/opts.go (1)
ApplyNixOpts(48-76)internal/configuration/flake.go (1)
FlakeRef(17-23)internal/cmd/nixopts/convert.go (1)
NixOptionsToArgsListByCategory(91-132)internal/configuration/legacy.go (1)
LegacyConfiguration(16-22)
internal/configuration/legacy.go (1)
internal/configuration/configuration.go (4)
BuildType(71-73)SystemBuildOptions(13-26)ImageBuild(99-101)SystemBuild(75-77)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Build/Test
🔇 Additional comments (14)
internal/cmd/opts/opts.go (2)
32-32: LGTM!The
BuildImagefield addition correctly supports the new image-building workflow, allowing users to specify image variants via the--imageflag.
45-76: LGTM!Extracting the inline
NixOptionsstruct into a publicApplyNixOptstype improves code organization and reusability. All fields andnixCategorytags are preserved correctly.cmd/apply/apply.go (9)
74-79: LGTM!The sentinel value
_listcorrectly triggers image listing when--imageis provided without a value, enabling the "list available images and exit" workflow described in the PR objectives.
103-103: LGTM!The flag definition properly integrates with the sentinel value logic and provides clear help text.
158-158: LGTM!Correctly prevents conflicting build types by marking
--vm,--vm-with-bootloader, and--imageas mutually exclusive.
212-219: LGTM!The BuildType construction correctly implements polymorphic dispatch, with proper logic for determining whether
SystemBuildshould activate based on theNoActivateandNoBootflags.
225-235: LGTM!Restricting root re-execution to local
SystemBuildwith activation is a good optimization that avoids unnecessary privilege escalation for VM and image builds.
386-391: LGTM!The dry-run logic now correctly preserves
--dryfor non-activating builds (VM, image) while ensuring that activating system builds always perform a real build. This addresses the concern raised in the previous review.
423-455: LGTM with a minor note.The finalization logic correctly handles each build type with appropriate messaging and early returns to prevent unwanted activation. The fallback messaging when
getImageNamefails is a good user experience touch.Note: If
imagePathreturned bygetImageNamecould be an absolute path, thefilepath.Joinat line 430 may produce unexpected results. Based on the implementation (line 756), it returns a trimmed string from Nix evaluation, which should be relative, so this is likely not an issue.
665-711: LGTM!The
getAvailableImageAttrshelper correctly handles both Flake and Legacy configurations, with proper error handling and JSON decoding. The use of category-specific Nix options (lockfor Flake,buildfor Legacy) aligns with the established pattern in the codebase.
713-757: LGTM!The
getImageNamehelper correctly resolves image file paths for both configuration types. The command logging (line 746) aids debugging, and the use of--rawensures clean string output. TheimgNameparameter is safely used because it's validated against available images before this function is called.internal/configuration/legacy.go (3)
116-137: LGTM!The
buildLocalSystemsignature update and attribute construction correctly handle theImageBuildcase with theconfig.system.build.prefix, while other build types use the attribute directly. The-kflag logic properly mirrorsnixos-rebuildbehavior by adding the flag for all non-activating builds.
229-231: LGTM!The
-kflag logic in the realise step correctly mirrors the pattern inbuildLocalSystem, ensuring consistent behavior for non-activating builds.
244-257: LGTM!The
BuildSystemsignature update correctly integrates with the polymorphicBuildTypeinterface, maintaining consistent dispatch logic to the appropriate internal build functions.
1e8a782 to
2b3816b
Compare
Description
nixos-rebuild.shsupports building images for various platforms, like standalone installer ISOs, Proxmox images, EC2 images, and the like through itsconfig.system.build.imagesattribute.This PR adds support for building those images, similar to the way that it can already build NixOS VMs.
Closes #113.
Summary by CodeRabbit
New Features
Bug Fixes
Refactor