Use runfiles path instead of root relative path in JavaStarlarkCommon.collectNativeLibsDirs

src/main/java/com/google/devtools/build/lib/rules/java/JavaStarlarkCommon.java

@@ -22,6 +22,7 @@
 import com.google.common.collect.ImmutableMap;
 import com.google.common.collect.Iterables;
 import com.google.devtools.build.lib.actions.Artifact;
+import com.google.devtools.build.lib.analysis.BazelRuleAnalysisThreadContext;
 import com.google.devtools.build.lib.analysis.ConfiguredTarget;
 import com.google.devtools.build.lib.analysis.Expander;
 import com.google.devtools.build.lib.analysis.RuleContext;
@@ -300,7 +301,7 @@ public Sequence<String> collectNativeLibsDirs(Depset libraries, StarlarkThread t
                   }
                   return true;
                 })
-            .map(artifact -> artifact.getRootRelativePath().getParentDirectory().getPathString())
+            .map(artifact -> artifact.getRunfilesPath().getParentDirectory().getPathString())
             .distinct()
             .collect(toImmutableList());
     return StarlarkList.immutableCopyOf(uniqueDirs);

src/test/shell/bazel/bazel_java_test.sh

@@ -1573,32 +1573,33 @@ EOF
 # Build and run a java_binary that calls a C++ function through JNI.
 # This test exercises the built-in @bazel_tools//tools/jdk:jni target.
 #
-# The java_binary wrapper script specifies -Djava.library.path=$runfiles/test,
+# The java_binary wrapper script specifies -Djava.library.path=$runfiles/jni,
 # and the Java program expects to find a DSO there---except on MS Windows,
 # which lacks support for symbolic links. Really there needs to
 # be a cleaner mechanism for finding and loading the JNI library (and better
 # hygiene around the library namespace). By contrast, Blaze links all the
 # native code and the JVM into a single executable, which is an elegant solution.
 #
-function test_jni() {
-  # Skip on MS Windows, as Bazel does not create a runfiles symlink tree.
-  # (MSYS_NT is the system name reported by MinGW uname.)
-  # TODO(adonovan): make this work.
-  uname -s | grep -q MSYS_NT && return
-
-  # Skip on Darwin, as System.loadLibrary looks for a file named
-  # .dylib, not .so, and that's not what the file is called.
-  # TODO(adonovan): make this just work.
-  uname -s | grep -q Darwin && return
-
-  mkdir -p test/
-  cat > test/BUILD <<'EOF'
-java_binary(
-  name = "app",
+function setup_jni_targets() {
+  repo="${1:-.}"
+  if [ "$repo" != "." ]; then
+    mkdir $repo
+    touch $repo/REPO.bazel
+    cat > $(setup_module_dot_bazel) <<EOF
+local_repository = use_repo_rule("@bazel_tools//tools/build_defs/repo:local.bzl", "local_repository")
+local_repository(
+  name="$repo",
+  path="./$repo",
+)
+EOF
+  fi
+  mkdir -p ${repo}/jni
+  cat > $repo/jni/BUILD <<EOF
+java_library(
+  name = "lib",
   srcs = ["App.java"],
-  main_class = 'foo.App',
-  data = [":libnative.so"],
-  jvm_flags = ["-Djava.library.path=test"],
+  deps = [":libnative.so"],
+  visibility = ["//visibility:public"],
 )
 cc_binary(
   name = "libnative.so",
@@ -1607,7 +1608,7 @@ cc_binary(
   deps = ["@bazel_tools//tools/jdk:jni"],
 )
 EOF
-  cat > test/App.java <<'EOF'
+  cat > ${repo}/jni/App.java <<'EOF'
 package foo;
 
 public class App {
@@ -1616,15 +1617,69 @@ public class App {
   private static native void f(int x);
 }
 EOF
-  cat > test/native.cc <<'EOF'
+  cat > ${repo}/jni/native.cc <<'EOF'
 #include <jni.h>
 #include <stdio.h>
 
 extern "C" JNIEXPORT void JNICALL Java_foo_App_f(JNIEnv *env, jclass clazz, jint x) {
   printf("hello %d\n", x);
 }
 EOF
-  bazel run //test:app > $TEST_log || {
+
+  mkdir -p test/
+  cat > test/BUILD <<EOF
+java_binary(
+  name = "app",
+  main_class = 'foo.App',
+  runtime_deps = ["@$1//jni:lib"],
+)
+EOF
+}
+
+function test_jni() {
+  # Skip on MS Windows, as Bazel does not create a runfiles symlink tree.
+  # (MSYS_NT is the system name reported by MinGW uname.)
+  # TODO(adonovan): make this work.
+  uname -s | grep -q MSYS_NT && return
+
+  # Skip on Darwin, as System.loadLibrary looks for a file named
+  # .dylib, not .so, and that's not what the file is called.
+  # TODO(adonovan): make this just work.
+  uname -s | grep -q Darwin && return
+
+  setup_jni_targets ""
+
+  bazel run //test:app >> $TEST_log || {
+    find bazel-bin/ | native # helpful for debugging
+    fail "bazel run command failed"
+  }
+  expect_log "hello 123"
+}
+
+function test_jni_external_repo_legacy_external_runfiles() {
+  # Skip on MS Windows, see details in test_jni
+  uname -s | grep -q MSYS_NT && return
+  # Skip on Darwin, see details in test_jni
+  uname -s | grep -q Darwin && return
+
+  setup_jni_targets "my_other_repo"
+
+  bazel run --legacy_external_runfiles //test:app >> $TEST_log || {
+    find bazel-bin/ | native # helpful for debugging
+    fail "bazel run command failed"
+  }
+  expect_log "hello 123"
+}
+
+function test_jni_external_repo_no_legacy_external_runfiles() {
+  # Skip on MS Windows, see details in test_jni
+  uname -s | grep -q MSYS_NT && return
+  # Skip on Darwin, see details in test_jni
+  uname -s | grep -q Darwin && return
+
+  setup_jni_targets "my_other_repo"
+
+  bazel run --nolegacy_external_runfiles //test:app >> $TEST_log || {
     find bazel-bin/ | native # helpful for debugging
     fail "bazel run command failed"
   }