Skip to content

nix flake check: Remove incorrect assertion#252

Merged
grahamc merged 1 commit intomainfrom
fix-nix-flake-check-crash
Nov 5, 2025
Merged

nix flake check: Remove incorrect assertion#252
grahamc merged 1 commit intomainfrom
fix-nix-flake-check-crash

Conversation

@edolstra
Copy link
Collaborator

@edolstra edolstra commented Nov 5, 2025

Motivation

The assumption that no unknown paths can be returned is incorrect. It can happen if a derivation has outputs that are substitutable, but that have references that cannot be substituted (i.e. an incomplete closure in the binary cache). This can easily happen with magic-nix-cache.

Context

Summary by CodeRabbit

  • Refactor
    • Removed a runtime validation check to streamline the check process, allowing the application to proceed without this strict enforcement.

The assumption that no unknown paths can be returned is incorrect. It
can happen if a derivation has outputs that are substitutable, but
that have references that cannot be substituted (i.e. an incomplete
closure in the binary cache). This can easily happen with
magic-nix-cache.
@coderabbitai
Copy link

coderabbitai bot commented Nov 5, 2025

Walkthrough

The change removes a runtime assertion from the flake check code path that previously validated missing.unknown was empty after computing missing derivations for drvPaths. The code now proceeds without this validation check.

Changes

Cohort / File(s) Summary
Assertion Removal
src/nix/flake.cc
Removed assertion enforcing missing.unknown to be empty after computing missing derivations

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~2 minutes

  • Straightforward removal of a single assertion; focus on understanding the rationale for removing the validation and confirming no correctness implications

Suggested reviewers

  • cole-h

Poem

A rabbit hops through flakes so grand,
Removing checks where none are planned,
One assertion freed, the path runs clear,
Trust the missing, have no fear! 🐰✨

Pre-merge checks and finishing touches

✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately describes the main change: removing an incorrect assertion from the nix flake check code, which is clearly supported by the file summary and PR objectives.
✨ 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 fix-nix-flake-check-crash

📜 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 51fa1ca and 7a77e87.

📒 Files selected for processing (1)
  • src/nix/flake.cc (0 hunks)
💤 Files with no reviewable changes (1)
  • src/nix/flake.cc
⏰ 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). (2)
  • GitHub Check: build_x86_64-linux / build
  • GitHub Check: build_aarch64-darwin / build

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

@github-actions
Copy link

github-actions bot commented Nov 5, 2025

@github-actions github-actions bot temporarily deployed to pull request November 5, 2025 14:25 Inactive
@grahamc grahamc added this pull request to the merge queue Nov 5, 2025
Merged via the queue into main with commit e6c5d56 Nov 5, 2025
35 checks passed
@grahamc grahamc deleted the fix-nix-flake-check-crash branch November 5, 2025 15:05
@coderabbitai coderabbitai bot mentioned this pull request Nov 5, 2025
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.

2 participants