CI: Always include macOS version in explicit triples #4553
Conversation
| llvm::SmallString<16> osname; | ||
| osname += "macosx"; | ||
| osname += "macos"; |
There was a problem hiding this comment.
I've stripped off the x; Apple uses macos{10.12,11} in https://developer.apple.com/documentation/apple-silicon/building-a-universal-macos-binary, and Google yields 4-5x more results for arm64-apple-macos11 than arm64-apple-macosx11.
kinke
force-pushed
the
macos_explicit_version
branch
from
476c67a to
1126e66
Compare
…et …` for clang linker driver In order not to have to hardcode an explicit macOS version.
kinke
force-pushed
the
macos_explicit_version
branch
from
b9893fb to
1650a5f
Compare
| \"-Xcc=-target\", | ||
| \"-Xcc=x86_64-apple-macos\", | ||
| \"-Xcc=-arch\", | ||
| \"-Xcc=x86_64\", |
There was a problem hiding this comment.
I think this is better than specifying a full triple with some hardcoded macOS version.
There was a problem hiding this comment.
This change leads to a problem on my mac. The compiler does not recognize -arch (it wants -march).
I'm testing what works best...
There was a problem hiding this comment.
ah cool... clang wants -arch and c++ wants -march ...
The problem is that -arch= is not accepted by c++ (I think kinke mentioned this before somewhere).
The fix in PR #4431 only works because -target= is accepted.
There was a problem hiding this comment.
And everything works when the target_link_options version of #4431 is used.
There was a problem hiding this comment.
(the solution with -target works, but runs into an annoying warning about unused -arch arm64 warning when linking LDC; so trying to find other solution)
There was a problem hiding this comment.
Is it possible to get the full triple that was matched and then insert it with the -target flag?
Another idea would be to not specify this flag in the config file and instead let LDC get the operating system version at runtime and use the full triple including the version. It's more flexible than a config file.
There was a problem hiding this comment.
Oh right - we could simply have LDC always add an appropriate -target option when linking an Apple target. Then we only need the -isysroot option in the config file, for iOS targets.
There was a problem hiding this comment.
#4431 now resolves building LDC with this, so from my side no change is needed (and I think the arch flag is better than triple, because the arch flag can't be wrong here)
|
@jacob-carlborg Can you verify that this now works for you? You can grab the package from https://github.com/ldc-developers/ldc/releases/tag/CI (wait until rebuild with this PR included) |
|
@JohanEngelen how do I know if this PR is included in the CI release? |
|
@JohanEngelen Seems to be working. I don't get any warnings and the object files contains the correct sections. |
|
@jacob-carlborg Thanks for confirming the fix. (the 8264f97 binaries contain this PR, Github says they were uploaded ~8hours ago) |
Wrt. issue #4501.