Fix various linker paths, variables and dependencies related to architecture specific library paths #16194
Conversation
|
@compnerd and then this goes in on top of the #16191 and https://github.com/apple/swift/pull/15560/files |
|
You'll have to rebase this, since there are conflicts. |
|
Yup, sorry, typo'd the branch name while pushing the update. |
cmake/modules/AddSwift.cmake
Outdated
| endif() | ||
| endif() | ||
|
|
||
| # For all other targets we just install the lib at the arch subdirectory |
There was a problem hiding this comment.
I'm not quite sure what this comment is referring to.
cmake/modules/AddSwift.cmake
Outdated
| set(file_permissions | ||
| OWNER_READ OWNER_WRITE OWNER_EXECUTE | ||
| GROUP_READ GROUP_EXECUTE | ||
| WORLD_READ WORLD_EXECUTE) |
There was a problem hiding this comment.
The library targets shouldn't need execute permissions, should they?
include/swift/Driver/ToolChain.h
Outdated
| @@ -195,6 +195,8 @@ class ToolChain { | |||
| /// relative to the compiler. | |||
| void getRuntimeLibraryPath(SmallVectorImpl<char> &runtimeLibPath, | |||
| const llvm::opt::ArgList &args, bool shared) const; | |||
| void getRuntimeLibraryPathWithArch(SmallVectorImpl<char> &runtimeLibPath, | |||
| const llvm::opt::ArgList &args, bool shared) const; | |||
There was a problem hiding this comment.
Nitpick: the indentation is supposed to match the open paren (while wrapping at 80 columns).
lib/Driver/ToolChains.h
Outdated
| @@ -86,6 +86,7 @@ class LLVM_LIBRARY_VISIBILITY GenericUnix : public ToolChain { | |||
| GenericUnix(const Driver &D, const llvm::Triple &Triple) | |||
| : ToolChain(D, Triple) {} | |||
| ~GenericUnix() = default; | |||
|
|
|||
There was a problem hiding this comment.
Nitpick: this is the only change in this file, so may as well leave it out.
lib/Driver/UnixToolChains.cpp
Outdated
| @@ -73,7 +73,7 @@ toolchains::GenericUnix::constructInvocation(const InterpretJobAction &job, | |||
| InvocationInfo II = ToolChain::constructInvocation(job, context); | |||
|
|
|||
| SmallString<128> runtimeLibraryPath; | |||
| getRuntimeLibraryPath(runtimeLibraryPath, context.Args, | |||
| getRuntimeLibraryPathWithArch(runtimeLibraryPath, context.Args, | |||
| /*Shared=*/true); | |||
There was a problem hiding this comment.
Nitpick: indentation again.
|
Build failed |
|
@swift-ci Please clean test |
|
Build failed |
|
Build failed |
…rrect architecture specific path Adjust the runtimeLibPath for the GenerixUnix toolchain to use the architecture specific path. Modify the `runtimeLibraryPath` in CompilerInvocation.cpp to use the architecture specific version for non-Darwin platforms. Modify a few tests to agree with these changes.
…tecture specific library paths On non-Darwin platforms we are now linking against libraries in their architecture specific sub-directories. (e.g. lib/swift/linux/x86_64). Fix various paths, variables and dependencies in AddSwift related to this change as well as removing the lipo functionality from platforms without fat binaries. Remove lipo targets for non-MachO binaries Remove the copying of binaries from their architecture specific paths to the lipo target's path on platforms that don't have universal binaries.
|
Thanks for the review. compnerd should check this in a few days, but he's on vacation at the moment. I'll follow up with him once he gets back. |
|
@swift-ci please clean test |
|
Build failed |
|
Build failed |
On non-Darwin platforms we are now linking against libraries in their
architecture specific sub-directories. (e.g. lib/swift/linux/x86_64).
Fix various paths, variables and dependencies in AddSwift related to
this change as well as removing the lipo functionality from platforms
without fat binaries.
Remove lipo targets for non-MachO binaries
Remove the copying of binaries from their architecture specific paths to
the lipo target's path on platforms that don't have universal binaries.