From 47ec56beb43cfcb37f35572401d28a6101274b7b Mon Sep 17 00:00:00 2001 From: "Kraemer, Benjamin" Date: Tue, 5 May 2020 11:34:45 +0200 Subject: [PATCH 01/12] Avoid collisions in cs files generated by Grpc.Tools --- .../Grpc.Tools.Tests/DepFileUtilTest.cs | 28 +++++++++ src/csharp/Grpc.Tools/DepFileUtil.cs | 62 ++++++++++++++++--- src/csharp/Grpc.Tools/GeneratorServices.cs | 11 ++-- src/csharp/Grpc.Tools/ProtoCompile.cs | 22 ++++++- 4 files changed, 106 insertions(+), 17 deletions(-) diff --git a/src/csharp/Grpc.Tools.Tests/DepFileUtilTest.cs b/src/csharp/Grpc.Tools.Tests/DepFileUtilTest.cs index e89a8f4b5d3..4da62f5773e 100644 --- a/src/csharp/Grpc.Tools.Tests/DepFileUtilTest.cs +++ b/src/csharp/Grpc.Tools.Tests/DepFileUtilTest.cs @@ -67,6 +67,34 @@ namespace Grpc.Tools.Tests Assert.AreNotEqual(unsame1, unsame2); } + [Test] + public void GetOutputDirWithHash_IsSane() + { + StringAssert.IsMatch(@"^out[\\/][a-f0-9]{16}$", + DepFileUtil.GetOutputDirWithHash("out", "foo.proto")); + StringAssert.IsMatch(@"^[a-f0-9]{16}$", + DepFileUtil.GetOutputDirWithHash("", "foo.proto")); + } + + [Test] + public void GetOutputDirWithHash_HashesDir() + { + string PickHash(string fname) => DepFileUtil.GetOutputDirWithHash("", fname); + + string same1 = PickHash("dir1/dir2/foo.proto"); + string same2 = PickHash("dir1/dir2/proto.foo"); + string same3 = PickHash("dir1/dir2/proto"); + string same4 = PickHash("dir1/dir2/.proto"); + string unsame1 = PickHash("dir2/foo.proto"); + string unsame2 = PickHash("/dir2/foo.proto"); + + Assert.AreEqual(same1, same2); + Assert.AreEqual(same1, same3); + Assert.AreEqual(same1, same4); + Assert.AreNotEqual(same1, unsame1); + Assert.AreNotEqual(unsame1, unsame2); + } + ////////////////////////////////////////////////////////////////////////// // Full file reading tests diff --git a/src/csharp/Grpc.Tools/DepFileUtil.cs b/src/csharp/Grpc.Tools/DepFileUtil.cs index 440d3d535c8..15ffed24546 100644 --- a/src/csharp/Grpc.Tools/DepFileUtil.cs +++ b/src/csharp/Grpc.Tools/DepFileUtil.cs @@ -138,6 +138,24 @@ namespace Grpc.Tools return result.ToArray(); } + /// + /// Construct the directory hash from a relative file name + /// + /// Relative path to the proto item, e. g. "foo/file.proto" + /// + /// Directory hash based on the file name, e. g. "deadbeef12345678" + /// + private static string GetDirectoryHash(string proto) + { + string dirname = Path.GetDirectoryName(proto); + if (Platform.IsFsCaseInsensitive) + { + dirname = dirname.ToLowerInvariant(); + } + + return HashString64Hex(dirname); + } + /// /// Construct relative dependency file name from directory hash and file name /// @@ -145,7 +163,7 @@ namespace Grpc.Tools /// Relative path to the proto item, e. g. "foo/file.proto" /// /// Full relative path to the dependency file, e. g. - /// "out/deadbeef12345678_file.protodep" + /// "out/deadbeef12345678/file.protodep" /// /// /// Since a project may contain proto files with the same filename but in different @@ -158,21 +176,45 @@ namespace Grpc.Tools /// project and solution directories, which are also some level deep from the root. /// Instead of creating long and unwieldy names for these proto sources, we cache /// the full path of the name without the filename, and append the filename to it, - /// as in e. g. "foo/file.proto" will yield the name "deadbeef12345678_file", where - /// "deadbeef12345678" is a presumed hash value of the string "foo/". This allows + /// as in e. g. "foo/file.proto" will yield the name "deadbeef12345678/file", where + /// "deadbeef12345678" is a presumed hash value of the string "foo". This allows /// the file names be short, unique (up to a hash collision), and still allowing /// the user to guess their provenance. /// public static string GetDepFilenameForProto(string protoDepDir, string proto) { - string dirname = Path.GetDirectoryName(proto); - if (Platform.IsFsCaseInsensitive) - { - dirname = dirname.ToLowerInvariant(); - } - string dirhash = HashString64Hex(dirname); + string outdir = GetOutputDirWithHash(protoDepDir, proto); string filename = Path.GetFileNameWithoutExtension(proto); - return Path.Combine(protoDepDir, $"{dirhash}_{filename}.protodep"); + return Path.Combine(outdir, $"{filename}.protodep"); + } + + /// + /// Construct relative output directory with directory hash + /// + /// Relative path to the output directory, e. g. "out" + /// Relative path to the proto item, e. g. "foo/file.proto" + /// + /// Full relative path to the directory, e. g. "out/deadbeef12345678" + /// + /// + /// Since a project may contain proto files with the same filename but in different + /// directories, a unique directory for the generated files is constructed based on the + /// proto file names directory. The directory path can be arbitrary, for example, + /// it can be outside of the project, or an absolute path including a drive letter, + /// or a UNC network path. A name constructed from such a path by, for example, + /// replacing disallowed name characters with an underscore, may well be over + /// filesystem's allowed path length, since it will be located under the project + /// and solution directories, which are also some level deep from the root. + /// Instead of creating long and unwieldy names for these proto sources, we cache + /// the full path of the name without the filename, as in e. g. "foo/file.proto" + /// will yield the name "deadbeef12345678", where that is a presumed hash value + /// of the string "foo". This allows the path to be short, unique (up to a hash + /// collision), and still allowing the user to guess their provenance. + /// + public static string GetOutputDirWithHash(string outputDir, string proto) + { + var dirhash = GetDirectoryHash(proto); + return Path.Combine(outputDir, dirhash); } // Get a 64-bit hash for a directory string. We treat it as if it were diff --git a/src/csharp/Grpc.Tools/GeneratorServices.cs b/src/csharp/Grpc.Tools/GeneratorServices.cs index 903dd3dacd9..4e2d2f748f5 100644 --- a/src/csharp/Grpc.Tools/GeneratorServices.cs +++ b/src/csharp/Grpc.Tools/GeneratorServices.cs @@ -67,19 +67,20 @@ namespace Grpc.Tools { bool doGrpc = GrpcOutputPossible(protoItem); var outputs = new string[doGrpc ? 2 : 1]; - string basename = Path.GetFileNameWithoutExtension(protoItem.ItemSpec); + var itemSpec = protoItem.ItemSpec; + string basename = Path.GetFileNameWithoutExtension(itemSpec); - string outdir = protoItem.GetMetadata(Metadata.OutputDir); + string outdir = DepFileUtil.GetOutputDirWithHash(protoItem.GetMetadata(Metadata.OutputDir), itemSpec); string filename = LowerUnderscoreToUpperCamelProtocWay(basename); outputs[0] = Path.Combine(outdir, filename) + ".cs"; if (doGrpc) { - // Override outdir if kGrpcOutputDir present, default to proto output. + // Override outdir if GrpcOutputDir present, default to proto output. string grpcdir = protoItem.GetMetadata(Metadata.GrpcOutputDir); + grpcdir = grpcdir == "" ? outdir : DepFileUtil.GetOutputDirWithHash(grpcdir, itemSpec); filename = LowerUnderscoreToUpperCamelGrpcWay(basename); - outputs[1] = Path.Combine( - grpcdir != "" ? grpcdir : outdir, filename) + "Grpc.cs"; + outputs[1] = Path.Combine(grpcdir, filename) + "Grpc.cs"; } return outputs; } diff --git a/src/csharp/Grpc.Tools/ProtoCompile.cs b/src/csharp/Grpc.Tools/ProtoCompile.cs index 36a0ea36cee..8625c2a5aab 100644 --- a/src/csharp/Grpc.Tools/ProtoCompile.cs +++ b/src/csharp/Grpc.Tools/ProtoCompile.cs @@ -18,6 +18,7 @@ using System; using System.Collections.Generic; +using System.IO; using System.Text; using System.Text.RegularExpressions; using Microsoft.Build.Framework; @@ -413,16 +414,21 @@ namespace Grpc.Tools // Called by the base ToolTask to get response file contents. protected override string GenerateResponseFileCommands() { + var outDir = TrimEndSlash(MaybeEnhanceOutputDir(OutputDir, Protobuf)); + var grpcOutDir = TrimEndSlash(MaybeEnhanceOutputDir(GrpcOutputDir, Protobuf)); + var cmd = new ProtocResponseFileBuilder(); - cmd.AddSwitchMaybe(Generator + "_out", TrimEndSlash(OutputDir)); + cmd.AddSwitchMaybe(Generator + "_out", outDir); cmd.AddSwitchMaybe(Generator + "_opt", OutputOptions); cmd.AddSwitchMaybe("plugin=protoc-gen-grpc", GrpcPluginExe); - cmd.AddSwitchMaybe("grpc_out", TrimEndSlash(GrpcOutputDir)); + cmd.AddSwitchMaybe("grpc_out", grpcOutDir); cmd.AddSwitchMaybe("grpc_opt", GrpcOutputOptions); if (ProtoPath != null) { foreach (string path in ProtoPath) + { cmd.AddSwitchMaybe("proto_path", TrimEndSlash(path)); + } } cmd.AddSwitchMaybe("dependency_out", DependencyOut); cmd.AddSwitchMaybe("error_format", "msvs"); @@ -433,6 +439,18 @@ namespace Grpc.Tools return cmd.ToString(); } + // If possible, disambiguate output dir by adding a hash of the proto file's path + static string MaybeEnhanceOutputDir(string outputDir, ITaskItem[] protobufs) + { + if (protobufs.Length != 1) + { + return outputDir; + } + + var protoFile = protobufs[0].ItemSpec; + return DepFileUtil.GetOutputDirWithHash(outputDir, protoFile); + } + // Protoc cannot digest trailing slashes in directory names, // curiously under Linux, but not in Windows. static string TrimEndSlash(string dir) From 31cd1000b2ea612904179bbb347f539afdf0ed54 Mon Sep 17 00:00:00 2001 From: "Kraemer, Benjamin" Date: Tue, 5 May 2020 11:36:05 +0200 Subject: [PATCH 02/12] Create generated directories --- .../Grpc.Tools/build/_protobuf/Google.Protobuf.Tools.targets | 1 + 1 file changed, 1 insertion(+) diff --git a/src/csharp/Grpc.Tools/build/_protobuf/Google.Protobuf.Tools.targets b/src/csharp/Grpc.Tools/build/_protobuf/Google.Protobuf.Tools.targets index 90e6ed7282c..40c5d5bd3dc 100644 --- a/src/csharp/Grpc.Tools/build/_protobuf/Google.Protobuf.Tools.targets +++ b/src/csharp/Grpc.Tools/build/_protobuf/Google.Protobuf.Tools.targets @@ -261,6 +261,7 @@ + From da4fe1e051a66b29d7c0c177f1bba28f9bc463c4 Mon Sep 17 00:00:00 2001 From: "Kraemer, Benjamin" Date: Tue, 5 May 2020 11:47:11 +0200 Subject: [PATCH 03/12] Updated unit tests --- src/csharp/Grpc.Tools.Tests/DepFileUtilTest.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/csharp/Grpc.Tools.Tests/DepFileUtilTest.cs b/src/csharp/Grpc.Tools.Tests/DepFileUtilTest.cs index 4da62f5773e..6b22e7f1a00 100644 --- a/src/csharp/Grpc.Tools.Tests/DepFileUtilTest.cs +++ b/src/csharp/Grpc.Tools.Tests/DepFileUtilTest.cs @@ -41,9 +41,9 @@ namespace Grpc.Tools.Tests [Test] public void GetDepFilenameForProto_IsSane() { - StringAssert.IsMatch(@"^out[\\/][a-f0-9]{16}_foo.protodep$", + StringAssert.IsMatch(@"^out[\\/][a-f0-9]{16}[\\/]foo.protodep$", DepFileUtil.GetDepFilenameForProto("out", "foo.proto")); - StringAssert.IsMatch(@"^[a-f0-9]{16}_foo.protodep$", + StringAssert.IsMatch(@"^[a-f0-9]{16}[\\/]foo.protodep$", DepFileUtil.GetDepFilenameForProto("", "foo.proto")); } From 6bcc37b07cb10d4882be3d3d707963ec39fbec43 Mon Sep 17 00:00:00 2001 From: "Kraemer, Benjamin" Date: Tue, 5 May 2020 14:41:40 +0200 Subject: [PATCH 04/12] Reverted GetDepFilenameForProto Otherwise it creates conflicts with non-standard $(Protobuf_DepFilesPath) --- .../Grpc.Tools.Tests/DepFileUtilTest.cs | 4 +- src/csharp/Grpc.Tools/DepFileUtil.cs | 52 ++++++++----------- 2 files changed, 23 insertions(+), 33 deletions(-) diff --git a/src/csharp/Grpc.Tools.Tests/DepFileUtilTest.cs b/src/csharp/Grpc.Tools.Tests/DepFileUtilTest.cs index 6b22e7f1a00..4da62f5773e 100644 --- a/src/csharp/Grpc.Tools.Tests/DepFileUtilTest.cs +++ b/src/csharp/Grpc.Tools.Tests/DepFileUtilTest.cs @@ -41,9 +41,9 @@ namespace Grpc.Tools.Tests [Test] public void GetDepFilenameForProto_IsSane() { - StringAssert.IsMatch(@"^out[\\/][a-f0-9]{16}[\\/]foo.protodep$", + StringAssert.IsMatch(@"^out[\\/][a-f0-9]{16}_foo.protodep$", DepFileUtil.GetDepFilenameForProto("out", "foo.proto")); - StringAssert.IsMatch(@"^[a-f0-9]{16}[\\/]foo.protodep$", + StringAssert.IsMatch(@"^[a-f0-9]{16}_foo.protodep$", DepFileUtil.GetDepFilenameForProto("", "foo.proto")); } diff --git a/src/csharp/Grpc.Tools/DepFileUtil.cs b/src/csharp/Grpc.Tools/DepFileUtil.cs index 15ffed24546..0c08fcb72e6 100644 --- a/src/csharp/Grpc.Tools/DepFileUtil.cs +++ b/src/csharp/Grpc.Tools/DepFileUtil.cs @@ -145,6 +145,21 @@ namespace Grpc.Tools /// /// Directory hash based on the file name, e. g. "deadbeef12345678" /// + /// + /// Since a project may contain proto files with the same filename but in different + /// directories, a unique directory for the generated files is constructed based on the + /// proto file names directory. The directory path can be arbitrary, for example, + /// it can be outside of the project, or an absolute path including a drive letter, + /// or a UNC network path. A name constructed from such a path by, for example, + /// replacing disallowed name characters with an underscore, may well be over + /// filesystem's allowed path length, since it will be located under the project + /// and solution directories, which are also some level deep from the root. + /// Instead of creating long and unwieldy names for these proto sources, we cache + /// the full path of the name without the filename, as in e. g. "foo/file.proto" + /// will yield the name "deadbeef12345678", where that is a presumed hash value + /// of the string "foo". This allows the path to be short, unique (up to a hash + /// collision), and still allowing the user to guess their provenance. + /// private static string GetDirectoryHash(string proto) { string dirname = Path.GetDirectoryName(proto); @@ -163,29 +178,16 @@ namespace Grpc.Tools /// Relative path to the proto item, e. g. "foo/file.proto" /// /// Full relative path to the dependency file, e. g. - /// "out/deadbeef12345678/file.protodep" + /// "out/deadbeef12345678_file.protodep" /// /// - /// Since a project may contain proto files with the same filename but in different - /// directories, a unique filename for the dependency file is constructed based on the - /// proto file name both name and directory. The directory path can be arbitrary, - /// for example, it can be outside of the project, or an absolute path including - /// a drive letter, or a UNC network path. A name constructed from such a path by, - /// for example, replacing disallowed name characters with an underscore, may well - /// be over filesystem's allowed path length, since it will be located under the - /// project and solution directories, which are also some level deep from the root. - /// Instead of creating long and unwieldy names for these proto sources, we cache - /// the full path of the name without the filename, and append the filename to it, - /// as in e. g. "foo/file.proto" will yield the name "deadbeef12345678/file", where - /// "deadbeef12345678" is a presumed hash value of the string "foo". This allows - /// the file names be short, unique (up to a hash collision), and still allowing - /// the user to guess their provenance. + /// See for notes on directory hash. /// public static string GetDepFilenameForProto(string protoDepDir, string proto) { - string outdir = GetOutputDirWithHash(protoDepDir, proto); + string dirhash = GetDirectoryHash(proto); string filename = Path.GetFileNameWithoutExtension(proto); - return Path.Combine(outdir, $"{filename}.protodep"); + return Path.Combine(protoDepDir, $"{dirhash}_{filename}.protodep"); } /// @@ -197,23 +199,11 @@ namespace Grpc.Tools /// Full relative path to the directory, e. g. "out/deadbeef12345678" /// /// - /// Since a project may contain proto files with the same filename but in different - /// directories, a unique directory for the generated files is constructed based on the - /// proto file names directory. The directory path can be arbitrary, for example, - /// it can be outside of the project, or an absolute path including a drive letter, - /// or a UNC network path. A name constructed from such a path by, for example, - /// replacing disallowed name characters with an underscore, may well be over - /// filesystem's allowed path length, since it will be located under the project - /// and solution directories, which are also some level deep from the root. - /// Instead of creating long and unwieldy names for these proto sources, we cache - /// the full path of the name without the filename, as in e. g. "foo/file.proto" - /// will yield the name "deadbeef12345678", where that is a presumed hash value - /// of the string "foo". This allows the path to be short, unique (up to a hash - /// collision), and still allowing the user to guess their provenance. + /// See for notes on directory hash. /// public static string GetOutputDirWithHash(string outputDir, string proto) { - var dirhash = GetDirectoryHash(proto); + string dirhash = GetDirectoryHash(proto); return Path.Combine(outputDir, dirhash); } From 906ce37a35427fee4df40c039dbe9ba74c0d9604 Mon Sep 17 00:00:00 2001 From: "Kraemer, Benjamin" Date: Tue, 5 May 2020 14:42:17 +0200 Subject: [PATCH 05/12] Improved folder generation --- .../build/_protobuf/Google.Protobuf.Tools.targets | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/csharp/Grpc.Tools/build/_protobuf/Google.Protobuf.Tools.targets b/src/csharp/Grpc.Tools/build/_protobuf/Google.Protobuf.Tools.targets index 40c5d5bd3dc..8b286593647 100644 --- a/src/csharp/Grpc.Tools/build/_protobuf/Google.Protobuf.Tools.targets +++ b/src/csharp/Grpc.Tools/build/_protobuf/Google.Protobuf.Tools.targets @@ -259,9 +259,11 @@ Condition=" '$(DisableProtobufDesignTimeBuild)' != 'true' " DependsOnTargets="Protobuf_PrepareCompileOptions;_Protobuf_GatherStaleFiles"> - - - + + <_Protobuf_ExpectedGenerated Include="@(Protobuf_ExpectedOutputs)" + Condition = " '%(Source)' != '' and '@(_Protobuf_OutOfDateProto)' != '' " /> + + From 5494c54cee7ef72b21cb68014cb6acba3facd3e6 Mon Sep 17 00:00:00 2001 From: "Kraemer, Benjamin" Date: Wed, 6 May 2020 15:07:21 +0200 Subject: [PATCH 06/12] Use same logic in csharp as in cpp --- .../Grpc.Tools.Tests/CSharpGeneratorTest.cs | 10 ++ src/csharp/Grpc.Tools/DepFileUtil.cs | 46 ++++++++++ src/csharp/Grpc.Tools/GeneratorServices.cs | 91 +++++++------------ src/csharp/Grpc.Tools/ProtoCompile.cs | 19 +--- src/csharp/Grpc.Tools/ProtoCompilerOutputs.cs | 16 ++++ .../_protobuf/Google.Protobuf.Tools.targets | 13 ++- 6 files changed, 117 insertions(+), 78 deletions(-) diff --git a/src/csharp/Grpc.Tools.Tests/CSharpGeneratorTest.cs b/src/csharp/Grpc.Tools.Tests/CSharpGeneratorTest.cs index 320bb6dc9fc..642ebf92cef 100644 --- a/src/csharp/Grpc.Tools.Tests/CSharpGeneratorTest.cs +++ b/src/csharp/Grpc.Tools.Tests/CSharpGeneratorTest.cs @@ -84,5 +84,15 @@ namespace Grpc.Tools.Tests Assert.AreEqual(1, poss.Length); Assert.That(poss[0], Is.EqualTo("out/Foo.cs") | Is.EqualTo("out\\Foo.cs")); } + + [Test] + public void OutputDirPatched() + { + var item = Utils.MakeItem("sub/foo.proto", "OutputDir", "out"); + _generator.PatchOutputDirectory(item); + var poss = _generator.GetPossibleOutputs(item); + Assert.AreEqual(1, poss.Length); + Assert.That(poss[0], Is.EqualTo("out/sub/Foo.cs") | Is.EqualTo("out\\sub\\Foo.cs")); + } }; } diff --git a/src/csharp/Grpc.Tools/DepFileUtil.cs b/src/csharp/Grpc.Tools/DepFileUtil.cs index 0c08fcb72e6..22fc2d00c24 100644 --- a/src/csharp/Grpc.Tools/DepFileUtil.cs +++ b/src/csharp/Grpc.Tools/DepFileUtil.cs @@ -301,5 +301,51 @@ namespace Grpc.Tools return new string[0]; } } + + // Calculate part of proto path relative to root. Protoc is very picky + // about them matching exactly, so can be we. Expect root be exact prefix + // to proto, minus some slash normalization. + internal static string GetRelativeDir(string root, string proto, TaskLoggingHelper log) + { + string protoDir = Path.GetDirectoryName(proto); + string rootDir = EndWithSlash(Path.GetDirectoryName(EndWithSlash(root))); + if (rootDir == s_dotSlash) + { + // Special case, otherwise we can return "./" instead of "" below! + return protoDir; + } + if (Platform.IsFsCaseInsensitive) + { + protoDir = protoDir.ToLowerInvariant(); + rootDir = rootDir.ToLowerInvariant(); + } + protoDir = EndWithSlash(protoDir); + if (!protoDir.StartsWith(rootDir)) + { + log.LogWarning("Protobuf item '{0}' has the ProtoRoot metadata '{1}' " + + "which is not prefix to its path. Cannot compute relative path.", + proto, root); + return ""; + } + return protoDir.Substring(rootDir.Length); + } + + // './' or '.\', normalized per system. + internal static string s_dotSlash = "." + Path.DirectorySeparatorChar; + + internal static string EndWithSlash(string str) + { + if (str == "") + { + return s_dotSlash; + } + + if (str[str.Length - 1] != '\\' && str[str.Length - 1] != '/') + { + return str + Path.DirectorySeparatorChar; + } + + return str; + } }; } diff --git a/src/csharp/Grpc.Tools/GeneratorServices.cs b/src/csharp/Grpc.Tools/GeneratorServices.cs index 4e2d2f748f5..af4b64c2479 100644 --- a/src/csharp/Grpc.Tools/GeneratorServices.cs +++ b/src/csharp/Grpc.Tools/GeneratorServices.cs @@ -55,7 +55,15 @@ namespace Grpc.Tools && !gsm.EqualNoCase("false"); } - public abstract string[] GetPossibleOutputs(ITaskItem proto); + // Update OutputDir and GrpcOutputDir for the item and all subsequent + // targets using this item. This should only be done if the real + // output directories for protoc should be modified. + public virtual void PatchOutputDirectory(ITaskItem protoItem) + { + // Nothing to do + } + + public abstract string[] GetPossibleOutputs(ITaskItem protoItem); }; // C# generator services. @@ -63,22 +71,40 @@ namespace Grpc.Tools { public CSharpGeneratorServices(TaskLoggingHelper log) : base(log) { } + public override void PatchOutputDirectory(ITaskItem protoItem) + { + string root = protoItem.GetMetadata(Metadata.ProtoRoot); + string proto = protoItem.ItemSpec; + string relative = DepFileUtil.GetRelativeDir(root, proto, Log); + + string outdir = protoItem.GetMetadata(Metadata.OutputDir); + string pathStem = Path.Combine(outdir, relative); + protoItem.SetMetadata(Metadata.OutputDir, pathStem); + + // Override outdir if GrpcOutputDir present, default to proto output. + string grpcdir = protoItem.GetMetadata(Metadata.GrpcOutputDir); + if (grpcdir != "") + { + pathStem = Path.Combine(grpcdir, relative); + } + protoItem.SetMetadata(Metadata.GrpcOutputDir, pathStem); + } + public override string[] GetPossibleOutputs(ITaskItem protoItem) { bool doGrpc = GrpcOutputPossible(protoItem); + string proto = protoItem.ItemSpec; + string basename = Path.GetFileNameWithoutExtension(proto); + var outputs = new string[doGrpc ? 2 : 1]; - var itemSpec = protoItem.ItemSpec; - string basename = Path.GetFileNameWithoutExtension(itemSpec); + string outdir = protoItem.GetMetadata(Metadata.OutputDir); - string outdir = DepFileUtil.GetOutputDirWithHash(protoItem.GetMetadata(Metadata.OutputDir), itemSpec); string filename = LowerUnderscoreToUpperCamelProtocWay(basename); outputs[0] = Path.Combine(outdir, filename) + ".cs"; if (doGrpc) { - // Override outdir if GrpcOutputDir present, default to proto output. string grpcdir = protoItem.GetMetadata(Metadata.GrpcOutputDir); - grpcdir = grpcdir == "" ? outdir : DepFileUtil.GetOutputDirWithHash(grpcdir, itemSpec); filename = LowerUnderscoreToUpperCamelGrpcWay(basename); outputs[1] = Path.Combine(grpcdir, filename) + "Grpc.cs"; } @@ -143,7 +169,7 @@ namespace Grpc.Tools string proto = protoItem.ItemSpec; string filename = Path.GetFileNameWithoutExtension(proto); // E. g., ("foo/", "foo/bar/x.proto") => "bar" - string relative = GetRelativeDir(root, proto); + string relative = DepFileUtil.GetRelativeDir(root, proto, Log); var outputs = new string[doGrpc ? 4 : 2]; string outdir = protoItem.GetMetadata(Metadata.OutputDir); @@ -152,7 +178,7 @@ namespace Grpc.Tools outputs[1] = fileStem + ".pb.h"; if (doGrpc) { - // Override outdir if kGrpcOutputDir present, default to proto output. + // Override outdir if GrpcOutputDir present, default to proto output. outdir = protoItem.GetMetadata(Metadata.GrpcOutputDir); if (outdir != "") { @@ -163,52 +189,5 @@ namespace Grpc.Tools } return outputs; } - - // Calculate part of proto path relative to root. Protoc is very picky - // about them matching exactly, so can be we. Expect root be exact prefix - // to proto, minus some slash normalization. - string GetRelativeDir(string root, string proto) - { - string protoDir = Path.GetDirectoryName(proto); - string rootDir = EndWithSlash(Path.GetDirectoryName(EndWithSlash(root))); - if (rootDir == s_dotSlash) - { - // Special case, otherwise we can return "./" instead of "" below! - return protoDir; - } - if (Platform.IsFsCaseInsensitive) - { - protoDir = protoDir.ToLowerInvariant(); - rootDir = rootDir.ToLowerInvariant(); - } - protoDir = EndWithSlash(protoDir); - if (!protoDir.StartsWith(rootDir)) - { - Log.LogWarning("Protobuf item '{0}' has the ProtoRoot metadata '{1}' " + - "which is not prefix to its path. Cannot compute relative path.", - proto, root); - return ""; - } - return protoDir.Substring(rootDir.Length); - } - - // './' or '.\', normalized per system. - static string s_dotSlash = "." + Path.DirectorySeparatorChar; - - static string EndWithSlash(string str) - { - if (str == "") - { - return s_dotSlash; - } - else if (str[str.Length - 1] != '\\' && str[str.Length - 1] != '/') - { - return str + Path.DirectorySeparatorChar; - } - else - { - return str; - } - } - }; + } } diff --git a/src/csharp/Grpc.Tools/ProtoCompile.cs b/src/csharp/Grpc.Tools/ProtoCompile.cs index 8625c2a5aab..e1f01a554bc 100644 --- a/src/csharp/Grpc.Tools/ProtoCompile.cs +++ b/src/csharp/Grpc.Tools/ProtoCompile.cs @@ -414,14 +414,11 @@ namespace Grpc.Tools // Called by the base ToolTask to get response file contents. protected override string GenerateResponseFileCommands() { - var outDir = TrimEndSlash(MaybeEnhanceOutputDir(OutputDir, Protobuf)); - var grpcOutDir = TrimEndSlash(MaybeEnhanceOutputDir(GrpcOutputDir, Protobuf)); - var cmd = new ProtocResponseFileBuilder(); - cmd.AddSwitchMaybe(Generator + "_out", outDir); + cmd.AddSwitchMaybe(Generator + "_out", TrimEndSlash(OutputDir)); cmd.AddSwitchMaybe(Generator + "_opt", OutputOptions); cmd.AddSwitchMaybe("plugin=protoc-gen-grpc", GrpcPluginExe); - cmd.AddSwitchMaybe("grpc_out", grpcOutDir); + cmd.AddSwitchMaybe("grpc_out", TrimEndSlash(GrpcOutputDir)); cmd.AddSwitchMaybe("grpc_opt", GrpcOutputOptions); if (ProtoPath != null) { @@ -439,18 +436,6 @@ namespace Grpc.Tools return cmd.ToString(); } - // If possible, disambiguate output dir by adding a hash of the proto file's path - static string MaybeEnhanceOutputDir(string outputDir, ITaskItem[] protobufs) - { - if (protobufs.Length != 1) - { - return outputDir; - } - - var protoFile = protobufs[0].ItemSpec; - return DepFileUtil.GetOutputDirWithHash(outputDir, protoFile); - } - // Protoc cannot digest trailing slashes in directory names, // curiously under Linux, but not in Windows. static string TrimEndSlash(string dir) diff --git a/src/csharp/Grpc.Tools/ProtoCompilerOutputs.cs b/src/csharp/Grpc.Tools/ProtoCompilerOutputs.cs index 24c0dd8482d..ea146ec090e 100644 --- a/src/csharp/Grpc.Tools/ProtoCompilerOutputs.cs +++ b/src/csharp/Grpc.Tools/ProtoCompilerOutputs.cs @@ -17,6 +17,7 @@ #endregion using System.Collections.Generic; +using System.IO; using Microsoft.Build.Framework; using Microsoft.Build.Utilities; @@ -40,6 +41,14 @@ namespace Grpc.Tools [Required] public ITaskItem[] Protobuf { get; set; } + /// + /// All Proto files in the project. A patched copy of all items from + /// Protobuf that might contain updated OutputDir and GrpcOutputDir + /// attributes. + /// + [Output] + public ITaskItem[] PatchedProtobuf { get; set; } + /// /// Output items per each potential output. We do not look at existing /// cached dependency even if they exist, since file may be refactored, @@ -68,8 +77,13 @@ namespace Grpc.Tools // Get language-specific possible output. The generator expects certain // metadata be set on the proto item. var possible = new List(); + var patched = new List(); foreach (var proto in Protobuf) { + // This operates on a local copy and has no effect on the MSBuild instance! + generator.PatchOutputDirectory(proto); + patched.Add(proto); + var outputs = generator.GetPossibleOutputs(proto); foreach (string output in outputs) { @@ -78,6 +92,8 @@ namespace Grpc.Tools possible.Add(ti); } } + + PatchedProtobuf = patched.ToArray(); PossibleOutputs = possible.ToArray(); return !Log.HasLoggedErrors; diff --git a/src/csharp/Grpc.Tools/build/_protobuf/Google.Protobuf.Tools.targets b/src/csharp/Grpc.Tools/build/_protobuf/Google.Protobuf.Tools.targets index 8b286593647..eaa228059dd 100644 --- a/src/csharp/Grpc.Tools/build/_protobuf/Google.Protobuf.Tools.targets +++ b/src/csharp/Grpc.Tools/build/_protobuf/Google.Protobuf.Tools.targets @@ -167,7 +167,13 @@ Protobuf="@(Protobuf_Compile)" Generator="$(Protobuf_Generator)"> + + + + + + - - <_Protobuf_ExpectedGenerated Include="@(Protobuf_ExpectedOutputs)" - Condition = " '%(Source)' != '' and '@(_Protobuf_OutOfDateProto)' != '' " /> - - + + From 18b4dd65621294756264c5693b50c11366164cfc Mon Sep 17 00:00:00 2001 From: "Kraemer, Benjamin" Date: Wed, 6 May 2020 15:10:38 +0200 Subject: [PATCH 07/12] Removed unused usings --- src/csharp/Grpc.Tools/Common.cs | 1 - src/csharp/Grpc.Tools/GeneratorServices.cs | 1 - src/csharp/Grpc.Tools/ProtoCompile.cs | 1 - src/csharp/Grpc.Tools/ProtoCompilerOutputs.cs | 1 - 4 files changed, 4 deletions(-) diff --git a/src/csharp/Grpc.Tools/Common.cs b/src/csharp/Grpc.Tools/Common.cs index 13cd6a32316..187ed9d84cb 100644 --- a/src/csharp/Grpc.Tools/Common.cs +++ b/src/csharp/Grpc.Tools/Common.cs @@ -19,7 +19,6 @@ using System; using System.IO; using System.Runtime.CompilerServices; -using System.Runtime.InteropServices; using System.Security; [assembly: InternalsVisibleTo("Grpc.Tools.Tests")] diff --git a/src/csharp/Grpc.Tools/GeneratorServices.cs b/src/csharp/Grpc.Tools/GeneratorServices.cs index af4b64c2479..0ade66972f1 100644 --- a/src/csharp/Grpc.Tools/GeneratorServices.cs +++ b/src/csharp/Grpc.Tools/GeneratorServices.cs @@ -16,7 +16,6 @@ #endregion -using System; using System.IO; using System.Text; using Microsoft.Build.Framework; diff --git a/src/csharp/Grpc.Tools/ProtoCompile.cs b/src/csharp/Grpc.Tools/ProtoCompile.cs index e1f01a554bc..68bd7ef2e02 100644 --- a/src/csharp/Grpc.Tools/ProtoCompile.cs +++ b/src/csharp/Grpc.Tools/ProtoCompile.cs @@ -18,7 +18,6 @@ using System; using System.Collections.Generic; -using System.IO; using System.Text; using System.Text.RegularExpressions; using Microsoft.Build.Framework; diff --git a/src/csharp/Grpc.Tools/ProtoCompilerOutputs.cs b/src/csharp/Grpc.Tools/ProtoCompilerOutputs.cs index ea146ec090e..3300b3ef0c7 100644 --- a/src/csharp/Grpc.Tools/ProtoCompilerOutputs.cs +++ b/src/csharp/Grpc.Tools/ProtoCompilerOutputs.cs @@ -17,7 +17,6 @@ #endregion using System.Collections.Generic; -using System.IO; using Microsoft.Build.Framework; using Microsoft.Build.Utilities; From 861cdf3d015d35bdd79ff730c9dd8c171cb497dc Mon Sep 17 00:00:00 2001 From: "Kraemer, Benjamin" Date: Wed, 6 May 2020 15:27:37 +0200 Subject: [PATCH 08/12] Reverted whitespace changes --- .../Grpc.Tools/build/_protobuf/Google.Protobuf.Tools.targets | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/csharp/Grpc.Tools/build/_protobuf/Google.Protobuf.Tools.targets b/src/csharp/Grpc.Tools/build/_protobuf/Google.Protobuf.Tools.targets index eaa228059dd..db299b29ff0 100644 --- a/src/csharp/Grpc.Tools/build/_protobuf/Google.Protobuf.Tools.targets +++ b/src/csharp/Grpc.Tools/build/_protobuf/Google.Protobuf.Tools.targets @@ -265,8 +265,8 @@ Condition=" '$(DisableProtobufDesignTimeBuild)' != 'true' " DependsOnTargets="Protobuf_PrepareCompileOptions;_Protobuf_GatherStaleFiles"> - - + + From d0c7eac97b4c34ced3703c88abeb22120c4361dc Mon Sep 17 00:00:00 2001 From: "Kraemer, Benjamin" Date: Wed, 6 May 2020 15:56:28 +0200 Subject: [PATCH 09/12] Readded InteropServices Needed #if NETCORE --- src/csharp/Grpc.Tools/Common.cs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/csharp/Grpc.Tools/Common.cs b/src/csharp/Grpc.Tools/Common.cs index 187ed9d84cb..13cd6a32316 100644 --- a/src/csharp/Grpc.Tools/Common.cs +++ b/src/csharp/Grpc.Tools/Common.cs @@ -19,6 +19,7 @@ using System; using System.IO; using System.Runtime.CompilerServices; +using System.Runtime.InteropServices; using System.Security; [assembly: InternalsVisibleTo("Grpc.Tools.Tests")] From 446f72d7a35b81c499e4a222f9ee1f5538e35d3a Mon Sep 17 00:00:00 2001 From: "Kraemer, Benjamin" Date: Tue, 16 Jun 2020 12:31:32 +0200 Subject: [PATCH 10/12] Bring closer to master --- src/csharp/Grpc.Tools/DepFileUtil.cs | 112 ++++++--------------- src/csharp/Grpc.Tools/GeneratorServices.cs | 54 +++++++++- 2 files changed, 82 insertions(+), 84 deletions(-) diff --git a/src/csharp/Grpc.Tools/DepFileUtil.cs b/src/csharp/Grpc.Tools/DepFileUtil.cs index 22fc2d00c24..c17f4553fcd 100644 --- a/src/csharp/Grpc.Tools/DepFileUtil.cs +++ b/src/csharp/Grpc.Tools/DepFileUtil.cs @@ -138,39 +138,6 @@ namespace Grpc.Tools return result.ToArray(); } - /// - /// Construct the directory hash from a relative file name - /// - /// Relative path to the proto item, e. g. "foo/file.proto" - /// - /// Directory hash based on the file name, e. g. "deadbeef12345678" - /// - /// - /// Since a project may contain proto files with the same filename but in different - /// directories, a unique directory for the generated files is constructed based on the - /// proto file names directory. The directory path can be arbitrary, for example, - /// it can be outside of the project, or an absolute path including a drive letter, - /// or a UNC network path. A name constructed from such a path by, for example, - /// replacing disallowed name characters with an underscore, may well be over - /// filesystem's allowed path length, since it will be located under the project - /// and solution directories, which are also some level deep from the root. - /// Instead of creating long and unwieldy names for these proto sources, we cache - /// the full path of the name without the filename, as in e. g. "foo/file.proto" - /// will yield the name "deadbeef12345678", where that is a presumed hash value - /// of the string "foo". This allows the path to be short, unique (up to a hash - /// collision), and still allowing the user to guess their provenance. - /// - private static string GetDirectoryHash(string proto) - { - string dirname = Path.GetDirectoryName(proto); - if (Platform.IsFsCaseInsensitive) - { - dirname = dirname.ToLowerInvariant(); - } - - return HashString64Hex(dirname); - } - /// /// Construct relative dependency file name from directory hash and file name /// @@ -207,6 +174,39 @@ namespace Grpc.Tools return Path.Combine(outputDir, dirhash); } + /// + /// Construct the directory hash from a relative file name + /// + /// Relative path to the proto item, e. g. "foo/file.proto" + /// + /// Directory hash based on the file name, e. g. "deadbeef12345678" + /// + /// + /// Since a project may contain proto files with the same filename but in different + /// directories, a unique directory for the generated files is constructed based on the + /// proto file names directory. The directory path can be arbitrary, for example, + /// it can be outside of the project, or an absolute path including a drive letter, + /// or a UNC network path. A name constructed from such a path by, for example, + /// replacing disallowed name characters with an underscore, may well be over + /// filesystem's allowed path length, since it will be located under the project + /// and solution directories, which are also some level deep from the root. + /// Instead of creating long and unwieldy names for these proto sources, we cache + /// the full path of the name without the filename, as in e. g. "foo/file.proto" + /// will yield the name "deadbeef12345678", where that is a presumed hash value + /// of the string "foo". This allows the path to be short, unique (up to a hash + /// collision), and still allowing the user to guess their provenance. + /// + private static string GetDirectoryHash(string proto) + { + string dirname = Path.GetDirectoryName(proto); + if (Platform.IsFsCaseInsensitive) + { + dirname = dirname.ToLowerInvariant(); + } + + return HashString64Hex(dirname); + } + // Get a 64-bit hash for a directory string. We treat it as if it were // unique, since there are not so many distinct proto paths in a project. // We take the first 64 bit of the string SHA1. @@ -301,51 +301,5 @@ namespace Grpc.Tools return new string[0]; } } - - // Calculate part of proto path relative to root. Protoc is very picky - // about them matching exactly, so can be we. Expect root be exact prefix - // to proto, minus some slash normalization. - internal static string GetRelativeDir(string root, string proto, TaskLoggingHelper log) - { - string protoDir = Path.GetDirectoryName(proto); - string rootDir = EndWithSlash(Path.GetDirectoryName(EndWithSlash(root))); - if (rootDir == s_dotSlash) - { - // Special case, otherwise we can return "./" instead of "" below! - return protoDir; - } - if (Platform.IsFsCaseInsensitive) - { - protoDir = protoDir.ToLowerInvariant(); - rootDir = rootDir.ToLowerInvariant(); - } - protoDir = EndWithSlash(protoDir); - if (!protoDir.StartsWith(rootDir)) - { - log.LogWarning("Protobuf item '{0}' has the ProtoRoot metadata '{1}' " + - "which is not prefix to its path. Cannot compute relative path.", - proto, root); - return ""; - } - return protoDir.Substring(rootDir.Length); - } - - // './' or '.\', normalized per system. - internal static string s_dotSlash = "." + Path.DirectorySeparatorChar; - - internal static string EndWithSlash(string str) - { - if (str == "") - { - return s_dotSlash; - } - - if (str[str.Length - 1] != '\\' && str[str.Length - 1] != '/') - { - return str + Path.DirectorySeparatorChar; - } - - return str; - } }; } diff --git a/src/csharp/Grpc.Tools/GeneratorServices.cs b/src/csharp/Grpc.Tools/GeneratorServices.cs index 0ade66972f1..efa67e02ab0 100644 --- a/src/csharp/Grpc.Tools/GeneratorServices.cs +++ b/src/csharp/Grpc.Tools/GeneratorServices.cs @@ -63,6 +63,52 @@ namespace Grpc.Tools } public abstract string[] GetPossibleOutputs(ITaskItem protoItem); + + // Calculate part of proto path relative to root. Protoc is very picky + // about them matching exactly, so can be we. Expect root be exact prefix + // to proto, minus some slash normalization. + protected static string GetRelativeDir(string root, string proto, TaskLoggingHelper log) + { + string protoDir = Path.GetDirectoryName(proto); + string rootDir = EndWithSlash(Path.GetDirectoryName(EndWithSlash(root))); + if (rootDir == s_dotSlash) + { + // Special case, otherwise we can return "./" instead of "" below! + return protoDir; + } + if (Platform.IsFsCaseInsensitive) + { + protoDir = protoDir.ToLowerInvariant(); + rootDir = rootDir.ToLowerInvariant(); + } + protoDir = EndWithSlash(protoDir); + if (!protoDir.StartsWith(rootDir)) + { + log.LogWarning("Protobuf item '{0}' has the ProtoRoot metadata '{1}' " + + "which is not prefix to its path. Cannot compute relative path.", + proto, root); + return ""; + } + return protoDir.Substring(rootDir.Length); + } + + // './' or '.\', normalized per system. + protected static string s_dotSlash = "." + Path.DirectorySeparatorChar; + + protected static string EndWithSlash(string str) + { + if (str == "") + { + return s_dotSlash; + } + + if (str[str.Length - 1] != '\\' && str[str.Length - 1] != '/') + { + return str + Path.DirectorySeparatorChar; + } + + return str; + } }; // C# generator services. @@ -74,7 +120,7 @@ namespace Grpc.Tools { string root = protoItem.GetMetadata(Metadata.ProtoRoot); string proto = protoItem.ItemSpec; - string relative = DepFileUtil.GetRelativeDir(root, proto, Log); + string relative = GetRelativeDir(root, proto, Log); string outdir = protoItem.GetMetadata(Metadata.OutputDir); string pathStem = Path.Combine(outdir, relative); @@ -92,12 +138,10 @@ namespace Grpc.Tools public override string[] GetPossibleOutputs(ITaskItem protoItem) { bool doGrpc = GrpcOutputPossible(protoItem); + var outputs = new string[doGrpc ? 2 : 1]; string proto = protoItem.ItemSpec; string basename = Path.GetFileNameWithoutExtension(proto); - - var outputs = new string[doGrpc ? 2 : 1]; string outdir = protoItem.GetMetadata(Metadata.OutputDir); - string filename = LowerUnderscoreToUpperCamelProtocWay(basename); outputs[0] = Path.Combine(outdir, filename) + ".cs"; @@ -168,7 +212,7 @@ namespace Grpc.Tools string proto = protoItem.ItemSpec; string filename = Path.GetFileNameWithoutExtension(proto); // E. g., ("foo/", "foo/bar/x.proto") => "bar" - string relative = DepFileUtil.GetRelativeDir(root, proto, Log); + string relative = GetRelativeDir(root, proto, Log); var outputs = new string[doGrpc ? 4 : 2]; string outdir = protoItem.GetMetadata(Metadata.OutputDir); From e165a33048e2dcb0a122bfddc7f2082133889e72 Mon Sep 17 00:00:00 2001 From: "Kraemer, Benjamin" Date: Tue, 16 Jun 2020 14:13:44 +0200 Subject: [PATCH 11/12] Add support for Protobuf_ProtoRoot --- .../Grpc.Tools.Tests/CSharpGeneratorTest.cs | 4 ++-- src/csharp/Grpc.Tools/GeneratorServices.cs | 19 +++++++++++-------- src/csharp/Grpc.Tools/ProtoCompilerOutputs.cs | 9 ++++----- .../_protobuf/Google.Protobuf.Tools.targets | 1 + 4 files changed, 18 insertions(+), 15 deletions(-) diff --git a/src/csharp/Grpc.Tools.Tests/CSharpGeneratorTest.cs b/src/csharp/Grpc.Tools.Tests/CSharpGeneratorTest.cs index 642ebf92cef..c53f0bdf8d4 100644 --- a/src/csharp/Grpc.Tools.Tests/CSharpGeneratorTest.cs +++ b/src/csharp/Grpc.Tools.Tests/CSharpGeneratorTest.cs @@ -89,8 +89,8 @@ namespace Grpc.Tools.Tests public void OutputDirPatched() { var item = Utils.MakeItem("sub/foo.proto", "OutputDir", "out"); - _generator.PatchOutputDirectory(item); - var poss = _generator.GetPossibleOutputs(item); + var output = _generator.PatchOutputDirectory(item); + var poss = _generator.GetPossibleOutputs(output); Assert.AreEqual(1, poss.Length); Assert.That(poss[0], Is.EqualTo("out/sub/Foo.cs") | Is.EqualTo("out\\sub\\Foo.cs")); } diff --git a/src/csharp/Grpc.Tools/GeneratorServices.cs b/src/csharp/Grpc.Tools/GeneratorServices.cs index efa67e02ab0..0608bb9d54d 100644 --- a/src/csharp/Grpc.Tools/GeneratorServices.cs +++ b/src/csharp/Grpc.Tools/GeneratorServices.cs @@ -57,9 +57,10 @@ namespace Grpc.Tools // Update OutputDir and GrpcOutputDir for the item and all subsequent // targets using this item. This should only be done if the real // output directories for protoc should be modified. - public virtual void PatchOutputDirectory(ITaskItem protoItem) + public virtual ITaskItem PatchOutputDirectory(ITaskItem protoItem) { // Nothing to do + return protoItem; } public abstract string[] GetPossibleOutputs(ITaskItem protoItem); @@ -116,23 +117,25 @@ namespace Grpc.Tools { public CSharpGeneratorServices(TaskLoggingHelper log) : base(log) { } - public override void PatchOutputDirectory(ITaskItem protoItem) + public override ITaskItem PatchOutputDirectory(ITaskItem protoItem) { - string root = protoItem.GetMetadata(Metadata.ProtoRoot); - string proto = protoItem.ItemSpec; + var outputItem = new TaskItem(protoItem); + string root = outputItem.GetMetadata(Metadata.ProtoRoot); + string proto = outputItem.ItemSpec; string relative = GetRelativeDir(root, proto, Log); - string outdir = protoItem.GetMetadata(Metadata.OutputDir); + string outdir = outputItem.GetMetadata(Metadata.OutputDir); string pathStem = Path.Combine(outdir, relative); - protoItem.SetMetadata(Metadata.OutputDir, pathStem); + outputItem.SetMetadata(Metadata.OutputDir, pathStem); // Override outdir if GrpcOutputDir present, default to proto output. - string grpcdir = protoItem.GetMetadata(Metadata.GrpcOutputDir); + string grpcdir = outputItem.GetMetadata(Metadata.GrpcOutputDir); if (grpcdir != "") { pathStem = Path.Combine(grpcdir, relative); } - protoItem.SetMetadata(Metadata.GrpcOutputDir, pathStem); + outputItem.SetMetadata(Metadata.GrpcOutputDir, pathStem); + return outputItem; } public override string[] GetPossibleOutputs(ITaskItem protoItem) diff --git a/src/csharp/Grpc.Tools/ProtoCompilerOutputs.cs b/src/csharp/Grpc.Tools/ProtoCompilerOutputs.cs index 3300b3ef0c7..0bd9c86a224 100644 --- a/src/csharp/Grpc.Tools/ProtoCompilerOutputs.cs +++ b/src/csharp/Grpc.Tools/ProtoCompilerOutputs.cs @@ -79,15 +79,14 @@ namespace Grpc.Tools var patched = new List(); foreach (var proto in Protobuf) { - // This operates on a local copy and has no effect on the MSBuild instance! - generator.PatchOutputDirectory(proto); - patched.Add(proto); + var patchedProto = generator.PatchOutputDirectory(proto); + patched.Add(patchedProto); - var outputs = generator.GetPossibleOutputs(proto); + var outputs = generator.GetPossibleOutputs(patchedProto); foreach (string output in outputs) { var ti = new TaskItem(output); - ti.SetMetadata(Metadata.Source, proto.ItemSpec); + ti.SetMetadata(Metadata.Source, patchedProto.ItemSpec); possible.Add(ti); } } diff --git a/src/csharp/Grpc.Tools/build/_protobuf/Google.Protobuf.Tools.targets b/src/csharp/Grpc.Tools/build/_protobuf/Google.Protobuf.Tools.targets index db299b29ff0..fa1cf8f9673 100644 --- a/src/csharp/Grpc.Tools/build/_protobuf/Google.Protobuf.Tools.targets +++ b/src/csharp/Grpc.Tools/build/_protobuf/Google.Protobuf.Tools.targets @@ -135,6 +135,7 @@ %(RelativeDir) + $(Protobuf_ProtoRoot) From 91e44c21bca4d08e531a50a8c880fe1fbc1ee1f6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Benjamin=20Kr=C3=A4mer?= Date: Tue, 23 Jun 2020 09:22:32 +0200 Subject: [PATCH 12/12] Fix whitespaces --- .../build/_protobuf/Google.Protobuf.Tools.targets | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/csharp/Grpc.Tools/build/_protobuf/Google.Protobuf.Tools.targets b/src/csharp/Grpc.Tools/build/_protobuf/Google.Protobuf.Tools.targets index fa1cf8f9673..fb7dc5fa03d 100644 --- a/src/csharp/Grpc.Tools/build/_protobuf/Google.Protobuf.Tools.targets +++ b/src/csharp/Grpc.Tools/build/_protobuf/Google.Protobuf.Tools.targets @@ -135,7 +135,7 @@ %(RelativeDir) - $(Protobuf_ProtoRoot) + $(Protobuf_ProtoRoot) @@ -170,11 +170,11 @@ - - + + - +