Add net 6 to optimize dependencies
@Havret this is a nice idea, though we probably want to optimise for the current LTS's only, @thompson-tomo did you want to update this to be only the current LTS versions, i think .net 5 is already out of LTS support.
All done
Thank you for your contribution and for taking the time to submit this PR!
After reviewing the changes, I don’t see a clear benefit to specifically targeting .NET 6 (or .NET 8) in this project. Since the library already targets .NET Standard, it remains fully compatible with .NET 6, .NET 8, and future runtimes. The runtime will utilize the most up-to-date implementations of dependencies when running on newer platforms, so adding a .NET 6 target doesn’t provide additional optimization or compatibility.
If there’s a specific issue or scenario you’re aiming to address with this change, could you please elaborate? Otherwise, I think we can keep the current approach.
Thanks again for your effort and looking forward to your thoughts!
@Havret by adding a newer TFM, you can remove a dependency which reduces your response to time for CVE'S & at the same time you are benefiting from performance improvements in the base library. Hence it is worthwhile to have.
@thompson-tomo The only dependency we could potentially remove (or make conditional with a compiler flag) is System.Threading.Tasks.Dataflow. However, I'm not sure it's worth the overhead of maintaining a specific .NET version for this. We're not using any APIs outside of .NET Standard 2.0.
If there's a need to use APIs available only in newer TFMs, I'm open to adding those alongside .NET Standard. But doing so just for the sake of it doesn’t make much sense, since any performance improvements come from the runtime, not the standard itself.
So for now, I’m not in favor of proceeding with this change.
@Havret I am sorry but you missed the point, System.Threading.Tasks.Dataflow is natively included in the newer dotnet frameworks hence libraries targeting the newer frameworks do not need to take an explicit dependency on it. There is no need for conditional compilation just a condition on the dependency hence no additional maintaince. In fact using packages which have framework dependencies can cause issues/conflicts and increase the time to remediate CVE'S.
In the case here you are by default getting a user to use a library from 7 years ago but with my changes the user if on net 6+ they immediately get the latest version in that sdk hence the performance benefit of using a library compiled for the newer TFM. As such this is a very efficient and common pattern to be followed
Exactly, that's my point. In a .NET 8 project, this library won't use the old System.Threading.Tasks.Dataflow package from NuGet. It should use the version bundled with the framework. Unless I'm completely missing something.
It doesn't do what you say it does without my change, hence why I made it. There has been talk about doing it but there is still open Issues to do it.
You can easily verify this with a quick test:
csproj:
<Project Sdk="Microsoft.NET.Sdk">
<PropertyGroup>
<OutputType>Exe</OutputType>
<TargetFramework>net8.0</TargetFramework>
</PropertyGroup>
<ItemGroup>
<PackageReference Include="Apache.NMS.AMQP" Version="2.3.0" />
</ItemGroup>
</Project>
Program.cs:
using System;
namespace ConsoleApp2;
class Program
{
static void Main(string[] args)
{
Console.WriteLine(typeof(System.Threading.Tasks.Dataflow.DataflowBlock).Assembly.Location);
}
}
Running this prints the path to the shared framework copy of System.Threading.Tasks.Dataflow.dll, e.g.:
/usr/local/share/dotnet/shared/Microsoft.NETCore.App/8.0.0/System.Threading.Tasks.Dataflow.dll
This demonstrates that the type is being resolved from the .NET 8 shared framework, not from a local NuGet package.