Skip to content
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

[5.9] Fix issues with excluding macros/plugins from dependency computation #6726

Merged
merged 1 commit into from
Jul 19, 2023

Conversation

neonichu
Copy link
Contributor

While fixing this, I also noticed the original issue also exists for executable targets, so this gets fixed here as well.

There's one unfortunate nuance here for test targets since using a macro/plugin is indistinguishable from needing to link it because it is being tested. We err on the side of caution here and will always link.

(sidenote: theoretically, plugins do distinguish between linkage and use in the package manifest, but this distinction is not carried forward into the actual model)

Partially fixes swiftlang/swift-source-compat-suite#833 since the underlying project also does not declare a dependency on the macro that is being tested.

(cherry picked from commit b31c19a)

While fixing this, I also noticed the original issue also exists for executable targets, so this gets fixed here as well.

There's one unfortunate nuance here for test targets since using a macro/plugin is indistinguishable from needing to link it because it is being tested. We err on the side of caution here and will always link.

(sidenote: theoretically, plugins *do* distinguish between linkage and use in the package manifest, but this distinction is not carried forward into the actual model)

Partially fixes https://github.com/apple/swift/issues/67371 since the underlying project also does not declare a dependency on the macro that is being tested.

(cherry picked from commit b31c19a)
@neonichu neonichu added the swift 5.9 This PR targets the 5.9 branch label Jul 19, 2023
@neonichu neonichu self-assigned this Jul 19, 2023
@neonichu neonichu requested a review from tomerd as a code owner July 19, 2023 03:11
@neonichu
Copy link
Contributor Author

@swift-ci please smoke test

@neonichu neonichu enabled auto-merge (squash) July 19, 2023 05:54
@neonichu neonichu merged commit 6ed7215 into release/5.9 Jul 19, 2023
@neonichu neonichu deleted the fix-plugin-exclusion-5.9 branch July 19, 2023 16:51
neonichu added a commit that referenced this pull request Sep 28, 2023
In #6726, I discovered that an issue I was fixing for macros and plugins could also apply to executables. That fix was incorrect and would drop legitimate linkage of transitive dependencies when testing executables. The logic here is pretty gnarly, so the most reasonable cause of action seems to be to revert that part of the change.

resolves #6940
@neonichu neonichu mentioned this pull request Sep 28, 2023
neonichu added a commit that referenced this pull request Sep 29, 2023
In #6726, I discovered that an issue I was fixing for macros and plugins could also apply to executables. That fix was incorrect and would drop legitimate linkage of transitive dependencies when testing executables. The logic here is pretty gnarly, so the most reasonable cause of action seems to be to revert that part of the change.

resolves #6940
neonichu added a commit that referenced this pull request Sep 29, 2023
In #6726, I discovered that an issue I was fixing for macros and plugins could also apply to executables. That fix was incorrect and would drop legitimate linkage of transitive dependencies when testing executables. The logic here is pretty gnarly, so the most reasonable cause of action seems to be to revert that part of the change.

resolves #6940

(cherry picked from commit 7c1cbeb)
neonichu added a commit that referenced this pull request Sep 29, 2023
In #6726, I discovered that an issue I was fixing for macros and plugins could also apply to executables. That fix was incorrect and would drop legitimate linkage of transitive dependencies when testing executables. The logic here is pretty gnarly, so the most reasonable cause of action seems to be to revert that part of the change.

resolves #6940

(cherry picked from commit 7c1cbeb)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
swift 5.9 This PR targets the 5.9 branch
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants