build: enable handling of alpine-linux-musl triple #61376

MaxDesiatov · 2022-09-29T22:53:51Z

Currently, when building LLVM/clang on Alpine Linux, CMake toolchain file specifies incorrect <cpu_arch>-unknown-linux-musl target, which makes the build immediately fail. Correct target that allows building on Alpine should be specified as <cpu_arch>-alpine-linux-musl, with <cpu_arch> replaced with respective CPU platform, e.g. aarch64.

MaxDesiatov · 2022-09-29T22:54:00Z

@swift-ci please smoke test

MaxDesiatov · 2022-09-30T12:54:39Z

@swift-ci please smoke test

tomerd · 2022-09-30T15:49:27Z

the general fix makes sense. is a more elegant way to detect it than shelling out to clang? how does clang do it?

MaxDesiatov · 2022-09-30T15:52:32Z

AFAIU, clang implements distro-specific parsing of /proc/version and /etc/*release files when running on Linux. Here we delegate to that logic, an alternative would be to reimplement it in our Python build scripts.

etcwilde

Small nits, lets use vendor instead of distro so that it keeps with existing triple terminology. Otherwise, LGTM.

How great it would be if build-script could just take a triple instead of trying to magically munge something together.

compnerd · 2022-09-30T18:39:06Z

utils/swift_build_support/swift_build_support/products/product.py

-        sysroot_arch, abi = self.get_linux_abi(arch)
-        return '{}-unknown-linux-{}'.format(sysroot_arch, abi)
+        sysroot_arch, distro, abi = self.get_linux_abi(arch)
+        return '{}-{}-linux-{}'.format(sysroot_arch, distro, abi)


The field being substituted is the "vendor". I agree with @etcwilde that we should stick to standard terminology here.

SuSe also depends on the vendor being proper IIRC. Keep in mind that this will make cross-compilation more complicated as the target triple is embedded into the serialised swiftmodule. Do we want to consider filtering this in the driver? That would allow us to use the same swiftmodule on different distributions but get the correct triple for the link phase.

It may be interesting to consider actually adding a new field into the swiftmodule to actually embed a different clang triple to the swift triple.

The field being substituted is the "vendor". I agree with @etcwilde that we should stick to standard terminology here.

Thanks, that's updated now.

SuSe also depends on the vendor being proper IIRC. Keep in mind that this will make cross-compilation more complicated as the target triple is embedded into the serialised swiftmodule. Do we want to consider filtering this in the driver? That would allow us to use the same swiftmodule on different distributions but get the correct triple for the link phase.

It may be interesting to consider actually adding a new field into the swiftmodule to actually embed a different clang triple to the swift triple.

I don't have an informed opinion on this. Should I tag someone else to review this aspect of the change?

I could have sworn that the name of the swift module helped in this regard. e.g. Blarpy.swiftmodule/x86_64-apple-macos.swiftmodule vs Blarpy.swiftmodule/arm64-apple-macos.swiftmodule are applied to their corresponding architectures on macOS. I haven't looked closely so take it with a grain of salt, but having the swiftmodule key'd on its full triple should work on Linux too. The build system probably needs to learn to do this on things that aren't just Apple platforms though.

@xymus, can you confirm that? Thanks.

Having a different target triple should help to avoid loading incompatible swiftmodules in the case of different distributions. I wouldn't trust loading a swiftmodule built on a different distro as they could refer to things from the C world that are organized differently. That being said, I don't work much in the Linux side of things so I'd be interested to know if it's an accepted practice there.

The default Linux toolchain distribution does not use the directory layout like Windows and macOS, so you lose that ability to differentiate. How much C code actually gets serialised into the object layout? The only case that I would actually be concerned about is if we are serialising opaque C structures (e.g. pthread_t) somehow. The remainder of the differences I believe should be handled implicitly as the macros are contained within the C module, which is re-constructed through the modulemap. The current loading behavior I feel may be a bit confusing for users. We should definitely include a test case for that along with a reasonable error message if we intend to ship this as is.

MaxDesiatov · 2022-09-30T19:38:00Z

@swift-ci please smoke test

compnerd · 2022-10-01T00:29:11Z

utils/swift_build_support/swift_build_support/products/product.py

+        vendor = 'unknown'
+
+        try:
+            output = shell.capture(["clang", "--version"])


This concerns me. This is dependent on the value of -D LLVM_DEFALT_TARGET_TRIPLE passed when building clang. Worse yet, it simply invokes clang without any guarantees on which clang is being invoked. So, if I were to build a clang and add it to my path even on alpine, this could give me unknown as the vendor or any other literal.

What would be a more reliable alternative to this invocation then, in your opinion?

I would say that you should consider exposing this as a flag that can be explicitly specified, defaulting to processing /etc/*-release, /etc/os-release, and lsb_release -b. The default would only work under a non-cross-compiling scenario of course, but that applies to most cases for the default. We could expose this as a completely generic --default-target flag perhaps.

I've changed it to use -print-target-triple, which reliably gives us the actual triple and that's the only output it produces, so we don't need to do any additional parsing.

Currently, when building LLVM/clang on Alpine Linux, CMake toolchain file specifies incorrect `<cpu_arch>-unknown-linux-musl` target, which makes the build immediately fail. Correct target that allows building on Alpine should be specified as `<cpu_arch>-alpine-linux-musl`, with `<cpu_arch>` replaced with respective CPU platform, e.g. `aarch64`.

MaxDesiatov · 2023-07-13T14:37:05Z

@swift-ci test

MaxDesiatov · 2023-07-13T14:37:19Z

@swift-ci build toolchain

MaxDesiatov requested review from gottesmm, tomerd and etcwilde September 29, 2022 22:55

MaxDesiatov force-pushed the maxd/build-alpine-product branch 2 times, most recently from 7b51100 to 32dac24 Compare September 30, 2022 12:54

etcwilde reviewed Sep 30, 2022

View reviewed changes

compnerd reviewed Sep 30, 2022

View reviewed changes

MaxDesiatov force-pushed the maxd/build-alpine-product branch from 32dac24 to 52593c5 Compare September 30, 2022 19:33

MaxDesiatov requested review from etcwilde and compnerd September 30, 2022 19:35

compnerd reviewed Oct 1, 2022

View reviewed changes

MaxDesiatov force-pushed the maxd/build-alpine-product branch from 52593c5 to d3768b3 Compare November 21, 2022 12:15

MaxDesiatov force-pushed the maxd/build-alpine-product branch 2 times, most recently from 26c62b6 to 1e37999 Compare January 11, 2023 16:26

MaxDesiatov force-pushed the maxd/build-alpine-product branch 2 times, most recently from 5e1b11c to 5db8cee Compare January 18, 2023 14:47

MaxDesiatov force-pushed the maxd/build-alpine-product branch 3 times, most recently from 37c40ec to 4181a4c Compare June 12, 2023 13:52

MaxDesiatov force-pushed the maxd/build-alpine-product branch from 4181a4c to b62eb2a Compare June 19, 2023 19:45

MaxDesiatov force-pushed the maxd/build-alpine-product branch from b62eb2a to 4ce56d3 Compare July 5, 2023 10:15

MaxDesiatov force-pushed the maxd/build-alpine-product branch from 4ce56d3 to 501a5da Compare July 5, 2023 21:14

Use clang -print-target-triple instead of clang --version

c1d2d85

MaxDesiatov requested a review from compnerd July 13, 2023 14:36

MaxDesiatov requested a review from al45tair July 13, 2023 14:36

al45tair approved these changes Jul 18, 2023

View reviewed changes

MaxDesiatov merged commit 0a1ae45 into main Jul 18, 2023

MaxDesiatov deleted the maxd/build-alpine-product branch July 18, 2023 12:05

finagolfin mentioned this pull request Jul 21, 2024

Add buildbot profile for cross-compiling for linux-armv7 using external sysroot #75367

Closed

build: enable handling of alpine-linux-musl triple #61376

build: enable handling of alpine-linux-musl triple #61376

Uh oh!

Conversation

MaxDesiatov commented Sep 29, 2022

Uh oh!

MaxDesiatov commented Sep 29, 2022

Uh oh!

MaxDesiatov commented Sep 30, 2022

Uh oh!

tomerd commented Sep 30, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

MaxDesiatov commented Sep 30, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

etcwilde left a comment

Choose a reason for hiding this comment

Uh oh!

compnerd Sep 30, 2022

Choose a reason for hiding this comment

Uh oh!

MaxDesiatov Sep 30, 2022

Choose a reason for hiding this comment

Uh oh!

etcwilde Sep 30, 2022

Choose a reason for hiding this comment

Uh oh!

xymus Sep 30, 2022

Choose a reason for hiding this comment

Uh oh!

compnerd Oct 1, 2022

Choose a reason for hiding this comment

Uh oh!

MaxDesiatov commented Sep 30, 2022

Uh oh!

compnerd Oct 1, 2022

Choose a reason for hiding this comment

Uh oh!

MaxDesiatov Oct 1, 2022

Choose a reason for hiding this comment

Uh oh!

compnerd Oct 2, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

MaxDesiatov Jul 13, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

MaxDesiatov commented Jul 13, 2023

Uh oh!

MaxDesiatov commented Jul 13, 2023

Uh oh!

Uh oh!

tomerd commented Sep 30, 2022 •

edited

Loading

MaxDesiatov commented Sep 30, 2022 •

edited

Loading

compnerd Oct 2, 2022 •

edited

Loading

MaxDesiatov Jul 13, 2023 •

edited

Loading