Skip to content

Add MinimalStdlib build-script product, build it in main CI jobs and PR testing jobs #64492

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

Conversation

kubamracek
Copy link
Contributor

@kubamracek kubamracek commented Mar 20, 2023

This adds the existing "minimal freestanding" stdlib variant preset (which builds a different, stripped-down version of the stdlib+runtime with minimal dependencies) into the main CI workflows, including PR testing that gates merging. The point of this is to block PRs that would regress the build of this "minimal freestanding" stdlib from merging.

For now, this doesn't run any tests, just builds. Testing will for now still only happen in the dedicated CI job (https://ci.swift.org/view/Dashboard/job/oss-swift-test-stdlib-with-toolchain-minimal/).

rdar://106456196

@kubamracek kubamracek changed the title Add MinimalStdlib build-script product, build is in main CI jobs and PR testing jobs Add MinimalStdlib build-script product, build it in main CI jobs and PR testing jobs Mar 20, 2023
@kubamracek
Copy link
Contributor Author

@swift-ci please test

1 similar comment
@kubamracek
Copy link
Contributor Author

@swift-ci please test

self.cmake_options.define('SWIFT_STDLIB_ASSERTIONS:BOOL', 'FALSE')

# Configure build to only build the stdlib, and only a static version of it
self.cmake_options.define('SWIFT_BUILD_DYNAMIC_SDK_OVERLAY:BOOL', 'FALSE')
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Note to self: This needs to somehow be unified with the settings in build-preset.ini

@kubamracek
Copy link
Contributor Author

@swift-ci please clean test

@kubamracek kubamracek force-pushed the build-minimal-stdlib-as-part-regular-ci-jobs branch from 29eadee to 90603ec Compare March 21, 2023 20:59
@kubamracek
Copy link
Contributor Author

@swift-ci please test

@kubamracek
Copy link
Contributor Author

From the PR testing build bot:

Build Percentage 	 Build Duration (sec) 	 Build Phase
================ 	 ==================== 	 ===========
27.1%             	 4212.93               	 macosx-x86_64-swift-test
20.1%             	 3128.13               	 macosx-x86_64-swift-build
6.0%              	 935.97                	 Running tests for swiftpm
4.9%              	 753.91                	 Running tests for swiftdriver
4.1%              	 638.08                	 Building swiftpm
4.1%              	 635.57                	 Running tests for swiftdocc
3.9%              	 602.32                	 macosx-x86_64-lldb-test
2.7%              	 424.7                 	 Building sourcekitlsp
2.6%              	 408.26                	 Running tests for swiftsyntax
2.3%              	 352.62                	 Building swiftsyntax
2.3%              	 350.8                 	 Building skstresstester
2.2%              	 341.94                	 Building llvm
2.2%              	 334.92                	 Building swiftformat
2.1%              	 323.74                	 Running tests for swiftformat
2.1%              	 320.19                	 Installing swiftpm
1.9%              	 301.6                 	 Building swiftevolve
1.8%              	 278.34                	 Building swiftdocc
1.8%              	 273.26                	 Installing swiftdocc
1.1%              	 171.65                	 Building swiftdriver
1.0%              	 159.79                	 Building minimalstdlib    // <<<=================
0.7%              	 110.14                	 Building earlyswiftdriver
0.6%              	 91.58                 	 Running tests for sourcekitlsp
0.5%              	 80.03                 	 Building earlyswiftsyntax
0.5%              	 78.48                 	 macosx-x86_64-lldb-build
0.4%              	 58.79                 	 macosx-x86_64-llbuild-build
0.3%              	 48.44                 	 Building indexstoredb
0.2%              	 29.27                 	 Running tests for indexstoredb
0.1%              	 18.85                 	 macosx-x86_64-swift-install
0.1%              	 18.11                 	 Building playgroundsupport
0.1%              	 13.88                 	 Running tests for skstresstester
0.1%              	 13.01                 	 macosx-x86_64-llbuild-test

So it takes an extra 2 minutes 40 seconds to build. CC @shahmishal @DougGregor @tbkka -- that feels like totally worth the value it adds?

Copy link
Contributor

@ktoso ktoso left a comment

Choose a reason for hiding this comment

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

Thanks this looks great

I'd really want this to be added, it is very tricky to get all these #ifs right while working on concurrency types, the additional validation will save a lot of time and fire-fighting from mistakes

@al45tair
Copy link
Contributor

Agreed, 2 minutes 40 seconds is well worth it.

Copy link
Member

@etcwilde etcwilde left a comment

Choose a reason for hiding this comment

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

From a code perspective, this looks fine. Isn't mostly a matter of being a different build configuration than an entirely separate product though? I suppose the static builds of Foundation and dispatch are kind of the same way, so maybe it's fine.

@kubamracek
Copy link
Contributor Author

I'm going to merge this as is, because the value seems very high right now, and follow up separately on the remaining things.

@kubamracek kubamracek merged commit 1a3cbfa into swiftlang:main Mar 28, 2023
@shahmishal
Copy link
Member

@kubamracek Can you cherry-pick this on to release/5.9?

@kubamracek
Copy link
Contributor Author

@shahmishal sure! #64677

etcwilde pushed a commit to etcwilde/swift that referenced this pull request Apr 19, 2023
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.

7 participants