build: introduce a new option to control library evolution #1307
Conversation
compnerd
requested review from
stmontgomery,
grynspan,
briancroom,
suzannaratcliff and
jerryjrchen
as code owners
| @@ -18,6 +18,7 @@ add_library(_Testing_Foundation | |||
| ReexportTesting.swift) | |||
|
|
|||
| target_link_libraries(_Testing_Foundation PUBLIC | |||
| _TestingInternals | |||
There was a problem hiding this comment.
The Foundation overlay shouldn't need to link to this module.
There was a problem hiding this comment.
I found that it did - there was a missing module error otherwise.
There was a problem hiding this comment.
The overlay doesn't import it and it's a static library, so if it's complaining, that probably aligns with what @stmontgomery noted in his comment.
| @@ -35,6 +36,7 @@ endif() | |||
| # interface, because Foundation does not have Library Evolution enabled for all | |||
| # platforms. | |||
| target_compile_options(_Testing_Foundation PRIVATE | |||
| -emit-module-interface -emit-module-interface-path $<TARGET_PROPERTY:_Testing_Foundation,Swift_MODULE_DIRECTORY>/_Testing_Foundation.swiftinterface) | |||
| -emit-module-interface | |||
There was a problem hiding this comment.
This is just a stylistic change, right?
There was a problem hiding this comment.
after the indentation for the conditional, yes.
| -enable-library-evolution | ||
| -emit-module-interface -emit-module-interface-path $<TARGET_PROPERTY:Testing,Swift_MODULE_DIRECTORY>/Testing.swiftinterface) | ||
|
|
||
| if(SwiftTesting_ENABLE_LIBRARY_EVOLUTION) |
There was a problem hiding this comment.
Pretty sure this module needs a .swiftinterface even without library evolution, no?
There was a problem hiding this comment.
The swift interface is not generated without library evolution AFAIK. @etcwilde?
There was a problem hiding this comment.
Swift interfaces are invalid without library evolution. Swift can't describe struct layouts and the interface doesn't include all properties resulting in ABI issues without library evolution.
There was a problem hiding this comment.
I understand Library Evolution (LE) is only supported on Apple platforms currently, but (based on past conversations with knowledgeable Swift compiler engineers) it is not considered problematic to use LE on other platforms as long as you ensure the code is only used with Swift runtime libraries aligned with the toolchain which built the code. Testing libraries and test-only modules have a notable difference from ordinary application and library targets: they're never intended for distribution to end users, and should only validate locally-built code. Because they're not distributed, historically we have enforced that users may only run tests using the same toolchain and runtime libraries which were used to build them.
We intentionally enable Library Evolution on all platforms for a couple reasons. For one thing, we make extensive use of @_spi for new features, and including binary .swiftmodules in the toolchain makes those SPIs freely available on the relevant platforms. Also, when LE is not enabled I believe all module dependencies must be visible to clients, which means things like our _TestingInternals module must be exposed.
For these reasons, I'm pretty hesitant to proceed with this since we've intentionally had things this way. I'd love to understand your motivation for this PR more though, to see what other options are available.
The appeal to authority is not helpful, but please do pull the engineers into the conversation here so that we can make an appropriate technical decision. Well, the issue is that with library evolution, swift-testing cannot be built on non-Apple platforms. This is only appearing as an issue now because the new runtimes build does not build the standard library runtime with library evolution. @etcwilde rightly pointed out that the library evolution without ABI stability doesn't offer any value. When the standard library is not built with library evolution, the client has no option - it cannot be built with library evolution. If Apple is able to lift the restriction and support the swift interface without library evolution, we should absolutely consider this. |
We've certainly been building successfully in nightlies. What's changed? |
Library evolution only makes sense on platforms where ABI stability is available. Introduce a new option which defaults to `TRUE` only on Darwin, the only platform currently providing ABI stability.
|
We had a discussion off GitHub about this PR and think we need to involve some folks from the core team and/or swift-build effort to help us out. |
|
Offline, we discussed this and believe it would be better resolved by a compiler change for which I filed swiftlang/swift#84241 |
Library evolution only makes sense on platforms where ABI stability is available. Introduce a new option which defaults to
TRUEonly on Darwin, the only platform currently providing ABI stability.