xoofx / dotnet-releaser

Easily build, run tests and coverage, cross-compile, package and publish your .NET library or application to NuGet and GitHub.
BSD 2-Clause "Simplified" License
687 stars 24 forks source link

Solution build fails if coverlet.collector is already added to project #59

Open FrediKats opened 4 months ago

FrediKats commented 4 months ago

Actual behavior: dotnet-releaser add coverlet.collector to test projects during build step. If project already has reference to coverlet.collector, build finish with:

error NU1504: Warning As Error: Duplicate 'PackageReference' items found. Remove the duplicate items or use the Update functionality to ensure a consistent  restore behavior. The duplicate 'PackageReference' items are: coverlet.collector , coverlet.collector . 

Expected behavior: dotner-releaser must check coverlet.collector reference before adding it to test projects.

FrediKats commented 4 months ago

@xoofx is this a good idea to remove reference (<PackageReference Remove="$(DotNetReleaserCoveragePackage)" />) before adding in this part of code? I can create PR but I'm not sure about this fix.

  <Choose>
    <When Condition="'$(ManagePackageVersionsCentrally)' == 'true' AND '$(CentralPackageVersionsFileImported)' == 'true' AND '$(DotNetReleaserCoverage)' == 'true' AND '$(IsTestProject)' == 'true'">
      <ItemGroup>
        <PackageReference Include="$(DotNetReleaserCoveragePackage)" VersionOverride="$(DotNetReleaserCoverageVersion)">
          <PrivateAssets>all</PrivateAssets>
          <IncludeAssets>runtime; build; native; contentfiles; analyzers</IncludeAssets>
        </PackageReference>
      </ItemGroup>
    </When>
    <When Condition="'$(DotNetReleaserCoverage)' == 'true' AND '$(IsTestProject)' == 'true'">
      <ItemGroup>
        <PackageReference Include="$(DotNetReleaserCoveragePackage)" Version="$(DotNetReleaserCoverageVersion)">
          <PrivateAssets>all</PrivateAssets>
          <IncludeAssets>runtime; build; native; contentfiles; analyzers</IncludeAssets>
        </PackageReference>
      </ItemGroup>
    </When>
  </Choose>
xoofx commented 4 months ago

@xoofx is this a good idea to remove reference (<PackageReference Remove="$(DotNetReleaserCoveragePackage)" />) before adding in this part of code? I can create PR but I'm not sure about this fix.

Dunno, usually, I prefer to remove manually the package myself from the project so that there are less surprises. Could it that in some cases, it might be tricky to remove it (e.g via transitive references)

FrediKats commented 4 months ago

Transitive references are not a problem. MSBuild can handle this. Moreover, this is common case for MSBuild when package added more than one time via transitive references: "Project A" uses SomePackage, "Project B" uses SomePackageand "Project B" uses "Project A".

MSBuild NU1504 is about treating duplicate references as accident. MSBuild can build successfully with duplicates with DisableCheckingDuplicateNuGetItems is set to true:

  <!--
    ============================================================
    CollectPackageReferences
    Gathers all PackageReference items from the project.
    This target may be used as an extension point to modify
    package references before NuGet reads them.
    ============================================================
  -->
  <Target Name="CollectPackageReferences" Returns="@(PackageReference)" >
    <!-- NOTE for design-time builds we need to ensure that we continue on error. -->
    <PropertyGroup>
      <CollectPackageReferencesContinueOnError>$(ContinueOnError)</CollectPackageReferencesContinueOnError>
      <CollectPackageReferencesContinueOnError Condition="'$(ContinueOnError)' == '' ">false</CollectPackageReferencesContinueOnError>
    </PropertyGroup>

    <CheckForDuplicateNuGetItemsTask
      Condition="'$(DisableCheckingDuplicateNuGetItems)' != 'true' "
      Items="@(PackageReference)"
      ItemName="PackageReference"
      LogCode="NU1504"
      MSBuildProjectFullPath="$(MSBuildProjectFullPath)"
      TreatWarningsAsErrors="$(TreatWarningsAsErrors)"
      WarningsAsErrors="$(WarningsAsErrors)"
      WarningsNotAsErrors="$(WarningsNotAsErrors)"
      NoWarn="$(NoWarn)"
      ContinueOnError="$(CollectPackageReferencesContinueOnError)"
      >
      <Output TaskParameter="DeduplicatedItems" ItemName="DeduplicatedPackageReferences" />
    </CheckForDuplicateNuGetItemsTask>

    <ItemGroup Condition="'@(DeduplicatedPackageReferences)' != ''">
      <PackageReference Remove="@(PackageReference)" />
      <PackageReference Include="@(DeduplicatedPackageReferences)" />
    </ItemGroup>

  </Target>

Now I have 3 ideas for fixing this case: