Skip to content
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

[Dependency Scanning] Record header dependencies of binary Swift module dependencies #66556

Merged
merged 3 commits into from
Jun 13, 2023

Conversation

artemcm
Copy link
Contributor

@artemcm artemcm commented Jun 12, 2023

When we encounter a pre-built Swift binary module dependency (without an interface file), such module may have been built with a bridging header, which must still be present and is referenced by the binary .swiftmodule as either a .h or, more-likely, a pre-built .pch in a fully-explicit build.

Clients must be able to know about such header dependencies in order to be able to import this binary module, because this binary module may be referencing types brought in via its bridging header. The build-system client (swift-driver) will then ensure these header dependencies are fed as inputs to all requiring compilation tasks.

@xedin xedin removed their request for review June 12, 2023 16:28
@artemcm artemcm force-pushed the TransitivePCHDepsOfBinaryModules branch from 8460fef to 7a769a0 Compare June 12, 2023 16:40
@artemcm
Copy link
Contributor Author

artemcm commented Jun 12, 2023

@swift-ci test

Copy link
Contributor

@nkcsgexi nkcsgexi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense to me!

@cachemeifyoucan
Copy link
Contributor

Is there any example use case for this? I assume you can't really ship a swift module that has bridging header dependencies?

The main reason to ask is this is going to a very hard to support corner case for caching, because this is not tracking the dependencies for the bridging header itself. Does build system/swift driver need to be aware of that in non-caching builds?

I think the only solution for support caching for this case will be encoding the CASID for the dependencies in swift module. Unfortunately it means that you can only depend on swift binary module builds from CAS when enable caching.

@artemcm
Copy link
Contributor Author

artemcm commented Jun 12, 2023

Is there any example use case for this? I assume you can't really ship a swift module that has bridging header dependencies?

PCHs started being a part of this field with Explicit modules, since we started feeding them in explicitly as inputs. We do have the alternative of referencing them as a .h file dependency, too.

The main reason to ask is this is going to a very hard to support corner case for caching, because this is not tracking the dependencies for the bridging header itself. Does build system/swift driver need to be aware of that in non-caching builds?

Perhaps binary modules built with a bridging header need to separately also track dependencies of the bridging header itself?

I think the only solution for support caching for this case will be encoding the CASID for the dependencies in swift module. Unfortunately it means that you can only depend on swift binary module builds from CAS when enable caching.

@cachemeifyoucan
Copy link
Contributor

Talk with Artem offline. Caching for this case needs a different solution orthogonal to this fix.

Nit: miss a test case for the dependency scanner that make sure the -include-pch option is generated.

Otherwise, LGTM.

@artemcm artemcm force-pushed the TransitivePCHDepsOfBinaryModules branch from 7a769a0 to e5b221f Compare June 12, 2023 18:30
artemcm added 3 commits June 12, 2023 14:30
…le dependencies

These are meant to capture paths to the PCH files that a given module was built with.
… dependencies in output and provide libSwiftScan API to query it
@artemcm artemcm force-pushed the TransitivePCHDepsOfBinaryModules branch from e5b221f to 92d9e61 Compare June 12, 2023 19:07
@artemcm
Copy link
Contributor Author

artemcm commented Jun 12, 2023

@swift-ci test

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants