Skip to content
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.

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

fuel-nightly build failing with linker error on macos-latest #64

Closed
mitchmindtree opened this issue May 30, 2023 · 10 comments
Closed

fuel-nightly build failing with linker error on macos-latest #64

mitchmindtree opened this issue May 30, 2023 · 10 comments
Labels
bug Something isn't working

Comments

@mitchmindtree
Copy link
Contributor

mitchmindtree commented May 30, 2023

Spotted while working on #61. See this comment.

It seems like the recent addition of the sysinfo crate in FuelLabs/sway#4564 may be causing issues on the macOS build of the latest forc-nightly and possibly some non-determinism.

Today's refresh-manifests job succeeded, however the current CI macos-latest matrix job is missing the cache, and failing with a linker error.

       >   = note: Undefined symbols for architecture x86_64:
       >             "_kCFURLVolumeAvailableCapacityForImportantUsageKey", referenced from:
       >                 _$LT$sysinfo..apple..disk..Disk$u20$as$u20$sysinfo..traits..DiskExt$GT$::refresh::ha090b62f02761b2b in libsysinfo-bfbd6883445a3969.rlib(sysinfo-bfbd6883445a3969.sysinfo.4f5fc5ae-cgu.15.rcgu.o)
       >                 sysinfo::apple::disk::get_disks::h63aaac400a8fb64f in libsysinfo-bfbd6883445a3969.rlib(sysinfo-bfbd6883445a3969.sysinfo.4f5fc5ae-cgu.15.rcgu.o)
       >           ld: symbol(s) not found for architecture x86_64
       >           clang-11: error: linker command failed with exit code 1 (use -v to see invocation)
       >           
       >
       > error: could not compile `forc` due to previous error
       For full logs, run 'nix log /nix/store/vgq1zhv6q43wbgbd5hifm26ab2wikwj9-forc-0.39.1.drv'.
error: 1 dependencies of derivation '/nix/store/jfxshavhlsxkkdfnjdqpxfcyjv1icj6f-fuel-nightly.drv' failed to build
@mitchmindtree mitchmindtree added the bug Something isn't working label May 30, 2023
@mitchmindtree
Copy link
Contributor Author

It seems this is now occurring as of the latest refresh-manifests run too: https://github.com/FuelLabs/fuel.nix/actions/runs/5127395153/jobs/9224233318

The fact that this started occurring in #61 which contains no changes to the set of manifests implies that the issue may come from elsewhere, e.g. a difference in the SDKs provided to the macos-latest runner, or some other source of non-determinism in the CI jobs like an unspecified Nix version 🤔

@mitchmindtree mitchmindtree mentioned this issue May 31, 2023
7 tasks
@mitchmindtree
Copy link
Contributor Author

This comment on a similar issue indicates the culprit function might be only available on macos 10.13+

rust-build/rust-build.action#61 (comment)

Also, I've cancelled today's refresh-manifests run in order to solve this first.

@mitchmindtree
Copy link
Contributor Author

mitchmindtree commented Jun 1, 2023

It seems that the GitHub runner does now use 10.13 and this switch might be what has caused us issues, as the nixpkgs Darwin SDK is still on 10.12.

I'm attempting to pin our runner to macos 10.12 in #68 to see if the sysinfo crate will automatically use an older API.

@mitchmindtree
Copy link
Contributor Author

mitchmindtree commented Jun 1, 2023

Ahh it seems I'm mistaken - when checking the action logs, it appears both macOS runners (macos-12 and macos-13 i.e. macos-latest) are still using macOS 12.6.5.

It seems I've gotten mixed up with the point versioning! The API documentation for _kCFURLVolumeAvailableCapacityForImportantUsageKey indicates the necessary version is macos 10.13+, not macos 13+, so we should be fine w.r.t. having necessary SDK versions available from both the GitHub runner and nixpkgs.

This means our issue stems from elsewhere.

The error itself indicates that we're not providing the CoreFoundation framework to the forc build, however we've already been providing it for darwin builds for a while via this entry in patches.nix:

fuel.nix/patches.nix

Lines 141 to 153 in 4cbf9a6

# We generally appear to require these frameworks on darwin.
{
condition = m: pkgs.lib.hasInfix "darwin" pkgs.system;
patch = m: {
buildInputs =
(m.buildInputs or [])
++ [
pkgs.darwin.apple_sdk.frameworks.CoreFoundation
pkgs.darwin.apple_sdk.frameworks.Security
pkgs.darwin.apple_sdk.frameworks.SystemConfiguration
];
};
}

mitchmindtree added a commit that referenced this issue Jun 1, 2023
Currently, the frameworks are provided as build inputs - this is an
attempt to provide the CoreFoundation framework as a nativeBuildInput to
see if it addresses the current CI error tracked in #64.
@mitchmindtree
Copy link
Contributor Author

Not having much luck chasing down this linker issue - I've opened a post on the forum to see if anyone has any ideas on what might be going on here: https://discourse.nixos.org/t/unable-to-link-to-corefoundation-symbols-even-though-corefoundation-framework-is-provided-as-buildinput-x86-64-darwin/28612

@mitchmindtree
Copy link
Contributor Author

After some more digging, it appears the recent forc-nightly-2023-05-30 manifest that is causing macos-latest fuel-nightly CI jobs to fail slipped through due to #70 and #71, and likely not to any non-determinism in the build process.

The simplest approach forward may be to:

  1. Revert the previous refresh-manifests runs that introduced the broken nightly to make sure fuel-nightly works on all platforms on master.
  2. Address Update refresh-manifests job to only commit manifests if *all* platforms succeed (not just Linux) #70 and The refresh-cache step in the refresh-manifests job uses previous master (not newly committed master with new manifests) #71 to ensure no new broken manifests make it into master.
  3. Address the issues in the latest forc-nightly in a dedicated PR to unblock new refresh-manifests runs.

@mitchmindtree mitchmindtree changed the title fuel-nightly non-deterministic failure on macos-latest fuel-nightly build failing with linker error on macos-latest Jun 1, 2023
mitchmindtree added a commit that referenced this issue Jun 1, 2023
…fuel-nightly` (#69)

Currently, the frameworks are provided as build inputs - this is an
attempt to provide the CoreFoundation framework as a nativeBuildInput to
see if it addresses the current CI error tracked in #64.

Edit: Also tried `propagatedBuildInputs` to see if propagating
CoreFoundation to the runtime inputs helped, but no luck.

Edit: Now attempting to remove the most recent manifest sets in order to
find the last working version of `fuel-nightly` so I can diff the two.
@mitchmindtree
Copy link
Contributor Author

OK #70 and #71 have been closed by #73!

Just as a reminder when looking into this again: the specific forc manifest commits (for the Sway repo) in which the linking error starts occurring are:

Working: af07c68f12b4ca780178b9e66f21af9173602b4b
Broken: 8844b4f60ef19ba5b1972e8e4932e023ec0415c0

The Sway PR that introduces it is: FuelLabs/sway#4564.

@mitchmindtree
Copy link
Contributor Author

It looks like this issue at the sysinfo repo is the same issue encountered by holochain devs: GuillaumeGomez/sysinfo#915 (comment).

@JoshuaBatty
Copy link
Member

Note, I tried applying the fix that @mitchmindtree suggested above. It basically involved specifying that we want to use the macos_sdk_11_0 version specifically.

While it may actually fix the sysinfo error that was occurring, we are now unable to build libgit2. See this failing CI log.

Note, this is all only happening when building for Intel mac architecture and is passing successfully when building for arm. I was able to use my M1 mac to test building locally for i386 though. If you right click on terminal and open properties, you can click the checkbox that says open using rosetta.

@JoshuaBatty
Copy link
Member

This is closed now that FuelLabs/sway#4737 landed and we no longer need to build sysinfo

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants