Move Grpc.Core.Api nuget from grpc/grpc #1651

jtattermusch · 2022-03-29T09:10:22Z

Add Grpc.Core.Api and relevant tests from grpc/grpc to grpc-dotnet.

copy the src/csharp/Grpc.Core.Api subtree from grpc/grpc
add the tests relevant to Grpc.Core.Api from src/csharp/Grpc.Core.Tests as Grpc.Core.Api.Tests
make necessary adjustments to fit the csproj layout in grpc-dotnet repo
fix analyzer warnings.
makes Grpc.Core.Api target net462 instead of net45 (net45 is past end of life). Based on my experiments, this still allows to reference newer Grpc.Core.Api versions from projects that use older versions of Grpc.Core (for which there will likely be no more major version releases). (the net462 version of Grpc.core.Api.dll assembly can still be used by an older Grpc.Core.dll assembly that targets net45 - as long as the project including both Grpc.Core.Api and Grpc.Core uses net462+, which is a fair limitation, since net45 is past end of support so there in practice there should really be no systems that don't support at least .NET Framework 4.6.2)
best reviewed commit-by-commit

jtattermusch · 2022-03-29T09:12:34Z

src/Grpc.Core.Api/VersionInfo.cs

+    /// </summary>
+    public static class VersionInfo
+    {
+        // TODO(jtattermusch): how to propagate versions from https://github.com/grpc/grpc-dotnet/blob/f238855fcb77cec01e87eb9cc08680c3420c132f/Directory.Build.props#L8-L10version into


Still need to figure out how to turn .csproj properties into string constants.
(in grpc/grpc we simply used codegen to generate contents of VersionInfo.cs)

@JamesNK any ideas how to do this?

No. I know how to easily embed properties as assembly attributes, which can then be fetched at runtime. But these are const values so that won't work.

Could write a source generator? https://docs.microsoft.com/en-us/dotnet/csharp/roslyn-sdk/source-generators-overview

src/Grpc.Core.Api/.gitignore

JamesNK · 2022-03-30T09:22:24Z

src/Grpc.Core.Api/AsyncAuthInterceptor.cs

+    /// </summary>
+    public class AuthInterceptorContext
+    {
+        readonly string serviceUrl;


What do you think of eventually following grpc-dotnet standards? e.g. underscore on private variables, no this, var, etc

It is something we can consider, but I think it's low priority. I understand that we'd want all the code to follow the same code-style, but I'd like us to refrain from making unnecessary changes / refactorings until the point when things settle down a bit (e.g. once we've done one or two successful releases of the new location and we have some verification that there aren't any unexpected problems).

src/Grpc.Core.Api/ContextPropagationToken.cs

src/Grpc.Core.Api/Internal/CodeAnalysisAttributes.cs

JamesNK · 2022-03-30T09:30:56Z

src/Grpc.Core.Api/Properties/AssemblyInfo.cs

+[assembly: InternalsVisibleTo("Grpc.Core,PublicKey=" +
+    "00240000048000009400000006020000002400005253413100040000010001002f5797a92c6fcde81bd4098f43" +
+    "0442bb8e12768722de0b0cb1b15e955b32a11352740ee59f2c94c48edc8e177d1052536b8ac651bce11ce5da3a" +
+    "27fc95aff3dc604a6971417453f9483c7b5e836756d5b271bf8f2403fe186e31956148c03d804487cf642f8cc0" +
+    "71394ee9672dfe5b55ea0f95dfd5a7f77d22c962ccf51320d3")]
+[assembly: InternalsVisibleTo("Grpc.Core.Tests,PublicKey=" +
+    "00240000048000009400000006020000002400005253413100040000010001002f5797a92c6fcde81bd4098f43" +
+    "0442bb8e12768722de0b0cb1b15e955b32a11352740ee59f2c94c48edc8e177d1052536b8ac651bce11ce5da3a" +
+    "27fc95aff3dc604a6971417453f9483c7b5e836756d5b271bf8f2403fe186e31956148c03d804487cf642f8cc0" +
+    "71394ee9672dfe5b55ea0f95dfd5a7f77d22c962ccf51320d3")]
+[assembly: InternalsVisibleTo("Grpc.Core.Testing,PublicKey=" +
+    "00240000048000009400000006020000002400005253413100040000010001002f5797a92c6fcde81bd4098f43" +
+    "0442bb8e12768722de0b0cb1b15e955b32a11352740ee59f2c94c48edc8e177d1052536b8ac651bce11ce5da3a" +
+    "27fc95aff3dc604a6971417453f9483c7b5e836756d5b271bf8f2403fe186e31956148c03d804487cf642f8cc0" +
+    "71394ee9672dfe5b55ea0f95dfd5a7f77d22c962ccf51320d3")]
+[assembly: InternalsVisibleTo("Grpc.IntegrationTesting,PublicKey=" +
+    "00240000048000009400000006020000002400005253413100040000010001002f5797a92c6fcde81bd4098f43" +
+    "0442bb8e12768722de0b0cb1b15e955b32a11352740ee59f2c94c48edc8e177d1052536b8ac651bce11ce5da3a" +
+    "27fc95aff3dc604a6971417453f9483c7b5e836756d5b271bf8f2403fe186e31956148c03d804487cf642f8cc0" +
+    "71394ee9672dfe5b55ea0f95dfd5a7f77d22c962ccf51320d3")]
+[assembly: InternalsVisibleTo("Grpc.Microbenchmarks,PublicKey=" +
+    "00240000048000009400000006020000002400005253413100040000010001002f5797a92c6fcde81bd4098f43" +
+    "0442bb8e12768722de0b0cb1b15e955b32a11352740ee59f2c94c48edc8e177d1052536b8ac651bce11ce5da3a" +
+    "27fc95aff3dc604a6971417453f9483c7b5e836756d5b271bf8f2403fe186e31956148c03d804487cf642f8cc0" +
+    "71394ee9672dfe5b55ea0f95dfd5a7f77d22c962ccf51320d3")]


Can be removed/replaced

We can only remove once we verify that Grpc.Core.Api assembly built using the grpc-dotnet repo still works with the tests from the grpc/grpc (since those tests aren't going to be deleted, we still need to be able to run them, even though they'd live on a "frozen" branch.)

src/Grpc.Core.Api/README.md

src/Grpc.Core.Api/Status.cs

src/Grpc.Core.Api/Utils/EncodingExtensions.cs

JamesNK · 2022-03-30T09:38:36Z

There are tests in Grpc.Core.Tests that relate to Grpc.Core.Api functionality. They should be extracted to a new test project.

jtattermusch · 2022-03-31T09:18:48Z

There are tests in Grpc.Core.Tests that relate to Grpc.Core.Api functionality. They should be extracted to a new test project.

Added Grpc.Core.Api.Tests with the tests from Grpc.Core.Tests that seemed relevant.

jtattermusch · 2022-03-31T09:20:43Z

test/Grpc.Core.Api.Tests/VersionInfoTest.cs

+    public class VersionInfoTest
+    {
+        [Test]
+        public void VersionInfoMatchesAssemblyProperties()


FYI I added this test to make sure that VersionInfo.cs' constants alway stay in sync with the .csproj settings regardless of what approach we choose for keeping the VersionInfo up to date.

jtattermusch · 2022-03-31T09:29:40Z

One more question: It seems feasible to add the original src/csharp/Grpc.Core.Api git subtree along with filtered commit history by using the git subtree split command. Do you think it would be worthwhile to import the code along with change history (the commit messages would have a --annotate prefix that would make it clear the the history is from a different repo) or is simply pointing to the original codebase somewhere in a readme good enough?

jtattermusch · 2022-04-05T08:51:20Z

One more question: It seems feasible to add the original src/csharp/Grpc.Core.Api git subtree along with filtered commit history by using the git subtree split command. Do you think it would be worthwhile to import the code along with change history (the commit messages would have a --annotate prefix that would make it clear the the history is from a different repo) or is simply pointing to the original codebase somewhere in a readme good enough?

Discussed and we decided to simply do a codedump and link to the original codebase (at a clearly documented revision) for simplicity.

jtattermusch · 2022-04-05T08:54:03Z

@JamesNK can you please do another pass and let me know if there's any other major concerns (besides the question of regenerating the constants in VersionInfo.cs) that would prevent this PR from accepting?
We still won't merge this, but I'd like to make sure that with some clean up this will be mergeable once we need it to.

JamesNK · 2022-04-25T06:34:26Z

test/Grpc.Core.Api.Tests/Grpc.Core.Api.Tests.csproj

@@ -0,0 +1,21 @@
+<Project Sdk="Microsoft.NET.Sdk">
+  <PropertyGroup>
+    <TargetFrameworks>net45;netcoreapp3.1</TargetFrameworks>


net45 -> net462

I think eventually these new projects should test with the same frameworks as existing test projects (e.g. + net5.0 and net6.0) but can leave it unchanged for now.

JamesNK · 2022-04-25T06:34:34Z

src/Grpc.Core.Api/Grpc.Core.Api.csproj

+
+    <IsGrpcPublishedPackage>true</IsGrpcPublishedPackage>
+    <GenerateDocumentationFile>true</GenerateDocumentationFile>
+    <TargetFrameworks>net45;netstandard1.5;netstandard2.0;netstandard2.1</TargetFrameworks>


net45 isn't supported anymore. See https://dotnet.microsoft.com/en-us/platform/support/policy/dotnet-framework

Should change net45 to net462 for various Grpc.Core projects.

ok, that seems doable. But there should also be a some follow-up cleanup after switching to net462 (#if defs and also there is some compatibility code that is no longer needed on net462)

jtattermusch · 2022-04-26T09:31:50Z

@JamesNK this is ready for final pass of review (even when approved, I still won't merge just yet, I'd like to synchronize removing stuff from grpc/grpc with adding the corresponding stuff to grpc-dotnet).

https://github.com/grpc/grpc/tree/dd14f803c3beb433c41be91672c6a7a6eeb8727c/src/csharp/Grpc.Core.Api

https://github.com/grpc/grpc/tree/e9ece8f7469e667d5202be134bd88980630dba95/src/csharp/Grpc.Core.Tests

…pc.Core

jtattermusch · 2022-04-28T08:13:43Z

I'm going to merge this to allow for easier review of #1652 and #1708.

the 2.47.x branch has already been cut on master, and the version 2.47 and above of Grpc.Core.Api will be released as part of the grpc-dotnet release. v2.46.x will be the last version that's going to be released from the grpc/grpc repository.

jtattermusch requested a review from JamesNK March 29, 2022 09:10

jtattermusch commented Mar 29, 2022

View reviewed changes

jtattermusch mentioned this pull request Mar 29, 2022

Move Grpc.Auth nuget from grpc/grpc #1652

Merged

JamesNK reviewed Mar 30, 2022

View reviewed changes

jtattermusch commented Mar 31, 2022

View reviewed changes

JamesNK reviewed Apr 25, 2022

View reviewed changes

jtattermusch force-pushed the add_grpc_core_api branch from 3024577 to 53e681c Compare April 25, 2022 15:06

jtattermusch marked this pull request as ready for review April 26, 2022 07:55

jtattermusch assigned JamesNK Apr 26, 2022

JamesNK approved these changes Apr 26, 2022

View reviewed changes

This was referenced Apr 26, 2022

Remove C# implementation (individual packages will continue to be maintained through v2.46.x patches or moved to grpc-dotnet) grpc/grpc#29225

Merged

Grpc.Core.Api nuget package add .net 5/6 support (seems required for C++/CLI .NET projects) #1722

Open

jtattermusch added 11 commits April 28, 2022 10:10

add Grpc.Core.Api tree (at SHA dd14f803c3beb433c41be91672c6a7a6eeb8727c)

cf8d55c

https://github.com/grpc/grpc/tree/dd14f803c3beb433c41be91672c6a7a6eeb8727c/src/csharp/Grpc.Core.Api

initial adjustments to make Grpc.Core.Api buildable

5b6ac01

fix analyzer warnings in Grpc.Core.Api

6213222

net462 fixup

00baf66

bin obj already in gitignore

7519304

add TODOs

d59b932

add Grpc.Core.Api to the solution

1527a9d

pack README.md

cd42434

start using net45 again

96527a7

add Grpc.Core.Tests tree at e9ece8f7469e667d5202be134bd88980630dba95

2154dbf

https://github.com/grpc/grpc/tree/e9ece8f7469e667d5202be134bd88980630dba95/src/csharp/Grpc.Core.Tests

remove gitignore

7adbfa3

jtattermusch added 20 commits April 28, 2022 10:10

initial adjustments

fd72388

more project adjustments

7a61b50

remove Grpc.Core specific tests

cc02ab6

Grpc.Core.Api: make internals visible to Grpc.Core.Api.Tests

dd65d76

remove ChannelOptionsTest (only relevant to Grpc.Core

63281bb

CallOptions.Normalize is only implemented in Grpc.Core

cddd0f2

CallCredential.ToNativeCredentials extension method only exists in Gr…

b1833bc

…pc.Core

CallOptionsTest: add TODO

30f2f6d

ChannelCredentials.ToNativeCredentials() only exists in Grpc.Core

73342ca

add Grpc.Core.Api.Tests to the solution

c3e9c44

cleanup Grpc.Core.Api.Tests.csproj

b04139f

fix versions in VersionInfo.cs

5f550ec

Version.cs no longer needed (same attributes set elsewhere

b938aa8

add VersionInfoTest to make sure VersionInfo.cs stays up to date

87d9841

cleanup Grpc.Core.Api.csproj

1b56c92

remove net45-compatibility code

6be4167

switch Grpc.Core.Api to net462

db5984f

use ProjectReference for Grpc.Core.Api

34ce0e9

add script for updating VersionInfo.cs

1e2d368

update VersionInfo.cs

c21d173

jtattermusch force-pushed the add_grpc_core_api branch from 1d4a5e1 to c21d173 Compare April 28, 2022 08:11

jtattermusch merged commit 819dea5 into grpc:master Apr 28, 2022

jtattermusch mentioned this pull request Apr 29, 2022

[C#] Update Grpc.Core target from 4.5 to 4.6.2 grpc/grpc#28619

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Move Grpc.Core.Api nuget from grpc/grpc #1651

Move Grpc.Core.Api nuget from grpc/grpc #1651

jtattermusch commented Mar 29, 2022 •

edited

Loading

jtattermusch Mar 29, 2022

jtattermusch Mar 30, 2022

JamesNK Mar 30, 2022

JamesNK Mar 30, 2022

JamesNK Mar 30, 2022 •

edited

Loading

jtattermusch Mar 30, 2022

JamesNK Mar 30, 2022

jtattermusch Mar 30, 2022

JamesNK commented Mar 30, 2022

jtattermusch commented Mar 31, 2022

jtattermusch Mar 31, 2022 •

edited

Loading

jtattermusch commented Mar 31, 2022

jtattermusch commented Apr 5, 2022

jtattermusch commented Apr 5, 2022

JamesNK Apr 25, 2022

JamesNK Apr 25, 2022

jtattermusch Apr 25, 2022

jtattermusch commented Apr 26, 2022

jtattermusch commented Apr 28, 2022

Move Grpc.Core.Api nuget from grpc/grpc #1651

Move Grpc.Core.Api nuget from grpc/grpc #1651

Conversation

jtattermusch commented Mar 29, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

JamesNK Mar 30, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

JamesNK commented Mar 30, 2022

jtattermusch commented Mar 31, 2022

jtattermusch Mar 31, 2022 • edited Loading

Choose a reason for hiding this comment

jtattermusch commented Mar 31, 2022

jtattermusch commented Apr 5, 2022

jtattermusch commented Apr 5, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jtattermusch commented Apr 26, 2022

jtattermusch commented Apr 28, 2022

jtattermusch commented Mar 29, 2022 •

edited

Loading

JamesNK Mar 30, 2022 •

edited

Loading

jtattermusch Mar 31, 2022 •

edited

Loading