Skip to content

[CMake] Require SWIFT_COMPONENT for add_swift_host_tool #21326

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

Merged
merged 1 commit into from
Dec 15, 2018

Conversation

bulbazord
Copy link
Contributor

Previously, all but one instance of add_swift_host_tool were specifying a SWIFT_COMPONENT. The one that did not (swift-stdlib-tool) was calling swift_install_in_component manually. This means that we never were adding swift-stdlib-tool to SWIFT_EXPORTS or SWIFT_BUILDTREE_EXPORTS.

In this PR I make swift-stdlib-tool specify SWIFT_COMPONENT in its add_swift_host_tool invocation and remove the manual call to swift_install_in_component.
Additionally, I change add_swift_host_tool to require a SWIFT_COMPONENT so that tool installations are always associated with at least one swift component.

@bulbazord
Copy link
Contributor Author

cc @compnerd @gottesmm

@compnerd
Copy link
Member

Seems reasonable, any objections @gottesmm?

@compnerd
Copy link
Member

@swift-ci please test

@gottesmm
Copy link
Contributor

@xiaobai +1 from me! The general design here is that all of the install in component calls should be hidden in the helper functions. I just have never had time to finish. So I am all for anything that helps us go towards that world.

@bulbazord
Copy link
Contributor Author

Awesome, thank you for the explanation!

@compnerd @gottesmm Would one of you mind committing this on my behalf? I don't have swift commit access.

@compnerd compnerd merged commit 7020740 into swiftlang:master Dec 15, 2018
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