-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Disable stage-2 builds #43439
Disable stage-2 builds #43439
Conversation
- ${{ if in(parameters.scope, 'full') }}: | ||
|
||
# This AlmaLinux leg is intended to build with the min supported glibc version | ||
- ${{ if parameters.enableStage2Builds }}: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't consider these to be stage 2 builds. These are stage 1 builds with a non-MSFT SDK.
Long term I don't see value in having a variable/parameter to disable non-MSFT sdk builds. Perhaps just commenting them out is appropriate for now.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The wording in dotnet/source-build#4605 should be updated accordingly.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should variable/parameter be used to condition CurrentSourceBuiltSdk
builds or should all non-MSFT builds be just commented out?
We would need to disable all non-MSFT builds as the very next re-bootstrapping would cause failures in PreviousSourceBuiltSdk
builds.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I lean towards commenting all non-msft builds for now with a comment. We likely will never use the variable/parameter again therefore it is adding unnecessary complexity IMO.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK - makes sense - will update this PR.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed with 935070d
runOnline: false # 🚫 | ||
useMonoRuntime: false # 🚫 | ||
withPreviousSDK: true # ✅ | ||
# Disabled until 9.0 GA - see https://github.com/dotnet/source-build/issues/4605 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This isn't coupled to the GA release. It is the standing up of net10.0 which is not coupled to the release.
# Disabled until 9.0 GA - see https://github.com/dotnet/source-build/issues/4605 | |
# Disabled until net9.0 -> net10.0 transition is complete - see https://github.com/dotnet/source-build/issues/4605 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks - I've made a local change, as there are 6 comments, one for each commented-out job. Fixed with 9c52eea
runOnline: false # 🚫 | ||
useMonoRuntime: false # 🚫 | ||
withPreviousSDK: true # ✅ | ||
# Disabled until 9.0 GA - see https://github.com/dotnet/source-build/issues/4605 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
# Disabled until 9.0 GA - see https://github.com/dotnet/source-build/issues/4605 | |
# Disabled until net9.0 -> net10.0 transition is complete - see https://github.com/dotnet/source-build/issues/4605 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks - I've made a local change, as there are 6 comments, one for each commented-out job. Fixed with 9c52eea
withPreviousSDK: false # 🚫 | ||
reuseBuildArtifactsFrom: | ||
- ${{ format('{0}_Mono_Offline_MsftSdk_x64', variables.centOSStreamName) }} | ||
# Disabled until 9.0 GA - see https://github.com/dotnet/source-build/issues/4605 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
# Disabled until 9.0 GA - see https://github.com/dotnet/source-build/issues/4605 | |
# Disabled until net9.0 -> net10.0 transition is complete - see https://github.com/dotnet/source-build/issues/4605 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks - I've made a local change, as there are 6 comments, one for each commented-out job. Fixed with 9c52eea
Resolves: dotnet/source-build#4586
Stage 2 builds are currently failing as is always the case during transition to a new release. The failures were originally introduced with #42969. We could modify those properties, to enable PVP version flow, and get stage 2 builds running, but that would only work for a short time, until
runtime
andaspnetcore
move to 10.0 versioning. At that point building 9.0 projects could break, as_NET90RuntimePackVersion
and other 9.0 'packs' properties insrc/Installer/redist-installer/targets/GenerateBundledVersions.targets
would reference 10.0 'packs'.Stage-2 builds can be enabled after 9.0 GA, at which point we will add 9.0 targeting pack to SBRP and update the properties introduced in #42969 to reference it.
Note that the diff is rather silly. My changes are simple. I am adding a new parameter
enableStage2Builds
and conditioning each of the stage-2 jobs, to only run if this parameter is set totrue
.