Skip to content

[CMake] add_swift_target_library shouldn't implicitly set INSTALL_IN_TARGET #26340

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
Aug 3, 2019

Conversation

bulbazord
Copy link
Contributor

This makes it more explicit what the install component of a target
library is if you don't see one (and its marked as IS_SDK_OVERLAY).
Explicit in this case makes more sense, as you don't have to rely on
knowledge of how add_swift_target_library is implemented to understand
what component is used to install the target.

cc @compnerd @gottesmm @Rostepher

@compnerd
Copy link
Member

@swift-ci please smoke test

@bulbazord bulbazord force-pushed the make-explicit-the-implicit branch from c4dce56 to 2603bef Compare August 2, 2019 20:27
@bulbazord
Copy link
Contributor Author

@swift-ci please smoke test

…TARGET

This makes it more explicit what the install component of a target
library is if you don't see one (and its marked as IS_SDK_OVERLAY).
Explicit in this case makes more sense, as you don't have to rely on
knowledge of how `add_swift_target_library` is implemented to understand
what component is used to install the target.
@bulbazord bulbazord force-pushed the make-explicit-the-implicit branch from 2603bef to 184d942 Compare August 2, 2019 20:52
@bulbazord
Copy link
Contributor Author

@swift-ci please smoke test

@bulbazord
Copy link
Contributor Author

@swift-ci please smoke test Linux platform

@Rostepher
Copy link
Contributor

I appreciate that you're making this explicit. LGTM!

@gmittert gmittert merged commit 1a92886 into swiftlang:master Aug 3, 2019
@bulbazord bulbazord deleted the make-explicit-the-implicit branch August 3, 2019 00:41
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.

4 participants