Poisoning support for shared components #1613
Conversation
eng/tools/tasks/Microsoft.DotNet.UnifiedBuild.Tasks/UpdateNuGetConfigPackageSourcesMappings.cs
Outdated
Show resolved
Hide resolved
eng/tools/tasks/Microsoft.DotNet.UnifiedBuild.Tasks/UpdateNuGetConfigPackageSourcesMappings.cs
Outdated
Show resolved
Hide resolved
eng/init-poison.proj
Outdated
| redistributed in the SDK. --> | ||
| <MSBuild Projects="$(MSBuildProjectFullPath)" | ||
| Targets="GetFilteredSharedComponentPackages" | ||
| Condition="'$(RepositoryName)' != 'source-build-reference-packages' and '$(RepositoryName)' != 'arcade'" |
There was a problem hiding this comment.
The number of places that are conditioning on SBRP and arcade continues to grow. These repos are the special UseBootStrapArcade repos. The UseBootStrapArcade property is defined in the repo-project and cannot be utilized here. Should we define a common itemGroup that defines these repo names that can be utilized in these places rather than hard coding the repos? I feel like this would help (marginally) the readability of the system.
eng/init-poison.proj
Outdated
| <Error Condition="'@(_FilteredSharedComponentPackages)' == ''" Text="Expected NuGet packages to be present from shared components artifacts." /> | ||
|
|
||
| <ItemGroup> | ||
| <_SharedComponentFilenames Include="@(_FilteredSharedComponentPackages)"> |
There was a problem hiding this comment.
Consider renaming to better indicate the contents - e.g. _SharedToolingComponentFilenames
eng/init-poison.proj
Outdated
| <_SharedComponentFilenames Include="@(_FilteredSharedComponentPackages)"> | ||
| <PackagePath>$(SharedComponentsArtifactsPath)$([System.IO.Path]::GetFileName('%(PipelineArtifactPath)'))</PackagePath> | ||
| </_SharedComponentFilenames> | ||
| <PreviouslySourceBuiltPackages Include="@(_SharedComponentFilenames->'%(PackagePath)')" /> |
There was a problem hiding this comment.
From a UX perspective when tracking down leaks, wouldn't it be more helpful to classify the shared components different than PSB? I feel like that would aid in the speed of tracking down leaks.
There was a problem hiding this comment.
The paths in the catalog show the difference in where they're coming from. But I've also updated to use a separate item group which will be useful when examining the binlog. Fixed in 95e7039.
| <_BuildSources Include="@(_CommonBuildSources)" /> | ||
| <_BuildSources Include="$(NetSdkSupportingFeedName)" | ||
| Condition="'$(AddNetSdkSupportingFeed)' == 'true'" /> | ||
| <_FallbackSources Include="$(PrebuiltNuGetSourceName); |
There was a problem hiding this comment.
Is this going to introduce non-determinism as well as potential poison leaks if the same package version exists in multiple feeds? Don't these sources need to be in priority order?
eng/VmrCommon.targets
Outdated
There was a problem hiding this comment.
So I was asking the other question before cause if no one other than D.B.targets imports this file, I would rather inline the target there then create a new file for which it isn't clear when to put stuff into it vs other files.
IOW I would recommend to move this logic into D.B.targets
Fixes #1112
This adds source build poisoning support for multi-band SDK scenarios in the VMR. In this scenario, runtime-related packages that are provided as input to a non-1xx branch via a shared components tarball are considered to be acceptable for redistribution by the SDK produced from that branch. That is, of course, the whole intention of providing these packages because they are part of the working product. However, the tooling-related packages that comes from the shared components tarball are not considered acceptable to redistribute because the non-1xx branch is the one that is producing the new tooling-related packages. This means that runtime-related packages are not marked for poisoning but the tooling-related packages are.
Notes on the changes:
UpdateNuGetConfigPackageSourcesMappingsso that it can handle an arbitrary number of "fallback" inputs (prebuilts, previously source built, shared components).GetFilteredSharedComponentPackagestarget that can filter the packages from the shared components according to whether they come from runtime- or tooling-related repos. This is shared by both repo-projects (for build input props generation) and poisoning. This is defined inVmrCommon.targets.