Revise how modules imports are done to better support import public
#1539
Conversation
- Add basic infrastructure to have Compile Tests.
- Add a specific MultiModule CompileTest.
- This ensure that with `import public` is properly handle and the generated
code compiles even when a needed type is coming from a public import.
- So the any relevant code changes are more visible, check the the generated
code so the diffs for an changes related to the test will be easily
reviewed.
- The SPM plugin doesn't support multi module generation
(apple#1450), which is also why the
code needs to be checked in, otherwise, it might simplify the tests.
Note: The whole concept of CompileTests doesn't have to be a set up as distinct
Swift Packages, but if we put the required targets in the main Package.swift,
all projects using swift-protobuf would end up having the "overhead" of those
test only targets, so I've errored on the side of a distinct package to make
sure there is no impact of these tests on the folks using swift-protobuf.
|
I split the setup of the compile test infra into the first comment, so the actually changes for |
|
The new |
Historically Swift didn't have a good way to model `import public` in .proto files. But by using `@_exported import`, we can get the majority of the way there. This makes restructuring of proto files less likely to break Swift source code; meaning Swift is less of a "special case" compared to other languages. If generating with `public` or `package` visibility, then instead of importing the whole module of for an `import public`, explicitly re-export the imports of the types from that file. That will make them usable for the given source file but will also let the types continue to be used by someone just importing this module. This also means a file with no `import public` usages becomes must simpler as it can just rely on its direct dependencies to provide all the types that could be used.
thomasvl
force-pushed
the
public_imports
branch
from
e4f5491
to
b27d019
Compare
This could all just be in the main |
There was a problem hiding this comment.
I like the changes here. The only thing I would like us to reconsider is checking in yet more generated proto files. In the PluginExamples, directory I have already used the Swift PM plugin to generate Swift files on the fly and it also does a compile check. The CI for that is already setup correctly and works.
Maybe we can merge the two setups into one since they are aiming to do the same thing. The added benefit of that would be that we make sure everything is doable via the Swift PM plugin as well.
I tried to cover this in CompileTests/MultiModule/README.md -
We have the request for this in #1450, but I thought the opinion was to not do this and instead wait on the hope that SwiftPM would eventually expose something so the SwiftPM plugin could eventually generate this info itself.
Since we don't support the mappings, I don't believe a developer could create such a set up. |
Oh sorry I missed that part. Makes sense then! Thanks for clarifying. |
@tbkka did you want to look at this PR in a little more detail, or am I good to merge it? |
|
If Franz and Tony are both okay with it, then please go ahead! |
|
Thanks! |
Historically Swift didn't have a good way to model
import publicin .protofiles. But by using
@_exported import, we can get the majority of the waythere. This makes restructuring of proto files less likely to break Swift source
code; meaning Swift is less of a "special case" compared to other languages.
If generating with
publicorpackagevisibility, then instead of importingthe whole module of for an
import public, explicitly re-export the imports ofthe types from that file. That will make them usable for the given source file
but will also let the types continue to be used by someone just importing this
module.
This also means a file with no
import publicusages becomes must simpler as itcan just rely on its direct dependencies to provide all the types that could be
used.