-
Notifications
You must be signed in to change notification settings - Fork 4.6k
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
Exclude packages from checksum generation #33722
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -7,10 +7,13 @@ | |||||||||||||
<SuppressFinalPackageVersion>true</SuppressFinalPackageVersion> | ||||||||||||||
</PropertyGroup> | ||||||||||||||
|
||||||||||||||
<!-- Create all the checksum files. Exclude anything that ends | ||||||||||||||
in .nupkg (symbol and regular packages). --> | ||||||||||||||
<Target Name="CreateChecksums"> | ||||||||||||||
<ItemGroup> | ||||||||||||||
<ArtifactsForGeneratingChecksums | ||||||||||||||
Include="@(UploadToBlobStorageFile)" | ||||||||||||||
Excldude="*.nupkg" | ||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't think this will work because the nupkgs aren't in the working dir. Maybe use a There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hrmm...that's odd then, because we're getting checksums for the blobs: I'll dig deeper. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Looks like it's the ultimate list of things to upload to the parepred artifacts location |
||||||||||||||
DestinationPath="%(FullPath)$(ChecksumExtension)" /> | ||||||||||||||
|
||||||||||||||
<GeneratedChecksumFile Include="@(ArtifactsForGeneratingChecksums -> '%(DestinationPath)')" /> | ||||||||||||||
|
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.
There is a typo in
Exclude
.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 we include *.snupkg so that when/if we add them, they're excluded?
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.
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).