roslyn-sdk icon indicating copy to clipboard operation
roslyn-sdk copied to clipboard

Fix Microsoft.CodeAnalysis.Analyzer.Testing nuget package dependencies for downstream clients

Open jmarolf opened this issue 3 years ago • 1 comments

In https://github.com/dotnet/roslyn-sdk/pull/1004 we pinned the version of Newtonsoft.Json to be 13.0.1 because older versions have security problems.

However, this meant that the nuget packages now also required consumers to only use that version of Newtonsoft.Json. This breaks downstream test runners who rely on older versions of this assembly.

This change make the nuspec look like this

Microsoft.CodeAnalysis.Analyzer.Testing.nuspec:

<package xmlns="http://schemas.microsoft.com/packaging/2013/05/nuspec.xsd">
  <metadata>
    <dependencies>
      <group targetFramework=".NETFramework4.7.2">
        <dependency id="DiffPlex" version="1.5.0" include="Runtime,Build,Native,ContentFiles,Analyzers,BuildTransitive" />
        <dependency id="Microsoft.CodeAnalysis.Analyzers" version="2.6.1" exclude="Build,Analyzers" />
        <dependency id="Microsoft.CodeAnalysis.Workspaces.Common" version="1.0.1" exclude="Build,Analyzers" />
        <dependency id="Microsoft.VisualBasic" version="10.0.1" exclude="Build,Analyzers" />
        <dependency id="Microsoft.VisualStudio.Composition" version="16.1.8" exclude="Build,Analyzers" />
-       <dependency id="Newtonsoft.Json" version="13.0.1" exclude="Build,Analyzers" />
        <dependency id="NuGet.Common" version="5.6.0" include="Runtime,Build,Native,ContentFiles,Analyzers,BuildTransitive" />
        <dependency id="NuGet.Packaging" version="5.6.0" include="Runtime,Build,Native,ContentFiles,Analyzers,BuildTransitive" />
        <dependency id="NuGet.Protocol" version="5.6.0" include="Runtime,Build,Native,ContentFiles,Analyzers,BuildTransitive" />
        <dependency id="NuGet.Resolver" version="5.6.0" include="Runtime,Build,Native,ContentFiles,Analyzers,BuildTransitive" />
        <dependency id="System.ValueTuple" version="4.5.0" exclude="Build,Analyzers" />
      </group>
    </dependencies>
  </metadata>
</package>

by adding ExcludeAssets="all" we can ensure that this does not appear in the package dependencies list.

  <!-- Needed to override the transitive 9.0.1 version brought in by the 16.1.1 Microsoft.NET.Test.Sdk -->
  <ItemGroup>
-    <PackageReference Include="Newtonsoft.Json" Version="$(NewtonsoftJsonVersion)" />
+    <PackageReference Include="Newtonsoft.Json" Version="$(NewtonsoftJsonVersion)" ExcludeAssets="all" />
  </ItemGroup>

which should ensure that we comply with security standards while also allowing runners to use whatever version of Newtonsoft.Json they require.

jmarolf avatar Oct 06 '22 20:10 jmarolf

can confirm this fixes running tests in visual studio:

image

jmarolf avatar Oct 09 '22 22:10 jmarolf

@jmarolf Will we have updated packages on NuGet.org soon?

Youssef1313 avatar Oct 11 '22 18:10 Youssef1313

@jmarolf Will we have updated packages on NuGet.org soon?

We haven't release 1.1.2 yet the packages on nuget are 1.1.1 and should not have this issue are you observing the same problem for 1.1.1?

jmarolf avatar Oct 11 '22 18:10 jmarolf

Ah sorry. It was a bad assumption on my end. I tested 1.1.1 and it works.

Youssef1313 avatar Oct 11 '22 18:10 Youssef1313