-
Notifications
You must be signed in to change notification settings - Fork 379
Add flags to support easy disablement of SB prebuilt checking #15721
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
Add flags to support easy disablement of SB prebuilt checking #15721
Conversation
src/Microsoft.DotNet.Arcade.Sdk/tools/SourceBuild/SourceBuildArcadeTools.targets
Outdated
Show resolved
Hide resolved
src/Microsoft.DotNet.Arcade.Sdk/tools/SourceBuild/SourceBuildArcadeTools.targets
Outdated
Show resolved
Hide resolved
Co-authored-by: Matt Thalman <mthalman@microsoft.com>
|
|
||
| <Import Project="SourceBuild/SourceBuildArcadeTools.targets" Condition="'$(DotNetBuildRepo)' == 'true' or | ||
| '$(SetUpSourceBuildIntermediateNupkgCache)' == 'true'" /> | ||
| <Import Project="SourceBuild/SourceBuildArcadeTools.targets" Condition="'$(DotNetBuildRepo)' == 'true'" /> |
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.
For targets in SourceBuild/SourceBuildArcadeTools.targets to run, SetUpSourceBuildIntermediateNupkgCache should not be false. Would it make sense to skip this import in that case?
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 decided against it. This is going away shortly all together once the flat flow is stabilized. The existing implementation felt odd in the sense that the import still happened in many scenarios where the targets were conditioned out.
|
@MichaelSimons does this need to be backported to release/9.0 for repositories that use the 9.0 Arcade SDK? cc @premun @akoeplinger |
|
No, we do not want to disable prebuilt detection for repos flowing into 9.0. |
|
Right. How should we handle repositories that use 9.0 Arcade but consume 10.0 SB intermediate packages? Example: https://github.com/dotnet/razor/blob/4ed93db9c39d970197c3410f9e878caaf8a92316/eng/Version.Details.xml#L8-L13 |
|
I see this commit is from main. Will main flow into a 9.0 sdk feature band? If so it really should be on a 9.0 SBRP version. If it doesn't flow to 9.0, then why isn't it on 10.0 arcade? I have no specific concern with backporting this, but we should not be disabling prebuilt detection on repos flowing into 9.0. Aside from disabling prebuilt detection, this will cause a ripple effect on other 9.0 repos that depend on it's intermediates. |
Discussed offline. Those are incorrect and should be on 9.0. @ViktorHofer is updating the subscriptions. |
|
OK only those three repos - puuuh |
Related to dotnet/source-build#5034
Setting
SetUpSourceBuildIntermediateNupkgCache=falsewill disable the intermediate restoration and settingReportPrebuiltUsage=falsewill disable the prebuilt validation. Both will be necessary to set during the flat flow enablement.SetUpSourceBuildIntermediateNupkgCachewas previously unused so I repurposed it.