Skip to content

Conversation

@lucperkins
Copy link
Member

@lucperkins lucperkins commented Nov 13, 2025

Summary by CodeRabbit

  • Refactor
    • Reworked project configuration for clearer per-platform handling; no functional user-facing changes.
  • CI / Automation
    • Upgraded GitHub Actions checkout versions and added job permissions to improve workflow reliability.
    • Simplified runner mapping by removing one platform entry and reordering defaults.
  • Documentation
    • Updated README examples to match the new default runner mapping.

@coderabbitai
Copy link

coderabbitai bot commented Nov 13, 2025

Walkthrough

Refactors flake outputs to a functional form that exposes self, replaces inputs.self usages with self across overlays, per-system derivations, rustToolchain, packages and devShells, and updates supported systems iteration. Also updates workflow YAMLs (checkout versions, permissions, runner-map formatting) and README runner-map example.

Changes

Cohort / File(s) Summary
Flake outputs & self usage
flake.nix
Change outputs = inputs:outputs = { self, ... }@inputs:. Replace inputs.self with self across overlays, package src references, devShells, rustToolchain, and per-system builders; restructure supportedSystems and forEachSupportedSystem to explicit lambdas; formatting/indentation tweaks.
CI workflows
.github/workflows/determinate-ci.yml, .github/workflows/nix.yml, .github/workflows/rust.yml
YAML literal quoting removed/normalized; added permissions blocks (id-token: write, contents: read) to jobs; bumped actions/checkout in two workflows (v3→v5, v4→v5); simplified/adjusted runner-map (removed x86_64-darwin entry, reordered entries).
Docs
README.md
Updated example/default JSON runner-map to remove x86_64-darwin and reorder remaining entries (aarch64-darwin before x86_64-linux).

Sequence Diagram(s)

sequenceDiagram
  participant Caller as flake caller
  participant Outputs as outputs fn ({ self, ... }@inputs)
  participant Self as self (flake)
  participant PerSys as per-system generator
  Caller->>Outputs: invoke with inputs
  Outputs->>Self: expose self to derivations
  alt per-system derivation
    Outputs->>PerSys: call forEachSupportedSystem (uses self.overlays.default)
    PerSys->>Self: reference self.overlays.default and self.packages
    PerSys-->>Outputs: return per-system pkgs/devShells
  end
  Outputs-->>Caller: return final flake outputs
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

  • Areas to inspect closely:
    • flake.nix: confirm self is in scope everywhere previously referencing inputs.self, and check for variable shadowing in the new forEachSupportedSystem lambdas.
    • Overlay and per-system package creation: ensure self.overlays.default resolves equivalently across systems.
    • CI workflow changes: validate permissions and upgraded actions/checkout versions are compatible with existing steps.
    • README: ensure example matches new defaults.

Poem

🐰 I hopped through flakes with cheerful stealth,

Swapped inputs.self for confident self,
Systems aligned, workflows trimmed of weight,
A tidy flake now hops to build and create. 🥕

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Title check ⚠️ Warning The title 'Switch to secure packages' is vague and does not clearly summarize the actual changes in the pull request. The changeset involves infrastructure updates (flake.nix refactoring, GitHub Actions versions, runner mapping changes, documentation updates), not package security changes. Revise the title to accurately reflect the main changes, such as 'Refactor flake.nix and update CI infrastructure' or 'Update flake outputs structure and CI workflows'.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch secure-packages

📜 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 c51eb00 and 8955f9d.

📒 Files selected for processing (5)
  • .github/workflows/determinate-ci.yml (2 hunks)
  • .github/workflows/nix.yml (1 hunks)
  • .github/workflows/rust.yml (1 hunks)
  • README.md (1 hunks)
  • flake.nix (1 hunks)
⏰ 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). (3)
  • GitHub Check: DeterminateCI / build (x86_64-linux, ubuntu-latest)
  • GitHub Check: DeterminateCI / build (aarch64-linux, ubuntu-24.04-arm)
  • GitHub Check: DeterminateCI / build (aarch64-darwin, macos-latest)
🔇 Additional comments (2)
.github/workflows/rust.yml (1)

15-15: Confirm runner version supports Checkout v5

actions/checkout@v5 runs on the Node 24 runtime and requires GitHub Actions runners at version 2.327.1 or newer. Because this job targets the custom UbuntuLatest32Cores128G label, please double-check that every self-hosted runner behind this label has been upgraded before merging, otherwise the workflow will fail immediately. (github.com)

.github/workflows/nix.yml (1)

15-15: Confirm runner version supports Checkout v5

The upgraded actions/checkout@v5 requires Actions runners at version 2.327.1 or newer because of its Node 24 runtime. Please ensure the UbuntuLatest32Cores128G runners meet this minimum before merging so the workflow doesn’t fail at startup. (github.com)


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

@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: 0

🧹 Nitpick comments (2)
flake.nix (2)

84-88: Consider removing duplicate devShell aliases.

The devShells a, b, c, and d all reference default. While the comment on line 91 mentions these are for local testing, consider removing these aliases or documenting their purpose to avoid confusion.

-          a = default;
-          b = default;
-          c = default;
-          d = default;

103-109: Consider removing test package aliases.

The packages a through f appear to be test artifacts. While the comment indicates these are for local testing, consider:

  1. Removing them from the committed flake
  2. Moving them to a separate test file or documentation
  3. Adding clear comments explaining their purpose

Having test packages like d = pkgs.jq, e = pkgs.ponysay, and f = pkgs.hello in the production flake may cause confusion.

-          a = default;
-          b = default;
-          c = default;
-          d = pkgs.jq;
-          e = pkgs.ponysay;
-          f = pkgs.hello;
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 7a099fa and c51eb00.

⛔ Files ignored due to path filters (1)
  • flake.lock is excluded by !**/*.lock
📒 Files selected for processing (1)
  • flake.nix (1 hunks)
⏰ 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: DeterminateCI / inventory
  • GitHub Check: rust-fmt-and-clippy
🔇 Additional comments (5)
flake.nix (5)

17-18: LGTM! Idiomatic Nix flake pattern.

Destructuring self from the inputs is a standard and cleaner pattern in Nix flakes, making the subsequent code more readable by using self directly instead of inputs.self.


26-36: LGTM! Consistent with the refactored signature.

The use of self.overlays.default instead of inputs.self.overlays.default is consistent with the new outputs signature and makes the code cleaner.


42-62: LGTM! Overlay definition is correct.

The rustToolchain overlay properly combines the stable Rust toolchain components with platform-specific musl targets for Linux systems.


98-98: LGTM! Correct source reference.

Using src = self; instead of src = inputs.self; is correct and consistent with the refactored outputs signature.


6-13: URLs are valid—no issues found.

All FlakeHub URLs have been verified:

  • nixpkgs points to DeterminateSystems' legitimate enterprise-grade secure nixpkgs flake with CVE monitoring and cryptographic signing
  • fenix version 0.1.1885 is a valid pinned release
  • crane version 0.20 (v0.20.0) is a valid pinned release

Copy link
Member

@RossComputerGuy RossComputerGuy left a comment

Choose a reason for hiding this comment

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

LGTM

@lucperkins lucperkins merged commit f7247bc into main Nov 13, 2025
7 of 8 checks passed
@lucperkins lucperkins deleted the secure-packages branch November 13, 2025 16:54
@coderabbitai coderabbitai bot mentioned this pull request Dec 16, 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.

3 participants