proto_library can depend on proto_library's from external repos. Fixes https://github.com/bazelbuild/bazel/issues/2039. -- PiperOrigin-RevId: 142672678 MOS_MIGRATED_REVID=142672678
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 8049af7..8936c24 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
@@ -327,7 +327,17 @@ ImmutableList.Builder<String> builder = ImmutableList.builder(); for (Artifact artifact : transitiveImports) { builder.add( - "-I" + artifact.getRootRelativePathString() + "=" + artifact.getExecPathString()); + "-I" + + artifact + .getRootRelativePath() + .relativeTo( + artifact + .getOwnerLabel() + .getPackageIdentifier() + .getRepository() + .getPathUnderExecRoot()) + + "=" + + artifact.getExecPathString()); } if (protosInDirectDependencies != null) { ArrayList<String> rootRelativePaths = new ArrayList<>();
diff --git a/src/test/java/com/google/devtools/build/lib/BUILD b/src/test/java/com/google/devtools/build/lib/BUILD index 60f781f..1bae10a 100644 --- a/src/test/java/com/google/devtools/build/lib/BUILD +++ b/src/test/java/com/google/devtools/build/lib/BUILD
@@ -1180,6 +1180,7 @@ name = "ProtoCompileActionBuilderTest", srcs = ["rules/proto/ProtoCompileActionBuilderTest.java"], deps = [ + ":actions_testutil", "//src/main/java/com/google/devtools/build/lib:build-base", "//src/main/java/com/google/devtools/build/lib:collect", "//src/main/java/com/google/devtools/build/lib:inmemoryfs", @@ -1187,6 +1188,7 @@ "//src/main/java/com/google/devtools/build/lib:util", "//src/main/java/com/google/devtools/build/lib:vfs", "//src/main/java/com/google/devtools/build/lib/actions", + "//src/main/java/com/google/devtools/build/lib/cmdline", "//third_party:guava", "//third_party:junit4", "//third_party:mockito",
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 8d05ad5..f024855 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
@@ -24,14 +24,15 @@ import com.google.common.collect.ImmutableList; import com.google.devtools.build.lib.actions.Artifact; import com.google.devtools.build.lib.actions.Root; +import com.google.devtools.build.lib.actions.util.LabelArtifactOwner; import com.google.devtools.build.lib.analysis.FilesToRunProvider; import com.google.devtools.build.lib.analysis.TransitiveInfoCollection; import com.google.devtools.build.lib.analysis.actions.CustomCommandLine; +import com.google.devtools.build.lib.cmdline.Label; import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder; import com.google.devtools.build.lib.rules.proto.ProtoCompileActionBuilder.ProtoCommandLineArgv; import com.google.devtools.build.lib.rules.proto.ProtoCompileActionBuilder.ToolchainInvocation; import com.google.devtools.build.lib.util.LazyString; -import com.google.devtools.build.lib.vfs.PathFragment; import com.google.devtools.build.lib.vfs.inmemoryfs.InMemoryFileSystem; import org.junit.Test; import org.junit.runner.RunWith; @@ -51,7 +52,7 @@ new FilesToRunProvider( ImmutableList.<Artifact>of(), null /* runfilesSupport */, - artifact("protoc-gen-javalite.exe")); + artifact("//:dont-care", "protoc-gen-javalite.exe")); ProtoLangToolchainProvider toolchainNoPlugin = ProtoLangToolchainProvider.create( @@ -70,10 +71,12 @@ SupportData supportData = SupportData.create( Predicates.<TransitiveInfoCollection>alwaysFalse(), - ImmutableList.of(artifact("source_file.proto")), + ImmutableList.of(artifact("//:dont-care", "source_file.proto")), null /* protosInDirectDeps */, NestedSetBuilder.create( - STABLE_ORDER, artifact("import1.proto"), artifact("import2.proto")), + STABLE_ORDER, + artifact("//:dont-care", "import1.proto"), + artifact("//:dont-care", "import2.proto")), true /* hasProtoSources */); CustomCommandLine cmdLine = @@ -112,10 +115,12 @@ SupportData supportData = SupportData.create( Predicates.<TransitiveInfoCollection>alwaysFalse(), - ImmutableList.of(artifact("source_file.proto")), - NestedSetBuilder.create(STABLE_ORDER, artifact("import1.proto")), + ImmutableList.of(artifact("//:dont-care", "source_file.proto")), + NestedSetBuilder.create(STABLE_ORDER, artifact("//:dont-care", "import1.proto")), NestedSetBuilder.create( - STABLE_ORDER, artifact("import1.proto"), artifact("import2.proto")), + STABLE_ORDER, + artifact("//:dont-care", "import1.proto"), + artifact("//:dont-care", "import2.proto")), true /* hasProtoSources */); CustomCommandLine cmdLine = @@ -259,40 +264,66 @@ public void testProtoCommandLineArgv() throws Exception { assertThat( new ProtoCommandLineArgv( - null /* directDependencies */, ImmutableList.of(derivedArtifact("foo.proto"))) + null /* directDependencies */, + ImmutableList.of(derivedArtifact("//:dont-care", "foo.proto"))) .argv()) .containsExactly("-Ifoo.proto=out/foo.proto"); assertThat( new ProtoCommandLineArgv( ImmutableList.<Artifact>of() /* directDependencies */, - ImmutableList.of(derivedArtifact("foo.proto"))) + ImmutableList.of(derivedArtifact("//:dont-care", "foo.proto"))) .argv()) .containsExactly("-Ifoo.proto=out/foo.proto", "--direct_dependencies="); assertThat( new ProtoCommandLineArgv( - ImmutableList.of(derivedArtifact("foo.proto")) /* directDependencies */, - ImmutableList.of(derivedArtifact("foo.proto"))) + ImmutableList.of( + derivedArtifact("//:dont-care", "foo.proto")) /* directDependencies */, + ImmutableList.of(derivedArtifact("//:dont-care", "foo.proto"))) .argv()) .containsExactly("-Ifoo.proto=out/foo.proto", "--direct_dependencies=foo.proto"); assertThat( new ProtoCommandLineArgv( ImmutableList.of( - derivedArtifact("foo.proto"), - derivedArtifact("bar.proto")) /* directDependencies */, - ImmutableList.of(derivedArtifact("foo.proto"))) + derivedArtifact("//:dont-care", "foo.proto"), + derivedArtifact("//:dont-care", "bar.proto")) /* directDependencies */, + ImmutableList.of(derivedArtifact("//:dont-care", "foo.proto"))) .argv()) .containsExactly("-Ifoo.proto=out/foo.proto", "--direct_dependencies=foo.proto:bar.proto"); } - private Artifact artifact(String path) { - return new Artifact(new PathFragment(path), root); + /** + * Include-maps are the -Ivirtual=physical arguments passed to proto-compiler. When including a + * file named 'foo/bar.proto' from an external repository 'bla', the include-map should be + * -Ifoo/bar.proto=external/bla/foo/bar.proto. That is - 'virtual' should be the path relative to + * the external repo root, and physical should be the physical file location. + */ + @Test + public void testIncludeMapsOfExternalFiles() throws Exception { + assertThat( + new ProtoCommandLineArgv( + null /* protosInDirectoDependencies */, + ImmutableList.of(artifact("@bla//foo:bar", "external/bla/foo/bar.proto"))) + .argv()) + .containsExactly("-Ifoo/bar.proto=external/bla/foo/bar.proto"); + } + + private Artifact artifact(String ownerLabel, String path) { + return new Artifact( + root.getPath().getRelative(path), + root, + root.getExecPath().getRelative(path), + new LabelArtifactOwner(Label.parseAbsoluteUnchecked(ownerLabel))); } /** Creates a dummy artifact with the given path, that actually resides in /out/<path>. */ - private Artifact derivedArtifact(String path) { - return new Artifact(new PathFragment(path), derivedRoot); + private Artifact derivedArtifact(String ownerLabel, String path) { + return new Artifact( + derivedRoot.getPath().getRelative(path), + derivedRoot, + derivedRoot.getExecPath().getRelative(path), + new LabelArtifactOwner(Label.parseAbsoluteUnchecked(ownerLabel))); } }