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

chore: skip failing packages on macOS #1006

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

alfonsograziano
Copy link
Contributor

Checklist
  • npm test passes
  • contribution guidelines followed
    here

Three packages are always failing during the install phase:

@codecov-commenter
Copy link

Codecov Report

All modified lines are covered by tests ✅

Comparison is base (c47ab10) 96.44% compared to head (70c4e20) 95.18%.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1006      +/-   ##
==========================================
- Coverage   96.44%   95.18%   -1.27%     
==========================================
  Files          28       28              
  Lines        2139     2139              
==========================================
- Hits         2063     2036      -27     
- Misses         76      103      +27     

see 4 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@RafaelGSS
Copy link
Member

Could you ping the maintainers of these packages?

@alfonsograziano
Copy link
Contributor Author

Sure! My fault actually:

bufferutil: @3rdeden, @einaros, @lpinca
leveldown: @ralphtheninja, @vweevers
microtime: @wadey

I'm pinging you since you're listed as the maintainer of the failing packages in the lookup file.
Your packages are constantly failing, can you please take a look at the CI reports in the PR description?

@lpinca
Copy link
Member

lpinca commented Oct 9, 2023

For bufferutil the issue is not in the module but in the machine the module is compiled on.

@alfonsograziano
Copy link
Contributor Author

Interesting! Maybe also the other packages can suffer of the same issue due to the machine (in fact, I can confirm that on my macOS machine it works perfectly). @lpinca do you have any insight on how we can fix the root issue?

@lpinca
Copy link
Member

lpinca commented Oct 9, 2023

It also seems that the same issue affects leveldown and microtime.

@lpinca
Copy link
Member

lpinca commented Oct 9, 2023

@lpinca do you have any insight on how we can fix the root issue?

I don't know, are the developer tools correctly installed? Is there any native module that is correctly compiled in that machine?

@lpinca
Copy link
Member

lpinca commented Oct 9, 2023

Maybe the clang version is too old to support the -arch arm64 flag. All three modules (bufferutil, leveldown, microtime) use it.

@alfonsograziano
Copy link
Contributor Author

Shall we open an issue in nodejs/build to ask for help on this topic? I don't have access to the machines

@richardlau
Copy link
Member

Maybe the clang version is too old to support the -arch arm64 flag. All three modules (bufferutil, leveldown, microtime) use it.

Quite possibly. The macOS 10.15 VMs are Xcode 11.

We're being forced by Apple to have to move off macOS 10.15 by 1 November 2023 but I'm not sure if that's just the release machines or if we'll also be migrating the test ones nodejs/build#3385.

@lpinca
Copy link
Member

lpinca commented Oct 9, 2023

I have just tested on a macOS 10.15 (10.15.7) VM with Xcode Command Line Tools installed via xcode-select --install and I can confirm that the issue is caused by the -arch arm64 option. Without it the module is compile correctly.

% clang --version
Apple clang version 12.0.0 (clang-1200.0.32.29)
Target: x86_64-apple-darwin19.6.0
Thread model: posix
InstalledDir: /Library/Developer/CommandLineTools/usr/bin

@vweevers
Copy link
Contributor

vweevers commented Oct 9, 2023

@lpinca Aye. I've seen that on rocksdb (has a similar setup as leveldown) some time ago but didn't get around to fixing it: Level/rocksdb#185 (comment) and Level/classic-level#108.

@lpinca
Copy link
Member

lpinca commented Oct 15, 2023

The problem should be fixed for bufferutil. See websockets/bufferutil@3af3375.

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.

7 participants