Skip to content

Commit 71149a3

Browse files
tonydnewellmario-vimal
authored andcommitted
[C#] Grpc.Tools - Remove TODOs for splitting package (grpc#33581)
It has been decided that we will no longer try and split Grpc.Tools NuGet package logic into two packages by adding logic to Google.Protobuf.Tools. See protocolbuffers/protobuf#5957 This PR removes some of the TODOs and redundant files. There is further simplification work that could be done to combine the props and targets files that were already split. But this is the first step in the tidying up the code.
1 parent 0ae55b0 commit 71149a3

9 files changed

+21
-44
lines changed

src/csharp/Grpc.Tools/Grpc.Tools.csproj

+8-14
Original file line numberDiff line numberDiff line change
@@ -9,17 +9,14 @@
99

1010
<Import Project="SourceLink.csproj.include" />
1111

12-
<PropertyGroup Label="Asset root folders. TODO(kkm): Change with package separation.">
13-
<!-- TODO(kkm): Rework whole section when splitting packages. -->
14-
<!-- GRPC: ../../third_party/protobuf/src/google/protobuf/ -->
15-
<!-- GPB: ../src/google/protobuf/ -->
12+
<PropertyGroup Label="Asset root folders.">
13+
<!-- Root of well known types .proto files -->
1614
<Assets_ProtoInclude>../../../third_party/protobuf/src/google/protobuf/</Assets_ProtoInclude>
1715

18-
<!-- GPB: ../protoc/ -->
19-
<!-- GRPC: ../protoc_plugins/protoc_ -->
16+
<!-- Protocol buffers compiler binaries -->
2017
<Assets_ProtoCompiler>../protoc_plugins/protoc_</Assets_ProtoCompiler>
2118

22-
<!-- GRPC: ../protoc_plugins/ -->
19+
<!-- Plugins binaries -->
2320
<Assets_GrpcPlugins>../protoc_plugins/</Assets_GrpcPlugins>
2421
</PropertyGroup>
2522

@@ -28,7 +25,6 @@
2825
</PropertyGroup>
2926

3027
<PropertyGroup Label="NuGet package definition" Condition=" '$(Configuration)' == 'Release' ">
31-
<!-- TODO(kkm): Change to "build\" after splitting. -->
3228
<BuildOutputTargetFolder>build\_protobuf\</BuildOutputTargetFolder>
3329
<DevelopmentDependency>true</DevelopmentDependency>
3430
<NoPackageAnalysis>true</NoPackageAnalysis>
@@ -51,22 +47,20 @@
5147
<ItemGroup Label="NuGet package assets">
5248
<None Pack="true" PackagePath="build\" Include="build\**\*.xml; build\**\*.props; build\**\*.targets;" />
5349

54-
<!-- Protobuf assets (for Google.Protobuf.Tools) -->
50+
<!-- Protobuf assets - well known types -->
5551
<_ProtoAssetName Include="any;api;descriptor;duration;empty;field_mask;
5652
source_context;struct;timestamp;type;wrappers" />
5753
<_Asset PackagePath="build/native/include/google/protobuf/" Include="@(_ProtoAssetName->'$(Assets_ProtoInclude)%(Identity).proto')" />
5854

59-
<!-- TODO(kkm): GPB builds assets into "macosx", GRPC into "macos". -->
60-
<!-- TODO(kkm): Do not place non-tools under tools/, use build/native/bin/. -->
61-
<!-- TODO(kkm): Do not package windows x64 builds (#13098). -->
55+
<!-- protocol buffers compiler -->
6256
<_Asset PackagePath="tools/windows_x86/" Include="$(Assets_ProtoCompiler)windows_x86/protoc.exe" />
6357
<_Asset PackagePath="tools/windows_x64/" Include="$(Assets_ProtoCompiler)windows_x64/protoc.exe" />
6458
<_Asset PackagePath="tools/linux_x86/" Include="$(Assets_ProtoCompiler)linux_x86/protoc" />
6559
<_Asset PackagePath="tools/linux_x64/" Include="$(Assets_ProtoCompiler)linux_x64/protoc" />
6660
<_Asset PackagePath="tools/linux_arm64/" Include="$(Assets_ProtoCompiler)linux_aarch64/protoc" />
67-
<_Asset PackagePath="tools/macosx_x64/" Include="$(Assets_ProtoCompiler)macos_x64/protoc" /> <!-- GPB: macosx-->
61+
<_Asset PackagePath="tools/macosx_x64/" Include="$(Assets_ProtoCompiler)macos_x64/protoc" />
6862

69-
<!-- gRPC assets (for Grpc.Tools) -->
63+
<!-- gRPC protocol buffer compiler plugins -->
7064
<_Asset PackagePath="tools/windows_x86/" Include="$(Assets_GrpcPlugins)protoc_windows_x86/grpc_csharp_plugin.exe" />
7165
<_Asset PackagePath="tools/windows_x64/" Include="$(Assets_GrpcPlugins)protoc_windows_x64/grpc_csharp_plugin.exe" />
7266
<_Asset PackagePath="tools/linux_x86/" Include="$(Assets_GrpcPlugins)protoc_linux_x86/grpc_csharp_plugin" />

src/csharp/Grpc.Tools/build/Grpc.Tools.props

+5-2
Original file line numberDiff line numberDiff line change
@@ -4,8 +4,11 @@
44
<MSBuildAllProjects>$(MSBuildAllProjects);$(MSBuildThisFileFullPath)</MSBuildAllProjects>
55
</PropertyGroup>
66

7-
<!-- Name of this file must match package ID. -->
8-
<!-- Packages will be split later. -->
7+
<!-- Name of this file (Grpc.Tools.props) must match package ID (Grpc.Tools). -->
8+
9+
<!-- TODO(tonydnewell) The files below were originally separate to allow
10+
Grpc.Tools and Google.Protobuf.Tools split, but since we abandoned that plan,
11+
the logic could now be simplified by combining these files together -->
912
<Import Project="_grpc/_Grpc.Tools.props"/>
1013
<Import Project="_protobuf/Google.Protobuf.Tools.props"/>
1114
</Project>

src/csharp/Grpc.Tools/build/Grpc.Tools.targets

+5-2
Original file line numberDiff line numberDiff line change
@@ -4,8 +4,11 @@
44
<MSBuildAllProjects>$(MSBuildAllProjects);$(MSBuildThisFileFullPath)</MSBuildAllProjects>
55
</PropertyGroup>
66

7-
<!-- Name of this file must match package ID. -->
8-
<!-- Packages will be split later. -->
7+
<!-- Name of this file (Grpc.Tools.targets) must match package ID (Grpc.Tools). -->
8+
9+
<!-- TODO(tonydnewell) The files below were originally separate to allow
10+
Grpc.Tools and Google.Protobuf.Tools split, but since we abandoned that plan,
11+
the logic could now be simplified by combining these files together -->
912
<Import Project="_grpc/_Grpc.Tools.targets"/>
1013
<Import Project="_protobuf/Google.Protobuf.Tools.targets"/>
1114
</Project>

src/csharp/Grpc.Tools/build/_grpc/README

-3
This file was deleted.

src/csharp/Grpc.Tools/build/_grpc/_Grpc.Tools.targets

-2
Original file line numberDiff line numberDiff line change
@@ -21,8 +21,6 @@
2121
<!-- This target is invoked in a C# project, or can be called in a customized project. -->
2222
<Target Name="gRPC_ResolvePluginFullPath" AfterTargets="Protobuf_ResolvePlatform">
2323
<PropertyGroup>
24-
<!-- TODO(kkm): Do not use Protobuf_PackagedToolsPath, roll gRPC's own. -->
25-
2624
<!-- First try environment variable. -->
2725
<gRPC_PluginFullPath Condition=" '$(gRPC_PluginFullPath)' == '' ">$(GRPC_PROTOC_PLUGIN)</gRPC_PluginFullPath>
2826

src/csharp/Grpc.Tools/build/_protobuf/Google.Protobuf.Tools.props

+3-2
Original file line numberDiff line numberDiff line change
@@ -6,9 +6,10 @@
66
<!-- Revision number of this package conventions (as if "API" version). -->
77
<Protobuf_ToolingRevision>1</Protobuf_ToolingRevision>
88

9-
<!-- TODO(kkm): Remove one "../" when separating packages. -->
10-
<!-- TODO(kkm): Do not place non-tools under tools/, use build/native/bin/. -->
9+
10+
<!-- Tools - protocol buffers compiler and plugins -->
1111
<Protobuf_PackagedToolsPath>$( [System.IO.Path]::GetFullPath($(MSBuildThisFileDirectory)../../tools) )</Protobuf_PackagedToolsPath>
12+
<!-- Protocol buffers Well Known Types -->
1213
<Protobuf_StandardImportsPath>$( [System.IO.Path]::GetFullPath($(MSBuildThisFileDirectory)../native/include) )</Protobuf_StandardImportsPath>
1314
</PropertyGroup>
1415

src/csharp/Grpc.Tools/build/_protobuf/Google.Protobuf.Tools.targets

-1
Original file line numberDiff line numberDiff line change
@@ -110,7 +110,6 @@
110110

111111
<!-- Do proto compilation by default in a C# project. In other types, the user invoke
112112
Protobuf_Compile directly where required. -->
113-
<!-- TODO(kkm): Do shared compile in outer multitarget project? -->
114113
<Target Name="_Protobuf_Compile_BeforeCsCompile"
115114
BeforeTargets="BeforeCompile"
116115
DependsOnTargets="Protobuf_Compile"

src/csharp/Grpc.Tools/build/_protobuf/README

-1
This file was deleted.

src/csharp/Grpc.Tools/build/native/Grpc.Tools.props

-17
This file was deleted.

0 commit comments

Comments
 (0)