-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
Allow unit test targets to import and link executable targets #3316
Allow unit test targets to import and link executable targets #3316
Conversation
@swift-ci please smoke test |
Sources/Build/BuildPlan.swift
Outdated
@@ -499,8 +499,7 @@ public final class SwiftTargetBuildDescription { | |||
|
|||
/// The path to the swiftmodule file after compilation. | |||
var moduleOutputPath: AbsolutePath { | |||
let dirPath = (target.type == .executable) ? tempsPath : buildParameters.buildPath | |||
return dirPath.appending(component: target.c99name + ".swiftmodule") | |||
return buildParameters.buildPath.appending(component: target.c99name + ".swiftmodule") |
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.
This restores the code from before a previous change to make it more obvious that tests couldn't import executable modules, by moving those modules into subdirectories of the main build directory.
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.
Because Clang targets don't produce a module but instead use a module map in the public headers directory, the more obvious error is preserved when trying to import a Clang executable module from a test target. It is not preserved when importing an executable module from another one, however. We should consider moving the module back into its subdirectory and instead adding additional search paths to test targets that declare a dependency on the executable target, to preserve the good error message if an executable tries to import another executable's module.
This would be good to do in general, so that a dependency between two targets needs to be declared before the module can be imported (otherwise it leads to hard-to-diagnose linker errors rather than a more obvious cannot-import-module error).
798d7d9
to
668f8d1
Compare
@swift-ci please smoke test |
668f8d1
to
685eb8d
Compare
@swift-ci please smoke test |
makes sense
cc @compnerd |
Some of the macOS failures are because of unit tests that need to be updated, but there's a real issue with how |
So with the nightly toolchain the general principle seems to work:
|
AFAIK, the Windows toolchain currently uses |
Interestingly, from the command line using |
Looks like this happens when linking the test suites themselves as executables. That's the case in which we don't want to rename back to |
685eb8d
to
5ef1e9e
Compare
@swift-ci please smoke test |
5ef1e9e
to
8fe92a5
Compare
@swift-ci please smoke test |
8fe92a5
to
c16815f
Compare
@swift-ci please smoke test |
c16815f
to
85db1a0
Compare
@swift-ci please smoke test |
85db1a0
to
787dc2e
Compare
@swift-ci please smoke test |
The self-hosed
This should be addressed by tying this to the package tools version. |
Actually, that won't help — we're testing the mainline SwiftPM (with a 999.0 tools version) but using a 5.3 Swift compiler version. So we would either need detect that the compiler we're using doesn't support it, or update the self-hosted macOS tools to one that has the mainline compiler. This won't be so interesting in practice since SwiftPM ships as part of the toolchain, so it will always be paired with a recent enough compiler. So this is mostly a testing problem. |
787dc2e
to
2ed483e
Compare
@swift-ci please smoke test |
looking good! can't wait to start using this |
96c5014
to
d43fd9e
Compare
@swift-ci please smoke test |
d43fd9e
to
a578ab1
Compare
@swift-ci please smoke test |
Hi @compnerd, do you know if there are any Windows linker options that could be used here for renaming |
I don't think that there is a good way to do a rename on a symbol on Windows. I cannot think of any option in link that would let you do that. You would need to generate a new source file to accomplish it, either way a thunk or some less than ideal horrible things to do the aliasing optionally as a replacement for main. |
Thanks, @compnerd. Would Otherwise the generated source file would always be a last resort. |
The
lld is meant to be 100% identical to link in invocation (and is how we use it in Swift as well). The For Windows, assume that the only valid way to test is link - lld should be a drop in replacement for link. |
… product is defined.
…ription to know the tools version of the package in which they are defined. This allows compiler flags and other semantically significant changes to be conditionalized on the tools version. In the cases where tools versions are synthesized, they get the tools version of the package defining the product or target for which they are being synthesized. The fallback for anything that cannot be determined at all is always `.vNext`, which is the same as has been the case until now.
…hat are implemented in Swift. This uses a new Swift compiler flag to set the name of the entry point when emitting object code, and then uses linker flags to rename the main executable module's entry point back to `_main` again when actually linking the executable. This is guarded by a tools version check, since packages written this way won't be testable on older toolchains. Also, this is currently only done on Darwin and Linux. A supplemental PR will generate a small stub containing code that implements `main` to call the per-module `<module>_main` function, which will be linked into the executable.
a578ab1
to
05babe9
Compare
@swift-ci please smoke test |
Thanks, @compnerd. For now I have updated the PR to only do this renaming on Darwin and Linux, but I will put up a supplemental PR to generate a source file to use as fallback. I tried an attempt at doing so in this PR, but the approach suggested in the compiler PR using a Swift source file didn't work. I will add a facility to compile and link C stub source files when building executables, which might be handy in any case, and then we can use that to compile and link in this:
when producing the executable, which will work on any platform. The stub was the original plan and the linker flags were only an optimization to avoid this extra code generation, but the stub will work for any platform without undocumented features or magic tricks. |
// XCTAssertEqual(String(cString: GetGreeting3()), "Hello, universe") | ||
|
||
// Some of the APIs that we use below are available in macOS 10.13 and above. | ||
guard #available(macOS 10.13, *) else { |
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.
@abertelrud does this stuff really not work on any platform but macOS?
Allow unit test targets to import and link executable targets. This allows them to test everything except the main function in those targets without having to run the executable as a subprocess. With these changes they can still run the subprocess if they want to.
Motivation:
Many package authors don’t want to split their code into executable targets vs library targets, but still want to test data structures and algorithms in their executables.
Modifications:
This uses a new Swift compiler flag introduced in swiftlang/swift#35595 to compile the main symbol to a different name than
_main
, so that it doesn't cause symbol collisions when linked into a test executable. When linking the actual executable, the entry point symbol is then renamed back to to_main
. This is done using linker flags for both Darwin and Linux in this implementation, but for linkers that don't support such options, a fallback would be to generate a stub source file along the lines of the compiler PR.This feature is guarded with a tools version check, since packages written this way won't be testable on older toolchains.
This PR supersedes the one at #3103, which modified object files after compilation. That was a much messier solution but the best possible without support from either the compiler or linker. Now that there is compiler support, we can do better.
Modifications:
_main
Result:
Unit test targets can now link executable targets that are implemented in Swift. Executables implemented using Clang targets would require a corresponding Clang flag, and are therefore currently not supported.
rdar://58122395