Use autolinking to pull in compatibility libraries. #25148
Conversation
|
@swift-ci Please test |
jckarter
force-pushed
the
runtime-compatibility-autolink
branch
from
2ed4d44 to
cd931ea
Compare
|
@swift-ci Please test |
|
@jrose-apple How's this look now? |
|
Build failed |
|
Build failed |
|
Build failed |
jckarter
force-pushed
the
runtime-compatibility-autolink
branch
from
cd931ea to
f415685
Compare
|
@swift-ci Please test |
|
Build failed |
|
Build failed |
There was a problem hiding this comment.
Looks good, with only a few remaining minor comments. Make sure you're coordinating with @Rostepher about how this will get installed into the toolchain.
lib/Driver/DarwinToolChains.cpp
Outdated
There was a problem hiding this comment.
Nitpick (here and elsewhere): double-indent for wrapped lines rather than reverse-indenting until it fits.
There was a problem hiding this comment.
You don't need to repeat this check because the entire NO-FORCE-LOAD check is negative.
There was a problem hiding this comment.
I'm surprised this works! Since it's matching in reverse order from what it needs. But if it works then okay. (If it doesn't actually work, and it's worth checking, then I think it's okay to just match the first line and trust that we wouldn't generate that for any other reason.)
There was a problem hiding this comment.
I thought that was what -DAG was for, to allow lines to be matched out of order from how they show up in the output.
There was a problem hiding this comment.
It is, but I've never tried using that with captures.
jckarter
force-pushed
the
runtime-compatibility-autolink
branch
from
f415685 to
2a20e28
Compare
|
@swift-ci Please test |
|
Build failed |
|
Build failed |
|
@swift-ci Please test |
|
Build failed |
|
Build failed |
lib/Basic/Platform.cpp
Outdated
There was a problem hiding this comment.
I think that you can write this more concisely as:
if (Major == 10 && Minor <= 14)
return llvm::VersionTuple(5, 0);
return None;
There was a problem hiding this comment.
I'm writing this with an eye toward it eventually growing more arms. There might be more macOS versions with Swift in them in the near future, but who knows
lib/Basic/Platform.cpp
Outdated
There was a problem hiding this comment.
No need for the else after return
lib/Basic/Platform.cpp
Outdated
There was a problem hiding this comment.
No need for the else after return
lib/Driver/DarwinToolChains.cpp
Outdated
Many build systems that support Swift don't use swiftc to drive the linker. To make things easier for these build systems, also use autolinking to pull in the needed compatibility libraries. This is less ideal than letting the driver add it at link time, since individual compile jobs don't know whether they're building an executable or not. Introduce a `-disable-autolink-runtime-compatibility` flag, which build systems that do drive the linker with swiftc can pass to avoid autolinking. rdar://problem/50057445
Although it's a static archive, its use is only relevant to dynamically linked builds of the standard library. (If you're statically linking a Swift runtime into your process, you don't need to worry about compatibility with older runtimes.)
jckarter
force-pushed
the
runtime-compatibility-autolink
branch
from
a2e0fe1 to
dd91bc2
Compare
|
@swift-ci Please test |
|
Build failed |
|
Build failed |
| BINARY_DIR ${SWIFT_RUNTIME_OUTPUT_INTDIR} | ||
| LIBRARY_DIR ${SWIFT_LIBRARY_OUTPUT_INTDIR}) | ||
|
|
||
| if(SWIFTLIB_INSTALL_WITH_SHARED) |
There was a problem hiding this comment.
For this sort of logic I usually prefer:
set(swift_lib_dir "${SWIFTSTATICLIB_DIR}")
if(SWIFTLIB_INSTALL_WITH_SHARED)
set(swift_lib_dir "${SWIFTLIB_DIR}")
endif()
| STATIC | ||
| TARGET_LIBRARY) | ||
| TARGET_LIBRARY | ||
| INSTALL_WITH_SHARED) |
There was a problem hiding this comment.
I'm not sure I agree with adding yet another single purpose flag to add_swift_target_library. Is there a more generic purpose here? Say maybe an INSTALL_PATH or something?
There was a problem hiding this comment.
Yeah, I'm torn about that too. YAGNI convinced me to stick with just the flag for now. A small generalization in between this and INSTALL_PATH might be to have an INSTALL_AS [SHARED|STATIC] flag, so you could also choose to send dylibs to the static dir and v.v.?
5 participants
Many build systems that support Swift don't use swiftc to drive the linker. To make things
easier for these build systems, also use autolinking to pull in the needed compatibility
libraries. This is less ideal than letting the driver add it at link time, since individual
compile jobs don't know whether they're building an executable or not. Introduce a
-disable-autolink-runtime-compatibilityflag, which build systems that do drive the linkerwith swiftc can pass to avoid autolinking.
rdar://problem/50057445