Link all compatibility libraries when cross compiling and bootstrapping #60728
Conversation
edymtt
requested review from
compnerd,
rintaro,
drexin,
gottesmm,
etcwilde and
shahmishal
|
@swift-ci please test |
|
@swift-ci please build toolchain |
There was a problem hiding this comment.
I think I would prefer having the list of libraries kept in a single place and tie them in with the b0_deps and b1_deps (https://github.com/apple/swift/blob/19831bea587afc9e2f8af07cda4c89ecbb90e4af/SwiftCompilerSources/CMakeLists.txt#L305-L311), that would help reduce the number of places I could possibly miss when I go and add a new compat lib.
edymtt
force-pushed
the
link-more-compat-libraries-when-crosscompiling
branch
from
bc4c425 to
02faa56
Compare
| add_subdirectory("Compatibility${compatibility_name}") | ||
| endforeach() | ||
|
|
||
| set(vsuffix "-${SWIFT_SDK_${SWIFT_HOST_VARIANT_SDK}_LIB_SUBDIR}-${SWIFT_HOST_VARIANT_ARCH}") |
There was a problem hiding this comment.
I preferred to keep the suffix here so target and architecture are explicit in the name of this target
|
@swift-ci please build toolchain |
stdlib/toolchain/CMakeLists.txt
Outdated
| add_subdirectory(Compatibility51) | ||
| add_subdirectory(CompatibilityDynamicReplacements) | ||
| add_subdirectory(CompatibilityConcurrency) | ||
| foreach(compatibility_name IN LISTS COMPATIBILITY_NAMES) |
There was a problem hiding this comment.
This section is way harder to read now.
I agree with @compnerd here. Something like the following is a bit more repetitive, though repetitions are kept close in the source, but it's way easier to read and know what's going on.
add_subdirectory(Compatibility50)
add_subdirectory(Compatibility51)
add_subdirectory(CompatibilityDynamicReplacements)
add_subdirectory(CompatibilityConcurrency)
add_library(HostCompatibilityLibs INTERFACE)
target_link_libraries(HostCompatibilityLibs INTERFACE
swiftCompatibilityConcurrency${vsuffix}
swiftCompatibilityDynamicReplacements${vsuffix}
swiftCompatibility50${vsuffix}
swiftCompatibility51${vsuffix})
As long as the repetitions are kept close together in the code, it's manageable. The issue is having to remember to keep multiple files in sync with each other. Then those who need the compat libs can link against the HostCompatibilityLibrary and it's easy.
There was a problem hiding this comment.
I admit in devising my initial change I thought I had to have a global variable (like in a general programming language) to reuse this in SwiftCompilerSources/CMakeLists.txt -- but in fact we can have that neatly defined in the proper place, and CMake will be able to find the target at generation time.
This has been reworked in 8b7d915 -- let me know if that looks better.
Refactor the logic so to have a single target to reference the compatibility libraries for the host, and use that when needed. The main driver for this change is supporting the cross-compilation of x86-64 on Apple Silicon. Supports rdar://90307965
edymtt
force-pushed
the
link-more-compat-libraries-when-crosscompiling
branch
from
02faa56 to
8b7d915
Compare
| # because ultimately is used to specify a dependency for a | ||
| # custom target and, unlike `target_link_libraries`, such dependency | ||
| # would be lost at the generation of the build system. | ||
| get_property(compatibility_libs |
There was a problem hiding this comment.
On a build on Apple Silicon, this expands to swiftCompatibilityConcurrency-macosx-arm64;swiftCompatibilityDynamicReplacements-macosx-arm64;swiftCompatibility50-macosx-arm64;swiftCompatibility51-macosx-arm64
swift/SwiftCompilerSources/CMakeLists.txt(304): get_property(compatibility_libs TARGET HostCompatibilityLibs PROPERTY INTERFACE_LINK_LIBRARIES )
swift/SwiftCompilerSources/CMakeLists.txt(307): list(APPEND b0_deps swiftCompatibilityConcurrency-macosx-arm64;swiftCompatibilityDynamicReplacements-macosx-arm64;swiftCompatibility50-macosx-arm64;swiftCompatibility51-macosx-arm64 )
swift/SwiftCompilerSources/CMakeLists.txt(308): list(APPEND b1_deps swiftCompatibilityConcurrency-macosx-arm64;swiftCompatibilityDynamicReplacements-macosx-arm64;swiftCompatibility50-macosx-arm64;swiftCompatibility51-macosx-arm64 )
|
@swift-ci please build toolchain |
|
@swift-ci please test |
edymtt
changed the title
|
@swift-ci please test macOS |
|
@swift-ci please build toolchain macOS |
|
@swift-ci please test macOS |
|
@swift-ci please build toolchain Linux |
|
@swift-ci please test Linux |
At the time I implemented swiftlang#60728, I did not realize that in all the bootstrapping mode we need to depend on the compatibility libraries we build -- otherwise when targeting a low deployment target we risk failing because we are not able to find such libraries. Addresses rdar://126552386
At the time I implemented swiftlang#60728, I did not realize that in all the bootstrapping mode we need to depend on the compatibility libraries we build -- otherwise when targeting a low deployment target we risk failing because we are not able to find such libraries. Addresses rdar://126552386
At the time I implemented swiftlang#60728, I did not realize that in all the bootstrapping mode we need to depend on the compatibility libraries we build -- otherwise when targeting a low deployment target we risk failing because we are not able to find such libraries. Addresses rdar://126552386
At the time I implemented swiftlang#60728, I did not realize that in all the bootstrapping mode we need to depend on the compatibility libraries we build -- otherwise when targeting a low deployment target we risk failing because we are not able to find such libraries. Addresses rdar://126552386
…s target At the time I implemented swiftlang#60728, I did not realize that in all the bootstrapping mode we need to depend on the compatibility libraries we build -- otherwise when targeting a low deployment target we risk failing because we are not able to find such libraries. Addresses rdar://126552386 (cherry picked from commit 4d14497)
…bs target At the time I implemented swiftlang#60728, I did not realize that in all the bootstrapping mode we need to depend on the compatibility libraries we build -- otherwise when targeting a low deployment target we risk failing because we are not able to find such libraries. Addresses rdar://126552386 (cherry picked from commit 4d14497)
Refactor the logic so to have a single target to reference the
compatibility libraries for the host, and use that when needed.
The main driver for this change is supporting the cross-compilation of
x86-64 on Apple Silicon.
Supports rdar://90307965