Skip to content

Delete the linkage tests #2189

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 15, 2023

Conversation

ahoppen
Copy link
Member

@ahoppen ahoppen commented Sep 13, 2023

The linkage tests have been causing more issues than they caught issues. And really what we want to test, is that the libraries built for the compiler using CMake don’t have any unexpected dependencies – it doesn’t really matter if the swift-syntax package itself gains a new link dependency.

Furthermore, the test was only checking SwiftSyntax, SwiftParser and SwiftSyntaxBuilder and was missing e.g. SwiftSyntaxMacros.

We should properly test that no dylibs in the toolchain have unintended linkages as part of integration testing in the apple/swift repo.

The linkage tests have been causing more issues than they caught issues. And really what we want to test, is that the libraries built for the compiler using CMake don’t have any unexpected dependencies – it doesn’t really matter if the swift-syntax package itself gains a new link dependency.

Furthermore, the test was only checking `SwiftSyntax`, `SwiftParser` and `SwiftSyntaxBuilder` and was missing e.g. `SwiftSyntaxMacros`.

We should properly test that no dylibs in the toolchain have unintended linkages as part of integration testing in the apple/swift repo.
@ahoppen ahoppen requested a review from bnbarham September 13, 2023 22:55
@ahoppen
Copy link
Member Author

ahoppen commented Sep 14, 2023

@swift-ci Please test

@ahoppen ahoppen merged commit dd8e936 into swiftlang:main Sep 15, 2023
@ahoppen ahoppen deleted the ahoppen/delete-linkage-tests branch February 13, 2024 21:02
ahoppen added a commit to ahoppen/swift-syntax that referenced this pull request Feb 13, 2024
* **Explanation**: The linkage tests have been causing more issues than they caught issues. And really what we want to test, is that the libraries built for the compiler using CMake don’t have any unexpected dependencies – it doesn’t really matter if the swift-syntax package itself gains a new link dependency.

Furthermore, the test was only checking `SwiftSyntax`, `SwiftParser` and `SwiftSyntaxBuilder` and was missing e.g. `SwiftSyntaxMacros`.

We should properly test that no dylibs in the toolchain have unintended linkages as part of integration testing in the apple/swift repo.
* **Scope**: This is a test-only change
* **Risk**: Very low, this is a test-only change and has already been reverted on the 509 package release of swift-syntax
* **Testing**: n/a
* **Issue**: rdar://122906543
* **Reviewer**: Ben Barham (@bnbarham) on swiftlang#2189
ahoppen added a commit to ahoppen/swift-syntax that referenced this pull request Feb 13, 2024
* **Explanation**: The linkage tests have been causing more issues than they caught issues. And really what we want to test, is that the libraries built for the compiler using CMake don’t have any unexpected dependencies – it doesn’t really matter if the swift-syntax package itself gains a new link dependency.
* **Scope**: This is a test-only change
* **Risk**: Very low, this is a test-only change and has already been reverted on the 509 package release of swift-syntax
* **Testing**: n/a
* **Issue**: rdar://122906543
* **Reviewer**: Ben Barham (@bnbarham) on swiftlang#2189
ahoppen added a commit to ahoppen/swift-syntax that referenced this pull request Feb 13, 2024
* **Explanation**: The linkage tests have been causing more issues than they caught issues. And really what we want to test, is that the libraries built for the compiler using CMake don’t have any unexpected dependencies – it doesn’t really matter if the swift-syntax package itself gains a new link dependency.
* **Scope**: This is a test-only change
* **Risk**: Very low, this is a test-only change and has already been removed on the 509 package release of swift-syntax
* **Testing**: n/a
* **Issue**: rdar://122906543
* **Reviewer**: Ben Barham (@bnbarham) on swiftlang#2189
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