Skip to content

[build.ps1] Revamp No-asserts toolchain build and packaging #81644

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 10 commits into from
Jun 11, 2025

Conversation

mhegazy
Copy link
Contributor

@mhegazy mhegazy commented May 20, 2025

This PR allows us to build the noasserts variant of the compilers along with the asserts variant by adding a new commandline argument: -IncludeNoAsserts. This is in preparation for packaging both in our installer and letting users choose which ones to use.

If the new option is not set, the build output should not change from today. if the new option is set compilers are built an extra time with asserts disabled.

Note to reviewers: Reviewing this change one commit at a time should give you a better experience than looking at all changes at once.

Changes:

  • Remove existing commandline option -Variant (added in #79313), that was added to allow building one variant at a time.
  • Add new command line option -IncludeNoAsserts to build a new version of the compiler with LLVM_ENABLE_ASSERTIONS=NO
  • Add an extra call to Build-Compilers when -IncludeNoAsserts is set
  • Add an extra step to Build-mimalloc to batch the tools in no-asserts output directory as well
  • Fix bug in argument handling in Invoke-BuildStep

Copy link
Member

@compnerd compnerd left a comment

Choose a reason for hiding this comment

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

This should likely be split out, this has no impact on the build/packaging only in the internals of the script.

@mhegazy mhegazy requested a review from compnerd May 21, 2025 01:09
@mhegazy mhegazy marked this pull request as ready for review May 27, 2025 18:26
@mhegazy mhegazy force-pushed the no-asserts-toolchain branch from 886310b to 3ba15e6 Compare May 28, 2025 17:13
@mhegazy mhegazy force-pushed the no-asserts-toolchain branch 2 times, most recently from a224b48 to 899402d Compare May 29, 2025 22:01
@mhegazy mhegazy force-pushed the no-asserts-toolchain branch from 899402d to 96b4038 Compare May 29, 2025 22:02
@mhegazy
Copy link
Contributor Author

mhegazy commented May 29, 2025

@compnerd updated based on your review feedback. mind taking another look

@mhegazy mhegazy force-pushed the no-asserts-toolchain branch from f471a28 to b8be792 Compare May 30, 2025 16:45
@mhegazy
Copy link
Contributor Author

mhegazy commented May 30, 2025

@compnerd i realized that PinnedToolchainVariant was causing trouble, speciallyt hat it was not fully supported before. so remvoing it and making it clear where we expect asserts and where we do not should address your comments

@mhegazy mhegazy requested a review from compnerd May 30, 2025 17:44
@mhegazy mhegazy requested a review from compnerd May 30, 2025 23:28
# /NC: Do not write file classes
# /NS: Do not write file sizes
# /NP: Do not show progress indicator
&robocopy $HostPlatform.ToolchainInstallRoot $HostPlatform.NoAssertsToolchainInstallRoot /E /XC /XN /XO /NS /NC /NFL /NDL /NJH
Copy link
Member

Choose a reason for hiding this comment

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

Invoke-Command would be better than &.

@compnerd
Copy link
Member

compnerd commented Jun 1, 2025

@swift-ci please smoke test

@compnerd
Copy link
Member

compnerd commented Jun 2, 2025

Error: Positional parameter 'Asserts' found. The Invoke-BuildStep function only supports named parameters after the required Name and Platform parameters.
    at Invoke-BuildStep, C:\Users\swift-ci\jenkins\workspace\swift-PR-windows\swift\utils\build.ps1: line 662
    at <ScriptBlock>, C:\Users\swift-ci\jenkins\workspace\swift-PR-windows\swift\utils\build.ps1: line 3473
  From System.Management.Automation.RuntimeException: Positional parameter 'Asserts' found. The Invoke-BuildStep function only supports named parameters after the required Name and Platform parameters.

@mhegazy
Copy link
Contributor Author

mhegazy commented Jun 2, 2025

@compnerd should be running clean now. mind rerunning the smoke test

@compnerd
Copy link
Member

compnerd commented Jun 4, 2025

@swift-ci please smoke test

Co-authored-by: Saleem Abdulrasool <compnerd@compnerd.org>
@compnerd
Copy link
Member

@swift-ci please smoke test

@compnerd compnerd merged commit 276261c into swiftlang:main Jun 11, 2025
3 checks passed
@mhegazy mhegazy deleted the no-asserts-toolchain branch June 11, 2025 16:44
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