Skip to content

WIP: Create a unified build for (swift-)foundation-tests #79862

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

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

ahoppen
Copy link
Member

@ahoppen ahoppen commented Mar 8, 2025

We can’t add the tests to the general unified build because they use testable imports and building a package with testable imports also re-builds all of its dependencies with testable imports (swiftlang/swift-package-manager#8344) and is thus incompatible with the other package’s unified build. But we should still be able to share build products between corelibs-foundation tests and swift-foundation-tests.

@ahoppen
Copy link
Member Author

ahoppen commented Mar 8, 2025

@swift-ci Please smoke test Linux

@ahoppen ahoppen force-pushed the unified-swiftfoundation-test branch from a4e3c3b to 47261ab Compare March 8, 2025 20:05
@ahoppen
Copy link
Member Author

ahoppen commented Mar 8, 2025

@swift-ci Please smoke test Linux

@ahoppen ahoppen force-pushed the unified-swiftfoundation-test branch from 47261ab to e78e5fe Compare March 9, 2025 02:01
@ahoppen
Copy link
Member Author

ahoppen commented Mar 9, 2025

@swift-ci Please test Linux

@ahoppen ahoppen force-pushed the unified-swiftfoundation-test branch from e78e5fe to 1170bc5 Compare March 9, 2025 08:56
@ahoppen
Copy link
Member Author

ahoppen commented Mar 9, 2025

@swift-ci Please smoke test Linux

We can’t add the tests to the general unified build because they use testable imports and building a package with testable imports also re-builds all of its dependencies with testable imports (swiftlang/swift-package-manager#8344) and is thus incompatible with the other package’s unified build. But we should still be able to share build products between corelibs-foundation tests and swift-foundation-tests.
@ahoppen ahoppen force-pushed the unified-swiftfoundation-test branch from 1170bc5 to db538f6 Compare March 9, 2025 09:00
@ahoppen
Copy link
Member Author

ahoppen commented Mar 9, 2025

@swift-ci Please smoke test Linux

@ahoppen
Copy link
Member Author

ahoppen commented Mar 10, 2025

OK, this seems to be trickier than I initially thought because: ' swift-corelibs-foundation' requires the 'plutil executable
product to be built for its tests to pass. When doing the unified build, we need to pass '--test-product to specify the targets that should be tested. The package's test product doesn't contain the 'plutil' executable product and thus 'plutil' is not getting built. Performing an independent build of plutil' using a separate 'swift build' invocation performs a build with testability disabled, which conflicts with the swift test build that has testability enabled, causing all targets (including dependencies) to get rebuilt. As far as I can tell, there is no way to build a non-test product with testability enabled.

Possible solutions are (in no particular order):

  1. Avoid using testable imports in swift-foundation and swift-corelibs-foundation, and instead use the package access level or @_spi(Testing) imports. Then we would perform all builds with testability disabled and wouldn’t have the conflict when building plutil. This would have the added advantage that swift-corelibs-foundation and swift-foundation could join the unified build of swift-syntax etc. and thus avoid rebuilding swift-syntax at all (saving ~30 minutes of CI time in total).
  2. Build and run tests in debug instead of release configuration. Debug builds always have testability enabled, so we shouldn’t have the conflict anymore. This shouldn’t have any effect on the produced toolchain because the version of swift-foundation and swift-corelibs-foundation being installed into the toolchain aren’t produced by the swiftfoundationtest and foundationtest build script products anyway. While this would still cause a re-build of swift-syntax in CI, since we wouldn’t be re-using its unified build, it would be significantly faster. I did not measure but I would expect any improvements in build time to outweigh slow downs in test execution because tests currently finish in ~1 minute while the build takes >15 minutes.
  3. Add an option to SwiftPM to perform a build that has testability enabled. That way we could build plutil with testability enabled and wouldn’t need to clean the build directory for it.
  4. Adopt SE-0455 in swift-(corelibs-)foundation and only build those targets with testability enabled that actually need it. Similar to (1), this means that swift-foundation could join the entire unified build. A catch seems to be that the PR implementing the proposal hasn’t been merged yet. Also, if swift-(corelibs-)foundation need to support building with older versions of Swift, they would need a [email protected] manifest that controls these flags
  5. Re-design TestProcess .test_plutil to not assume that plutil is present in the build directory. I don’t know much about this test but as far as I can tell, it is not really testing behavior of plutil itself, so this might be possible to re-design the test to use some other executable to test the Process type, which seems to be the test’s purpose.

@finagolfin
Copy link
Member

I implemented 2. in #78390, some data on how much CI time it saved there.

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.

2 participants