Remove extra installed copy of libFoundation.so #1776
Conversation
- Currently libFoundation.so exists in 2 places in the distribution tarfile: ./usr/lib/swift/linux/x86_64/libFoundation.so ./usr/lib/swift/linux/libFoundation.so - ld.so will load the first one located in x86_64 subdirectory which will then set $ORIGIN to this directory. Consequently, the libICU shared libraries required by libFoundation.so will not be found. - Update CMakeLists.txt to not install libFoundation.so in the x86_64 directory. - Also remove an unneeded RUNPATH pointing to the build directory for libdispatch. This is already reached using LD_LIBRARY_PATH during testing.
|
@swift-ci test |
Note that even though the |
|
Wasn't the plan on this to put all the libraries under the x86_64 directory instead? Or to put it another way, why isn't the correct fix to stop putting the libFoundation at the top level, and move the ICU libraries to under the x86_64 directory? |
|
Looking at the current layout: I think the only thing that is important is that all of the libraries are in either the |
|
I concur – having them in only one place is the right solution. But I thought the re-org had happened; maybe it was rolled back in the Swift repo but not here? ISTR @compnerd was doing some refactoring in this area, so wanted to get visibility on what the right fix was. |
|
Yeah, we want to move the libraries into the architecture subdirectory. Darwin is the only platform that supports fat binaries due to the the MachO feature. This prevents us from having multiple architectures supported by a single toolchain distribution which seems like a horrible limitation (having a 1GiB+ image for every target, no longer really doing cross-compilation, etc). If we want to begin the de-duplication process, we should be removing the version in the os directory. |
|
I think the only issue will be that some changes will be required in the main swift repository to move the |
|
@compnerd I tried a test removing the version in the os directory but it failed because the linker commands used by Note the |
|
Hmm, in that case, why not commit directly into just the swift-5.0-branch rather than in master, and go ahead and start making the other changes for master? @drodriguez was working on this which is why I haven't been pushing on it ... there were a couple of patches that still need to be merged and need to be coordinated across repositories. |
|
Oh, but, the change to the |
|
@compnerd I wanted to get both |
|
I really would like to try to see if we can get the subdirectory move in the 5.0 timeframe. Because we are installing the runtime libraries into the toolchain (why are we doing that btw?), the RPATHs will have to change. It seems that it would be nicer to have the paths corrected for the 5.0 release. I believe that all the patches for this are already up, its a matter of coordinating and merging them. The problem is that getting the build changes through tends to be very difficult (I've had build changes get reverted multiple times for various reasons, including Apple internal merge conflicts). I don't see a good way to ensure that we can get such changes in quickly because of the wonkiness in the build system that has spread out through all the repositories (primarily due to build-script and swift's resistance to using CMake's support for cross-compiling and installation). |
|
I don't think you will get anymore changes into the main swift repo for swift-5 as I believe only essential patches are now being taken for the main repo and I believe at least 2 changes are required for that:
So I think it will end up being a Swift6 change unfortunately. |
|
We really do need to figure out how to get more of these changes which potentially may break Apple's builds in more easily. In this particular case, the tree structure is not suitable for non-Darwin targets :-(. |
|
Isn't it only an issue when we want to ship multi architecture toolchains? How is it not suitable for non -Darwin targets at the moment ? |
|
@swift-ci test and merge |
|
@spevans - you cannot target x86 and x86_64 for Linux right now from the same toolchain. |
|
It is unfortunate that this change was merged, as it has undone one of the preparation steps to merge the changes in the compiler. The full process, was adding this duplication workaround, merge the compiler changes and remove the workarounds. Sadly the compiler change is talking a long time to be reviewed and merged, so this was catch in the middle. As @compnerd explains, it is impossible to target with the same toolchain two architectures for the same os, except you use the special support for fat libraries from Mach-O. That means that it is not possible to have a toolchain building for two Android architectures, two Linux architectures, etc. The installation process will copy both architectures into one destination with the same name, which fails. The changes in the compiler will allow to look into the right architecture subdirectory instead, not needing to do any copy (except that Darwin keeps working as always). It is also very strange that libraries that are not exactly related to the compiler are installed into the compiler files I’m really sorry I didn’t see this PR before, and I also sorry I didn’t realize that ICU was being installed in the OS directory instead of the architecture. I would have provided the same workaround until the compiler changes were merged. We will try to figure out if there’s a way of doing this in a simple way without too many inter-repo dependencies, but it would be awesome if this is not limited to “after 5.0”, since this only affects platforms that are not Darwin (we were very careful about it) and it makes distributing toolchains outside of Darwin a really complicated process. |
Currently libFoundation.so exists in 2 places in the distribution
tarfile:
./usr/lib/swift/linux/x86_64/libFoundation.so
./usr/lib/swift/linux/libFoundation.so
ld.so will load the first one located in x86_64 subdirectory which
will then set $ORIGIN to this directory. Consequently, the libICU
shared libraries required by libFoundation.so will not be found.
Update CMakeLists.txt to not install libFoundation.so in the x86_64
directory.
Also remove an unneeded RUNPATH pointing to the build directory for
libdispatch. This is already reached using LD_LIBRARY_PATH during
testing.