search

crystal-lang / distribution-scripts

40 stars 24 forks source link

Fix macos triple #296

Closed hovsater closed 7 months ago

hovsater commented 7 months ago

I've been investigating this issue and it's related to the default target triple produced by Crystal during compilation. It seems like in order to fix a bug back in 2015, we stripped the minimum deployment target from the target triple in this commit. With the release of Xcode 15, a new linker was introduced and with it came these warning messages. It appears that the new linker is stricter in which target triples it considers valid. Specifically, it looks like the minimum deployment target is required. E.g., aarch64-apple-darwin23.3.0 is valid, while aarch64-apple-darwin is not.

We still need to handle the issue of normalization that's happening here, probably by not normalizing the macOS target triple at all and I'll do that in a follow up pull request.

hovsater commented 7 months ago

@straight-shoota would be possible to run a test build with these changes to see how the default target triple comes out?

straight-shoota commented 7 months ago

https://app.circleci.com/pipelines/github/crystal-lang/crystal/14567/workflows/6f82a2a4-06ce-42d6-9035-6b0319ab31af/jobs/82546

hovsater commented 7 months ago

Seems like it's working! Here's the output of the generated artefact:

$ crystal -v
Crystal 1.12.0-dev [2800d9462] (2024-04-06)                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                             LLVM: 15.0.7
Default target: aarch64-apple-macosx11.0

It subsequently also silences the warning messages as the default target triple is arm64-apple-macosx11.0 and thus, it bypasses the normalization process. The only reason Crystal report it as aarch64-apple-macosx11.0 is due to the architecture normalization done in Crystal::Codegen::Target.

@straight-shoota like you mentioned in https://github.com/crystal-lang/crystal/issues/13846#issuecomment-2041048568, we should probably discuss whether or not it makes sense to keep the normalization done in LLVM.default_target_triple. At least on macOS.

ysbaddaden commented 7 months ago

@hovsater as far as I recall the normalization was meant to 1. make sure we use a single flag, whatever the triple name (e.g. armhf -> arm, amd64 -> x86_64) and 2. generate a triple for choosing the correct LibC binding (src/lib_c).

Now, forcing the normalization of the actual elements was probably not a good idea. We might want to have #normalize and #normalized_architecture instead to handle the above cases, and keep the explicit triple.

hovsater commented 7 months ago

@ysbaddaden that makes sense. We do make some normalization in Crystal::Codegen::Target which gets passed whatever the default target triple is. With this change, it seems to be working fine. The architecture is normalized for any consumers of the host target, while the actual triple is left alone. 🙂

Of course, this is only applicable on macOS currently, since the arm64 prefix bypasses the normalization done in LLVM.default_target_triple.

straight-shoota commented 7 months ago

FTR: This change only affects the ARM build of the compiler. It's not expected to remove linker warnings on Intel macs.

hovsater commented 7 months ago

Yeah, you're right. We do not set the target triple explicitly for x86_64, so I assume we're already using an appropriate target triple. When we follow up with the pull request addressing normalization, it should sort itself out, I believe.