Skip to content

feat: specify parameters for file and attribute for legacy-style CLIs#129

Merged
water-sucks merged 7 commits intonix-community:mainfrom
water-sucks:legacy-cli-args
Nov 9, 2025
Merged

feat: specify parameters for file and attribute for legacy-style CLIs#129
water-sucks merged 7 commits intonix-community:mainfrom
water-sucks:legacy-cli-args

Conversation

@water-sucks
Copy link
Copy Markdown
Collaborator

@water-sucks water-sucks commented Nov 9, 2025

For already-instantiated configurations, it can be hard to retrieve their values or build with them purely using Nix include arguments (aka -I) or by setting $NIXOS_CONFIG.

This gives users the explicit ability to pass already-instantiated user configurations as positional arguments, similar to flake refs, but for legacy-style configurations only, for the following commands:

  • nixos apply
  • nixos install
  • nixos option
  • nixos repl

Closes #120.

Summary by CodeRabbit

  • New Features

    • Apply, Install, Option, and REPL commands accept optional FILE and ATTR arguments to target legacy (non-flake) configurations.
    • Commands can load explicit file paths and attributes and infer system names when using flake refs.
  • Improvements

    • More robust resolution of nix configuration paths and clearer debug logging.
  • Documentation

    • Manual pages updated with FILE/ATTR usage, examples, and option descriptions.

@coderabbitai
Copy link
Copy Markdown

coderabbitai bot commented Nov 9, 2025

Walkthrough

This PR adds legacy file-based configuration support across CLI commands (apply, install, option, repl): accepts FILE and ATTR positional args and --file/--attr flags in non-flake mode, resolves file paths via a new ResolveNixFilename utility, and extends LegacyConfiguration with explicit path and attribute handling used throughout configuration resolution and build flows.

Changes

Cohort / File(s) Summary
Command CLIs (args & help)
cmd/apply/apply.go, cmd/install/install.go, cmd/option/option.go, cmd/repl/repl.go
Updated usage/help and arg parsing to accept FILE and ATTR (positional) and --file/--attr (flags) in non-flake mode; map args into opts.File/opts.Attr; construct LegacyConfiguration from resolved file paths and attributes; added debug logging for chosen config sources.
Options / CLI opts structs
internal/cmd/opts/opts.go
Added public File string and Attr string fields to ApplyOpts, InstallOpts, OptionOpts, ReplOpts; moved/adjusted BuildImage placement in ApplyOpts.
Legacy configuration core
internal/configuration/legacy.go, internal/configuration/configuration.go
Replaced directory-based legacy handling with explicit path fields: ConfigPath, Attribute, UseExplicitPath; added Dirname(); altered Eval/Build flows to construct attribute paths for explicit-path mode; included evaluation output in AttributeEvaluationError.Error and simplified BuildAttr to always return "toplevel".
Flake support helpers
internal/configuration/flake.go
Added String() method on FlakeRef to render URI#System.
Path resolution utility
internal/utils/utils.go
Added ResolveNixFilename(input string) (string, error) to resolve file vs directory (append default.nix), resolve symlinks, and return absolute file paths.
Documentation (man pages)
doc/man/nixos-cli-apply.1.scd, doc/man/nixos-cli-install.1.scd, doc/man/nixos-cli-option.1.scd, doc/man/nixos-cli-repl.1.scd
Added/updated SYNOPSIS, ARGUMENTS, OPTIONS and EXAMPLES to document FILE and ATTR for legacy CLIs and note environment fallback semantics.

Sequence Diagram(s)

sequenceDiagram
    actor User
    participant CLI as CLI command
    participant Parser as Arg Parser
    participant Util as ResolveNixFilename
    participant Config as Config Resolver
    participant Build as Build System

    User->>CLI: invoke with [FILE] [ATTR]
    CLI->>Parser: parse args / flags

    alt Flake mode
        Parser->>Config: create/use FlakeRef
        Config->>Build: evaluate flake reference (URI#system)
    else Non-flake mode
        alt FILE provided (or --file)
            Parser->>Util: ResolveNixFilename(FILE)
            Util-->>Config: resolved absolute file path
            Config->>Config: construct LegacyConfiguration{ConfigPath, Attribute, UseExplicitPath=true}
        else no FILE
            Config->>Config: discover config via NIXOS_CONFIG / NIX_PATH / env
            Config->>Config: construct LegacyConfiguration{ConfigPath, UseExplicitPath=false, Includes}
        end
        Config->>Build: evaluate using LegacyConfiguration (explicit or discovered)
    end

    Build-->>User: result / output
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

  • Wide spread across CLI commands, configuration core, utils, and docs.
  • Mixed changes: repeated arg-parsing pattern but significant, non-uniform logic in legacy config handling and build attribute construction.

Areas needing extra attention:

  • internal/configuration/legacy.go — correctness of UseExplicitPath branching, makeBuildAttrPath usage, and interactions with nix-instantiate/nix-build.
  • cmd/repl/repl.go — changed execLegacyRepl signature and dynamic system-expression construction.
  • Consistency of ResolveNixFilename usage across commands and correct symlink/dir handling.

Possibly related PRs

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed Title accurately reflects the main change: adding file and attribute parameters for legacy-style CLI commands as positional arguments.
Linked Issues check ✅ Passed PR implements all coding requirements from issue #120: adds --file/--attr support to legacy CLI commands (apply, install, option, repl), enables positional argument parsing, updates configuration resolution logic, and adds supporting infrastructure (ResolveNixFilename utility, LegacyConfiguration fields).
Out of Scope Changes check ✅ Passed All changes align with issue #120 objectives: command implementations use File/Attr opts, configuration structs updated to support explicit paths, utilities added for path resolution, and documentation updated to reflect new parameters.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 58249e3 and e1e8172.

📒 Files selected for processing (7)
  • cmd/option/option.go (3 hunks)
  • cmd/repl/repl.go (7 hunks)
  • doc/man/nixos-cli-option.1.scd (2 hunks)
  • doc/man/nixos-cli-repl.1.scd (2 hunks)
  • internal/cmd/opts/opts.go (3 hunks)
  • internal/configuration/configuration.go (3 hunks)
  • internal/configuration/legacy.go (6 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-10-20T02:59:26.118Z
Learnt from: water-sucks
Repo: nix-community/nixos-cli PR: 94
File: cmd/activate/run.go:245-281
Timestamp: 2025-10-20T02:59:26.118Z
Learning: Go 1.25 introduced the sync.WaitGroup.Go(f func()) method, which starts f in a new goroutine and automatically handles Add(1) and defer Done(). This is valid Go 1.25+ syntax and should not be flagged as an error.

Applied to files:

  • internal/configuration/legacy.go
🧬 Code graph analysis (4)
cmd/option/option.go (3)
internal/configuration/flake.go (2)
  • FlakeRefFromString (25-52)
  • FlakeRef (17-23)
internal/utils/utils.go (1)
  • ResolveNixFilename (72-111)
internal/configuration/legacy.go (1)
  • LegacyConfiguration (17-32)
internal/configuration/legacy.go (3)
internal/utils/utils.go (1)
  • ResolveNixFilename (72-111)
internal/system/runner.go (1)
  • CommandRunner (10-14)
internal/configuration/configuration.go (1)
  • BuildType (71-73)
cmd/repl/repl.go (6)
internal/build/vars/vars.go (1)
  • Flake (9-9)
internal/build/build.go (1)
  • Flake (32-34)
internal/configuration/configuration.go (2)
  • Configuration (28-32)
  • FindConfiguration (43-69)
internal/configuration/flake.go (2)
  • FlakeRef (17-23)
  • FlakeRefFromString (25-52)
internal/utils/utils.go (1)
  • ResolveNixFilename (72-111)
internal/configuration/legacy.go (1)
  • LegacyConfiguration (17-32)
internal/cmd/opts/opts.go (1)
internal/configuration/flake.go (1)
  • FlakeRef (17-23)
⏰ 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 (4)
cmd/repl/repl.go (4)

16-16: LGTM!

The import of internal/utils and conditional usage string setup correctly support the new legacy file-based arguments.

Also applies to: 25-30


36-60: LGTM!

Argument validation correctly handles both flake mode (max 1 arg) and legacy mode (max 2 args) with proper assignment to option fields.


70-86: LGTM!

Help template documentation clearly explains the legacy [FILE] and [ATTR] arguments and their fallback behavior.


160-188: LGTM!

Configuration resolution logic correctly handles explicit file/attribute arguments using ResolveNixFilename for path canonicalization and properly constructs the LegacyConfiguration with UseExplicitPath=true.


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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
doc/man/nixos-cli-install.1.scd (1)

50-50: Fix typo in section heading.

"EXMAPLES" should be "EXAMPLES".

Apply this diff:

-# EXMAPLES
+# EXAMPLES
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 41d0481 and 58249e3.

📒 Files selected for processing (13)
  • cmd/apply/apply.go (5 hunks)
  • cmd/install/install.go (6 hunks)
  • cmd/option/option.go (3 hunks)
  • cmd/repl/repl.go (7 hunks)
  • doc/man/nixos-cli-apply.1.scd (3 hunks)
  • doc/man/nixos-cli-install.1.scd (3 hunks)
  • doc/man/nixos-cli-option.1.scd (2 hunks)
  • doc/man/nixos-cli-repl.1.scd (2 hunks)
  • internal/cmd/opts/opts.go (3 hunks)
  • internal/configuration/configuration.go (3 hunks)
  • internal/configuration/flake.go (1 hunks)
  • internal/configuration/legacy.go (6 hunks)
  • internal/utils/utils.go (2 hunks)
🧰 Additional context used
🧬 Code graph analysis (7)
internal/configuration/flake.go (1)
internal/system/system.go (1)
  • System (9-14)
internal/cmd/opts/opts.go (1)
internal/configuration/flake.go (1)
  • FlakeRef (17-23)
cmd/apply/apply.go (4)
internal/build/vars/vars.go (1)
  • Flake (9-9)
internal/build/build.go (1)
  • Flake (32-34)
internal/utils/utils.go (1)
  • ResolveNixFilename (72-111)
internal/configuration/legacy.go (1)
  • LegacyConfiguration (17-32)
cmd/option/option.go (3)
internal/configuration/flake.go (2)
  • FlakeRefFromString (25-52)
  • FlakeRef (17-23)
internal/utils/utils.go (1)
  • ResolveNixFilename (72-111)
internal/configuration/legacy.go (1)
  • LegacyConfiguration (17-32)
cmd/install/install.go (5)
internal/cmd/opts/opts.go (1)
  • InstallOpts (153-166)
internal/build/build.go (1)
  • Flake (32-34)
internal/configuration/flake.go (1)
  • FlakeRef (17-23)
internal/utils/utils.go (1)
  • ResolveNixFilename (72-111)
internal/configuration/legacy.go (1)
  • LegacyConfiguration (17-32)
internal/configuration/legacy.go (3)
internal/utils/utils.go (1)
  • ResolveNixFilename (72-111)
internal/system/runner.go (1)
  • CommandRunner (10-14)
internal/configuration/configuration.go (1)
  • BuildType (71-73)
cmd/repl/repl.go (6)
internal/build/vars/vars.go (1)
  • Flake (9-9)
internal/build/build.go (1)
  • Flake (32-34)
internal/configuration/configuration.go (2)
  • Configuration (28-32)
  • FindConfiguration (43-69)
internal/configuration/flake.go (2)
  • FlakeRef (17-23)
  • FlakeRefFromString (25-52)
internal/utils/utils.go (1)
  • ResolveNixFilename (72-111)
internal/configuration/legacy.go (1)
  • LegacyConfiguration (17-32)
⏰ 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 (12)
doc/man/nixos-cli-install.1.scd (2)

9-16: Clear differentiation between CLI modes.

The updated synopsis clearly distinguishes between flake-enabled and legacy CLIs, making it easier for users to understand which arguments apply to their use case.


111-130: Comprehensive FILE and ATTR documentation.

The documentation for the new FILE and ATTR arguments is clear and thorough, including fallback behavior and usage notes.

doc/man/nixos-cli-option.1.scd (1)

48-68: Well-documented legacy CLI options.

The --attr and --file options are clearly documented with appropriate fallback behavior and legacy CLI restrictions.

doc/man/nixos-cli-repl.1.scd (1)

48-65: Consistent documentation pattern.

The FILE and ATTR argument documentation follows the same clear pattern established in other man pages, ensuring consistency across the CLI.

doc/man/nixos-cli-apply.1.scd (2)

47-56: Helpful legacy CLI examples.

The examples demonstrate practical usage of FILE and ATTR arguments, including both simple file and attribute set scenarios.


101-120: Complete FILE and ATTR documentation.

The argument documentation is thorough and consistent with the pattern used across all man pages.

internal/cmd/opts/opts.go (1)

24-28: Consistent field additions for legacy CLI support.

The File and Attr fields are added consistently across option structs to support the new legacy CLI arguments.

internal/configuration/flake.go (1)

80-82: Simple and correct String() implementation.

The String() method provides a standard string representation of FlakeRef in the format "URI#System", which is useful for logging and debugging.

internal/configuration/configuration.go (3)

40-40: Improved error diagnostics.

Including the evaluation trace in the error message will help users debug configuration issues more effectively.


65-65: Updated logging for legacy configuration.

The change from ConfigDirname to ConfigPath aligns with the new explicit path support in LegacyConfiguration.


79-81: Simplified BuildAttr() method.

The method now unconditionally returns "toplevel". Ensure this simplification is intentional and doesn't affect any build scenarios where the previous conditional logic was necessary.

internal/utils/utils.go (1)

72-111: Well-implemented path resolution function.

The ResolveNixFilename function correctly handles:

  • File vs directory input
  • Directory resolution to default.nix
  • Validation that default.nix is not itself a directory
  • Symlink resolution and absolute path conversion

The error handling is appropriate, propagating os.Stat errors and path resolution errors to the caller.

@water-sucks water-sucks merged commit c499246 into nix-community:main Nov 9, 2025
2 checks passed
@water-sucks water-sucks deleted the legacy-cli-args branch November 9, 2025 08:23
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.

support direct parameters for configuration for legacy CLI

1 participant