Skip to content

Conversation

@grahamc
Copy link
Member

@grahamc grahamc commented Oct 25, 2025

Implements https://determinate.systems/blog/installer-dropping-upstream/

Checklist
  • Formatted with cargo fmt
  • Built with nix build
  • Ran flake checks with nix flake check
  • Added or updated relevant tests (leave unchecked if not applicable)
  • Added or updated relevant documentation (leave unchecked if not applicable)
  • Linked to related issues (leave unchecked if not applicable)
Validating with install.determinate.systems

If a maintainer has added the upload to s3 label to this PR, it will become available for installation via install.determinate.systems:

curl --proto '=https' --tlsv1.2 -sSf -L https://install.determinate.systems/nix/pr/$PR_NUMBER | sh -s -- install

Summary by CodeRabbit

  • Changes

    • Removed interactive prompt for Determinate Nix installation; the installer now defaults directly to Determinate Nix without user interaction.
    • Simplified installation workflow to a streamlined, non-interactive process.
  • Chores

    • Updated configuration defaults and test fixtures to align with new installer behavior.

@coderabbitai
Copy link

coderabbitai bot commented Oct 25, 2025

Walkthrough

Removes the interactive prompt_for_determinate function and related prompting logic from Determinate Nix installation, makes the planner binding immutable in mod.rs, eliminates post-install message handling, changes the default --prefer-upstream-nix flag from true to false, and updates test fixtures to reflect the new defaults.

Changes

Cohort / File(s) Summary
Interactive Prompt Removal
src/cli/subcommand/install/determinate.rs
Removes prompt_for_determinate async function and all related interactive logic, imports (e.g., PromptChoice), and constants; keeps simplified inform_macos_about_pkg function that only performs OS check and logging.
Install Flow Refactoring
src/cli/subcommand/install/mod.rs
Changes planner binding from mutable to immutable, removes post_install_message tracking and prompt_for_determinate call, adds error mapping for planner construction, and eliminates post-install message printing logic.
Default Settings Change
src/settings.rs
Changes default value of --prefer-upstream-nix CLI flag from true to false, making Determinate Nix the default distribution when determinate_nix is false.
Test Fixture Updates
tests/fixtures/linux/linux.json, tests/fixtures/linux/steam-deck.json, tests/fixtures/macos/macos.json
Updates planner settings across all platform fixtures: sets determinate_nix to true and prefer_upstream to false, reflecting new default behavior.

Sequence Diagram

sequenceDiagram
    participant User
    participant CLI
    participant Planner
    participant Installer
    
    rect rgb(200, 220, 200)
    Note over User,Installer: New Non-Interactive Flow
    User->>CLI: Run install command
    CLI->>Planner: Create planner with settings<br/>(prefer_upstream=false)
    Planner-->>CLI: Planner initialized<br/>(Determinate Nix selected)
    CLI->>Installer: Execute plan
    Installer-->>CLI: Installation complete
    CLI-->>User: Done (no post-install prompts)
    end
    
    Note over User,Installer: Old flow removed:<br/>- No interactive prompting<br/>- No post-install messaging<br/>- Direct execution path
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

  • Potential attention areas:
    • Verify that removing the prompt_for_determinate function and post-install message flow doesn't break any callers or downstream code that may expect those messages
    • Confirm the planner mutability change in mod.rs doesn't affect any header-level manipulations previously dependent on mutability
    • Ensure test fixtures comprehensively cover the new default behavior with prefer_upstream=false and determinate_nix=true
    • Check that inform_macos_about_pkg still functions correctly after the interactive wrapper removal

Possibly related PRs

Suggested reviewers

  • cole-h

Poem

🐰 No more prompts to fill the screen,
Determinate's the new routine,
With settings set and flags so true,
The choice is made for me and you!
Auto-pilot, smooth and clean,

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The title "Default to Determinate Nix" directly and clearly captures the primary objective of this changeset. The core changes involve modifying default settings in src/settings.rs to set prefer_upstream to false, which causes the distribution() method to default to DeterminateNix, alongside removal of interactive prompting logic and updates to test fixtures to reflect these new defaults. The title is concise, specific, and provides sufficient clarity for a developer scanning the history to understand that this PR changes the installer's default Nix distribution preference.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch grahamc/default-to-determinate-nix

📜 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 096b735 and 155af92.

📒 Files selected for processing (6)
  • src/cli/subcommand/install/determinate.rs (0 hunks)
  • src/cli/subcommand/install/mod.rs (1 hunks)
  • src/settings.rs (2 hunks)
  • tests/fixtures/linux/linux.json (1 hunks)
  • tests/fixtures/linux/steam-deck.json (1 hunks)
  • tests/fixtures/macos/macos.json (1 hunks)
💤 Files with no reviewable changes (1)
  • src/cli/subcommand/install/determinate.rs
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: grahamc
PR: DeterminateSystems/nix-installer#1664
File: src/settings.rs:56-67
Timestamp: 2025-09-10T15:49:03.281Z
Learning: In the nix-installer codebase, the prefer_upstream CLI flag is intentionally designed as a presence-only flag. Users wanting Determinate Nix should use --determinate instead of --prefer-upstream=false, as this was a deliberate UX design choice to keep the interface simpler.
📚 Learning: 2025-09-10T15:49:03.281Z
Learnt from: grahamc
PR: DeterminateSystems/nix-installer#1664
File: src/settings.rs:56-67
Timestamp: 2025-09-10T15:49:03.281Z
Learning: In the nix-installer codebase, the prefer_upstream CLI flag is intentionally designed as a presence-only flag. Users wanting Determinate Nix should use --determinate instead of --prefer-upstream=false, as this was a deliberate UX design choice to keep the interface simpler.

Applied to files:

  • src/settings.rs
🔇 Additional comments (6)
src/settings.rs (2)

63-63: Default changed to Determinate Nix.

The CLI default for --prefer-upstream-nix is now false, aligning with the PR objective to default to Determinate Nix. This pairs with the distribution() method logic (lines 324-333) where both flags being false now results in Distribution::DeterminateNix.


243-243: Programmatic default updated consistently.

The default() implementation now sets prefer_upstream: false, matching the CLI default change at line 63. Both changes work together to ensure Determinate Nix is the default distribution.

tests/fixtures/linux/steam-deck.json (1)

1175-1176: Fixture updated to reflect new defaults.

The test fixture now correctly represents the new default behavior where determinate_nix: true and prefer_upstream: false, consistent with the changes in src/settings.rs.

tests/fixtures/macos/macos.json (1)

1247-1248: macOS fixture aligned with new defaults.

The macOS test fixture correctly reflects the new default behavior, matching the changes in other platform fixtures and the updated settings in src/settings.rs.

tests/fixtures/linux/linux.json (1)

1147-1148: Linux fixture updated consistently.

The standard Linux fixture now matches the new defaults, completing the fixture updates across all platforms (Linux, Steam Deck, macOS).

src/cli/subcommand/install/mod.rs (1)

126-131: Planner binding simplified appropriately.

The planner is now immutable, which is correct since the interactive prompting logic has been removed (per the AI summary). The error handling has also been improved with map_err(|e| eyre::eyre!(e))? for better error reporting.


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

@grahamc grahamc force-pushed the grahamc/default-to-determinate-nix branch from 2cc1fde to 155af92 Compare October 25, 2025 00:49
Copy link
Member

@colemickens colemickens left a comment

Choose a reason for hiding this comment

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

Looks good; requesting changes til late on November 9th or early November 10th.

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