diff options
author | elenairina <elenairina@google.com> | 2018-02-20 06:26:33 -0800 |
---|---|---|
committer | Copybara-Service <copybara-piper@google.com> | 2018-02-20 06:27:57 -0800 |
commit | 5deca4cf88f5568771f2c836a9b8c693b88bd749 (patch) | |
tree | 1ab038e4bf10301e0e8878c21e9693091b78e304 /src/main/java/com/google/devtools/build/lib/rules/proto | |
parent | 9ac10696da052d1327b3f1cd276b2ab50fe1fee1 (diff) |
Accept proto paths relative to proto_source_root as direct dependencies.
This will make protoc see as direct dependencies the .proto files that were included using the proto_source_root flag.
Until now, Bazel passed to protoc the direct dependencies of a target as the path relative to the WORKSPACE, which made it fail when a shorter path, relative to the package was used.
Progress on #4544.
RELNOTES: None.
PiperOrigin-RevId: 186294997
Diffstat (limited to 'src/main/java/com/google/devtools/build/lib/rules/proto')
5 files changed, 107 insertions, 19 deletions
diff --git a/src/main/java/com/google/devtools/build/lib/rules/proto/BazelProtoLibrary.java b/src/main/java/com/google/devtools/build/lib/rules/proto/BazelProtoLibrary.java index d1cd914016..a4fd57525b 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/proto/BazelProtoLibrary.java +++ b/src/main/java/com/google/devtools/build/lib/rules/proto/BazelProtoLibrary.java @@ -44,10 +44,14 @@ public class BazelProtoLibrary implements RuleConfiguredTargetFactory { NestedSet<Artifact> transitiveImports = ProtoCommon.collectTransitiveImports(ruleContext, protoSources); - NestedSet<String> protoPathFlags = ProtoCommon.collectTransitiveProtoPathFlags(ruleContext); - NestedSet<Artifact> protosInDirectDeps = ProtoCommon.computeProtosInDirectDeps(ruleContext); + String protoSourceRoot = ProtoCommon.getProtoSourceRoot(ruleContext); + NestedSet<String> directProtoSourceRoots = + ProtoCommon.getProtoSourceRootsOfDirectDependencies(ruleContext, protoSourceRoot); + NestedSet<String> protoPathFlags = + ProtoCommon.collectTransitiveProtoPathFlags(ruleContext, protoSourceRoot); + final SupportData supportData = SupportData.create( Predicates.<TransitiveInfoCollection>alwaysTrue() /* nonWeakDepsPredicate */, @@ -55,6 +59,7 @@ public class BazelProtoLibrary implements RuleConfiguredTargetFactory { protosInDirectDeps, transitiveImports, protoPathFlags, + directProtoSourceRoots, !protoSources.isEmpty()); Artifact descriptorSetOutput = @@ -74,7 +79,8 @@ public class BazelProtoLibrary implements RuleConfiguredTargetFactory { descriptorSetOutput, true /* allowServices */, dependenciesDescriptorSets, - protoPathFlags); + protoPathFlags, + directProtoSourceRoots); Runfiles dataRunfiles = ProtoCommon.createDataRunfilesProvider(transitiveImports, ruleContext) @@ -90,7 +96,8 @@ public class BazelProtoLibrary implements RuleConfiguredTargetFactory { checkDepsProtoSources, descriptorSetOutput, transitiveDescriptorSetOutput, - protoPathFlags); + protoPathFlags, + protoSourceRoot); return new RuleConfiguredTargetBuilder(ruleContext) .setFilesToBuild(NestedSetBuilder.create(STABLE_ORDER, descriptorSetOutput)) diff --git a/src/main/java/com/google/devtools/build/lib/rules/proto/ProtoCommon.java b/src/main/java/com/google/devtools/build/lib/rules/proto/ProtoCommon.java index 374b534cd9..2b6f88e2e6 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/proto/ProtoCommon.java +++ b/src/main/java/com/google/devtools/build/lib/rules/proto/ProtoCommon.java @@ -100,29 +100,66 @@ public class ProtoCommon { } /** - * Returns all proto source roots in this lib and in its transitive dependencies, each prefixed - * by {@code --proto_path}. + * Returns all proto source roots in this lib ({@code currentProtoSourceRoot}) and in its + * transitive dependencies, each prefixed by {@code --proto_path}. * - * Build will fail if the {@code proto_source_root} of the current lib is different than the - * package name. + * Assumes {@code currentProtoSourceRoot} is the same as the package name. */ - public static NestedSet<String> collectTransitiveProtoPathFlags(RuleContext ruleContext) { + public static NestedSet<String> collectTransitiveProtoPathFlags( + RuleContext ruleContext, String currentProtoSourceRoot) { NestedSetBuilder<String> protoPathFlags = NestedSetBuilder.stableOrder(); // first add the protoSourceRoot of the current target, if any + if (currentProtoSourceRoot != null && !currentProtoSourceRoot.isEmpty()) { + protoPathFlags.add("--proto_path=" + currentProtoSourceRoot); + } + + for (ProtoSourcesProvider provider : ruleContext.getPrerequisites( + "deps", Mode.TARGET, ProtoSourcesProvider.class)) { + protoPathFlags.addTransitive(provider.getTransitiveProtoPathFlags()); + } + + return protoPathFlags.build(); + } + + /** + * Returns the {@code proto_source_root} of the current library or null if none is specified. + * + * Build will fail if the {@code proto_source_root} of the current library is different than the + * package name. + */ + @Nullable + public static String getProtoSourceRoot(RuleContext ruleContext) { String protoSourceRoot = ruleContext.attributes().get("proto_source_root", Type.STRING); if (protoSourceRoot != null && !protoSourceRoot.isEmpty()) { checkProtoSourceRootIsTheSameAsPackage(protoSourceRoot, ruleContext); - protoPathFlags.add("--proto_path=" + protoSourceRoot); + } + return protoSourceRoot; + } + + /** + * Returns a set of the {@code proto_source_root} collected from the current library and the + * direct dependencies. + * + * Assumes {@code currentProtoSourceRoot} is the same as the package name. + */ + public static NestedSet<String> getProtoSourceRootsOfDirectDependencies( + RuleContext ruleContext, String currentProtoSourceRoot) { + NestedSetBuilder<String> protoSourceRoots = NestedSetBuilder.stableOrder(); + if (currentProtoSourceRoot != null && !currentProtoSourceRoot.isEmpty()) { + protoSourceRoots.add(currentProtoSourceRoot); } for (ProtoSourcesProvider provider : ruleContext.getPrerequisites( - "deps", Mode.TARGET, ProtoSourcesProvider.class)) { - protoPathFlags.addTransitive(provider.getTransitiveProtoPathFlags()); + "deps", Mode.TARGET, ProtoSourcesProvider.class)) { + String protoSourceRoot = provider.getProtoSourceRoot(); + if (protoSourceRoot != null && !protoSourceRoot.isEmpty()) { + protoSourceRoots.add(provider.getProtoSourceRoot()); + } } - return protoPathFlags.build(); + return protoSourceRoots.build(); } private static void checkProtoSourceRootIsTheSameAsPackage( diff --git a/src/main/java/com/google/devtools/build/lib/rules/proto/ProtoCompileActionBuilder.java b/src/main/java/com/google/devtools/build/lib/rules/proto/ProtoCompileActionBuilder.java index 374bf9b9fe..015c02c3f0 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/proto/ProtoCompileActionBuilder.java +++ b/src/main/java/com/google/devtools/build/lib/rules/proto/ProtoCompileActionBuilder.java @@ -322,6 +322,7 @@ public class ProtoCompileActionBuilder { addIncludeMapArguments( result, areDepsStrict ? supportData.getProtosInDirectDeps() : null, + supportData.getDirectProtoSourceRoots(), supportData.getTransitiveImports()); if (areDepsStrict) { @@ -360,7 +361,8 @@ public class ProtoCompileActionBuilder { Artifact output, boolean allowServices, NestedSet<Artifact> transitiveDescriptorSets, - NestedSet<String> protoSourceRoots) { + NestedSet<String> protoSourceRoots, + NestedSet<String> directProtoSourceRoots) { if (protosToCompile.isEmpty()) { ruleContext.registerAction( FileWriteAction.createEmptyWithInputs( @@ -376,6 +378,7 @@ public class ProtoCompileActionBuilder { transitiveSources, protosInDirectDeps, protoSourceRoots, + directProtoSourceRoots, ruleContext.getLabel(), ImmutableList.of(output), "Descriptor Set", @@ -423,6 +426,7 @@ public class ProtoCompileActionBuilder { NestedSet<Artifact> transitiveSources, NestedSet<Artifact> protosInDirectDeps, NestedSet<String> protoSourceRoots, + NestedSet<String> directProtoSourceRoots, Label ruleLabel, Iterable<Artifact> outputs, String flavorName, @@ -435,6 +439,7 @@ public class ProtoCompileActionBuilder { transitiveSources, protosInDirectDeps, protoSourceRoots, + directProtoSourceRoots, ruleLabel, outputs, flavorName, @@ -452,6 +457,7 @@ public class ProtoCompileActionBuilder { NestedSet<Artifact> transitiveSources, @Nullable NestedSet<Artifact> protosInDirectDeps, NestedSet<String> protoSourceRoots, + NestedSet<String> directProtoSourceRoots, Label ruleLabel, Iterable<Artifact> outputs, String flavorName, @@ -487,6 +493,7 @@ public class ProtoCompileActionBuilder { protosToCompile, transitiveSources, protoSourceRoots, + directProtoSourceRoots, areDepsStrict(ruleContext) ? protosInDirectDeps : null, ruleLabel, allowServices, @@ -524,11 +531,13 @@ public class ProtoCompileActionBuilder { Iterable<Artifact> protosToCompile, NestedSet<Artifact> transitiveSources, NestedSet<String> transitiveProtoPathFlags, + NestedSet<String> directProtoSourceRoots, @Nullable NestedSet<Artifact> protosInDirectDeps, Label ruleLabel, boolean allowServices, ImmutableList<String> protocOpts) { CustomCommandLine.Builder cmdLine = CustomCommandLine.builder(); + cmdLine.addAll(transitiveProtoPathFlags); cmdLine.addAll(transitiveProtoPathFlags); @@ -565,7 +574,7 @@ public class ProtoCompileActionBuilder { cmdLine.addAll(protocOpts); // Add include maps - addIncludeMapArguments(cmdLine, protosInDirectDeps, transitiveSources); + addIncludeMapArguments(cmdLine, protosInDirectDeps, directProtoSourceRoots, transitiveSources); if (protosInDirectDeps != null) { cmdLine.addFormatted(STRICT_DEPS_FLAG_TEMPLATE, ruleLabel); @@ -586,6 +595,7 @@ public class ProtoCompileActionBuilder { static void addIncludeMapArguments( CustomCommandLine.Builder commandLine, @Nullable NestedSet<Artifact> protosInDirectDependencies, + NestedSet<String> directProtoSourceRoots, NestedSet<Artifact> transitiveImports) { commandLine.addAll( VectorArg.of(transitiveImports) @@ -596,7 +606,14 @@ public class ProtoCompileActionBuilder { "--direct_dependencies", VectorArg.join(":") .each(protosInDirectDependencies) - .mapped(ProtoCompileActionBuilder::expandToPathIgnoringRepository)); + .mapped((Artifact proto, Consumer<String> args) -> { + for (String directProtoSourceRoot : directProtoSourceRoots) { + expandToPathIgnoringSourceRoot(proto, directProtoSourceRoot, args); + } + expandToPathIgnoringRepository(proto, args); + }) + ); + } else { // The proto compiler requires an empty list to turn on strict deps checking commandLine.add("--direct_dependencies="); @@ -627,6 +644,20 @@ public class ProtoCompileActionBuilder { .toString(); } + private static void expandToPathIgnoringSourceRoot( + Artifact artifact, String directProtoSourceRoot, Consumer<String> args) { + try { + String relativePath = artifact.getRootRelativePath() + .relativeTo( + artifact.getOwnerLabel().getPackageIdentifier().getRepository().getPathUnderExecRoot()) + .relativeTo(directProtoSourceRoot) + .toString(); + args.accept(relativePath); + } catch (IllegalArgumentException exception) { + // do nothing + } + } + /** * Describes a toolchain and the value to replace for a $(OUT) that might appear in its * commandLine() (e.g., "bazel-out/foo.srcjar"). diff --git a/src/main/java/com/google/devtools/build/lib/rules/proto/ProtoSourcesProvider.java b/src/main/java/com/google/devtools/build/lib/rules/proto/ProtoSourcesProvider.java index bf2ed12b3b..25ea43b090 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/proto/ProtoSourcesProvider.java +++ b/src/main/java/com/google/devtools/build/lib/rules/proto/ProtoSourcesProvider.java @@ -42,7 +42,8 @@ public abstract class ProtoSourcesProvider implements TransitiveInfoProvider { NestedSet<Artifact> checkDepsProtoSources, Artifact directDescriptorSet, NestedSet<Artifact> transitiveDescriptorSets, - NestedSet<String> protoPathFlags) { + NestedSet<String> protoPathFlags, + String protoSourceRoot) { return new AutoValue_ProtoSourcesProvider( transitiveImports, transitiveProtoSources, @@ -50,7 +51,8 @@ public abstract class ProtoSourcesProvider implements TransitiveInfoProvider { checkDepsProtoSources, directDescriptorSet, transitiveDescriptorSets, - protoPathFlags); + protoPathFlags, + protoSourceRoot); } /** @@ -135,5 +137,10 @@ public abstract class ProtoSourcesProvider implements TransitiveInfoProvider { */ public abstract NestedSet<String> getTransitiveProtoPathFlags(); + /** + * The {@code proto_source_root} of the current library. + */ + public abstract String getProtoSourceRoot(); + ProtoSourcesProvider() {} } diff --git a/src/main/java/com/google/devtools/build/lib/rules/proto/SupportData.java b/src/main/java/com/google/devtools/build/lib/rules/proto/SupportData.java index 9e02d190d5..ad2c09c3ee 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/proto/SupportData.java +++ b/src/main/java/com/google/devtools/build/lib/rules/proto/SupportData.java @@ -34,10 +34,11 @@ public abstract class SupportData { NestedSet<Artifact> protosInDirectDeps, NestedSet<Artifact> transitiveImports, NestedSet<String> transitiveProtoPathFlags, + NestedSet<String> directProtoSourceRoots, boolean hasProtoSources) { return new AutoValue_SupportData( nonWeakDepsPredicate, protoSources, transitiveImports, protosInDirectDeps, - transitiveProtoPathFlags, hasProtoSources); + transitiveProtoPathFlags, directProtoSourceRoots, hasProtoSources); } public abstract Predicate<TransitiveInfoCollection> getNonWeakDepsPredicate(); @@ -57,6 +58,11 @@ public abstract class SupportData { */ public abstract NestedSet<String> getTransitiveProtoPathFlags(); + /** + * The {@code proto_source_root}'s collected from the current library and the direct dependencies. + */ + public abstract NestedSet<String> getDirectProtoSourceRoots(); + public abstract boolean hasProtoSources(); SupportData() {} |