Do not add a --proto_path argument to the proto compiler invocation there are no .proto sources in it.
This prevents protoc from complaining about a nonexistent directory in sandboxed mode and is good hygiene in any case.
Fixes #7157.
RELNOTES: None.
PiperOrigin-RevId: 256208923
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 1463bbf..cbeeaa2 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
@@ -19,6 +19,7 @@
import static com.google.devtools.build.lib.syntax.Type.STRING;
import com.google.common.annotations.VisibleForTesting;
+import com.google.common.base.Optional;
import com.google.common.collect.ImmutableList;
import com.google.devtools.build.lib.actions.Artifact;
import com.google.devtools.build.lib.actions.ArtifactRoot;
@@ -136,10 +137,13 @@
* <p>Assumes {@code currentProtoSourceRoot} is the same as the package name.
*/
private static NestedSet<String> computeTransitiveProtoSourceRoots(
- RuleContext ruleContext, String currentProtoSourceRoot) {
+ RuleContext ruleContext, Optional<String> currentProtoSourceRoot) {
NestedSetBuilder<String> protoPath = NestedSetBuilder.stableOrder();
- protoPath.add(currentProtoSourceRoot);
+ if (currentProtoSourceRoot.isPresent()) {
+ protoPath.add(currentProtoSourceRoot.get());
+ }
+
for (ProtoInfo provider :
ruleContext.getPrerequisites("deps", Mode.TARGET, ProtoInfo.PROVIDER)) {
protoPath.addTransitive(provider.getTransitiveProtoSourceRoots());
@@ -159,11 +163,11 @@
static final class Library {
private final ImmutableList<Artifact> sources;
private final ImmutableList<Pair<Artifact, String>> importPathSourcePair;
- private final String sourceRoot;
+ private final Optional<String> sourceRoot;
Library(
ImmutableList<Artifact> sources,
- String sourceRoot,
+ Optional<String> sourceRoot,
ImmutableList<Pair<Artifact, String>> importPathSourcePair) {
this.sources = sources;
this.sourceRoot = sourceRoot;
@@ -178,7 +182,7 @@
return importPathSourcePair;
}
- public String getSourceRoot() {
+ public Optional<String> getSourceRoot() {
return sourceRoot;
}
}
@@ -201,22 +205,38 @@
return null;
}
- // This is the same as getPackageIdentifier().getPathUnderExecRoot() due to the check above for
- // protoSourceRoot == package name, but it's a bit more future-proof.
- String result =
- ruleContext
- .getLabel()
- .getPackageIdentifier()
- .getRepository()
- .getPathUnderExecRoot()
- .getRelative(protoSourceRoot)
- .getPathString();
+ boolean hasNonGeneratedSources = false;
+ for (Artifact artifact : directSources) {
+ if (artifact.isSourceArtifact()) {
+ hasNonGeneratedSources = true;
+ break;
+ }
+ }
+
+ String sourceRoot = "";
+ // Otherwise, we'll direct the proto compiler to the source files using -Ifoo=bar arguments.
+ // Then let's not spam the command line.
+ if (hasNonGeneratedSources) {
+ // This is the same as getPackageIdentifier().getPathUnderExecRoot() due to the check above
+ // for protoSourceRoot == package name, but it's a bit more future-proof.
+ sourceRoot =
+ ruleContext
+ .getLabel()
+ .getPackageIdentifier()
+ .getRepository()
+ .getPathUnderExecRoot()
+ .getRelative(protoSourceRoot)
+ .getPathString();
+ }
ImmutableList.Builder<Pair<Artifact, String>> builder = ImmutableList.builder();
for (Artifact protoSource : directSources) {
builder.add(new Pair<Artifact, String>(protoSource, null));
}
- return new Library(directSources, result.isEmpty() ? "." : result, builder.build());
+ return new Library(
+ directSources,
+ sourceRoot.isEmpty() ? Optional.absent() : Optional.of(sourceRoot),
+ builder.build());
}
private static PathFragment getPathFragmentAttribute(
@@ -327,7 +347,7 @@
.getExecPath()
.getRelative(sourceRootPath)
.getPathString();
- return new Library(symlinks.build(), sourceRoot, protoSourceImportPair.build());
+ return new Library(symlinks.build(), Optional.of(sourceRoot), protoSourceImportPair.build());
}
private static Pair<PathFragment, Artifact> computeImports(
@@ -361,9 +381,11 @@
* <p>Assumes {@code currentProtoSourceRoot} is the same as the package name.
*/
private static NestedSet<String> getProtoSourceRootsOfAttribute(
- RuleContext ruleContext, String currentProtoSourceRoot, String attributeName) {
+ RuleContext ruleContext, Optional<String> currentProtoSourceRoot, String attributeName) {
NestedSetBuilder<String> protoSourceRoots = NestedSetBuilder.stableOrder();
- protoSourceRoots.add(currentProtoSourceRoot);
+ if (currentProtoSourceRoot.isPresent()) {
+ protoSourceRoots.add(currentProtoSourceRoot.get());
+ }
for (ProtoInfo provider :
ruleContext.getPrerequisites(attributeName, Mode.TARGET, ProtoInfo.PROVIDER)) {
@@ -380,7 +402,7 @@
* <p>Assumes {@code currentProtoSourceRoot} is the same as the package name.
*/
private static NestedSet<String> computeStrictImportableProtoSourceRoots(
- RuleContext ruleContext, String currentProtoSourceRoot) {
+ RuleContext ruleContext, Optional<String> currentProtoSourceRoot) {
return getProtoSourceRootsOfAttribute(ruleContext, currentProtoSourceRoot, "deps");
}
@@ -391,7 +413,7 @@
* <p>Assumes {@code currentProtoSourceRoot} is the same as the package name.
*/
private static NestedSet<String> computeExportedProtoSourceRoots(
- RuleContext ruleContext, String currentProtoSourceRoot) {
+ RuleContext ruleContext, Optional<String> currentProtoSourceRoot) {
return getProtoSourceRootsOfAttribute(ruleContext, currentProtoSourceRoot, "exports");
}
@@ -490,7 +512,7 @@
ProtoInfo protoInfo =
new ProtoInfo(
library.getSources(),
- library.getSourceRoot(),
+ library.getSourceRoot().or("."),
transitiveProtoSources,
transitiveProtoSourceRoots,
strictImportableProtosForDependents,
diff --git a/src/test/java/com/google/devtools/build/lib/rules/proto/BazelProtoLibraryTest.java b/src/test/java/com/google/devtools/build/lib/rules/proto/BazelProtoLibraryTest.java
index 225134e..447763b 100644
--- a/src/test/java/com/google/devtools/build/lib/rules/proto/BazelProtoLibraryTest.java
+++ b/src/test/java/com/google/devtools/build/lib/rules/proto/BazelProtoLibraryTest.java
@@ -340,8 +340,34 @@
);
ConfiguredTarget protoTarget = getConfiguredTarget("//x/foo:withdeps");
ProtoInfo sourcesProvider = protoTarget.get(ProtoInfo.PROVIDER);
- assertThat(sourcesProvider.getTransitiveProtoSourceRoots())
- .containsExactly("x/foo", "x/bar", ".");
+ assertThat(sourcesProvider.getTransitiveProtoSourceRoots()).containsExactly("x/foo", "x/bar");
+ }
+
+ @Test
+ public void testExternalRepoWithGeneratedProto() throws Exception {
+ if (!isThisBazel()) {
+ return;
+ }
+
+ FileSystemUtils.appendIsoLatin1(
+ scratch.resolve("WORKSPACE"), "local_repository(name = 'foo', path = '/foo')");
+ invalidatePackages();
+
+ scratch.file("/foo/WORKSPACE");
+ scratch.file(
+ "/foo/x/BUILD",
+ "proto_library(name='x', srcs=['generated.proto'])",
+ "genrule(name='g', srcs=[], outs=['generated.proto'], cmd='')");
+
+ scratch.file("a/BUILD", "proto_library(name='a', srcs=['a.proto'], deps=['@foo//x:x'])");
+
+ ConfiguredTarget a = getConfiguredTarget("//a:a");
+ ProtoInfo aInfo = a.get(ProtoInfo.PROVIDER);
+ assertThat(aInfo.getTransitiveProtoSourceRoots()).isEmpty();
+
+ ConfiguredTarget x = getConfiguredTarget("@foo//x:x");
+ ProtoInfo xInfo = x.get(ProtoInfo.PROVIDER);
+ assertThat(xInfo.getTransitiveProtoSourceRoots()).isEmpty();
}
@Test
