Skip to content

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