-
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
Update Source-Build SDK Diff Tests Baselines and Exclusions #41929
Conversation
…ld/results?buildId=2487166 (internal Microsoft link)
Will investigate in the morning of 7/3 |
...tent/test/Microsoft.DotNet.SourceBuild.SmokeTests/assets/ArtifactsSizeTests/centos.9-x64.txt
Outdated
Show resolved
Hide resolved
...tent/test/Microsoft.DotNet.SourceBuild.SmokeTests/assets/ArtifactsSizeTests/centos.9-x64.txt
Show resolved
Hide resolved
...tent/test/Microsoft.DotNet.SourceBuild.SmokeTests/assets/ArtifactsSizeTests/centos.9-x64.txt
Outdated
Show resolved
Hide resolved
./host/fxr/x.y.z/ | ||
./host/fxr/x.y.z/libhostfxr.so | ||
+./library-packs/ |
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.
Will update the exclusions list to account for this one. It should've been included in the last PR update since it went along with those changes.
./sdk-manifests/ | ||
./sdk-manifests/x.y.z/ | ||
./sdk-manifests/x.y.z/ | ||
-./sdk-manifests/x.y.z/ |
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 microsoft sdk has the following content in sdk-manifests/
:
- 8.0.100/
- 9.0.100-preview.1/
- 9.0.100-preview7/
The source-build sdk has the following content in sdk-manifests/
:
- 8.0.100/
- 9.0.100-preview.7/
@MiYanni - Do you know if this is expected? Is the microsoft sdk supposed to have the preview 1 manifests?
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.
Sorry, but I don't actually know the composition of the SDK. @marcpopMSFT Does that look like it makes sense?
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 comes from here and is used in BundledManifests.targets.
This should definitely get updated to the latest, i.e. https://www.nuget.org/packages/Microsoft.NET.Sdk.Maui.Manifest-9.0.100-preview.5/9.0.0-preview.5.24307.10#readme-body-tab
The last time this was done was in dotnet/installer#18693.
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.
These always lag behind 1 release, as we want publicly listed packages. There is kind of a circular dependency here, .NET SDK -> MAUI -> .NET SDK. So, when .NET 9 GAs, we'll probably have .NET 9 RC 2 manifests in here.
We could update it now, though: it's been a while.
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.
Why are we currently lagging 5 preview releases behind? I assume there is no automated process in place that makes sure those get updated? And what's the customer impact here?
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'm not aware of any customer impact. I suppose you wouldn't get the latest workloads if you used the --skip-manifest-update
switch.
To automate this, we might be able to use the same Maestro subscription that this repo will be using:
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.
@ellahathaway if the source-build SDK doesn't contain maui workloads, then the missing 9.0.100-preview.1/
folder is expected and should be ignored. Note as discussed above, the folder name will change whenever the workload manifests for maui are updated.
@jonathanpeppers can you please file an issue in dotnet/sdk? We should probably also get the manifest updated from P1 to P5.
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.
if the source-build SDK doesn't contain maui workloads, then the missing 9.0.100-preview.1/ folder is expected and should be ignored. Note as discussed above, the folder name will change whenever the workload manifests for maui are updated.
Sounds good. I think the best course of action here is to add this diff to the baseline. I'll update this PR 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.
@jonathanpeppers can you please file an issue in dotnet/sdk? We should probably also get the manifest updated from P1 to P5.
I just opened a PR with the builds that shipped yesterday:
...tent/test/Microsoft.DotNet.SourceBuild.SmokeTests/assets/ArtifactsSizeTests/centos.9-x64.txt
Show resolved
Hide resolved
...tent/test/Microsoft.DotNet.SourceBuild.SmokeTests/assets/ArtifactsSizeTests/centos.9-x64.txt
Show resolved
Hide resolved
...tent/test/Microsoft.DotNet.SourceBuild.SmokeTests/assets/ArtifactsSizeTests/centos.9-x64.txt
Show resolved
Hide resolved
Merged before #42022 since I did not want the PR updater tool to undo these changes tomorrow morning. |
Fixes dotnet/source-build#4493
This PR was created by the
CreateBaselineUpdatePR
tool for build 2490356.The updated test results can be found at https://dev.azure.com/dnceng/internal/_build/results?buildId=2490356 (internal Microsoft link)