Sharp edges when `CentralPackageTransitivePinningEnabled=true` in source generator projects
austindrenski opened this issue · 2 comments
This isn't necessarily a bug, but it had me stumped all morning, so wanted to flag it for future readers, and maybe for the Roslyn team to consider a diagnostic warning as part of EnforceExtendedAnalyzerRules.
Basically, turning on CentralPackageTransitivePinningEnabled caused our source generators to fail in Rider and Visual Studio (e.g. unable to find System.Collections.Immutable), but notably they did not fail when running dotnet build.
The source generators in question are bare bones and only depend on Microsoft.CodeAnalysis.CSharp so it took a while to realize that CentralPackageTransitivePinningEnabled was causing our source generator package to include a reference (but not as a dependency!) to the RC version of System.Collections.Immutable instead of the compiler-and-IDE-safe reference that usually flows from Microsoft.CodeAnalysis.CSharp.
So again, not a bug since this is exactly what CentralPackageTransitivePinningEnabled is supposed to do, but was a confusing triage given how source generator failures are displayed in the IDEs.
What might be nice is a meta-analyzer to warn you when a source generator project has CentralPackageTransitivePinningEnabled=true. In a perfect world, this meta-analyzer would only warn when CentralPackageTransitivePinningEnabled=true and one of the project's transitive dependencies was actually being pinned, but not sure if the analyzer would have sufficient context for that.
// global.json
{
"sdk": {
"version": "9.0.100-rc.2.24474.11"
}
}<!-- Directory.Packages.props -->
<Project>
<PropertyGroup>
<CentralPackageTransitivePinningEnabled>true</CentralPackageTransitivePinningEnabled>
<ManagePackageVersionsCentrally>true</ManagePackageVersionsCentrally>
</PropertyGroup>
<ItemGroup>
<!-- included for source generator project -->
<PackageVersion Include="Microsoft.CodeAnalysis.CSharp" Version="4.11.0" />
<!-- included for normal project -->
<PackageVersion Include="System.Collections.Immutable" Version="9.0.0-rc.2.24473.5" />
</ItemGroup>
</Project><!-- SomeSourceGeneratorProject.csproj -->
<Project Sdk="Microsoft.NET.Sdk">
<PropertyGroup>
<EnforceExtendedAnalyzerRules>true</EnforceExtendedAnalyzerRules>
<IncludeBuildOutput>false</IncludeBuildOutput>
<IncludeSymbols>false</IncludeSymbols>
<NoPackageAnalysis>true</NoPackageAnalysis>
<TargetFramework>netstandard2.0</TargetFramework>
</PropertyGroup>
<ItemGroup>
<None Include="_._" Pack="true" PackagePath="lib/$(TargetFramework)" Visible="false" />
<None Include="$(OutputPath)$(AssemblyName).dll" Pack="true" PackagePath="analyzers/dotnet/cs" Visible="false" />
</ItemGroup>
<ItemGroup>
<PackageReference Include="Microsoft.CodeAnalysis.CSharp" PrivateAssets="all" />
</ItemGroup>
</Project>
edit: including our work-around below:
<!-- SomeSourceGeneratorProject.csproj --> <Project Sdk="Microsoft.NET.Sdk"> <PropertyGroup> + <CentralPackageTransitivePinningEnabled>false</CentralPackageTransitivePinningEnabled> <EnforceExtendedAnalyzerRules>true</EnforceExtendedAnalyzerRules> <IncludeBuildOutput>false</IncludeBuildOutput> <IncludeSymbols>false</IncludeSymbols> <NoPackageAnalysis>true</NoPackageAnalysis> <TargetFramework>netstandard2.0</TargetFramework> </PropertyGroup> <ItemGroup> <None Include="_._" Pack="true" PackagePath="lib/$(TargetFramework)" Visible="false" /> <None Include="$(OutputPath)$(AssemblyName).dll" Pack="true" PackagePath="analyzers/dotnet/cs" Visible="false" /> </ItemGroup> <ItemGroup> <PackageReference Include="Microsoft.CodeAnalysis.CSharp" PrivateAssets="all" /> </ItemGroup> </Project>
edit: including a (perhaps) better work-around based on the discussion below:
<!-- Directory.Packages.props --> <Project> <PropertyGroup> <CentralPackageTransitivePinningEnabled>true</CentralPackageTransitivePinningEnabled> <ManagePackageVersionsCentrally>true</ManagePackageVersionsCentrally> </PropertyGroup> <ItemGroup> <!-- included for source generator project --> <PackageVersion Include="Microsoft.CodeAnalysis.CSharp" Version="4.11.0" /> <!-- included for normal project --> - <PackageVersion Include="System.Collections.Immutable" Version="9.0.0-rc.2.24473.5" /> + <PackageVersion Include="System.Collections.Immutable" Version="9.0.0-rc.2.24473.5" GeneratePackagePath="true" /> </ItemGroup> </Project> <!-- SomeSourceGeneratorProject.csproj --> <Project Sdk="Microsoft.NET.Sdk"> <PropertyGroup> <EnforceExtendedAnalyzerRules>true</EnforceExtendedAnalyzerRules> <IncludeBuildOutput>false</IncludeBuildOutput> <IncludeSymbols>false</IncludeSymbols> <NoPackageAnalysis>true</NoPackageAnalysis> <TargetFramework>netstandard2.0</TargetFramework> </PropertyGroup> <ItemGroup> <None Include="_._" Pack="true" PackagePath="lib/$(TargetFramework)" Visible="false" /> <None Include="$(OutputPath)$(AssemblyName).dll" Pack="true" PackagePath="analyzers/dotnet/cs" Visible="false" /> + <None Include="$(PkgSystem_Collections_Immutable)/lib/$(TargetFramework)/*.dll" Pack="true" PackagePath="analyzers/dotnet/cs" Visible="false" /> </ItemGroup> <ItemGroup> <PackageReference Include="Microsoft.CodeAnalysis.CSharp" PrivateAssets="all" /> </ItemGroup> </Project>
edit: including a target that can be dropped into the csproj and does this in a less icky way
<Target Name="_IncludeAnalyzerDependencies" AfterTargets="GetTargetPath"> <ItemGroup> <_AnalyzerDependency Include="@(ReferenceCopyLocalPaths)" Condition="'%(Extension)' == '.dll'" /> <_AnalyzerDependency Include="@(RuntimeCopyLocalItems)" Exclude="@(ReferencesFromSDK)" /> <_AnalyzerDependency Include="@(TargetPathWithTargetPlatformMoniker)" /> </ItemGroup> <ItemGroup> <None Include="@(_AnalyzerDependency)" Pack="true" PackagePath="analyzers/dotnet/cs" Visible="false" /> </ItemGroup> </Target><!-- Directory.Packages.props --> <Project> <PropertyGroup> <CentralPackageTransitivePinningEnabled>true</CentralPackageTransitivePinningEnabled> <ManagePackageVersionsCentrally>true</ManagePackageVersionsCentrally> </PropertyGroup> <ItemGroup> <!-- included for source generator project --> <PackageVersion Include="Microsoft.CodeAnalysis.CSharp" Version="4.11.0" /> <!-- included for normal project --> <PackageVersion Include="System.Collections.Immutable" Version="9.0.0-rc.2.24473.5" /> </ItemGroup> </Project> <!-- SomeSourceGeneratorProject.csproj --> <Project Sdk="Microsoft.NET.Sdk"> <PropertyGroup> <EnforceExtendedAnalyzerRules>true</EnforceExtendedAnalyzerRules> <IncludeBuildOutput>false</IncludeBuildOutput> <IncludeSymbols>false</IncludeSymbols> <NoPackageAnalysis>true</NoPackageAnalysis> <TargetFramework>netstandard2.0</TargetFramework> </PropertyGroup> <ItemGroup> <None Include="_._" Pack="true" PackagePath="lib/$(TargetFramework)" Visible="false" /> - <None Include="$(OutputPath)$(AssemblyName).dll" Pack="true" PackagePath="analyzers/dotnet/cs" Visible="false" /> </ItemGroup> <ItemGroup> <PackageReference Include="Microsoft.CodeAnalysis.CSharp" PrivateAssets="all" /> </ItemGroup> + + <Target Name="_IncludeAnalyzerDependencies" AfterTargets="GetTargetPath"> + + <ItemGroup> + <_AnalyzerDependency Include="@(ReferenceCopyLocalPaths)" Condition="'%(Extension)' == '.dll'" /> + <_AnalyzerDependency Include="@(RuntimeCopyLocalItems)" Exclude="@(ReferencesFromSDK)" /> + <_AnalyzerDependency Include="@(TargetPathWithTargetPlatformMoniker)" /> + </ItemGroup> + + <ItemGroup> + <None Include="@(_AnalyzerDependency)" Pack="true" PackagePath="analyzers/dotnet/cs" Visible="false" /> + </ItemGroup> + + </Target> </Project>
edit: continuing to update this for future readers...
<!-- Directory.Build.targets --> <Project> <PropertyGroup Condition="'$(IsAnalyzer)' == 'true'"> <!-- TODO: https://github.com/dotnet/roslyn-analyzers/issues/7460 --> <CentralPackageTransitivePinningEnabled>false</CentralPackageTransitivePinningEnabled> <EnforceExtendedAnalyzerRules>true</EnforceExtendedAnalyzerRules> <GetTargetPathDependsOn>$(GetTargetPathDependsOn);_IncludeAnalyzerDependencies</GetTargetPathDependsOn> <IncludeBuildOutput>false</IncludeBuildOutput> <IncludeSymbols>false</IncludeSymbols> <NoPackageAnalysis>true</NoPackageAnalysis> </PropertyGroup> <!-- ============================================================ _IncludeAnalyzerDependencies ============================================================ Include analyzer runtime dependencies that are not provided or otherwise resolvable by Roslyn for design-time builds. ============================================================ --> <Target Name="_IncludeAnalyzerDependencies" Returns="@(_AnalyzerDependency)"> <ItemGroup> <_RoslynDependency Include="@(RuntimeCopyLocalItems)" Condition="$([System.Text.RegularExpressions.Regex]::IsMatch(%(Identity), '^.*[/\\]Microsoft\.CodeAnalysis(\.[^./\\]+)*.dll$'))" /> </ItemGroup> <ItemGroup> <_AnalyzerDependency Include="@(RuntimeCopyLocalItems)" Exclude="@(_RoslynDependency)" /> <_AnalyzerDependency Include="@(ReferencePath)" Condition="'%(ReferencePath.ReferenceSourceTarget)' == 'ProjectReference'" /> </ItemGroup> <ItemGroup> <None Include="@(_AnalyzerDependency);$(TargetPath)" Pack="true" PackagePath="analyzers/dotnet/cs" Visible="false" /> </ItemGroup> <ItemGroup> <TargetPathWithTargetPlatformMoniker Include="@(_AnalyzerDependency)" IncludeRuntimeDependency="false" ResolveableAssembly="false" /> </ItemGroup> </Target> </Project>
What might be nice is a meta-analyzer to warn you when a source generator project has CentralPackageTransitivePinningEnabled=true. I
This is a very unlikely course for us to be taking. The guidance for .NET is pushing the ecosystem into using CPM and transitive pinning because it a much easier system for managing dependencies. There is nothing special about source generators that would lead us to giving different guidance here.
What might be nice is a meta-analyzer to warn you when a source generator project has CentralPackageTransitivePinningEnabled=true
The root problem here is that your analyzer / generator ended up taking a dependency on a version of System.Collections.Immutable that was higher than what the compiler used. I can see us wanting thinking about a warning for that as it does represent a likely issue in your build.
What might be nice is a meta-analyzer to warn you when a source generator project has CentralPackageTransitivePinningEnabled=true. I
This is a very unlikely course for us to be taking. The guidance for .NET is pushing the ecosystem into using CPM and transitive pinning because it a much easier system for managing dependencies. There is nothing special about source generators that would lead us to giving different guidance here.
Of course! Just to clarify, I didn't mean to suggest a warning about CPM/CPTPE, but more so that due to a CPTPE-induced dependency version change, an additional artifact needs to be packed into analyzers/dotnet/cs.
For the time-being, I've just slapped CentralPackageTransitivePinningEnabled=false into the affected projects (i.e. overriding the default from Directory.Packages.props), which does the job, but feels like a sledgehammer of a solution.
So the alternative is that I could pack the bumped dependencies into my NuGet package, but there's not much guidance on how that might affect things for consumers.
I haven't seen much about this in the analyzer/generator design docs, but I assume there's a reason they don't recommend shipping analyzer/generator packages fully self-contained with all in analyzers/dotnet/cs. For example, if all of the analyzers/generators I ship now pack their own copy of System.Collections.Immutable v9.0.0-rc.2, will devs consuming these feel the impact in their IDEs?¹
What might be nice is a meta-analyzer to warn you when a source generator project has CentralPackageTransitivePinningEnabled=true
The root problem here is that your analyzer / generator ended up taking a dependency on a version of System.Collections.Immutable that was higher than what the compiler used. I can see us wanting thinking about a warning for that as it does represent a likely issue in your build.
+1
¹(System.Collections.Immutable is perhaps a poor example here, since this particular issue goes away once .NET 9 GA ships next month. But all the same, would still be curious to hear your thoughts on this.)