Skip to content

fix(ci): Do not use mold on macos #7321

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 3 commits into from
Jul 17, 2025
Merged

fix(ci): Do not use mold on macos #7321

merged 3 commits into from
Jul 17, 2025
+28 −7

Conversation

tolbrino
Copy link
Contributor

@tolbrino tolbrino self-assigned this Jul 17, 2025
@Copilot Copilot AI review requested due to automatic review settings July 17, 2025 08:09
@coderabbitai coderabbitai
Copy link
Contributor

coderabbitai bot commented Jul 17, 2025 •
edited
Loading

📝 Walkthrough

Walkthrough

The update refines platform-specific handling in Nix build configurations by conditionally adding the mold linker only for Linux builds and removing a special ARMv7 Linux interpreter path. It also adjusts the cargoTarget variable to map "arm64-apple-darwin" to "aarch64-apple-darwin" in multiple files.

Changes

File(s) Change Summary
nix/rust-package.nix Removed ARMv7 Linux interpreter path special case; introduced linuxNativeBuildInputs to add mold only on Linux builds.
nix/mkShell.nix, nix/rust-builder.nix Modified cargoTarget assignment to map "arm64-apple-darwin" to "aarch64-apple-darwin"; conditionally include mold in Linux build inputs.

Sequence Diagram(s)

sequenceDiagram
    participant User
    participant NixBuild
    participant LinuxPlatform
    participant DarwinPlatform

    User->>NixBuild: Initiate build
    NixBuild->>LinuxPlatform: Check platform
    alt Platform is Linux
        LinuxPlatform-->>NixBuild: true
        NixBuild->>NixBuild: Add mold to nativeBuildInputs
    else Platform is Darwin
        DarwinPlatform-->>NixBuild: true
        NixBuild->>NixBuild: Do not add mold
    end
    NixBuild->>User: Build proceeds with platform-specific inputs
Loading

Suggested labels

toolchain

Suggested reviewers

  • Teebor-Choka
📜 Recent review details

Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between f7f6330 and 280134e.

📒 Files selected for processing (2)
  • nix/mkShell.nix (2 hunks)
  • nix/rust-builder.nix (1 hunks)
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings 
Learnt from: NumberFour8
PR: hoprnet/hoprnet#6244
File: transport/protocol/src/ack/processor.rs:31-32
Timestamp: 2024-07-28T07:26:06.634Z
Learning: The GitHub issue to standardize the usage of `METRIC_TICKETS_COUNT` across the codebase was successfully created and can be tracked at https://github.com/hoprnet/hoprnet/issues/6245.

Learnt from: NumberFour8
PR: hoprnet/hoprnet#6244
File: transport/protocol/src/ack/processor.rs:31-32
Timestamp: 2024-10-09T06:16:18.170Z
Learning: The GitHub issue to standardize the usage of `METRIC_TICKETS_COUNT` across the codebase was successfully created and can be tracked at https://github.com/hoprnet/hoprnet/issues/6245.

Learnt from: NumberFour8
PR: hoprnet/hoprnet#6630
File: crypto/packet/src/por.rs:84-84
Timestamp: 2024-11-13T17:56:37.536Z
Learning: In PR #6630, the changes include the removal of ticket validation from the last hop and fixes for Index Offset handling as specified in the PR objectives.

Learnt from: ausias-armesto
PR: hoprnet/hoprnet#7312
File: docs/changelog/changelog-3.0.0-rc.1.md:39-46
Timestamp: 2025-07-15T15:28:59.124Z
Learning: The changelog files in the hoprnet/hoprnet project are auto-generated from PRs, so manual editing suggestions for deduplication are not appropriate. Any improvements to prevent duplicate entries would need to be handled at the generation level.

Learnt from: Teebor-Choka
PR: hoprnet/hoprnet#6545
File: transport/p2p/src/swarm.rs:491-494
Timestamp: 2024-10-11T10:43:34.869Z
Learning: When reviewing revert pull requests in the `hoprnet/hoprnet` repository, avoid making code recommendations or suggestions.

Learnt from: tolbrino
PR: hoprnet/hoprnet#6651
File: nix/rust-package.nix:35-39
Timestamp: 2024-11-20T08:43:53.099Z
Learning: In the `nix/rust-package.nix` file, replacing hardcoded interpreter paths with `${stdenv.hostPlatform.platform.dynamicLinker}` doesn't work as intended, so hardcoded interpreter paths are necessary.

Learnt from: tolbrino
PR: hoprnet/hoprnet#7110
File: nix/mkShell.nix:27-27
Timestamp: 2025-05-06T07:42:34.550Z
Learning: In the Nix package ecosystem, the LLD linker (`lld`) is part of the `llvmPackages.bintools` package, not a separate package that needs to be added independently.

Learnt from: tolbrino
PR: hoprnet/hoprnet#7070
File: nix/testShell.nix:22-23
Timestamp: 2025-04-15T19:24:23.570Z
Learning: In the HOPR project, the `autoPatchelf` command from Nix can handle the case where the target directory (like ./.venv) doesn't exist, so no additional directory existence check is needed before running it.

Learnt from: Teebor-Choka
PR: hoprnet/hoprnet#6886
File: nix/rust-package.nix:84-84
Timestamp: 2025-02-25T00:35:33.437Z
Learning: When transitioning from vendored dependencies to a private proxy registry in Rust projects, GitHub Actions workflows need to be refactored to accommodate the new update strategy, particularly workflows handling dependency updates like renovate-cargo-update.yaml.
nix/rust-builder.nix (3) 
Learnt from: tolbrino
PR: hoprnet/hoprnet#6651
File: nix/rust-package.nix:35-39
Timestamp: 2024-11-20T08:43:53.099Z
Learning: In the `nix/rust-package.nix` file, replacing hardcoded interpreter paths with `${stdenv.hostPlatform.platform.dynamicLinker}` doesn't work as intended, so hardcoded interpreter paths are necessary.

Learnt from: tolbrino
PR: hoprnet/hoprnet#6651
File: nix/rust-package.nix:19-19
Timestamp: 2024-11-20T08:30:15.619Z
Learning: In the `nix/rust-package.nix` file, the `rev` parameter is always expected to be provided, and it's acceptable to not assign a default value to it.

Learnt from: Teebor-Choka
PR: hoprnet/hoprnet#6886
File: nix/rust-package.nix:84-84
Timestamp: 2025-02-25T00:35:33.437Z
Learning: When transitioning from vendored dependencies to a private proxy registry in Rust projects, GitHub Actions workflows need to be refactored to accommodate the new update strategy, particularly workflows handling dependency updates like renovate-cargo-update.yaml.
nix/mkShell.nix (5) 
Learnt from: tolbrino
PR: hoprnet/hoprnet#6651
File: nix/rust-package.nix:35-39
Timestamp: 2024-11-20T08:43:53.099Z
Learning: In the `nix/rust-package.nix` file, replacing hardcoded interpreter paths with `${stdenv.hostPlatform.platform.dynamicLinker}` doesn't work as intended, so hardcoded interpreter paths are necessary.

Learnt from: tolbrino
PR: hoprnet/hoprnet#6651
File: nix/rust-package.nix:19-19
Timestamp: 2024-11-20T08:30:15.619Z
Learning: In the `nix/rust-package.nix` file, the `rev` parameter is always expected to be provided, and it's acceptable to not assign a default value to it.

Learnt from: tolbrino
PR: hoprnet/hoprnet#7110
File: nix/mkShell.nix:27-27
Timestamp: 2025-05-06T07:42:34.550Z
Learning: In the Nix package ecosystem, the LLD linker (`lld`) is part of the `llvmPackages.bintools` package, not a separate package that needs to be added independently.

Learnt from: tolbrino
PR: hoprnet/hoprnet#7070
File: nix/testShell.nix:22-23
Timestamp: 2025-04-15T19:24:23.570Z
Learning: In the HOPR project, the `autoPatchelf` command from Nix can handle the case where the target directory (like ./.venv) doesn't exist, so no additional directory existence check is needed before running it.

Learnt from: Teebor-Choka
PR: hoprnet/hoprnet#6886
File: nix/rust-package.nix:84-84
Timestamp: 2025-02-25T00:35:33.437Z
Learning: When transitioning from vendored dependencies to a private proxy registry in Rust projects, GitHub Actions workflows need to be refactored to accommodate the new update strategy, particularly workflows handling dependency updates like renovate-cargo-update.yaml.
⏰ 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). (15)
  • GitHub Check: hoprd-aarch64-darwin / binary
  • GitHub Check: hopli-x86_64-darwin / binary
  • GitHub Check: hopli-aarch64-darwin / binary
  • GitHub Check: hoprd-x86_64-darwin / binary
  • GitHub Check: Zizmor / Zizmor
  • GitHub Check: Lint / Lint
  • GitHub Check: Audit / Audit
  • GitHub Check: hopli / docker
  • GitHub Check: Pre-commit / Pre-commit
  • GitHub Check: hoprd / docker
  • GitHub Check: Docs / Rust
  • GitHub Check: tests-unit
  • GitHub Check: Build Candidate Binaries (hopli)
  • GitHub Check: Build Candidate Binaries (hoprd)
  • GitHub Check: tests-unit-nightly
🔇 Additional comments (4)
nix/rust-builder.nix (1)

57-58: LGTM: Correct platform target mapping for Apple ARM64.

The conditional mapping from "arm64-apple-darwin" to "aarch64-apple-darwin" ensures consistent platform target naming for Apple ARM64 builds, which aligns with Rust toolchain expectations.

nix/mkShell.nix (3)

12-12: LGTM: Good refactoring to avoid repetition.

Introducing the buildPlatform variable improves code readability and maintainability by avoiding repetition of pkgs.stdenv.buildPlatform.

13-17: LGTM: Consistent platform target mapping.

The conditional mapping from "arm64-apple-darwin" to "aarch64-apple-darwin" is consistent with the implementation in nix/rust-builder.nix and ensures proper platform target naming for Apple ARM64 builds.

26-34: LGTM: Correct platform-specific mold handling.

The linuxMinimumPackages implementation correctly addresses the PR objective by conditionally including the mold package only on Linux platforms. This prevents the macOS build issues mentioned in the PR objectives while maintaining the performance benefits of mold on Linux.

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
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Explain this complex logic.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai explain this code block.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and explain its main purpose.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Support

Need help? Create a ticket on our support page for assistance with any issues or questions.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (Invoked using PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR.
  • @coderabbitai generate sequence diagram to generate a sequence diagram of the changes in this PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copilot
Copilot AI reviewed Jul 17, 2025
View reviewed changes
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR fixes a macOS build issue by preventing the mold linker from being used on macOS platforms. The mold linker is Linux-specific and causes build failures when included in macOS builds.

  • Removes mold from the universal native build inputs list
  • Creates a Linux-specific build inputs list that includes mold only for Linux builds
  • Removes unused ARMv7 Linux dynamic linker path configuration
Comments suppressed due to low confidence (1)

nix/rust-package.nix:74

  • [nitpick] The variable name 'linuxNativeBuildInputs' is inconsistent with the existing naming pattern. Consider renaming it to 'linuxNativeBuildInputs' to match the pattern of 'darwinNativeBuildInputs' and 'darwinBuildInputs'.
  linuxNativeBuildInputs =

@tolbrino tolbrino added binary:aarch64-darwin Build aarch64-darwin binary binary:x86_64-darwin Build x86_64-darwin binary labels Jul 17, 2025
@tolbrino tolbrino added this to the 3.0.0-temporary milestone Jul 17, 2025
@tolbrino tolbrino requested a review from a team July 17, 2025 09:19
@tolbrino tolbrino enabled auto-merge (squash) July 17, 2025 09:19
@tolbrino tolbrino changed the title fix(macos): Do not use mold on macos fix(ci): Do not use mold on macos Jul 17, 2025
Teebor-Choka
Teebor-Choka approved these changes Jul 17, 2025
View reviewed changes
Copy link
Contributor

@Teebor-Choka Teebor-Choka left a comment

Choose a reason for hiding this comment

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

What the hell is arm64 darwin build?

@tolbrino tolbrino merged commit e207ffa into master Jul 17, 2025
44 of 46 checks passed
@tolbrino tolbrino deleted the tb/202507-fix-macos-mold branch July 17, 2025 09:43
@tolbrino
Copy link
Contributor Author

What the hell is arm64 darwin build?

nixpkgs has changed the platform name to consolidate

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

Copilot code review Copilot Copilot left review comments

@Teebor-Choka Teebor-Choka Teebor-Choka approved these changes

Assignees

@tolbrino tolbrino

Labels
binary:aarch64-darwin Build aarch64-darwin binary binary:x86_64-darwin Build x86_64-darwin binary
Projects
None yet
Milestone
3.0.0
Development

Successfully merging this pull request may close these issues.
3 participants
@tolbrino @Teebor-Choka @ausias-armesto