Skip to content

Conversation

maxwellE
Copy link
Contributor

@maxwellE maxwellE commented Aug 14, 2025

  • Explanation:

Create a module map for the CSKTestSupport module, exporting the header "CSKTestSupport.h". This allows for better modularization and encapsulation of the support functionalities.

Signed-off-by: Maxwell Elliott maxwell@elliott.now

  • Scope:

Existing SPM users appear to be not impacted, this change should not affect their builds

  • Issues:

Some build systems cannot build this logic without this modulemap file being valid

  • Original PRs:

  • Risk:

Low

  • Testing:

Does it build

  • Reviewers:

Create a module map for the CSKTestSupport module, exporting the header "CSKTestSupport.h". This allows for better modularization and encapsulation of the support functionalities.

Signed-off-by: Maxwell Elliott <maxwell@elliott.now>
Copy link
Member

@ahoppen ahoppen left a comment

Choose a reason for hiding this comment

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

Out of curiosity: Which build system are you using that requires the module map?

@bnbarham
Copy link
Contributor

bnbarham commented Aug 14, 2025

My guess is bazel (?), but I'm still wondering why the modulemap is needed - nothing is importing CSKTestSupport, it's just being linked into SKTestSupport to not call static destructors. I assume the modulemap is there at all purely to appease SwiftPM.

And if we do need to specify something in the module, my preference would be for it to still be empty rather than adding this header. There's no intention that anything would ever import and call this function.

Signed-off-by: Maxwell Elliott <maxwell@elliott.now>
@maxwellE
Copy link
Contributor Author

This was done to support using this library via https://github.com/cgrindel/rules_swift_package_manager building with Bazel

Signed-off-by: Maxwell Elliott <maxwell@elliott.now>
Copy link
Member

@ahoppen ahoppen left a comment

Choose a reason for hiding this comment

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

Thanks for clarifying our questions. I don’t see any reason not to add the module map, so happy to take it.

@ahoppen
Copy link
Member

ahoppen commented Aug 15, 2025

@swift-ci Please test

@ahoppen
Copy link
Member

ahoppen commented Aug 15, 2025

@swift-ci Please test Windows

3 similar comments
@ahoppen
Copy link
Member

ahoppen commented Aug 17, 2025

@swift-ci Please test Windows

@ahoppen
Copy link
Member

ahoppen commented Aug 18, 2025

@swift-ci Please test Windows

@ahoppen
Copy link
Member

ahoppen commented Aug 18, 2025

@swift-ci Please test Windows

@ahoppen ahoppen merged commit 520c734 into swiftlang:main Aug 18, 2025
3 checks passed
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