Fixing issue with external j2objc protos

The output files are created without a repository, but the expected
filenames have them

This resolves issues when having a proto_library from an external build
file.

This seems to be a regression, so maybe should go into the 0.8.0 branch?

Note: Work at Square and we have a signed CLA with google

Note, without this fix we get errors like

```
ERROR: /private/var/tmp/_bazel_lewis/4a25cfc2b9b758043413ac58525ef6b4/external/AllProtos/BUILD.bazel:27:1: output 'external/AllProtos/squareup/objc/objc.j2objc.pb.m' was not created
ERROR: /private/var/tmp/_bazel_lewis/4a25cfc2b9b758043413ac58525ef6b4/external/AllProtos/BUILD.bazel:27:1: output 'external/AllProtos/squareup/objc/objc.j2objc.pb.h' was not created
```

Closes #4058.

PiperOrigin-RevId: 187480864

5 files changed, 158 insertions, 18 deletions

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..54393cb212 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
@@ -183,7 +183,7 @@ public class ProtoCommon {
     ArtifactRoot genfiles =
         ruleContext.getConfiguration().getGenfilesDirectory(ruleContext.getRule().getRepository());
     for (Artifact src : protoSources) {
-      PathFragment srcPath = src.getRootRelativePath();
+      PathFragment srcPath = getPathIgnoringRepository(src);
       if (pythonNames) {
         srcPath = srcPath.replaceName(srcPath.getBaseName().replace('-', '_'));
       }
@@ -251,4 +251,18 @@ public class ProtoCommon {
     }
     return (flagValue == BuildConfiguration.StrictDepsMode.STRICT);
   }
+
+  /**
+   * Gets the artifact's path relative to the root, ignoring the external repository the artifact is
+   * at. For example, <code>
+   * //a:b.proto --> a/b.proto
+   * {@literal @}foo//a:b.proto --> a/b.proto
+   * </code>
+   */
+  public static PathFragment getPathIgnoringRepository(Artifact artifact) {
+    return artifact
+        .getRootRelativePath()
+        .relativeTo(
+            artifact.getOwnerLabel().getPackageIdentifier().getRepository().getPathUnderExecRoot());
+  }
 }
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 6fdcc040f0..c0767acdf0 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
@@ -611,26 +611,15 @@ public class ProtoCompileActionBuilder {
   }
 
   private static void expandTransitiveImportArg(Artifact artifact, Consumer<String> args) {
-    args.accept("-I" + getPathIgnoringRepository(artifact) + "=" + artifact.getExecPathString());
+    args.accept(
+        "-I"
+            + ProtoCommon.getPathIgnoringRepository(artifact).toString()
+            + "="
+            + artifact.getExecPathString());
   }
 
   private static void expandToPathIgnoringRepository(Artifact artifact, Consumer<String> args) {
-    args.accept(getPathIgnoringRepository(artifact));
-  }
-
-  /**
-   * Gets the artifact's path relative to the root, ignoring the external repository the artifact is
-   * at. For example, <code>
-   * //a:b.proto --> a/b.proto
-   * {@literal @}foo//a:b.proto --> a/b.proto
-   * </code>
-   */
-  private static String getPathIgnoringRepository(Artifact artifact) {
-    return artifact
-        .getRootRelativePath()
-        .relativeTo(
-            artifact.getOwnerLabel().getPackageIdentifier().getRepository().getPathUnderExecRoot())
-        .toString();
+    args.accept(ProtoCommon.getPathIgnoringRepository(artifact).toString());
   }
 
   /**
diff --git a/src/test/java/com/google/devtools/build/lib/BUILD b/src/test/java/com/google/devtools/build/lib/BUILD
index 72f1f9fd6c..831dbb37dd 100644
--- a/src/test/java/com/google/devtools/build/lib/BUILD
+++ b/src/test/java/com/google/devtools/build/lib/BUILD
@@ -1332,6 +1332,21 @@ java_test(
 )
 
 java_test(
+    name = "ProtoCommonTest",
+    srcs = ["rules/proto/ProtoCommonTest.java"],
+    deps = [
+        ":actions_testutil",
+        ":analysis_testutil",
+        "//src/main/java/com/google/devtools/build/lib:proto-rules",
+        "//src/main/java/com/google/devtools/build/lib/actions",
+        "//src/main/java/com/google/devtools/build/lib/cmdline",
+        "//src/main/java/com/google/devtools/build/lib/vfs",
+        "//src/test/java/com/google/devtools/build/lib:testutil",
+        "//third_party:truth",
+    ],
+)
+
+java_test(
     name = "ProtoCompileActionBuilderTest",
     srcs = ["rules/proto/ProtoCompileActionBuilderTest.java"],
     deps = [
diff --git a/src/test/java/com/google/devtools/build/lib/rules/objc/BazelJ2ObjcLibraryTest.java b/src/test/java/com/google/devtools/build/lib/rules/objc/BazelJ2ObjcLibraryTest.java
index 6a72c21ef9..45f55abe31 100644
--- a/src/test/java/com/google/devtools/build/lib/rules/objc/BazelJ2ObjcLibraryTest.java
+++ b/src/test/java/com/google/devtools/build/lib/rules/objc/BazelJ2ObjcLibraryTest.java
@@ -47,6 +47,7 @@ import com.google.devtools.build.lib.rules.cpp.CppCompileActionTemplate;
 import com.google.devtools.build.lib.rules.cpp.CppModuleMapAction;
 import com.google.devtools.build.lib.rules.cpp.UmbrellaHeaderAction;
 import com.google.devtools.build.lib.testutil.TestConstants;
+import com.google.devtools.build.lib.vfs.FileSystemUtils;
 import com.google.devtools.build.lib.vfs.PathFragment;
 import java.io.ByteArrayOutputStream;
 import java.util.List;
@@ -277,6 +278,50 @@ public class BazelJ2ObjcLibraryTest extends J2ObjcLibraryTest {
   }
 
   @Test
+  public void testJavaProtoLibraryWithProtoLibrary_external() throws Exception {
+    scratch.file("/bla/WORKSPACE");
+    // Create the rule '@bla//foo:test_proto'.
+    scratch.file(
+        "/bla/foo/BUILD",
+        "package(default_visibility=['//visibility:public'])",
+        j2ObjcCompatibleProtoLibrary("    name = 'test_proto',", "    srcs = ['test.proto'],"),
+        "java_proto_library(",
+        "    name = 'test_java_proto',",
+        "    deps = [':test_proto'])",
+        "");
+
+    String existingWorkspace =
+        new String(FileSystemUtils.readContentAsLatin1(rootDirectory.getRelative("WORKSPACE")));
+    scratch.overwriteFile(
+        "WORKSPACE", "local_repository(name = 'bla', path = '/bla/')", existingWorkspace);
+    invalidatePackages(); // A dash of magic to re-evaluate the WORKSPACE file.
+
+    scratch.file(
+        "x/BUILD",
+        "",
+        "java_library(",
+        "    name = 'test',",
+        "    srcs = ['test.java'],",
+        "    deps = ['@bla//foo:test_java_proto'])");
+
+    ConfiguredTarget target = getJ2ObjCAspectConfiguredTarget("//x:test");
+    ConfiguredTarget test =
+        getConfiguredTarget("@bla//foo:test_proto", getAppleCrosstoolConfiguration());
+
+    J2ObjcMappingFileProvider provider = target.getProvider(J2ObjcMappingFileProvider.class);
+
+    Artifact classMappingFile = getGenfilesArtifact("test.clsmap.properties", test);
+    assertThat(provider.getClassMappingFiles()).containsExactly(classMappingFile);
+
+    ObjcProvider objcProvider = target.get(ObjcProvider.SKYLARK_CONSTRUCTOR);
+
+    Artifact headerFile = getGenfilesArtifact("test.j2objc.pb.h", test);
+    Artifact sourceFile = getGenfilesArtifact("test.j2objc.pb.m", test);
+    assertThat(objcProvider.get(ObjcProvider.HEADER)).contains(headerFile);
+    assertThat(objcProvider.get(ObjcProvider.SOURCE)).contains(sourceFile);
+  }
+
+  @Test
   public void testJ2ObjcInfoExportedInJavaImport() throws Exception {
     scratch.file("java/com/google/transpile/BUILD",
         "java_import(name = 'dummy',",
diff --git a/src/test/java/com/google/devtools/build/lib/rules/proto/ProtoCommonTest.java b/src/test/java/com/google/devtools/build/lib/rules/proto/ProtoCommonTest.java
new file mode 100644
index 0000000000..808bb53966
--- /dev/null
+++ b/src/test/java/com/google/devtools/build/lib/rules/proto/ProtoCommonTest.java
@@ -0,0 +1,77 @@
+// Copyright 2018 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.
+
+package com.google.devtools.build.lib.rules.proto;
+
+import static com.google.common.truth.Truth.assertThat;
+
+import com.google.devtools.build.lib.actions.Artifact;
+import com.google.devtools.build.lib.actions.ArtifactRoot;
+import com.google.devtools.build.lib.actions.util.LabelArtifactOwner;
+import com.google.devtools.build.lib.cmdline.Label;
+import com.google.devtools.build.lib.cmdline.LabelSyntaxException;
+import com.google.devtools.build.lib.cmdline.PackageIdentifier;
+import com.google.devtools.build.lib.cmdline.RepositoryName;
+import com.google.devtools.build.lib.testutil.Scratch;
+import com.google.devtools.build.lib.vfs.Path;
+import com.google.devtools.build.lib.vfs.PathFragment;
+import java.io.IOException;
+import org.junit.Before;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.JUnit4;
+
+/**
+ * Unit tests for {@link ProtoCommon}.
+ */
+@RunWith(JUnit4.class)
+public class ProtoCommonTest {
+
+  private Scratch scratch;
+  private Path execDir;
+  private ArtifactRoot rootDir;
+
+  @Before
+  public final void setRootDir() throws Exception {
+    scratch = new Scratch();
+    execDir = scratch.dir("/exec");
+    rootDir = ArtifactRoot.asDerivedRoot(execDir, scratch.dir("/exec/root"));
+  }
+
+  @Test
+  public void getPathIgnoringRepository_main() throws IOException, LabelSyntaxException {
+    Path f1 = scratch.file("/exec/root/foo/bar");
+
+    PackageIdentifier ownerPackage =
+        PackageIdentifier.create(RepositoryName.MAIN, PathFragment.create("//foo"));
+
+    LabelArtifactOwner owner = new LabelArtifactOwner(Label.create(ownerPackage, "owner_a"));
+    Artifact a1 = new Artifact(rootDir, f1.relativeTo(execDir), owner);
+    PathFragment pathIgnoringRepository = ProtoCommon.getPathIgnoringRepository(a1);
+    assertThat(pathIgnoringRepository).isEqualTo(PathFragment.create("foo/bar"));
+  }
+
+  @Test
+  public void getPathIgnoringRepository_external() throws IOException, LabelSyntaxException {
+    Path f1 = scratch.file("/exec/root/external/repo_a/foo/bar");
+
+    PackageIdentifier ownerPackage =
+        PackageIdentifier.create("@repo_a", PathFragment.create("//foo"));
+
+    LabelArtifactOwner owner = new LabelArtifactOwner(Label.create(ownerPackage, "owner_a"));
+    Artifact a1 = new Artifact(rootDir, f1.relativeTo(execDir), owner);
+    PathFragment pathIgnoringRepository = ProtoCommon.getPathIgnoringRepository(a1);
+    assertThat(pathIgnoringRepository).isEqualTo(PathFragment.create("foo/bar"));
+  }
+}