sveawebpay / svea-sdk-dotnet

Svea WebPay SDK for .NET
3 stars 3 forks source link

Microsoft.Extension.* and System.Text.Json version constraints #300

Closed csi-lund closed 2 months ago

csi-lund commented 2 months ago

Both a question and an issue

In this commit https://github.com/sveawebpay/svea-sdk-dotnet/commit/02837fffba891df4f152af98e22ea7009196f161 a change was made to scope the System.Text.Json and Microsoft.Extensions.Logging.Abstractions version range to specific TFMs with the upper bound for 6 and 7 excluding version 8+

  <ItemGroup Condition="'$(TargetFramework)' == 'net8.0'">
    <PackageReference Include="Microsoft.Extensions.Logging.Abstractions" Version="[8.0.0,9.0)" />
    <PackageReference Include="System.Text.Json" Version="[8.0.0,9.0)" />
  </ItemGroup>
  <ItemGroup Condition="'$(TargetFramework)' == 'net7.0'">
    <PackageReference Include="Microsoft.Extensions.Logging.Abstractions" Version="[7.0.0,8.0)" />
    <PackageReference Include="System.Text.Json" Version="[7.0.0,8.0)" />
  </ItemGroup>  
  <ItemGroup Condition="'$(TargetFramework)' == 'net6.0'">
    <PackageReference Include="Microsoft.Extensions.Logging.Abstractions" Version="[6.0.0,8.0)" />
    <PackageReference Include="System.Text.Json" Version="[6.0.0,8.0)" />
  </ItemGroup>

Checking these packages in nuget they seem to be backward compatible and do not seem to be coupled to the runtimes other than lower bound, the major LTS version releases, adds additional support for the latest runtime being released, which you have supported this in your constraints above.

https://www.nuget.org/packages/System.Text.Json/8.0.4#supportedframeworks-body-tab https://www.nuget.org/packages/Microsoft.Extensions.Logging.Abstractions/#supportedframeworks-body-tab

image

We have encountered an issue where a package we are consuming is targeting 8.0.0+ version of these and when we are consuming the svea SDK and this other package it will cause a build error due to the exclusive upper bound constraints and the level these packages come in at with dependency resolution. If we work around the issue by explicitly defining the version we want with a parent project, we still will get a warning which we would need to suppress.

This is an undesirable behaviour since we have to document the reasons for this work around. It is hard to know what changes would be for latest version of these before its released and if there are breaking changes but its seems like the upper bound could be overly restrictive, and needs updating to reflect the supposed backward compatibility of these packages, so for instance TFM for dotnet 6 could change from "[6.0.0,8.0)", which currently exclude 8.0.0 from the NET 6 TFM to in this case to '[6.0.0,9.0)' or maybe even '[6.0.0,)' perhaps' to avoid the max constraint altogether.

  <ItemGroup Condition="'$(TargetFramework)' == 'net8.0'">
    <PackageReference Include="Microsoft.Extensions.Logging.Abstractions" Version="[8.0.0,9.0)" />
    <PackageReference Include="System.Text.Json" Version="[8.0.0,9.0)" />
  </ItemGroup>
  <ItemGroup Condition="'$(TargetFramework)' == 'net7.0'">
    <PackageReference Include="Microsoft.Extensions.Logging.Abstractions" Version="[7.0.0,9.0)'" />
    <PackageReference Include="System.Text.Json" Version="[7.0.0,9.0)" />
  </ItemGroup>  
  <ItemGroup Condition="'$(TargetFramework)' == 'net6.0'">
    <PackageReference Include="Microsoft.Extensions.Logging.Abstractions" Version="[6.0.0,9.0)'" />
    <PackageReference Include="System.Text.Json" Version="[6.0.0,9.0)" />
  </ItemGroup>

That is unless you encountered a specific issue that required this restriction, or its just and abundance of caution?

dusanbobicic commented 2 months ago

Hey, thanks for reporting the issue. Yes, we could remove this upper constraint.