[release/6.0] Add a target to force packages which reference serviced packages to also be serviced

[ericstj]

Putting this out as a suggestion and demonstrating how it would work.

This would make it so that any other packages in dotnet/runtime would also ship whenever one of their dependencies ship.

[ghost]

Tagging subscribers to this area: @dotnet/area-infrastructure-libraries
See info in area-owners.md if you want to be subscribed.

Issue Details

---

Putting this out as a suggestion and demonstrating how it would work.

This would make it so that any other packages in dotnet/runtime would also ship whenever one of their dependencies ship.

Author:	ericstj
Assignees:	ericstj
Labels:	area-Infrastructure-libraries
Milestone:	-

[carlossanlop]

@ericstj for my own education, would you mind sharing some more context?

[ericstj]

I'm carrying on an internal discussion around our incremental servicing policies and this is a prototype of one of the options (2). Other options are what 1) we do today (single package when it changes) and 3) what up-stack repos do (every package always).

[ericstj]

We could even do something similar to this for telling people to set GeneratePackageOnBuild in the first place. If we could see the files they touched in their PR, intersect those with the input to the compiler, and if any were found error we'd have PR validation for the case where folks forgot to set GeneratePackageOnBuild.

[joperezr]

If we come to place where we are able to detect that, (and thinking a bit outside of the box here) would it be possible to get rid of GeneratePackageOnBuild all together and calculate it on the fly? This would reduce mistakes on servicing PRs of only editing the code but forgetting to set the property, and I'm thinking we would have some sort of checked in file that gets modified by the build (similar to what we used to have with packageIndex.json) where we would have the build add entries there of packages that have been edited for this servicing release, and then branding PRs would only go and empty that file to go back to not generate any package.

[ericstj]

Here's an example of the errors we'll see when all packages in a dependency chain aren't shipped:
https://dev.azure.com/dnceng/public/_build/results?buildId=1641172&view=logs&j=833f538a-ed58-511b-3c52-f2ba7b5799bd&t=416c8fb5-e98c-5538-8919-f35da5198b2e&l=1904

/__w/1/s/eng/packaging.targets(301,7): error : This project did not set GeneratePackageOnBuild, but dependencies '/__w/1/s/src/libraries/Microsoft.Extensions.Configuration/src/Microsoft.Extensions.Configuration.csproj' did.  Please ship this project by setting GeneratePackageOnBuild to true. [/__w/1/s/src/libraries/Microsoft.Extensions.Configuration.FileExtensions/src/Microsoft.Extensions.Configuration.FileExtensions.csproj]
/__w/1/s/eng/packaging.targets(301,7): error : This project did not set GeneratePackageOnBuild, but dependencies '/__w/1/s/src/libraries/Microsoft.Extensions.Logging.Abstractions/src/Microsoft.Extensions.Logging.Abstractions.csproj' did.  Please ship this project by setting GeneratePackageOnBuild to true. [/__w/1/s/src/libraries/Microsoft.Extensions.Logging/src/Microsoft.Extensions.Logging.csproj]
/__w/1/s/eng/packaging.targets(301,7): error : This project did not set GeneratePackageOnBuild, but dependencies '/__w/1/s/src/libraries/Microsoft.Extensions.Logging.Abstractions/src/Microsoft.Extensions.Logging.Abstractions.csproj' did.  Please ship this project by setting GeneratePackageOnBuild to true. [/__w/1/s/src/libraries/Microsoft.Extensions.Caching.Memory/src/Microsoft.Extensions.Caching.Memory.csproj]
/__w/1/s/eng/packaging.targets(301,7): error : This project did not set GeneratePackageOnBuild, but dependencies '/__w/1/s/src/libraries/Microsoft.Extensions.Configuration/src/Microsoft.Extensions.Configuration.csproj' did.  Please ship this project by setting GeneratePackageOnBuild to true. [/__w/1/s/src/libraries/Microsoft.Extensions.Configuration.CommandLine/src/Microsoft.Extensions.Configuration.CommandLine.csproj]
/__w/1/s/eng/packaging.targets(301,7): error : This project did not set GeneratePackageOnBuild, but dependencies '/__w/1/s/src/libraries/Microsoft.Extensions.Configuration/src/Microsoft.Extensions.Configuration.csproj' did.  Please ship this project by setting GeneratePackageOnBuild to true. [/__w/1/s/src/libraries/Microsoft.Extensions.Configuration.EnvironmentVariables/src/Microsoft.Extensions.Configuration.EnvironmentVariables.csproj]
/__w/1/s/eng/packaging.targets(301,7): error : This project did not set GeneratePackageOnBuild, but dependencies '/__w/1/s/src/libraries/Microsoft.Extensions.Configuration/src/Microsoft.Extensions.Configuration.csproj' did.  Please ship this project by setting GeneratePackageOnBuild to true. [/__w/1/s/src/libraries/Microsoft.Extensions.Configuration.EnvironmentVariables/src/Microsoft.Extensions.Configuration.EnvironmentVariables.csproj]
/__w/1/s/eng/packaging.targets(301,7): error : This project did not set GeneratePackageOnBuild, but dependencies '/__w/1/s/src/libraries/Microsoft.Extensions.Configuration/src/Microsoft.Extensions.Configuration.csproj' did.  Please ship this project by setting GeneratePackageOnBuild to true. [/__w/1/s/src/libraries/Microsoft.Extensions.Configuration.FileExtensions/src/Microsoft.Extensions.Configuration.FileExtensions.csproj]
/__w/1/s/eng/packaging.targets(301,7): error : This project did not set GeneratePackageOnBuild, but dependencies '/__w/1/s/src/libraries/System.Reflection.Metadata/src/System.Reflection.Metadata.csproj' did.  Please ship this project by setting GeneratePackageOnBuild to true. [/__w/1/s/src/libraries/System.Reflection.MetadataLoadContext/src/System.Reflection.MetadataLoadContext.csproj]
/__w/1/s/eng/packaging.targets(301,7): error : This project did not set GeneratePackageOnBuild, but dependencies '/__w/1/s/src/libraries/System.Reflection.Metadata/src/System.Reflection.Metadata.csproj' did.  Please ship this project by setting GeneratePackageOnBuild to true. [/__w/1/s/src/libraries/System.Reflection.MetadataLoadContext/src/System.Reflection.MetadataLoadContext.csproj]
/__w/1/s/eng/packaging.targets(301,7): error : This project did not set GeneratePackageOnBuild, but dependencies '/__w/1/s/src/libraries/System.Reflection.Metadata/src/System.Reflection.Metadata.csproj' did.  Please ship this project by setting GeneratePackageOnBuild to true. [/__w/1/s/src/libraries/System.Reflection.MetadataLoadContext/src/System.Reflection.MetadataLoadContext.csproj]
/__w/1/s/eng/packaging.targets(301,7): error : This project did not set GeneratePackageOnBuild, but dependencies '/__w/1/s/src/libraries/Microsoft.Extensions.Configuration/src/Microsoft.Extensions.Configuration.csproj' did.  Please ship this project by setting GeneratePackageOnBuild to true. [/__w/1/s/src/libraries/Microsoft.Extensions.Configuration.CommandLine/src/Microsoft.Extensions.Configuration.CommandLine.csproj]
/__w/1/s/eng/packaging.targets(301,7): error : This project did not set GeneratePackageOnBuild, but dependencies '/__w/1/s/src/libraries/Microsoft.Extensions.Logging.Abstractions/src/Microsoft.Extensions.Logging.Abstractions.csproj' did.  Please ship this project by setting GeneratePackageOnBuild to true. [/__w/1/s/src/libraries/Microsoft.Extensions.Logging/src/Microsoft.Extensions.Logging.csproj]
/__w/1/s/eng/packaging.targets(301,7): error : This project did not set GeneratePackageOnBuild, but dependencies '/__w/1/s/src/libraries/Microsoft.Extensions.Logging.Abstractions/src/Microsoft.Extensions.Logging.Abstractions.csproj' did.  Please ship this project by setting GeneratePackageOnBuild to true. [/__w/1/s/src/libraries/Microsoft.Extensions.Caching.Memory/src/Microsoft.Extensions.Caching.Memory.csproj]
/__w/1/s/eng/packaging.targets(301,7): error : This project did not set GeneratePackageOnBuild, but dependencies '/__w/1/s/src/libraries/Microsoft.Extensions.Logging.Abstractions/src/Microsoft.Extensions.Logging.Abstractions.csproj' did.  Please ship this project by setting GeneratePackageOnBuild to true. [/__w/1/s/src/libraries/Microsoft.Extensions.Logging/src/Microsoft.Extensions.Logging.csproj]
/__w/1/s/eng/packaging.targets(301,7): error : This project did not set GeneratePackageOnBuild, but dependencies '/__w/1/s/src/libraries/System.Reflection.Metadata/src/System.Reflection.Metadata.csproj' did.  Please ship this project by setting GeneratePackageOnBuild to true. [/__w/1/s/src/libraries/System.Reflection.MetadataLoadContext/src/System.Reflection.MetadataLoadContext.csproj]

Probably I'll include the project name for the actual project and reduce the dependency down to a project name to make it more readable. Also provide a link to the servicing MD and describe how to address the error.

[ViktorHofer]

This file is only imported when a project sets IsPackable true.

Suggested change

	Condition="'$(PreReleaseVersionLabel)' == 'servicing' and '$(IsPackable)' == 'true'"
	Condition="'$(PreReleaseVersionLabel)' == 'servicing'"

[ViktorHofer]

Consider refining this target's name as the ValidateServicingVersionIsPropertlySet target also performs servicing validation. Alternatively consider merging both targets together.

[ViktorHofer]

Source build doesn't do incremental servicing, it doesn't make sense to run this target for it.

[ViktorHofer]

Suggested change

	Text="This project did not set GeneratePackageOnBuild, but dependencies '%(ReferencePath.OriginalProjectReferenceItemSpec)' did. Please ship this project by setting GeneratePackageOnBuild to true." />
	Text="This project did not set GeneratePackageOnBuild, but dependencies '%(ReferencePath.OriginalProjectReferenceItemSpec)' did. Please ship this project by setting GeneratePackageOnBuild to true and increment the `ServicingVersion` property." />

[ViktorHofer]

The additional validation does make sense to me. Please let me know when you want another review round from me as it sounds like you are considering hooking the validation onto another extension point (compile inputs).

[joperezr]

Probably I'll include the project name for the actual project and reduce the dependency down to a project name to make it more readable. Also provide a link to the servicing MD and describe how to address the error.

That sounds good to me, one error per project missing the property that shows all of the upstack packages that are bieng built which are causing the warning makes sense. As a bonus, it would also be great if we provide an additional target that automatically adds the property to all projects that need it for you, and for us to include the instructions on how to run this target in the error message.

[bartonjs]

OK... so I need to service S.S.C.Pkcs. We mark that for publication, build, and the build fails because S.S.C.Xml depends on S.S.C.Pkcs, and S.S.C.Xml wasn't marked for servicing. So it's now serviced merely to convey the higher dependency. OK, that's an approach. But now we hit the repository boundary, and the build clears.

There are downstream repositories (winforms and aspnet) that both ingest S.S.C.Pkcs and/or S.S.C.Xml. Are we designing a solution for them for how to understand that they want to publish a new version to merely talk about their dependency bump?

(This specific example is perhaps moot since both of these runtimes pull the NuGet packages into their shared runtimes... but let's assume a pure OOB in aspnet references a pure OOB from runtime... what's our story there?)

[joperezr]

For the downstream repos case, they should use Arcade's dependency flow using darc based on subscriptions which automatically creates PRs for them every time we service a package that they consume. This happens already for repos that use darc dependency flow (which for your particular example, both winforms and aspnet do use dependency flow). What this PR is meant to address is the inter-repo dependencies which obviously won't be using dependency flow, in order to make sure we catch those cases as well.

Of course this means that downstream repos will only get a PR, which won't necessarily break their build if they don't service themselves, but this should be controlled by the downstream repo itself. The problem with the inter-repo, is that because there is no automatic PR from dependency flow, it is hard to notice that you probably want to service a specific package given your dependency is being serviced.

[ghost]

Draft Pull Request was automatically closed for inactivity. Please let us know if you'd like to reopen it.