Unix toolchains will not try to use fat binaries. #19432
Conversation
|
Please review @compnerd, @gottesmm, @jrose-apple |
|
I thought someone else was working on this too. @lanza, maybe? |
|
I talked offline with @lanza and he is OK with me taking over this. From what I have talked with him, my solution is completely forgetting the installation phase and some other changes in the toolchains. I will try to include those improvements here as soon as I can. |
Depends on swiftlang/swift#19432 Unix (other than Darwin) and Windows do not support fat binaries. However, the build system was placing the build results in the platform/OS folder, instead of the per-architecture folder. Having two architectures side by side was impossible. After applying the above patch in the Swift compiler, non-Darwin platforms will look into the per-architecture folders, so the sibling projects need to leave the products in the right place.
Depends on swiftlang/swift#19432 Unix (other than Darwin) and Windows do not support fat binaries. However, the build system was placing the build results in the platform/OS folder, instead of the per-architecture folder. Having two architectures side by side was impossible. After applying the above patch in the Swift compiler, non-Darwin platforms will look into the per-architecture folders, so the sibling projects need to leave the products in the right place.
Depends on swiftlang/swift#19432 Unix (other than Darwin) and Windows do not support fat binaries. However, the build system was placing the build results in the platform/OS folder, instead of the per-architecture folder. Having two architectures side by side was impossible. After applying the above patch in the Swift compiler, non-Darwin platforms will look into the per-architecture folders, so the sibling projects need to leave the products in the right place. The change is performed in the CMakeList.txt file (which as far as I understand is not used to build Foundation at the moment), and the Python script.
drodriguez
force-pushed
the
thin-platforms
branch
from
27c7834 to
dee8f66
Compare
|
The latest version of the commit includes the changes in Nathan’s version that I didn’t catch, it also includes slightly modified version to correctly generate the install scripts for non-Darwin platforms. To build a full package, changes in other projects are necessary:
With all those changes a |
drodriguez
force-pushed
the
thin-platforms
branch
from
dee8f66 to
83a3141
Compare
|
(not reviewing, just testing) swiftlang/swift-corelibs-libdispatch#392 |
|
Build failed |
|
Build failed |
|
What about adding a flag to emit this path and changing the corelibs buildsystems to use the result of this rather than duplicating this logic across all three buildsystems. For example (names bikesheddable): You could also do this as an installed CMake config file or a pkg-config file. The advantage is that other build systems that need this knowledge (swifpm, a hypothetical swift library installer) could ask this tool as well. |
|
Seems that the two builds failed because they tried to find the original commit from this PR, instead of the last version. Is there something I need to do for the CI to pick up the right one? @kevints: sounds like a great idea, but I don’t see this |
|
Ah, those get automatically retried by our CI system; they're not the real issue. The "Details" section of the actual checks shows the real failures: Apple platforms are failing to configure, and Linux build-script is trying to run a command that might not exist anymore. |
|
I see the problems now. I change that check to a precondition as one of the last things. I will try to find some time to check what’s happening on Darwin. The Linux one is stranger. I will see if I can reproduce and fix it. Thanks! |
|
@drodriguez you'd have to introduce it. The advantage is that this tool would be built from this repository and so it would always have up-to-date information about these paths and downstream projects (like s-c-f and s-c-d) would just need to query this tool rather than duplicate its logic in the language of their own buildsystem). I suspect it would be less total code but I don't want to hold this change up if it proves more difficult than expected to implement. |
drodriguez
force-pushed
the
thin-platforms
branch
from
83a3141 to
7043165
Compare
|
I tested with a Darwin machine again and find two small problems during configuration which should be fixed in the last commit. I will try to reproduce the Linux problem tomorrow (sadly the details are already gone from the CI server, and I didn’t save them anywhere). @kevints: I don’t disagree that the tool is a good idea, but I wasn’t planning into creating a tool for this, since previously all the path calculation was already duplicated (but I agree that it was simpler). Anyway, I can try to create such tool as a follow up, because a new tool will add a lot more code to an already complicated diff. |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
drodriguez
force-pushed
the
thin-platforms
branch
from
7043165 to
c46d1f4
Compare
|
Build failed |
|
@drodriguez seems that the failures are related to the integration tests failing during s-p-m tests due to not linking Foundation and dispatch (can't find it is my guess). |
|
@compnerd: I think you are missing the related PRs in your CI command. Like: swiftlang/swift-corelibs-libdispatch#392 The missing symbols are XCTest, which currently leaves the products in the root directory, but the compiler is looking into the per-arch directory (to support multiple architectures without fat libraries support). |
drodriguez
force-pushed
the
thin-platforms
branch
from
4530157 to
4f8ab60
Compare
|
Please test with the following PRs: @swift-ci please test |
|
Build failed |
|
Build failed |
|
Seems that I can reproduce the errors in CI, but I don’t know where to start. The problems disappear if I compile without my code. For The problem is the line In the case of And finally |
drodriguez
force-pushed
the
thin-platforms
branch
from
4f8ab60 to
cd2bcce
Compare
|
With one the last changes in the CMake module I forgot to change one usage of |
|
Please test with the following PRs: @swift-ci please test |
|
Build failed |
|
Build failed |
Depends on swiftlang/swift#19432 Unix (other than Darwin) and Windows do not support fat binaries. However, the build system was placing the build results in the platform/OS folder, instead of the per-architecture folder. Having two architectures side by side was impossible. After applying the above patch in the Swift compiler, non-Darwin platforms will look into the per-architecture folders, so the sibling projects need to leave the products in the right place.
Depends on swiftlang/swift#19432 Unix (other than Darwin) and Windows do not support fat binaries. However, the build system was placing the build results in the platform/OS folder, instead of the per-architecture folder. Having two architectures side by side was impossible. After applying the above patch in the Swift compiler, non-Darwin platforms will look into the per-architecture folders, so the sibling projects need to leave the products in the right place.
Depends on swiftlang/swift#19432 Unix (other than Darwin) and Windows do not support fat binaries. However, the build system was placing the build results in the platform/OS folder, instead of the per-architecture folder. Having two architectures side by side was impossible. After applying the above patch in the Swift compiler, non-Darwin platforms will look into the per-architecture folders, so the sibling projects need to leave the products in the right place.
Support for Android aarch64 in many parts of the build-script. Most of the changes are reuse variables/parameters that already existed for Android ARMv7. There is also a new parameter to specify the ICU data library, which is used by swiftlang#19503. With this one can build either armv7 or aarch64, since building both at the same time requires more changes like swiftlang#19432 (and probably more work to support two set of paths).
drodriguez
force-pushed
the
thin-platforms
branch
2 times, most recently
from
f44e551 to
e330c8a
Compare
Depends on swiftlang/swift#19432 Unix (other than Darwin) and Windows do not support fat binaries. However, the build system was placing the build results in the platform/OS folder, instead of the per-architecture folder. Having two architectures side by side was impossible. After applying the above patch in the Swift compiler, non-Darwin platforms will look into the per-architecture folders, so the sibling projects need to leave the products in the right place.
Depends on swiftlang/swift#19432 Unix (other than Darwin) and Windows do not support fat binaries. However, the build system was placing the build results in the platform/OS folder, instead of the per-architecture folder. Having two architectures side by side was impossible. After applying the above patch in the Swift compiler, non-Darwin platforms will look into the per-architecture folders, so the sibling projects need to leave the products in the right place.
Depends on swiftlang/swift#19432 Unix (other than Darwin) and Windows do not support fat binaries. However, the build system was placing the build results in the platform/OS folder, instead of the per-architecture folder. Having two architectures side by side was impossible. After applying the above patch in the Swift compiler, non-Darwin platforms will look into the per-architecture folders, so the sibling projects need to leave the products in the right place.
drodriguez
force-pushed
the
thin-platforms
branch
from
e330c8a to
09f73f3
Compare
Unix (other than Darwin) and Windows do not support fat binaries. However, the build system was trying to "lipo" several architectures into one file even for those targets (only Windows was exercising that part of the code, it seems). This patch removes the copy for a bogus empty target, and modifies the Unix toolchain to look into the architecture subdirectory, instead of trying to use the platform directory. The Windows toolchain already checked the architecture directory, and not the platform one. This should allow building two Linux or Android SDKs for different architures side by side. Some tests are modified to handle the new structure. The tests are hardcoded to use x86_64 because their targets were originally x86_64.
drodriguez
force-pushed
the
thin-platforms
branch
from
09f73f3 to
336ede3
Compare
Support for Android aarch64 in many parts of the build-script. Most of the changes are reuse variables/parameters that already existed for Android ARMv7. There is also a new parameter to specify the ICU data library, which is used by swiftlang#19503. With this one can build either armv7 or aarch64, since building both at the same time requires more changes like swiftlang#19432 (and probably more work to support two set of paths).
Unix (other than Darwin) and Windows do not support fat binaries.
However, the build system was trying to "lipo" several
architectures into one file even for those targets (only Windows
was exercising that part of the code, it seems).
This patch removes the copy for a bogus empty target, and modifies
the Unix toolchain to look into the architecture subdirectory,
instead of trying to use the platform directory. The Windows
toolchain already checked the architecture directory, and not the
platform one.
This should allow building two Linux or Android SDKs for different
architures side by side.
Some tests are modified to handle the new structure. The tests are
hardcoded to use x86_64 because their targets were originally
x86_64.