[Driver] Add -static flag for generating static archives #25202
Conversation
include/swift/Driver/Action.h
Outdated
There was a problem hiding this comment.
I would like to get @jrose-apple's opinion on naming these: DyamicLinkJob and StaticLinkJob
There was a problem hiding this comment.
Yeah, I think I like @compnerd's names better (sans typo).
There was a problem hiding this comment.
Doing it in a separate commit (but still the same PR) might be nice for reviewing purposes, but not necessary.
lib/Driver/DarwinToolChains.cpp
Outdated
There was a problem hiding this comment.
You should be able to use ar to do the operation. The only time that libtool is really required is for fat libraries.
There was a problem hiding this comment.
I don't really have an opinion here, but from the previous review: #25088 (comment)
There was a problem hiding this comment.
Commented on that - I think that we can safely get away with it with llvm-ar at least
There was a problem hiding this comment.
Commented against it. :-(
There was a problem hiding this comment.
I think that we can get away with it using the stock Xcode tools as well (do we really care about the original AT&T Unix?).
lib/Driver/DarwinToolChains.cpp
Outdated
There was a problem hiding this comment.
Rather than "invalid link kind" can you clarify that cannot perform archiving with the dynamic linker?
lib/Driver/Driver.cpp
Outdated
There was a problem hiding this comment.
Actually, please prepend lib on Windows for the static archives.
lib/Driver/Driver.cpp
Outdated
lib/Driver/UnixToolChains.cpp
Outdated
There was a problem hiding this comment.
You can combine this with the previous clause with an else:
if (const char *ar = llvm::sys::findProgramByName("llvm-ar", {toolchainPath}))
Arguments.push_back(context.Args.MakeArgString(ar));
else if (const char *ar = llvm::sys::findProgramByName("ar", {toolchainPath}))
Arguments.push_back(context.Args.MakeArgString(ar));
lib/Driver/UnixToolChains.cpp
Outdated
There was a problem hiding this comment.
mmm I don't think that the traditional archiver permits response files. This should be equivalent to whether llvm-ar is in use.
lib/Driver/WindowsToolChains.cpp
Outdated
There was a problem hiding this comment.
Similar to Darwin and Linux cases
lib/Driver/WindowsToolChains.cpp
Outdated
There was a problem hiding this comment.
Please use llvm-ar if available, otherwise fall back to lld-link (lld-link -lib), otherwise try lib. I don't think that we need to bother with link /LIB as a means of invoking the librarian (lib).
There was a problem hiding this comment.
That seems a lot of complexity – three tools with different invocations. I assume you want llvm-ar and lld-link for cross-compilation, so would it be enough to just pick one of those and say it's required for cross-compilation?
There was a problem hiding this comment.
I don't want to depend on lld-link yet. Until we have swift-lld created and tracking, please do not depend on lld-link. However, lld-link and link should be compatible enough, and link -lib is the same as lib.exe. So if you want to just use link to invoke the librarian, I am fine with that for Windows. You can use either lld-link or link then to invoke lib.
test/Driver/static-archive.swift
Outdated
There was a problem hiding this comment.
Please add a test case for the invalid combination (-emit-executable -static) as well as tests for the fallback path using mocked toolchains.
There was a problem hiding this comment.
Could you clarify what you mean by "tests for the fallback path using mocked toolchains", or point to existing similar tests? I assume you mean ensure that we use the correct tools for e.g. lld-link, but I can't see where that's actually tested for the dynamic linker path – there's a line referencing config.link in lit.cfg but I'm not sure how that feeds into the tests.
troughton
force-pushed
the
static-libraries-driver
branch
from
cf350b2 to
4f391b4
Compare
lib/Driver/DarwinToolChains.cpp
Outdated
There was a problem hiding this comment.
I don't think libtool does allow response files. It does have a form of filelist, though, which you could borrow from the dynamic linking logic on Darwin.
lib/Driver/Driver.cpp
Outdated
There was a problem hiding this comment.
Nitpick: indentation should align by the open-paren.
troughton
force-pushed
the
static-libraries-driver
branch
3 times, most recently
from
19585e2 to
78b593f
Compare
…kJob # Conflicts: # lib/Driver/DarwinToolChains.cpp
troughton
force-pushed
the
static-libraries-driver
branch
from
78b593f to
e5ea42d
Compare
|
I've removed the parts of this PR which used non-system-default linkers. @compnerd, I know that won't meet your needs, but it's probably best to get the uncontroversial parts of this PR in before dealing with the complexity of customising the choice of linker. Provided this looks okay with you, @jrose-apple, this should be okay to merge. (I've rebased off master but can't build locally since I don't have Catalina installed – hopefully all the tests still pass CI.) |
|
I'll take a look soon, but note that you shouldn't need Catalina installed to build, just Xcode 11. |
|
@swift-ci Please test |
|
@compnerd, are you okay with this base thing going in, and we can discuss libtool/ar/llvm-ar later? |
|
@jrose-apple - okay, lets do that. At least we have a starting point then to make improvements. |
3 participants
Add a
-staticflag to the driver that, when combined with-emit-library, generates a static library using the platform archiver.In the future, this
-staticflag will also be used on Windows to determine whether a Swift module should have DLLImport linkage applied.