Drop x86_64-darwin by grahamc · Pull Request #207 · DeterminateSystems/nix-installer-action

grahamc · 2025-11-08T20:03:18Z

Implements DeterminateSystems/nix-src#224

Description

Checklist

Tested changes against a test repository
Added or updated relevant documentation (leave unchecked if not applicable)
(If this PR is for a release) Updated README to point to the new tag (leave unchecked if not applicable)

Summary by CodeRabbit

Chores
- Removed Intel macOS from supported platforms, reducing targets that receive development shells and per-system settings.
- CI: Devshell rendering step now skips the two large macOS runners.
Bug Fixes / Behavior
- Installer creation on Intel macOS now fails with an error instead of proceeding.
- If no installer tag is provided, the process falls back to tag v3.12.2.

Implements DeterminateSystems/nix-src#224

coderabbitai · 2025-11-08T20:03:27Z

Walkthrough

Removed x86_64-darwin from flake.nix supportedSystems, added constructor-side platform handling in src/index.ts that errors on Intel macOS and sets INPUT_SOURCE-TAG=v3.12.2 if source-tag is absent, and restricted a CI step to skip two large macOS runners.

Changes

Cohort / File(s)	Summary
System Support Configuration `flake.nix`	Removed the `x86_64-darwin` entry from `supportedSystems`, reducing supported platforms from four to three.
Installer action platform handling `src/index.ts`	In `NixInstallerAction` constructor, detect x86_64 macOS (Intel): emit a hard error via `actionsCore.error` indicating unsupported platform; read input `source-tag` and if undefined emit a notice and set environment variable `INPUT_SOURCE-TAG = "v3.12.2"`.
CI workflow condition `.github/workflows/ci.yml`	Updated the "Render the devshell" step condition to skip when `matrix.runner` is `macos-13-large` or `macos-14-large`, narrowing when the step runs.

Sequence Diagram(s)

sequenceDiagram
  participant Runner
  participant Action as NixInstallerAction
  participant Env as Environment

  Runner->>Action: instantiate action (runtime info)
  Note right of Action `#ddeeff`: detect platform (os + arch)
  alt x86_64 macOS (Intel)
    Action->>Runner: emit hard error (Intel macOS unsupported)
    Note right of Runner `#ffeecc`: constructor may abort
    Action->>Action: attempt read input "source-tag"
    alt input undefined
      Action->>Runner: emit notice about pinning to v3.12.2
      Action->>Env: set INPUT_SOURCE-TAG = "v3.12.2"
    else input provided
      Action->>Runner: continue with provided tag
    end
  else other platform
    Action->>Runner: proceed normally
  end

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Check src/index.ts for correct use of actionsCore.error vs. failing behavior and scope of the env assignment.
Verify no remaining references or tests expecting x86_64-darwin in flake.nix.
Confirm CI condition logic in .github/workflows/ci.yml matches intended runner semantics.

Poem

🐰 I nibbled flakes and pruned a line,
Intel macs now stop at the sign.
A fallback tag I gently sowed,
v3.12.2 — so builds still go.
Hoppity cheers for tidy code.

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 'Drop x86_64-darwin' directly and accurately summarizes the main change: removing x86_64-darwin from supported systems across the codebase.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✨ 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 push-pokunnsooyqs

📜 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 d88c2e8 and 540d4ad.

⛔ Files ignored due to path filters (1)

dist/index.js is excluded by !**/dist/**

📒 Files selected for processing (1)

src/index.ts (1 hunks)

🚧 Files skipped from review as they are similar to previous changes (1)

src/index.ts

⏰ 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). (9)

GitHub Check: Test: macos-14-xlarge with determinate
GitHub Check: Test: macos-14-xlarge
GitHub Check: Test: macos-14-large
GitHub Check: Test: macos-14-large with determinate
GitHub Check: Test: macos-13-large with determinate
GitHub Check: Test: macos-13-large
GitHub Check: Test: namespace-profile-default-arm64
GitHub Check: Test: ubuntu-latest
GitHub Check: Test: nscloud-ubuntu-22.04-amd64-4x16

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

coderabbitai

Actionable comments posted: 0

🧹 Nitpick comments (1)

src/index.ts (1)
93-105: Consider refactoring this platform-specific guard for maintainability and code quality.

While this approach is functional for handling end-of-support for Intel Macs, there are several improvements to consider:
Unprofessional comment (line 94): The comment "Holy guacamole this is ugly" should be removed or rewritten professionally. Comments like this undermine code quality and don't explain why this approach was necessary.
Hardcoded version string (line 103): The version "v3.12.2" is hardcoded, making it harder to maintain. Consider extracting it as a constant at the file level:
const LAST_INTEL_MAC_SUPPORTED_VERSION = "v3.12.2";
Direct environment manipulation: Setting process.env["INPUT_source-tag"] directly bypasses normal input mechanisms. While this works for GitHub Actions (which reads inputs from INPUT_* environment variables), it's unconventional and could be confusing. Consider documenting why this approach is necessary, or explore if detsys-ts provides a cleaner way to set default inputs.
Constructor placement: While technically correct (no this access before super()), having complex conditional logic in the constructor is less than ideal. Consider extracting to a private static method:
private static applyIntelMacWorkaround(): void {
  if (platform.getArchOs() === "X64-macOS") {
    actionsCore.warning("...");
    const sourceTag = inputs.getStringOrUndefined("source-tag");
    if (sourceTag === undefined) {
      actionsCore.notice("...");
      process.env["INPUT_source-tag"] = LAST_INTEL_MAC_SUPPORTED_VERSION;
    }
  }
}
Given the PR comment "CI needs attention," please verify that the pinned version works correctly on Intel Macs and that appropriate tests exist for this code path.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 9fc343c and c8bf775.

⛔ Files ignored due to path filters (1)

dist/index.js is excluded by !**/dist/**

📒 Files selected for processing (1)

src/index.ts (1 hunks)

🧰 Additional context used

🧬 Code graph analysis (1)

src/index.ts (1)

dist/index.js (4)

platform (99922-99922)

sourceTag (101443-101443)

inputs (3395-3397)

process (24980-24980)

⏰ 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). (10)

GitHub Check: Test: macos-14-large
GitHub Check: Test: macos-14-xlarge
GitHub Check: Test: macos-14-xlarge with determinate
GitHub Check: Test: macos-14-large with determinate
GitHub Check: Test: namespace-profile-default-arm64 with determinate
GitHub Check: Test: macos-13-large with determinate
GitHub Check: Test: macos-13-large
GitHub Check: Test: namespace-profile-default-arm64
GitHub Check: Test: nscloud-ubuntu-22.04-amd64-4x16
GitHub Check: Test: ubuntu-latest

coderabbitai

Actionable comments posted: 1

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 3b0637f and d88c2e8.

⛔ Files ignored due to path filters (1)

dist/index.js is excluded by !**/dist/**

📒 Files selected for processing (1)

src/index.ts (1 hunks)

🧰 Additional context used

🧬 Code graph analysis (1)

src/index.ts (1)

dist/index.js (4)

platform (99922-99922)

sourceTag (101443-101443)

inputs (3395-3397)

process (24980-24980)

⏰ 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). (9)

GitHub Check: Test: macos-14-xlarge with determinate
GitHub Check: Test: macos-13-large
GitHub Check: Test: macos-14-xlarge
GitHub Check: Test: macos-14-large
GitHub Check: Test: ubuntu-latest
GitHub Check: Test: macos-14-large with determinate
GitHub Check: Test: macos-13-large with determinate
GitHub Check: Test: nscloud-ubuntu-22.04-amd64-4x16
GitHub Check: Test: namespace-profile-default-arm64

src/index.ts

colemickens · 2025-11-09T20:17:08Z

.github/workflows/ci.yml


      - name: Render the devshell
-        if: success() || failure()
+        if: (success() || failure()) && matrix.runner != 'macos-13-large' && matrix.runner != 'macos-14-large'


why? Does the devshell not build for macOS? That's surprising?

macos-13-large and macos-14-large are Intel macs, which no longer has a dev shell

why are they not just removed from the matrix, if we're dropping them after all?

I don't want to cause all CI runs to immediately fail as soon as we merge and cause a stop-the-world emergency for people all at once. That'll include us, since we haven't deleted x86_64-darwin from all our CI workflows. I'm thinking we have this pin-back warning, and then in a couple weeks or so move to failing.

(...which implies making sure it works :P)

Ok, the reason we're keeping this around is so that we actually test our "fall back to version that supports x86" code, so we don't immediately break all users who were using x86 macs

And since there's no devshell for x86 macs in this repo anymore we have to skip this step

But we still want to test that x86 isn't hard-broken just yet.

cole-h · 2025-11-10T03:33:23Z

src/index.ts

+      actionsCore.error(
+        "Determinate Nix Installer no longer supports macOS on Intel. Please migrate to Apple Silicon, and use Nix's built-in Rosetta support to build for Intel. See: https://github.com/DeterminateSystems/nix-src/issues/224",
+      );
+      const sourceTag = inputs.getStringOrUndefined("source-tag");


What if they're setting source-rev, or source-pr, or any of our other source-..... args? We probably shouldn't override those too.

as far as I can tell, absolutely nobody does that for x86_64-darwin:

Drop x86_64-darwin

9fc343c

Implements DeterminateSystems/nix-src#224

This comment was marked as outdated.

Sign in to view

coderabbitai bot reviewed Nov 9, 2025

View reviewed changes

Force back to v3.12.2 for users who don't otherwise specify a tag

2387849

grahamc force-pushed the push-pokunnsooyqs branch from c8bf775 to 2387849 Compare November 9, 2025 19:52

grahamc added 2 commits November 9, 2025 14:58

ci: don't try to render the devshell on intel macOS

3b0637f

Try an error?

d88c2e8

coderabbitai bot reviewed Nov 9, 2025

View reviewed changes

src/index.ts Show resolved Hide resolved

colemickens reviewed Nov 9, 2025

View reviewed changes

grahamc requested a review from colemickens November 9, 2025 20:22

Update messaging to be more action-oriented

540d4ad

cole-h reviewed Nov 10, 2025

View reviewed changes

cole-h approved these changes Nov 10, 2025

View reviewed changes

grahamc merged commit 89b1f59 into main Nov 10, 2025
17 checks passed

grahamc deleted the push-pokunnsooyqs branch November 10, 2025 03:55

coderabbitai bot mentioned this pull request Nov 10, 2025

Update to determinate by default #208

Merged

3 tasks

coderabbitai bot mentioned this pull request Jan 28, 2026

Tidy up the macos runner list #224

Merged

3 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Drop x86_64-darwin#207

Drop x86_64-darwin#207
grahamc merged 5 commits intomainfrom
push-pokunnsooyqs

grahamc commented Nov 8, 2025 •

edited by coderabbitai bot

Loading

Uh oh!

coderabbitai bot commented Nov 8, 2025 •

edited

Loading

Uh oh!

This comment was marked as outdated.

coderabbitai bot left a comment

Uh oh!

coderabbitai bot left a comment

Uh oh!

Uh oh!

colemickens Nov 9, 2025

Uh oh!

grahamc Nov 9, 2025

Uh oh!

colemickens Nov 9, 2025

Uh oh!

grahamc Nov 9, 2025

Uh oh!

grahamc Nov 9, 2025

Uh oh!

cole-h Nov 10, 2025

Uh oh!

cole-h Nov 10, 2025

Uh oh!

grahamc Nov 10, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

grahamc commented Nov 8, 2025 • edited by coderabbitai bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Checklist

Summary by CodeRabbit

Uh oh!

coderabbitai bot commented Nov 8, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Sequence Diagram(s)

Estimated code review effort

Poem

Pre-merge checks and finishing touches

Uh oh!

This comment was marked as outdated.

coderabbitai bot left a comment

Choose a reason for hiding this comment

Uh oh!

coderabbitai bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

grahamc commented Nov 8, 2025 •

edited by coderabbitai bot

Loading

coderabbitai bot commented Nov 8, 2025 •

edited

Loading