ghost
commented
1 year ago Tagging subscribers to this area: @dotnet/area-infrastructure-libraries See info in area-owners.md if you want to be subscribed.
Issue Details
Today it is manual process to include the source gen projects into the [transitive package](https://github.com/dotnet/runtime/blob/913a8447ae0cb0300ba91077988dbc0ab40301a7/src/libraries/Microsoft.Internal.Runtime.AspNetCore.Transport/src/Microsoft.Internal.Runtime.AspNetCore.Transport.proj). This issue tracks if we can improve this process. **Viktor Hofer** If the expectation is to include the entire package content, then we can just define a package dependency from Microsoft.DotNet.Runtime.AspNetCore.Transport -> i.e. Microsoft.Extensions.Configuration.Binder. That will cause troubles during servicing though. In servicing, we always create the transport packages for the partner teams but libraries packages like System.Text.Json are only created and published on-demand (when something in them changes). Also, that would mean that we hand-off more assemblies and package assets to partner teams than today. I.e. the aspnetcore team would then also get the netstandard2.x and net462 assemblies, all the other roslyn version targeted source generators (roslyn3.11, roslyn4.0, ...) and other potential assets from our packages. This would enlargen our current contract and I'm not sure if that's what we want. Today, our transport packages are hand-tailored, meaning that they only include what's needed: all the reference assemblies, source assemblies and source generator assemblies (for the current roslyn version). If we want to change that, please file an issue. Note that the current state (including how to add a source generator) is documented here: [runtime/docs/coding-guidelines/libraries-packaging.md at main · dotnet/runtime (github.com)](https://github.com/dotnet/runtime/blob/main/docs/coding-guidelines/libraries-packaging.md#microsoftinternalruntimetargetrepositorynametransport) **Eric St. John** Viktor Hofer I wonder if we could drive the generator project references in the transport package off a list in NetCoreAppLibrary.props instead (EG: NetCoreAppAnalyzer, AspNetCoreAppAnalyzer). Then when we build a package, we could see if that package includes a generator and the library is in NetCoreAppLibrary or AspNetCoreAppLibrary, but the generator is missing from the respective list NetCoreAppAnalyzer or AspNetCoreAppAnalyzer. If missing we could raise a suppressible error. **Viktor Hofer** How would you identify the generators that should be added to the transport package? Most of our source generator aware packages include multiple ones, for different compiler versions. **Eric St. John** Good point, I think it would need to be an "Any" vs "None" check. **Viktor Hofer** I only NuGet would allow a dependency to not be promoted to a package dependency without making it non transitive. That would allow the latest targeting source generator to be a transitive dependency without promoting it as a package dependency... **Eric St. John** We could create our own extension to project-reference protocol to flow transitive source generators. **Eric St. John** In metadata that only we look at. **Viktor Hofer** NuGet calculates transitives and package dependencies based on the project's project.assets.json file. AFAIK it's not possible to adjust that algorithm. **Eric St. John** But don't we already use build targets to compose those transport packages, since we're redistributing project references? **Eric St. John** So it doesn't really matter what NuGet does, all we need to do is change the content we provide from the project reference. **Viktor Hofer** Correct. But the transport package still needs the source generator project dependency. The source generator dependency is marked as PrivateAssets="all" in the source project that distributes it which results in it not being listed in the transport package's project.assets.json. **Viktor Hofer** Your proposal above works. I was saying that it would be nice to not have to list source generators dependency and automatically receive it as transitive of the referenced source project.
| Author: | tarekgh |
|---|---|
| Assignees: | - |
| Labels: | `area-Infrastructure-libraries`, `untriaged` |
| Milestone: | - |
ericstj
ViktorHofer
Today it is manual process to include the source gen projects into the transport package. This issue tracks if we can improve this process.
Viktor Hofer If the expectation is to include the entire package content, then we can just define a package dependency from Microsoft.DotNet.Runtime.AspNetCore.Transport -> i.e. Microsoft.Extensions.Configuration.Binder. That will cause troubles during servicing though. In servicing, we always create the transport packages for the partner teams but libraries packages like System.Text.Json are only created and published on-demand (when something in them changes). Also, that would mean that we hand-off more assemblies and package assets to partner teams than today. I.e. the aspnetcore team would then also get the netstandard2.x and net462 assemblies, all the other roslyn version targeted source generators (roslyn3.11, roslyn4.0, ...) and other potential assets from our packages. This would enlargen our current contract and I'm not sure if that's what we want. Today, our transport packages are hand-tailored, meaning that they only include what's needed: all the reference assemblies, source assemblies and source generator assemblies (for the current roslyn version). If we want to change that, please file an issue. Note that the current state (including how to add a source generator) is documented here: runtime/docs/coding-guidelines/libraries-packaging.md at main · dotnet/runtime (github.com)
Eric St. John Viktor Hofer I wonder if we could drive the generator project references in the transport package off a list in NetCoreAppLibrary.props instead (EG: NetCoreAppAnalyzer, AspNetCoreAppAnalyzer). Then when we build a package, we could see if that package includes a generator and the library is in NetCoreAppLibrary or AspNetCoreAppLibrary, but the generator is missing from the respective list NetCoreAppAnalyzer or AspNetCoreAppAnalyzer. If missing we could raise a suppressible error.
Viktor Hofer How would you identify the generators that should be added to the transport package? Most of our source generator aware packages include multiple ones, for different compiler versions.
Eric St. John Good point, I think it would need to be an "Any" vs "None" check.
Viktor Hofer I only NuGet would allow a dependency to not be promoted to a package dependency without making it non transitive. That would allow the latest targeting source generator to be a transitive dependency without promoting it as a package dependency...
Eric St. John We could create our own extension to project-reference protocol to flow transitive source generators.
Eric St. John In metadata that only we look at.
Viktor Hofer NuGet calculates transitives and package dependencies based on the project's project.assets.json file. AFAIK it's not possible to adjust that algorithm.
Eric St. John But don't we already use build targets to compose those transport packages, since we're redistributing project references?
Eric St. John So it doesn't really matter what NuGet does, all we need to do is change the content we provide from the project reference.
Viktor Hofer Correct. But the transport package still needs the source generator project dependency. The source generator dependency is marked as PrivateAssets="all" in the source project that distributes it which results in it not being listed in the transport package's project.assets.json.
Viktor Hofer Your proposal above works. I was saying that it would be nice to not have to list source generators dependency and automatically receive it as transitive of the referenced source project.