-
Notifications
You must be signed in to change notification settings - Fork 10.5k
[DO NOT MERGE] remove reference to swiftImageInspectionShared static library #14860
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Even the name of this library shows that it is not a static library.... The static-stdlib-args.lnk file is generated by the gen-static-stdlib-link-args script. The previous change for SR-7038 (PR 14772) only fixed the reference in the static-executable-args.lnk file. rdar://problem/37710244
@swift-ci please test and merge |
Can we add a regression test that attempts to link (and if |
It looks like that's what Driver/static-stdlib-linux.swift is, since it just failed. My next question is how it passed before. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure how this is supposed to work. The swiftImageInspectionShared is what contains the runtime and the rest of the image inspection parts. If this is being removed, which I'm fine with conceptually, we need to ensure that we do not remove ImageInspectionELF.cpp from the runtime sources.
The static standard library is supposed to be completely static. It doesn't get to link against |
@jrose-apple okay, so that is what I would assume too. However, there was linkage to the shared inspection component. The way that we currently are handling the static build seems pretty complicated overall. Contrary to what you would think, the shared image inspection is used ONLY for static builds. |
Who wrote this originally? Was it @jckarter? |
I was just looking at this. The "Shared" library is actually built as a static library: add_swift_library(swiftImageInspectionShared STATIC ....) |
/me throws up hands, quits guessing about the problem without looking |
I just downloaded the latest dev toolchain for Linux and there is no copy of that library installed. That would explain why we're getting link failures. This might just be a CMake problem. |
@jrose-apple @spevans wrote that originally |
I don't know the reason for the complexity, lets see what breaks on PR#14877. Perhaps we have evolved enough to avoid a lot of the complexity that necessitated that back when it was introduced? |
I think the static linking needs to be reworked as I think it broke when the ELF section changes were reworked. The problem with adding a regression test for it was that it also relied on building a fully static libICU (the one that ships with ubuntu needs -ldl) so a test couldnt be added to reliably test it. |
It used to be that we needed a different archive for the image inspection part of things because a static binary doesn't have access to |
Ross and I looked at a build log from one of the Linux bots. The swiftImageInspectionShared library is not even being built. |
See #14880. I'm going to close this PR |
Even the name of this library shows that it is not a static library....
The static-stdlib-args.lnk file is generated by the
gen-static-stdlib-link-args script. The previous change for SR-7038
(PR 14772) only fixed the reference in the static-executable-args.lnk file.
rdar://problem/37710244