Skip to content

Conversation

@artemcm
Copy link
Contributor

@artemcm artemcm commented Aug 12, 2020

Ensure the flag is propagated from a given swift module's dependency graph details to its swift module info passed in as an explicit module map to frontend jobs.

This is a companion PR to:
swiftlang/swift#33430

@artemcm artemcm requested review from DougGregor and nkcsgexi August 12, 2020 21:54
type: .swiftModule)
// Since this module has already been built, it is no longer relevant
// whether it is a framework or not.
isFramework = false
Copy link
Member

Choose a reason for hiding this comment

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

Is this comment true? An already-built .swiftmodule can be part of a .framework. This happens both with libraries built for distribution (the .swiftmodule and .swiftinterface are installed together) and with libraries not built for distribution (where it's just the .swiftmodule).

Copy link
Member

Choose a reason for hiding this comment

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

... and even if we don't "need" to know when it's a framework, is it better to have that information for consistency reasons?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The explicitCompiledModulePath is used only during placeholder dependency resolution, which tells us that this .swiftmodule is built as a part of an external dependency (separate from the compiledModuleCandidates). With that in mind, I guess my thought process was that it is the responsibility of the builder/planner of that external dependency to ensure that it is treated correctly w.r.t. whether or not it is a framework. Hence the "no longer relevant" part.

I do agree that if we don't need to know it's a framework, it is better to have that information; though, I am not yet sure how we can get this information. Perhaps we can have SwiftPM query it from the swift-driver after planning and propagate it to the dependees along with the .swiftmodule path and the dependency graph.

Copy link
Contributor

Choose a reason for hiding this comment

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

If it’s not needed you could change the variable name to something like isNonPreCompiledFramework. Then this false isn’t a lie 😅

@artemcm artemcm force-pushed the ExplicitFrameworkBuilds branch from 1758095 to 0b05b03 Compare August 20, 2020 22:12
@artemcm
Copy link
Contributor Author

artemcm commented Aug 20, 2020

@swift-ci please test

@artemcm artemcm force-pushed the ExplicitFrameworkBuilds branch from 0b05b03 to 5e9ad89 Compare August 21, 2020 01:42
…ph and swift module info.

Ensure the flag is propagated from a given swift module's dependency graph details to its swift module info passed in as an explicit module map to frontend jobs.
@artemcm artemcm force-pushed the ExplicitFrameworkBuilds branch from 5e9ad89 to 70f502b Compare August 21, 2020 01:43
@artemcm
Copy link
Contributor Author

artemcm commented Aug 24, 2020

@swift-ci please test

1 similar comment
@artemcm
Copy link
Contributor Author

artemcm commented Aug 24, 2020

@swift-ci please test

@artemcm artemcm merged commit 25116dc into swiftlang:master Aug 24, 2020
@artemcm artemcm deleted the ExplicitFrameworkBuilds branch January 20, 2021 19:01
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