Add process to check in prune package data after a major release #51992
Conversation
There was a problem hiding this comment.
Pull request overview
This PR adds a new process to check in prune package data after major .NET releases instead of downloading targeting packs during the SDK build. This avoids adding targeting packs to source-build externals. The implementation conditionally downloads targeting packs only when the data doesn't exist in the repository, and generates an error directing developers to copy the data into source control.
Key Changes
- Check in prune package data to
src\Layout\redist\PrunePackageDataafter each major release - Conditionally download targeting packs only when data is missing from the repo
- Add error handling with clear instructions to copy missing data using the
CopyPrunePackageDataToSourceproperty
| Build with CopyPrunePackageDataToSource set to true to copy these files into the source tree. Then check them in."/> | ||
|
|
||
| <Copy SourceFiles="%(PrunePackageDataToCopyToSource.Source)" | ||
| DestinationFiles="%(RepoSourcePath)" |
There was a problem hiding this comment.
The metadata reference should be qualified with the item name for clarity and to ensure correct batching behavior. Change %(RepoSourcePath) to %(PrunePackageDataToCopyToSource.RepoSourcePath) to be consistent with the qualified %(PrunePackageDataToCopyToSource.Source) used in SourceFiles.
| DestinationFiles="%(RepoSourcePath)" | |
| DestinationFiles="%(PrunePackageDataToCopyToSource.RepoSourcePath)" |
| <!-- Add metadata for downloading and binplacing --> | ||
| <TargetingPackForPruneData Version="$(TargetingPackPruneVersion).0.0" TargetFrameworkVersion="$(TargetingPackPruneVersion).0" PackageName="%(Identity).Ref" /> | ||
|
|
||
| <!-- Add metadata for where the PackagesOverride file should be in repo source --> |
There was a problem hiding this comment.
Typo in comment: "PackagesOverride" should be "PackageOverrides" to match the actual filename used in the metadata.
| <!-- Add metadata for where the PackagesOverride file should be in repo source --> | |
| <!-- Add metadata for where the PackageOverrides file should be in repo source --> |
| </ItemGroup> | ||
|
|
||
| <Error Condition="'@(PrunePackageDataToCopyToSource)' != '' And '$(CopyPrunePackageDataToSource)' != 'true'" | ||
| Text="Prune package data not found: %(PrunePackageDataToCopyToSource.RepoSourcePath) |
There was a problem hiding this comment.
The error message uses batched metadata %(PrunePackageDataToCopyToSource.RepoSourcePath), which will only display one item's path if multiple files are missing. Consider using @(PrunePackageDataToCopyToSource->'%(RepoSourcePath)', '%0A') to list all missing files in the error message, making it more helpful when multiple targeting packs are missing.
| Text="Prune package data not found: %(PrunePackageDataToCopyToSource.RepoSourcePath) | |
| Text="Prune package data not found: | |
| @(PrunePackageDataToCopyToSource->'%(RepoSourcePath)', '%0A') |
ViktorHofer
left a comment
ViktorHofer
left a comment
There was a problem hiding this comment.
Looks good so far. Only questions:
Should the existing data for previous versions of .NETCoreApp and .NETStandard also be migrated to the PackageOverrides.txt checked-in format, away from C#?
Can you please also include the files for 10.0? Can be in a follow-up but we need the data for the branding PR.
There's been a lot of manual cleanup of the data from previous versions, so we'd have to use the C# data to generate PackageOverrides.txt data, which I don't think is necessary.
I think it would be better to let this PR flow into the VMR, then in the branding PR we would push a commit that would add the pruning data. That's how I think the process would work each year, so we should try to test it now. Does that make sense? |
|
C# representation should be minimal and faster to load. If anything you might consider source-generating the stuff that isn't expected to change to improve load times. Of course that's not a requirement for this PR. |
The inner loop in the VMR is a bit rough. It will work but it won't be intuitive how to do that with the correct SDK, etc. Let's give it a try. |
|
@ViktorHofer I can certainly add the data to this PR or create a separate one in the SDK repo if necessary. I'm just trying to create a process that will be as simple as possible and won't require us to remember much from year to year to keep it working. To create the data in the SDK repo before it can target .NET 10, I need to temporarily modify the MaxNetVersion logic. I thought it would be simpler to run a build with a special property set during the rebranding process. But let me know what you think or how it goes and we can adjust. |
|
Sounds good. I'm absolutely in favor of giving this a try in the VMR PR. Is it possible to invoke the target that is responsible for copying the content into the project directory manually so that we don't have to do a full build? I assume so. |
|
I assume that you tested this locally? :) |
It should be possible, but you'll have to also run a restore first to download the packages where the files will be copied from.
Yes, I tested the flow where it gives the error message, you build with the property and it copies the files, then when you build again it doesn't error and doesn't download the files. |
Cherrypicked commits 8bb9aac61e53a495eae43b6789acc2aa4fbd5ea4 and efa4308f6a29f7dc41de6747109c30babb991e1c from dotnet/sdk#51992
3 participants
Per discussion in dotnet/dotnet#2271, we don't want to download targeting packs to get prune package data during the SDK build, because that would require adding them to source build externals.
Instead, we will check the prune package data into the SDK after each major release. This PR adds support for that.
src\Layout\redist\PrunePackageData, it will be used from there without being downloaded