diff options
Diffstat (limited to 'src')
12 files changed, 572 insertions, 19 deletions
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/proto/CcProtoAspect.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/proto/CcProtoAspect.java index 17f5ac8842..f48e1de8f9 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/proto/CcProtoAspect.java +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/proto/CcProtoAspect.java @@ -343,6 +343,7 @@ public abstract class CcProtoAspect extends NativeAspectClass implements Configu supportData.getTransitiveImports(), supportData.getProtosInDirectDeps(), supportData.getTransitiveProtoPathFlags(), + supportData.getDirectProtoSourceRoots(), ruleContext.getLabel(), outputs, "C++", diff --git a/src/main/java/com/google/devtools/build/lib/rules/java/proto/JavaLiteProtoAspect.java b/src/main/java/com/google/devtools/build/lib/rules/java/proto/JavaLiteProtoAspect.java index 9e5b521b16..1de36b18e1 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/java/proto/JavaLiteProtoAspect.java +++ b/src/main/java/com/google/devtools/build/lib/rules/java/proto/JavaLiteProtoAspect.java @@ -252,6 +252,7 @@ public class JavaLiteProtoAspect extends NativeAspectClass implements Configured supportData.getTransitiveImports(), supportData.getProtosInDirectDeps(), supportData.getTransitiveProtoPathFlags(), + supportData.getDirectProtoSourceRoots(), ruleContext.getLabel(), ImmutableList.of(sourceJar), "JavaLite", diff --git a/src/main/java/com/google/devtools/build/lib/rules/java/proto/JavaProtoAspect.java b/src/main/java/com/google/devtools/build/lib/rules/java/proto/JavaProtoAspect.java index 3c64f91a7d..2df7c12d15 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/java/proto/JavaProtoAspect.java +++ b/src/main/java/com/google/devtools/build/lib/rules/java/proto/JavaProtoAspect.java @@ -281,6 +281,7 @@ public class JavaProtoAspect extends NativeAspectClass implements ConfiguredAspe supportData.getTransitiveImports(), supportData.getProtosInDirectDeps(), supportData.getTransitiveProtoPathFlags(), + supportData.getDirectProtoSourceRoots(), ruleContext.getLabel(), ImmutableList.of(sourceJar), "Java (Immutable)", diff --git a/src/main/java/com/google/devtools/build/lib/rules/java/proto/JavaProtoSkylarkCommon.java b/src/main/java/com/google/devtools/build/lib/rules/java/proto/JavaProtoSkylarkCommon.java index cbd866a72f..e658fdf447 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/java/proto/JavaProtoSkylarkCommon.java +++ b/src/main/java/com/google/devtools/build/lib/rules/java/proto/JavaProtoSkylarkCommon.java @@ -56,6 +56,7 @@ public class JavaProtoSkylarkCommon supportData.getTransitiveImports(), supportData.getProtosInDirectDeps(), supportData.getTransitiveProtoPathFlags(), + supportData.getDirectProtoSourceRoots(), skylarkRuleContext.getLabel(), ImmutableList.of(sourceJar), "JavaLite", diff --git a/src/main/java/com/google/devtools/build/lib/rules/objc/J2ObjcAspect.java b/src/main/java/com/google/devtools/build/lib/rules/objc/J2ObjcAspect.java index 1d19107f67..f0b5a15081 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/objc/J2ObjcAspect.java +++ b/src/main/java/com/google/devtools/build/lib/rules/objc/J2ObjcAspect.java @@ -640,6 +640,7 @@ public class J2ObjcAspect extends NativeAspectClass implements ConfiguredAspectF supportData.getTransitiveImports(), supportData.getProtosInDirectDeps(), supportData.getTransitiveProtoPathFlags(), + supportData.getDirectProtoSourceRoots(), ruleContext.getLabel(), outputs, "j2objc", 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 e3687be773..1c8a7d52cb 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 @@ -45,10 +45,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( /* nonWeakDepsPredicate= */ Predicates.<TransitiveInfoCollection>alwaysTrue(), @@ -56,6 +60,7 @@ public class BazelProtoLibrary implements RuleConfiguredTargetFactory { protosInDirectDeps, transitiveImports, protoPathFlags, + directProtoSourceRoots, !protoSources.isEmpty()); Artifact descriptorSetOutput = @@ -75,7 +80,8 @@ public class BazelProtoLibrary implements RuleConfiguredTargetFactory { descriptorSetOutput, /* allowServices= */ true, dependenciesDescriptorSets, - protoPathFlags); + protoPathFlags, + directProtoSourceRoots); Runfiles dataRunfiles = ProtoCommon.createDataRunfilesProvider(transitiveImports, ruleContext) @@ -91,7 +97,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 a86bdda6dc..f3cc3d6751 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,28 +100,66 @@ public class ProtoCommon { } /** - * Returns all proto source roots in this lib and in its transitive dependencies. + * Returns all proto source roots in this lib ({@code currentProtoSourceRoot}) and in its + * transitive dependencies. * - * Build will fail if the {@code proto_source_root} of the current lib is different than the - * package name. + * <p>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> protoPath = NestedSetBuilder.stableOrder(); // first add the protoSourceRoot of the current target, if any + if (currentProtoSourceRoot != null && !currentProtoSourceRoot.isEmpty()) { + protoPath.add(currentProtoSourceRoot); + } + + for (ProtoSourcesProvider provider : + ruleContext.getPrerequisites("deps", Mode.TARGET, ProtoSourcesProvider.class)) { + protoPath.addTransitive(provider.getTransitiveProtoPathFlags()); + } + + return protoPath.build(); + } + + /** + * Returns the {@code proto_source_root} of the current library or null if none is specified. + * + * <p>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); - protoPath.add(protoSourceRoot); } + return protoSourceRoot; + } - for (ProtoSourcesProvider provider : ruleContext.getPrerequisites( - "deps", Mode.TARGET, ProtoSourcesProvider.class)) { - protoPath.addTransitive(provider.getTransitiveProtoPathFlags()); + /** + * Returns a set of the {@code proto_source_root} collected from the current library and the + * direct dependencies. + * + * <p>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); } - return protoPath.build(); + for (ProtoSourcesProvider provider : + ruleContext.getPrerequisites("deps", Mode.TARGET, ProtoSourcesProvider.class)) { + String protoSourceRoot = provider.getProtoSourceRoot(); + if (protoSourceRoot != null && !protoSourceRoot.isEmpty()) { + protoSourceRoots.add(provider.getProtoSourceRoot()); + } + } + + 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 808c2a309a..ed138d0381 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 @@ -28,6 +28,7 @@ import com.google.common.collect.ImmutableMap; import com.google.devtools.build.lib.actions.Action; import com.google.devtools.build.lib.actions.Artifact; import com.google.devtools.build.lib.actions.CommandLineItem; +import com.google.devtools.build.lib.actions.CommandLineItem.CapturingMapFn; import com.google.devtools.build.lib.actions.ParamFileInfo; import com.google.devtools.build.lib.actions.ParameterFile.ParameterFileType; import com.google.devtools.build.lib.actions.ResourceSet; @@ -50,6 +51,7 @@ import java.util.Collection; import java.util.HashSet; import java.util.List; import java.util.Map; +import java.util.function.Consumer; import javax.annotation.Nullable; /** Constructs actions to run the protocol compiler to generate sources from .proto files. */ @@ -296,6 +298,7 @@ public class ProtoCompileActionBuilder { addIncludeMapArguments( result, areDepsStrict ? supportData.getProtosInDirectDeps() : null, + supportData.getDirectProtoSourceRoots(), supportData.getTransitiveImports()); if (areDepsStrict) { @@ -331,7 +334,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( @@ -347,6 +351,7 @@ public class ProtoCompileActionBuilder { transitiveSources, protosInDirectDeps, protoSourceRoots, + directProtoSourceRoots, ruleContext.getLabel(), ImmutableList.of(output), "Descriptor Set", @@ -394,6 +399,7 @@ public class ProtoCompileActionBuilder { NestedSet<Artifact> transitiveSources, NestedSet<Artifact> protosInDirectDeps, NestedSet<String> protoSourceRoots, + NestedSet<String> directProtoSourceRoots, Label ruleLabel, Iterable<Artifact> outputs, String flavorName, @@ -406,6 +412,7 @@ public class ProtoCompileActionBuilder { transitiveSources, protosInDirectDeps, protoSourceRoots, + directProtoSourceRoots, ruleLabel, outputs, flavorName, @@ -423,6 +430,7 @@ public class ProtoCompileActionBuilder { NestedSet<Artifact> transitiveSources, @Nullable NestedSet<Artifact> protosInDirectDeps, NestedSet<String> protoSourceRoots, + NestedSet<String> directProtoSourceRoots, Label ruleLabel, Iterable<Artifact> outputs, String flavorName, @@ -458,6 +466,7 @@ public class ProtoCompileActionBuilder { protosToCompile, transitiveSources, protoSourceRoots, + directProtoSourceRoots, areDepsStrict(ruleContext) ? protosInDirectDeps : null, ruleLabel, allowServices, @@ -495,6 +504,7 @@ public class ProtoCompileActionBuilder { Iterable<Artifact> protosToCompile, NestedSet<Artifact> transitiveSources, NestedSet<String> transitiveProtoPathFlags, + NestedSet<String> directProtoSourceRoots, @Nullable NestedSet<Artifact> protosInDirectDeps, Label ruleLabel, boolean allowServices, @@ -537,7 +547,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); @@ -558,6 +568,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).mapped(EXPAND_TRANSITIVE_IMPORT_ARG)); if (protosInDirectDependencies != null) { @@ -566,7 +577,8 @@ public class ProtoCompileActionBuilder { "--direct_dependencies", VectorArg.join(":") .each(protosInDirectDependencies) - .mapped(EXPAND_TO_PATH_IGNORING_REPOSITORY)); + .mapped(new ExpandToPathFn(directProtoSourceRoots))); + } else { // The proto compiler requires an empty list to turn on strict deps checking commandLine.add("--direct_dependencies="); @@ -584,6 +596,24 @@ public class ProtoCompileActionBuilder { args.accept( "-I" + getPathIgnoringRepository(artifact) + "=" + artifact.getExecPathString()); + @AutoCodec + @AutoCodec.VisibleForSerialization + static final class ExpandToPathFn implements CapturingMapFn<Artifact> { + private final NestedSet<String> directProtoSourceRoots; + + public ExpandToPathFn(NestedSet<String> directProtoSourceRoots) { + this.directProtoSourceRoots = directProtoSourceRoots; + } + + @Override + public void expandToCommandLine(Artifact proto, Consumer<String> args) { + for (String directProtoSourceRoot : directProtoSourceRoots) { + expandToPathIgnoringSourceRoot(proto, directProtoSourceRoot, args); + } + EXPAND_TO_PATH_IGNORING_REPOSITORY.expandToCommandLine(proto, args); + } + } + @AutoCodec @AutoCodec.VisibleForSerialization static final CommandLineItem.MapFn<Artifact> EXPAND_TO_PATH_IGNORING_REPOSITORY = (artifact, args) -> args.accept(getPathIgnoringRepository(artifact)); @@ -603,6 +633,29 @@ public class ProtoCompileActionBuilder { .toString(); } + private static void expandToPathIgnoringSourceRoot( + Artifact artifact, String directProtoSourceRoot, Consumer<String> args) { + // TODO(bazel-team): IAE is caught here because every artifact is relativized against every + // directProtoSourceRoot. Instead of catching the exception, a check should be performed + // to see if the artifact has the root as a substring before relativizing. + 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 26075cf07f..d976b4d121 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 @@ -44,7 +44,8 @@ public abstract class ProtoSourcesProvider NestedSet<Artifact> checkDepsProtoSources, Artifact directDescriptorSet, NestedSet<Artifact> transitiveDescriptorSets, - NestedSet<String> transitiveProtoPathFlags) { + NestedSet<String> transitiveProtoPathFlags, + String protoSourceRoot) { return new AutoValue_ProtoSourcesProvider( transitiveImports, transitiveProtoSources, @@ -52,7 +53,8 @@ public abstract class ProtoSourcesProvider checkDepsProtoSources, directDescriptorSet, transitiveDescriptorSets, - transitiveProtoPathFlags); + transitiveProtoPathFlags, + protoSourceRoot); } /** @@ -108,5 +110,8 @@ public abstract class ProtoSourcesProvider @Override 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 d21a4d9d60..c3bc3aa553 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 @@ -35,6 +35,7 @@ public abstract class SupportData { NestedSet<Artifact> protosInDirectDeps, NestedSet<Artifact> transitiveImports, NestedSet<String> transitiveProtoPathFlags, + NestedSet<String> directProtoSourceRoots, boolean hasProtoSources) { return new AutoValue_SupportData( nonWeakDepsPredicate, @@ -42,6 +43,7 @@ public abstract class SupportData { transitiveImports, protosInDirectDeps, transitiveProtoPathFlags, + directProtoSourceRoots, hasProtoSources); } @@ -62,6 +64,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() {} diff --git a/src/test/java/com/google/devtools/build/lib/rules/proto/ProtoCompileActionBuilderTest.java b/src/test/java/com/google/devtools/build/lib/rules/proto/ProtoCompileActionBuilderTest.java index e33ed9a47e..1e2311fa84 100644 --- a/src/test/java/com/google/devtools/build/lib/rules/proto/ProtoCompileActionBuilderTest.java +++ b/src/test/java/com/google/devtools/build/lib/rules/proto/ProtoCompileActionBuilderTest.java @@ -83,6 +83,7 @@ public class ProtoCompileActionBuilderTest { artifact("//:dont-care", "import1.proto"), artifact("//:dont-care", "import2.proto")), /*transitiveProtoPathFlags=*/ NestedSetBuilder.<String>emptySet(STABLE_ORDER), + /*directProtoSourceRoots=*/ NestedSetBuilder.<String>stableOrder().build(), /* hasProtoSources= */ true); CustomCommandLine cmdLine = @@ -94,6 +95,7 @@ public class ProtoCompileActionBuilderTest { supportData.getDirectProtoSources(), supportData.getTransitiveImports(), /*transitiveProtoPathFlags=*/ NestedSetBuilder.<String>stableOrder().build(), + /*directProtoSourceRoots=*/ NestedSetBuilder.<String>stableOrder().build(), null /* protosInDirectDeps */, Label.parseAbsoluteUnchecked("//foo:bar"), true /* allowServices */, @@ -120,6 +122,7 @@ public class ProtoCompileActionBuilderTest { /* protosInDirectDeps= */ NestedSetBuilder.<Artifact>emptySet(STABLE_ORDER), /* transitiveImports= */ NestedSetBuilder.<Artifact>emptySet(STABLE_ORDER), /*transitiveProtoPathFlags=*/ NestedSetBuilder.<String>emptySet(STABLE_ORDER), + /*directProtoSourceRoots=*/ NestedSetBuilder.<String>stableOrder().build(), /* hasProtoSources= */ true); CustomCommandLine cmdLine = @@ -128,6 +131,7 @@ public class ProtoCompileActionBuilderTest { supportData.getDirectProtoSources(), supportData.getTransitiveImports(), /*transitiveProtoPathFlags=*/ NestedSetBuilder.<String>emptySet(STABLE_ORDER), + /*directProtoSourceRoots=*/ NestedSetBuilder.<String>emptySet(STABLE_ORDER), null /* protosInDirectDeps */, Label.parseAbsoluteUnchecked("//foo:bar"), true /* allowServices */, @@ -155,6 +159,7 @@ public class ProtoCompileActionBuilderTest { artifact("//:dont-care", "import1.proto"), artifact("//:dont-care", "import2.proto")), /*transitiveProtoPathFlags=*/ NestedSetBuilder.<String>emptySet(STABLE_ORDER), + /*directProtoSourceRoots=*/ NestedSetBuilder.<String>stableOrder().build(), /* hasProtoSources= */ true); CustomCommandLine cmdLine = @@ -163,6 +168,7 @@ public class ProtoCompileActionBuilderTest { supportData.getDirectProtoSources(), supportData.getTransitiveImports(), /*transitiveProtoPathFlags=*/ NestedSetBuilder.<String>emptySet(STABLE_ORDER), + /*directProtoSourceRoots=*/ NestedSetBuilder.<String>emptySet(STABLE_ORDER), supportData.getProtosInDirectDeps(), Label.parseAbsoluteUnchecked("//foo:bar"), true /* allowServices */, @@ -189,6 +195,7 @@ public class ProtoCompileActionBuilderTest { /* protosInDirectDeps= */ NestedSetBuilder.<Artifact>emptySet(STABLE_ORDER), NestedSetBuilder.<Artifact>emptySet(STABLE_ORDER), /*transitiveProtoPathFlags=*/ NestedSetBuilder.<String>emptySet(STABLE_ORDER), + /*directProtoSourceRoots=*/ NestedSetBuilder.<String>stableOrder().build(), /* hasProtoSources= */ true); CustomCommandLine cmdLine = @@ -197,6 +204,7 @@ public class ProtoCompileActionBuilderTest { supportData.getDirectProtoSources(), supportData.getTransitiveImports(), /*transitiveProtoPathFlags=*/ NestedSetBuilder.<String>emptySet(STABLE_ORDER), + /*directProtoSourceRoots=*/ NestedSetBuilder.<String>emptySet(STABLE_ORDER), supportData.getProtosInDirectDeps(), Label.parseAbsoluteUnchecked("//foo:bar"), false /* allowServices */, @@ -233,6 +241,7 @@ public class ProtoCompileActionBuilderTest { /* protosInDirectDeps= */ NestedSetBuilder.<Artifact>emptySet(STABLE_ORDER), NestedSetBuilder.<Artifact>emptySet(STABLE_ORDER), /*transitiveProtoPathFlags=*/ NestedSetBuilder.<String>emptySet(STABLE_ORDER), + /*directProtoSourceRoots=*/ NestedSetBuilder.<String>stableOrder().build(), /* hasProtoSources= */ true); CustomCommandLine cmdLine = @@ -241,6 +250,7 @@ public class ProtoCompileActionBuilderTest { supportData.getDirectProtoSources(), supportData.getTransitiveImports(), /*transitiveProtoPathFlags=*/ NestedSetBuilder.<String>emptySet(STABLE_ORDER), + /*directProtoSourceRoots=*/ NestedSetBuilder.<String>emptySet(STABLE_ORDER), supportData.getProtosInDirectDeps(), Label.parseAbsoluteUnchecked("//foo:bar"), true /* allowServices */, @@ -264,6 +274,7 @@ public class ProtoCompileActionBuilderTest { /* protosInDirectDeps= */ NestedSetBuilder.<Artifact>emptySet(STABLE_ORDER), NestedSetBuilder.<Artifact>emptySet(STABLE_ORDER), /*transitiveProtoPathFlags=*/ NestedSetBuilder.<String>emptySet(STABLE_ORDER), + /*directProtoSourceRoots=*/ NestedSetBuilder.<String>stableOrder().build(), /* hasProtoSources= */ true); ProtoLangToolchainProvider toolchain1 = @@ -288,6 +299,7 @@ public class ProtoCompileActionBuilderTest { supportData.getDirectProtoSources(), supportData.getTransitiveImports(), /*transitiveProtoPathFlags=*/ NestedSetBuilder.<String>emptySet(STABLE_ORDER), + /*directProtoSourceRoots=*/ NestedSetBuilder.<String>emptySet(STABLE_ORDER), supportData.getProtosInDirectDeps(), Label.parseAbsoluteUnchecked("//foo:bar"), true /* allowServices */, @@ -383,7 +395,10 @@ public class ProtoCompileActionBuilderTest { NestedSet<Artifact> transitiveImportsNestedSet = NestedSetBuilder.wrap(STABLE_ORDER, transitiveImports); ProtoCompileActionBuilder.addIncludeMapArguments( - commandLine, protosInDirectDependenciesBuilder, transitiveImportsNestedSet); + commandLine, + protosInDirectDependenciesBuilder, + NestedSetBuilder.emptySet(STABLE_ORDER), + transitiveImportsNestedSet); return commandLine.build().arguments(); } } diff --git a/src/test/shell/bazel/bazel_proto_library_test.sh b/src/test/shell/bazel/bazel_proto_library_test.sh new file mode 100755 index 0000000000..0446d49639 --- /dev/null +++ b/src/test/shell/bazel/bazel_proto_library_test.sh @@ -0,0 +1,423 @@ +#!/bin/bash +# +# Copyright 2016 The Bazel Authors. All rights reserved. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +# +# Tests the examples provided in Bazel +# + +# Load the test setup defined in the parent directory +CURRENT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" +source "${CURRENT_DIR}/../integration_test_setup.sh" \ + || { echo "integration_test_setup.sh not found!" >&2; exit 1; } + +# Appends to "WORKSPACE" the declaration of 2 local repositories. +# Assumes the main content of WORKSPACE was created previously. +function add_local_repos_to_workspace() { + cat >> WORKSPACE <<EOF +local_repository( + name = "repo", + path = "a/b" +) + +local_repository( + name = "main_repo", + path = "c/d" +) +EOF +} + +# Appends to the WORKSPACE file under a given path (the first argument) the dependencies needed +# for proto_library. +function write_workspace() { + workspace="" + if [ ! -z "$1" ]; + then + workspace=$1 + mkdir -p "$workspace" + fi + + cat >> "$workspace"WORKSPACE << EOF +# proto_library, cc_proto_library, and java_proto_library rules implicitly +# depend on @com_google_protobuf for protoc and proto runtimes. +# This statement defines the @com_google_protobuf repo. +http_archive( + name = "com_google_protobuf", + sha256 = "cef7f1b5a7c5fba672bec2a319246e8feba471f04dcebfe362d55930ee7c1c30", + strip_prefix = "protobuf-3.5.0", + urls = ["https://github.com/google/protobuf/archive/v3.5.0.zip"], +) + +# java_lite_proto_library rules implicitly depend on @com_google_protobuf_javalite//:javalite_toolchain, +# which is the JavaLite proto runtime (base classes and common utilities). +http_archive( + name = "com_google_protobuf_javalite", + sha256 = "d8a2fed3708781196f92e1e7e7e713cf66804bd2944894401057214aff4f468e", + strip_prefix = "protobuf-5e8916e881c573c5d83980197a6f783c132d4276", + urls = ["https://github.com/google/protobuf/archive/5e8916e881c573c5d83980197a6f783c132d4276.zip"], +) +EOF +} + +# Creates directories and files with the structure: +# x/ +# person/ +# BUILD +# person.proto (imports "bar/bar.proto", has proto_source_root = "x/person") +# phonenumber/ +# phonenumber.proto +# phonebook/ +# BUILD +# phonebook.proto (imports "person.proto" & "phonenumber/phonenumber.proto") +# +# The BUILD files could use directly "proto_library" rules or a macro called +# "proto_library_macro" that should be created beforehand using write_macro(). +# +# Expected arguments: +# 1. The name of the proto library rule. Can be "proto_library" or +# "proto_library_macro". +# 2. A row in the BUILD file that specifies the proto_source_root attribute on +# proto_library. Should be left empty if a macro is used. +# 3. A load statement that includes a macro containing a wrapper around +# proto_library. +function write_setup() { + mkdir -p x/person/phonenumber + proto_library_name=$1 + proto_source_root=$2 + include_macro=$3 + + cat > x/person/BUILD << EOF +package(default_visibility = ["//visibility:public"]) +$include_macro +$proto_library_name( + name = "person_proto", + srcs = ["person.proto"], + deps = [":phonenumber_proto"], + $proto_source_root +) + +$proto_library_name( + name = "phonenumber_proto", + srcs = ["phonenumber/phonenumber.proto"], +) +EOF + + cat > x/person/person.proto << EOF +syntax = "proto2"; + +option java_package = "person"; +option java_outer_classname = "Person"; + +import "phonenumber/phonenumber.proto"; + +message PersonProto { + optional string name = 1; + + optional PhoneNumberProto phone = 2; +} +EOF + + cat > x/person/phonenumber/phonenumber.proto << EOF +syntax = "proto2"; + +option java_package = "person.phonenumber"; +option java_outer_classname = "PhoneNumber"; + +message PhoneNumberProto { + required string number = 1; +} +EOF + + mkdir -p x/phonebook + + cat > x/phonebook/BUILD << EOF +$proto_library_name( + name = "phonebook", + srcs = ["phonebook.proto"], + deps = ["//x/person:person_proto"], +) +EOF + + cat > x/phonebook/phonebook.proto << EOF +import "person.proto"; +import "phonenumber/phonenumber.proto"; + +message Agenda { + required PersonProto person = 1; + required PhoneNumberProto number = 2; +} +EOF +} + +# Creates the files in the following directory structure: +# proto_library/ +# BUILD <- empty +# src/ +# BUILD <- has target "all" for all .proto +# address.proto <- imports "person.proto" +# person.proto <- imports "address.proto" +# zip_code.proto +function write_regression_setup() { + mkdir -p proto_library/src + touch proto_library/BUILD + + cat > proto_library/src/BUILD << EOF +proto_library( + name = "all", + srcs = glob(["*.proto"]), + proto_source_root = package_name(), +) +EOF + + cat > proto_library/src/address.proto <<EOF +syntax = "proto3"; + +package demo; // Required to generate valid code. + +// Always import protos with a full path relative to the WORKSPACE file. +import "zip_code.proto"; + +message Address { + string city = 1; + ZipCode zip_code = 2; +} +EOF + + cat > proto_library/src/person.proto <<EOF +syntax = "proto3"; + +package demo; // Required to generate valid code. + +// Always import protos with a full path relative to the WORKSPACE file. +import "address.proto"; + +message Person { + string name = 1; + int32 id = 2; + string email = 3; + Address address = 4; +} +EOF + + cat > proto_library/src/zip_code.proto <<EOF +syntax = "proto3"; + +package demo; // Requried to generate valid code. + +message ZipCode { + string code = 1; +} +EOF +} + +# Creates the directories and files in following the structure: +# a/b/ +# WORKSPACE <- workspace referenced as "repo" +# BUILD <- empty +# src/ +# BUILD <- "all_protos" with proto_source_root +# address.proto <- imports "zip_code.proto" +# zip_code.proto +# c/d/ +# WORKSPACE <- workspace referenced as "main_repo" +# BUILD <- empty +# src/ +# BUILD <- "all_protos" with proto_source_root +# person.proto <- imports "address.proto" and depends on @repo//src:all_protos +function write_workspaces_setup() { + mkdir -p a/b/src + + touch a/b/BUILD + cat > a/b/src/BUILD <<EOF +package(default_visibility = ["//visibility:public"]) +proto_library( + name = "all_protos", + srcs = glob(["*.proto"]), + proto_source_root = package_name() +) +EOF + + + cat > a/b/src/address.proto <<EOF +syntax = "proto3"; + +package demo; // Required to generate valid code. + +// Always import protos with a full path relative to the WORKSPACE file. +import "zip_code.proto"; + +message Address { + string city = 1; + ZipCode zip_code = 2; +} +EOF + + cat > a/b/src/zip_code.proto <<EOF +syntax = "proto3"; + +package demo; // Requried to generate valid code. + +message ZipCode { + string code = 1; +} +EOF + + #### WORKSPACE(c/d) #### + mkdir -p c/d/src + + touch c/d/BUILD + + cat > c/d/src/BUILD <<EOF +package(default_visibility = ["//visibility:public"]) +proto_library( + name = "all_protos", + srcs = glob(["*.proto"]), + proto_source_root = package_name(), + deps = ["@repo//src:all_protos"] +) +EOF + + cat > c/d/src/person.proto <<EOF +syntax = "proto3"; + +package demo; // Required to generate valid code. + +// Always import protos with a full path relative to the WORKSPACE file. +import "address.proto"; + +message Person { + string name = 1; + int32 id = 2; + string email = 3; + Address address = 4; +} +EOF + +} + + +# Creates macros/BUILD and macros/proto_library_macro.bzl, which contains a +# macro that wraps the proto_library rule. The macro passes to proto_library the +# same "name", "srcs", "deps" and adds "proto_source_root = native.package_name()". +# This will be a common use case for the "proto_source_root" attribute. +function write_macro() { + mkdir macros + cat > macros/BUILD << EOF +export_files(["proto_library_macro.bzl]) +EOF + + cat > macros/proto_library_macro.bzl << EOF +def proto_library_macro(name, srcs, deps = []): + native.proto_library( + name = name, + srcs = srcs, + deps = deps, + proto_source_root = native.package_name() + ) +EOF +} + +# Creates the directories and files from the following structure +# java/com/google/src/ +# BUILD +# A.java +function write_java_library() { + # should depend on x/foo:foo + mkdir -p java/com/google/src + cat > java/com/google/src/BUILD << EOF +java_library( + name = "top", + srcs = ["A.java"], + deps = [":jpl"] +) + +java_proto_library( + name = "jpl", + deps = ["//x/person:person_proto"] +) +EOF + + cat > java/com/google/src/A.java << EOF +import person.Person.PersonProto; +import person.phonenumber.PhoneNumber.PhoneNumberProto; + +public class A { + private PersonProto person; + + public A(PersonProto person) { + this.person = person; + PhoneNumberProto number = person.getPhone(); + } +} + +EOF + +} + +############# TESTS ############# + +function test_proto_source_root() { + write_workspace "" + write_setup "proto_library" "proto_source_root = 'x/person'" "" + bazel build //x/person:person_proto > "$TEST_log" || fail "Expected success" +} + +function test_proto_source_root_fails() { + write_workspace "" + # Don't specify the "proto_source_root" attribute and expect failure. + write_setup "proto_library" "" "" + bazel build //x/person:person_proto >& "$TEST_log" && fail "Expected failure" + expect_log "phonenumber/phonenumber.proto: File not found." +} + +function test_proto_source_root_macro() { + write_workspace "" + write_macro + write_setup "proto_library_macro" "" "load('//macros:proto_library_macro.bzl', 'proto_library_macro')" + bazel build //x/person:person_proto > "$TEST_log" || fail "Expected success" +} + +function test_proto_source_root_with_java_library() { + write_workspace "" + write_setup "proto_library" "proto_source_root = 'x/person'" "" + write_java_library + bazel build //java/com/google/src:top \ + --strict_java_deps=off > "$TEST_log" || fail "Expected success" +} + +function test_proto_source_root_glob() { + write_workspace "" + write_regression_setup + bazel build //proto_library/src:all >& "$TEST_log" || fail "Expected success" +} + +function test_proto_source_root_glob() { + write_workspace "" + write_regression_setup + bazel build //proto_library/src:all --strict_proto_deps=off >& "$TEST_log" \ + || fail "Expected success" +} + +# TODO(elenairina): Enable this after #4665 is fixed. +function DISABLED_test_proto_source_root_multiple_workspaces() { + write_workspace "a/b/" + write_workspace "c/d/" + write_workspace "" + add_local_repos_to_workspace + write_workspaces_setup + + bazel build @main_repo//src:all_protos >& "$TEST_log" || fail "Expected success" +} + +run_suite "Integration tests for proto_library"
\ No newline at end of file |