grpc / grpc

The C based gRPC (C++, Python, Ruby, Objective-C, PHP, C#)
https://grpc.io
Apache License 2.0
41.77k stars 10.52k forks source link

[C#] grpc_csharp_ext.x86.dll and grpc_csharp_ext.x64.dll not copied to output in netstandard2.0 #25285

Closed minglunli closed 3 years ago

minglunli commented 3 years ago

Is your feature request related to a problem? Please describe.

A clear and concise description of what the problem is. Ex. I'm always frustrated when [...]

This request is specifically for the Grpc.Core package on nuget: https://www.nuget.org/packages/Grpc.Core/2.35.0

We recently transitioned to netstandard2.0 and encountered errors because grpc_csharp_ext.x86.dll and grpc_csharp_ext.x64.dll were not found in output directory. After a bit of investigating, we found that the build step to copy these files only exists in net45 and not netstandard2.0 (Or any other versions). We are currently doing it manually to ensure our projects actually work, but this feels like a bug as in the net45 targets file it explicitly states that users should only do it manually if they really know what they are doing and in netstandard2.0 they have to do it themselves. It is also undesirable as it could cause further issues if the file structure changes in a future update, etc.

Are there any plans to update the build steps for the nuget packages?

Describe the solution you'd like

A clear and concise description of what you want to happen.

For Grpc.Core to add the same copy to output directory build step for netstandard2.0 as well

Describe alternatives you've considered

A clear and concise description of any alternative solutions or features you've considered.

We are currently doing the output manually but this is undesirable for many reasons. Including future directory tree modification could cause the files to not be found.

Additional context

Add any other context about the feature request here.

image

dfederm commented 3 years ago

It looks like the problem here is that build logic is not transitive across project references.

So if Proj1 depends on Proj2 depends on Grpc.Core, Proj1 does not import Gprc.Core.targets.

The reason this works when both projects use .NET Framework is the Proj2 would import Grpc.Core.target which adds Content items. Content items are transitive for projects, so Proj1 does not import Grpc.Core.targets, but does transitively copy Proj2's content.

When Proj2 moves to netstandard2.0 however, it no longer imports Grpc.Core.targets and so no longer has the Content items for Proj1 to inherit.

My suggestion would either be to move Gprc.Core.targets to the root og the /build dir (so it applies to netstandard/netcoreapp/net5 projects), or move it under /buildTransitive instead of /build so that Grpc.Core.targets would be imported in Proj1 (see NuGet spec). Or maybe a combination of both since you presumably want to support netcoreapp/net5.

jtattermusch commented 3 years ago

The Grpc.Core.targets file is only needed when running on .NET Framework ("netXYZ" targets). .NET Core support the concept of native libraries automatically so it doesn't require any special handling for them to get copied.

Can you provide a ready to run reproduction (e.g. as a github repository)? At this point it's not really clear how your project is setup (which library targets what and what are its dependencies) and if there's anything wrong about this.

dfederm commented 3 years ago

@jtattermusch You're right about netcore due to the runtimes dir, although that does require a publish, which is reasonable.

In that case my suggestion is simply to move /build to /buildTransitive.

Here's a simple repro

In Library\Library.csproj:

<Project Sdk="Microsoft.NET.Sdk">
  <PropertyGroup>
    <TargetFramework>netstandard2.0</TargetFramework>
  </PropertyGroup>
  <ItemGroup>
    <PackageReference Include="Grpc.Core" Version="*" />
  </ItemGroup>
</Project>

In ConsoleApp\ConsoleApp.csproj:

<Project Sdk="Microsoft.NET.Sdk">
  <PropertyGroup>
    <OutputType>Exe</OutputType>
    <TargetFramework>net472</TargetFramework>
  </PropertyGroup>
  <ItemGroup>
    <ProjectReference Include="..\Library\Library.csproj" />
  </ItemGroup>
</Project>

Upon building ConsoleApp, you will see that the native binaries are not present in the output folder. This is because build logic from packages is not transitive across projects. IE, Library is eligible to import Grpc.Core's targets (although doesn't because it's in the net45 dir), while ConsoleApp is not.

If you change Library's TFM to net472, it works, but not in the way you'd expect. In that case, ConsoleApp still doesn't import Grpc.Core.targets, but the files get copied due to Library importing it and having Content items, and Content items are transitive.

I just tested it locally and if you literally just rename the build dir to buildTransitive, it works as expected (ConsoleApp imports Grpc.Core.targets directly).

jtattermusch commented 3 years ago

The buildTransitive is only supported in nuget5.0+ https://docs.microsoft.com/en-us/nuget/consume-packages/package-references-in-project-files

https://github.com/NuGet/Home/wiki/Allow-package--authors-to-define-build-assets-transitive-behavior

dfederm commented 3 years ago

Yea, the .net5 sdk. Where projects can still target .net framework. .net 5 sdk just means you're sdk style and using Visual Studio 16.8+

dfederm commented 3 years ago

Er sorry that's Nuget 5.0, not .net sdk 5.0. the versions don't align, and Nuget 5.0 is pretty old at this point (April 2019).

jtattermusch commented 3 years ago

FTR this old issue https://github.com/grpc/grpc/issues/11279 seems related.