Exclude packages from checksum generation #33722
Conversation
|
I couldn't add an area label to this PR. Checkout this page to find out which area owner to ping, or please add exactly one area label to help train me in the future. |
|
Fix is currently in testing here: https://dev.azure.com/dnceng/internal/_build/results?buildId=563934&view=results |
| <Target Name="CreateChecksums"> | ||
| <ItemGroup> | ||
| <ArtifactsForGeneratingChecksums | ||
| Include="@(UploadToBlobStorageFile)" | ||
| Excldude="*.nupkg" |
There was a problem hiding this comment.
There is a typo in Exclude.
| Excldude="*.nupkg" | |
| Exclude="*.nupkg" |
There was a problem hiding this comment.
Should we include *.snupkg so that when/if we add them, they're excluded?
There was a problem hiding this comment.
They won't be found by any of this by default, so nothing needs to be done to prevent that:
runtime/src/installer/publish/Directory.Build.targets
Lines 21 to 24 in 188243a
FWIW, I have concerns that snupkg doesn't meet dotnet/runtime requirements based on the spec last time I looked: dotnet/arcade#167 (comment).
| <Target Name="CreateChecksums"> | ||
| <ItemGroup> | ||
| <ArtifactsForGeneratingChecksums | ||
| Include="@(UploadToBlobStorageFile)" | ||
| Excldude="*.nupkg" |
There was a problem hiding this comment.
I don't think this will work because the nupkgs aren't in the working dir. Maybe use a Condition on %(Extension) instead, or exclude @(DownloadedSymbolNupkgFile);@(DownloadedNupkgFile)?
There was a problem hiding this comment.
Oh, this has actually already been done:
runtime/src/installer/publish/Directory.Build.targets
Lines 26 to 29 in f2e6260
This PR should be a no-op.
There was a problem hiding this comment.
Hrmm...that's odd then, because we're getting checksums for the blobs:
I'll dig deeper.
There was a problem hiding this comment.
Oh, that's strange. Later in that file the nupkgs are added back in:
I don't know why we'd be doing that.
There was a problem hiding this comment.
Looks like it's the ultimate list of things to upload to the parepred artifacts location
|
Changed this around:
|
|
Test official build passed and did the right thing, merging |
ghost
locked as resolved and limited conversation to collaborators
No description provided.