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

The options to the linker need to be separate elements in the list, n… #427

Closed
wants to merge 1 commit into from

Conversation

JGailor
Copy link

@JGailor JGailor commented Nov 9, 2023

The options to the linker were a single element in a list instead of two elements. This was breaking build on MacOS

…ot a single element. This was breaking build on MacOS
@sagudev
Copy link
Member

sagudev commented Nov 10, 2023

@JGailor
Copy link
Author

JGailor commented Nov 10, 2023

Can you give some more context on this? The parameters look wrong in the original code and making the list 2 separate elements was the way I was able to get mozjs and then Servo to compile on a Mac OS Ventura.

@sagudev
Copy link
Member

sagudev commented Nov 10, 2023

mozjs-sys/mozjs folder is from mozilla, so it's always good to check if upstream has similar patch. But from what I know -Wl,--version is correct and should be passed as one argument (see: https://stackoverflow.com/a/6562437) also android CI fails if it's not.

We do run tests on Mac OS Venturain CI, so something might be wrong on your end. Which compiler/linker do you use? Clang with lld linker should work.

@nicoburns
Copy link

This patch also fixes building (cargo build) for me on macOS (13.6). I have made no special effort to configure the linker, so I suspect it's using ld64.

@sagudev
Copy link
Member

sagudev commented Nov 21, 2023

Hm, I did some more digging and apparently apple changed their default linker: mesonbuild/meson#12282 (comment) and we should be able to get old one using -Wl,-ld_classic and get version information more regularly using -v option. There is also some interesting info here: mesonbuild/meson#11958 how compiler outputs now differ and mesonbuild/meson#12282 (comment) with fix for mason.

Good old clang should still work and maybe it's only supported configuration: https://firefox-source-docs.mozilla.org/contributing/build/supported.html#tier-1-targets or because they switch Firefox builds to macOS 14.0 SDK just two months ago, so it might be unsupported configuration altogether. Anyhow this code should select lld on too latest xcode; so how exactly does error looks?

@mrobinson
Copy link
Member

This is the failure message:

  DEBUG: Executing: `/usr/bin/cc -isysroot /Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX14.sdk -mmacosx-version-min=11.0 -std=gnu99 --target=arm64-apple-darwin -Wl,--version`
  ERROR: Failed to find an adequate linker

@sagudev sagudev mentioned this pull request Dec 6, 2023
@sagudev
Copy link
Member

sagudev commented Dec 6, 2023

Closed in favor of #436

@sagudev sagudev closed this Dec 6, 2023
@Naist4869
Copy link

Closed in favor of #436以#436 结束

["-Wl", "--version"] is work for me ,

return try_linker(linker, "-Wl,-v") not work.

@sagudev
Copy link
Member

sagudev commented Jan 29, 2024

Closed in favor of #436以#436 结束

["-Wl", "--version"] is work for me ,

return try_linker(linker, "-Wl,-v") not work.

Does current main branch work for you? It firstly tries -Wl,--version and then fallback to -Wl,-v.

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.

5 participants