Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

System.ValueTuple and System.ComponentModel.Annotations #183

Open
segor opened this issue Nov 18, 2024 · 9 comments
Open

System.ValueTuple and System.ComponentModel.Annotations #183

segor opened this issue Nov 18, 2024 · 9 comments
Assignees

Comments

@segor
Copy link

segor commented Nov 18, 2024

What's home repository for System.ValueTuple and System.ComponentModel.Annotations?
Can you please consider to migrate them into the repository?
Thanks!

@carlossanlop
Copy link
Member

Hi @segor

Both System.ComponentModel.Annotations and System.ValueTuple nuget packages indeed contain actual implementations inside them, but they are not used on any of the currently supported frameworks. The types in those nuget package are already part of the shared framework of the currently supported frameworks (.NET 8.0+, .NET Framework 4.6.2+).

@segor
Copy link
Author

segor commented Nov 19, 2024

@carlossanlop Let me explain the reason of my question. We do not reference these packages directly in our multitargeted project (.net 4.8 and .net 8), but they are transitive packages and the corresponding assemblies are being copied to the output binaries folder. According to our security compliance all the distributed open source libraries have to be tracked including their license, authors , repository, etc. The code repository has to be up to date and maintained. As for me, these requirements are reasonable.

The types in those nuget package are already part of the shared framework

Do I understand correctly that these assemblies can be excluded from distribution and the app will work without them? It looks like the libraries contain the type forwarding instructions. If so,.where are these instructions located?

Thanks

@ericstj
Copy link
Member

ericstj commented Nov 19, 2024

In both cases the framework contains the assembly. On NET 4.8 the assembly is part of .NETFramework and loaded from the GAC (C:\WINDOWS\Microsoft.Net\assembly\GAC_MSIL\System.ValueTuple\v4.0_4.0.0.0__cc7b13ffcd2ddd51\System.ValueTuple.dll). In .NET 8 the assembly is part of the shared framework and loaded from that directory (EG: C:\Program Files\dotnet\shared\Microsoft.NETCore.App\8.0.11\System.ValueTuple.dll)

You can see this if you try to run the following:
project.csproj

<Project Sdk="Microsoft.NET.Sdk">

  <PropertyGroup>
    <OutputType>Exe</OutputType>
    <TargetFrameworks>net4.8;net8.0</TargetFrameworks>
    <LangVersion>latest</LangVersion>
  </PropertyGroup>

  <ItemGroup>
    <PackageReference Include="System.ValueTuple" Version="4.5.0" />
  </ItemGroup>

</Project>

program.cs

using System;
using System.Reflection;
Console.WriteLine(Assembly.Load("System.ValueTuple").Location);
Console.WriteLine(Type.GetType("System.ValueTuple, System.ValueTuple, Version=4.0.3.0, Culture=neutral, PublicKeyToken=cc7b13ffcd2ddd51").Assembly.Location);

Output

C:\scratch\vt>bin\Debug\net4.8\vt.exe
C:\WINDOWS\Microsoft.Net\assembly\GAC_MSIL\System.ValueTuple\v4.0_4.0.0.0__cc7b13ffcd2ddd51\System.ValueTuple.dll
C:\Windows\Microsoft.NET\Framework64\v4.0.30319\mscorlib.dll

C:\scratch\vt>bin\Debug\net8.0\vt.exe
C:\Program Files\dotnet\shared\Microsoft.NETCore.App\8.0.11\System.ValueTuple.dll
C:\Program Files\dotnet\shared\Microsoft.NETCore.App\8.0.11\System.Private.CoreLib.dll

@segor
Copy link
Author

segor commented Nov 20, 2024

@ericstj thanks for the response and the code snippet. Yes, for the net4.8 it looks like the assemblies System.ValueTuple.dll and System.ComponentModel.Annotations.dll are being copied to the output folder and but they are never loaded from the folder when an application runs.

So I think the issue can be closed but I would add that placing these unnecessary(?) assemblies into the output folder may confuse developers and it would be interesting to know why it happens.

PS
To prove that these assemblies are unnecessary, I removed them from the output folder in the PostBuild event and the application worked as expected. The code is the following.
project.csproj:

<Project Sdk="Microsoft.NET.Sdk">
  <PropertyGroup>
    <OutputType>Exe</OutputType>
    <TargetFrameworks>net4.8;net8.0</TargetFrameworks>
    <LangVersion>latest</LangVersion>
  </PropertyGroup>
  <ItemGroup>
    <PackageReference Include="System.ValueTuple" Version="4.5.0" />
    <PackageReference Include="System.ComponentModel.Annotations" Version="5.0.0" />
  </ItemGroup>
  <Target Name="PostBuild" AfterTargets="PostBuildEvent"> <!--remove the unnecessary assemblies-->
    <Exec Command="del /s System.ValueTuple.dll System.ComponentModel.Annotations.dll" />
  </Target>
</Project>

program.cs:

using System;
Console.WriteLine(typeof(System.ValueTuple).Assembly.Location);
Console.WriteLine(typeof(System.ComponentModel.DataAnnotations.DataTypeAttribute).Assembly.Location);

Output for net4.8:

C:\Windows\Microsoft.NET\Framework64\v4.0.30319\mscorlib.dll
C:\WINDOWS\Microsoft.Net\assembly\GAC_MSIL\System.ComponentModel.DataAnnotations\v4.0_4.0.0.0__31bf3856ad364e35\System.ComponentModel.DataAnnotations.dll

Output for net8.0:

C:\Program Files\dotnet\shared\Microsoft.NETCore.App\8.0.10\System.Private.CoreLib.dll
C:\Program Files\dotnet\shared\Microsoft.NETCore.App\8.0.10\System.ComponentModel.Annotations.dll

@ericstj
Copy link
Member

ericstj commented Nov 20, 2024

You can avoid a target by using ExcludeAssets="Runtime" on the PackageReference

So I think the issue can be closed but I would add that placing these unnecessary(?) assemblies into the output folder may confuse developers and it would be interesting to know why it happens.

They won't be copied on .NET 6+. On .NETFramework the files are copied because the packages have newer versions than the assemblies from the NET4.8 targeting pack, so they don't get dropped, however their assembly versions are still within the range that's unified by the framework's loader, so they are ignored at runtime in favor of the framework copy.

Here's the relevant snippet from the build log:

Target Name=_HandlePackageFileConflicts
ResolvePackageFileConflicts
...
    Encountered conflict between 'Reference:C:\Users\ericstj\.nuget\packages\system.valuetuple\4.5.0\ref\net47\System.ValueTuple.dll' and 'Platform:System.ValueTuple.dll'. Choosing 'Reference:C:\Users\ericstj\.nuget\packages\system.valuetuple\4.5.0\ref\net47\System.ValueTuple.dll' because AssemblyVersion '4.0.3.0' is greater than '4.0.2.0'.
    Encountered conflict between 'Platform:System.ValueTuple.dll' and 'CopyLocal:C:\Users\ericstj\.nuget\packages\system.valuetuple\4.5.0\lib\net47\System.ValueTuple.dll'. Choosing 'CopyLocal:C:\Users\ericstj\.nuget\packages\system.valuetuple\4.5.0\lib\net47\System.ValueTuple.dll' because AssemblyVersion '4.0.3.0' is greater than '4.0.2.0'.

We could ship new copies of these packages, just to make it so that they aren't copied on .NETFramework versions, but we'd prefer folks just stop using the packages altogether since the types are built into the framework and the packages aren't required.

@chrarnoldus
Copy link

Fwiw .NET 4.6.2 is still supported and doesn't have inbox System.ValueTuple.

@segor
Copy link
Author

segor commented Nov 21, 2024

@ericstj
1.

You can avoid a target by using ExcludeAssets="Runtime" on the PackageReference

This option works well if I use it for System.ValueTuple but fails for System.ComponentModel.Annotations with compilation error:

1>C:\...\Program.cs(3,64,3,81): error CS1069: The type name 'DataTypeAttribute' could not be found in the namespace 'System.ComponentModel.DataAnnotations'. This type has been forwarded to assembly 'System.ComponentModel.DataAnnotations, Version=4.0.0.0, Culture=neutral, PublicKeyToken=31bf3856ad364e35' Consider adding a reference to that assembly.

To reproduce it just add ExcludeAssets="Runtime" in the code from my previous comment #183 (comment) .

We could ship new copies of these packages, just to make it so that they aren't copied on .NETFramework versions, but we'd prefer folks just stop using the packages altogether since the types are built into the framework and the packages aren't required.

The problem is that these libraries come mostly from other .NET Runtime libraries . For example, in my project, System.ValueTuple comes from Microsoft.Bcl.TimeProvider (v8.0.0) and System.Text.Json (v8.0.5):

PS C:\...> dotnet nuget why .\xxx.csproj System.ValueTuple
Project 'xxx' has the following dependency graph(s) for 'System.ValueTuple':

  [net48]
   │
   ├─ Polly (v8.2.1)
   │  └─ Polly.Core (v8.2.1)
   │     ├─ Microsoft.Bcl.TimeProvider (v8.0.0)
   │     │  └─ System.ValueTuple (v4.5.0)
   │     └─ System.ValueTuple (v4.5.0)
   ├─ RestSharp.Serializers.NewtonsoftJson (v112.0.0)
   │  └─ RestSharp (v112.0.0)
   │     └─ System.Text.Json (v8.0.5)
   │        └─ System.ValueTuple (v4.5.0)
   └─ System.Text.Json (v8.0.5)
      └─ System.ValueTuple (v4.5.0)

So question is what is better, to request folks just stop using the unnecessary packages in Microsoft.Bcl.TimeProvider (v8.0.0) and System.Text.Json (v8.0.5) and in other .NET runtime libraries or just to release a new version of System.ValueTuple.

@ericstj
Copy link
Member

ericstj commented Nov 21, 2024

To reproduce it just add ExcludeAssets="Runtime" in the code from my previous comment #183 (comment)

To use types from System.ComponentModel.DataAnnotations you need to reference that assembly on .NET Framework. I would not have expected ExcludeAssets="Runtime" to change the behavior of the following references from the ComponentModel.Annotations package:

      <frameworkAssembly assemblyName="System.ComponentModel.DataAnnotations" targetFramework=".NETFramework4.5" />
      <frameworkAssembly assemblyName="System.ComponentModel.DataAnnotations" targetFramework=".NETFramework4.6.1" />

But it does seem to do that. I think that's a NuGet bug. I filed NuGet/Home#13951

So question is what is better, to request folks just stop using the unnecessary packages in Microsoft.Bcl.TimeProvider (v8.0.0) and System.Text.Json (v8.0.5) and in other .NET runtime libraries or just to release a new version of System.ValueTuple.

That's a very good point. We do still need the reference on .NET 4.6.2 for these libraries, there's no alternative. We could have them multi-target their .NETFramework targets to drop the package on 4.7 and later. @ViktorHofer what are your thoughts on this?

@ViktorHofer
Copy link
Member

ViktorHofer commented Nov 21, 2024

Certainly possible but Microsoft.Bcl.* aren't the only packages that transitively bring System.ValueTuple in. I see multiple hits just in the dotnet/runtime repo: https://github.com/search?q=repo%3Adotnet%2Fruntime%20%3CPackageReference%20Include%3D%22System.ValueTuple%22&type=code

Given that .NET Framework 4.6.2 is still in-support and doesn't have System.ValueTuple inbox, I would prefer to add the library to this repository.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants