Skip to content

Invoke build tool plugins based on allTargets #6776

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
Sep 14, 2023

Conversation

neonichu
Copy link
Contributor

@neonichu neonichu commented Aug 4, 2023

Currently, we're only invoking build tool plugins for reachable targets which does not seem correct based on the fact that we're willing to build targets out of reachable, e.g. when executing a command plugin.

rdar://113264168

@neonichu neonichu self-assigned this Aug 4, 2023
@neonichu
Copy link
Contributor Author

neonichu commented Aug 4, 2023

@swift-ci please smoke test

@MaxDesiatov
Copy link
Contributor

@swift-ci smoke test linux

Copy link
Contributor

@MaxDesiatov MaxDesiatov left a comment

Choose a reason for hiding this comment

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

Can a test for this be added?

@neonichu neonichu marked this pull request as draft August 25, 2023 22:34
Currently, we're only invoking build tool plugins for reachable targets which does not seem correct based on the fact that we're willing to build targets out of reachable, e.g. when executing a command plugin.

rdar://113264168
@neonichu neonichu force-pushed the invoke-plugins-for-all-targets branch from 695fd11 to 3a8dbfa Compare September 12, 2023 23:07
@neonichu neonichu marked this pull request as ready for review September 12, 2023 23:07
@neonichu
Copy link
Contributor Author

@swift-ci please test

@neonichu
Copy link
Contributor Author

Can a test for this be added?

Not really sure how to add a test case for this since it requires a quite elaborate setup to demonstrate the original issue. I think that's ok though since using allTargets is obviously correct and reachableTargets is not.

@tomerd
Copy link
Contributor

tomerd commented Sep 14, 2023

would be possible to add a test?

@neonichu
Copy link
Contributor Author

would be possible to add a test?

See comment above. In principle yes, but the cost seems high for something that's obviously just wrong.

@neonichu neonichu merged commit 9306b79 into main Sep 14, 2023
@neonichu neonichu deleted the invoke-plugins-for-all-targets branch September 14, 2023 20:58
MaxDesiatov pushed a commit that referenced this pull request Sep 28, 2023
Currently, we're only invoking build tool plugins for reachable targets which does not seem correct based on the fact that we're willing to build targets out of reachable, e.g. when executing a command plugin.

rdar://113264168
MaxDesiatov pushed a commit that referenced this pull request Sep 28, 2023
Currently, we're only invoking build tool plugins for reachable targets which does not seem correct based on the fact that we're willing to build targets out of reachable, e.g. when executing a command plugin.

rdar://113264168
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