Set the correct include path for r13b's llvm-libc++ headers and fix compilation with @androidndk//:toolchain-libcpp with missing link time files.

This fix needs a way to compare revision numbers, so the type of NdkRelease.majorRevision has been changed to Integer. This also paves the way for r15+ support.

Fixes https://github.com/bazelbuild/bazel/issues/3641
Fixes https://github.com/bazelbuild/bazel/issues/3923
Fixes https://github.com/bazelbuild/bazel/issues/4677

TESTED=bazel test //src/test/shell/bazel/android:android_ndk_integration_test with r11, r12, r13, r14, r15

RELNOTES: Fixed include paths for NDK r13+ llvm-libc++ headers to `ndk/sources/cxx-stl/llvm-libc++/include` and `ndk/sources/cxx-stl/llvm-libc++abi/include`
PiperOrigin-RevId: 186783465

8 files changed, 182 insertions, 38 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/bazel/rules/android/AndroidNdkRepositoryFunction.java b/src/main/java/com/google/devtools/build/lib/bazel/rules/android/AndroidNdkRepositoryFunction.java
index 0f39042e49..faae014e40 100644
--- a/src/main/java/com/google/devtools/build/lib/bazel/rules/android/AndroidNdkRepositoryFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/bazel/rules/android/AndroidNdkRepositoryFunction.java
@@ -207,7 +207,7 @@ public class AndroidNdkRepositoryFunction extends AndroidRepositoryFunction {
     try {
 
       String hostPlatform = AndroidNdkCrosstools.getHostPlatform(ndkRelease);
-      NdkPaths ndkPaths = new NdkPaths(ruleName, hostPlatform, apiLevel);
+      NdkPaths ndkPaths = new NdkPaths(ruleName, hostPlatform, apiLevel, ndkRelease.majorRevision);
 
       for (StlImpl stlImpl : StlImpls.get(ndkPaths)) {
         CrosstoolRelease crosstoolRelease =
diff --git a/src/main/java/com/google/devtools/build/lib/bazel/rules/android/ndkcrosstools/AndroidNdkCrosstools.java b/src/main/java/com/google/devtools/build/lib/bazel/rules/android/ndkcrosstools/AndroidNdkCrosstools.java
index 64a55d1df3..ff1127da5c 100644
--- a/src/main/java/com/google/devtools/build/lib/bazel/rules/android/ndkcrosstools/AndroidNdkCrosstools.java
+++ b/src/main/java/com/google/devtools/build/lib/bazel/rules/android/ndkcrosstools/AndroidNdkCrosstools.java
@@ -32,16 +32,16 @@ public final class AndroidNdkCrosstools {
 
   // NDK minor revisions should be backwards compatible within a major revision, so all that needs
   // to be tracked here are the major revision numbers.
-  public static final ImmutableMap<String, NdkMajorRevision> KNOWN_NDK_MAJOR_REVISIONS =
+  public static final ImmutableMap<Integer, NdkMajorRevision> KNOWN_NDK_MAJOR_REVISIONS =
       ImmutableMap.of(
-          "10", new NdkMajorRevisionR10(),
-          "11", new NdkMajorRevisionR11(),
-          "12", new NdkMajorRevisionR12(),
-          "13", new NdkMajorRevisionR13("3.8.256229"),
+          10, new NdkMajorRevisionR10(),
+          11, new NdkMajorRevisionR11(),
+          12, new NdkMajorRevisionR12(),
+          13, new NdkMajorRevisionR13("3.8.256229"),
           // The only difference between the NDK13 and NDK14 CROSSTOOLs is the version of clang in
           // built-in includes paths, so we can reuse everything else.
-          "14", new NdkMajorRevisionR13("3.8.275480"));
-  public static final Map.Entry<String, NdkMajorRevision> LATEST_KNOWN_REVISION =
+          14, new NdkMajorRevisionR13("3.8.275480"));
+  public static final Map.Entry<Integer, NdkMajorRevision> LATEST_KNOWN_REVISION =
       Iterables.getLast(KNOWN_NDK_MAJOR_REVISIONS.entrySet());
 
   /**
diff --git a/src/main/java/com/google/devtools/build/lib/bazel/rules/android/ndkcrosstools/NdkPaths.java b/src/main/java/com/google/devtools/build/lib/bazel/rules/android/ndkcrosstools/NdkPaths.java
index 45b62deea6..5d6b72d289 100644
--- a/src/main/java/com/google/devtools/build/lib/bazel/rules/android/ndkcrosstools/NdkPaths.java
+++ b/src/main/java/com/google/devtools/build/lib/bazel/rules/android/ndkcrosstools/NdkPaths.java
@@ -39,11 +39,14 @@ public class NdkPaths {
   private final String repositoryName;
   private final String hostPlatform;
   private final ApiLevel apiLevel;
+  private final Integer majorRevision;
 
-  public NdkPaths(String repositoryName, String hostPlatform, ApiLevel apiLevel) {
+  public NdkPaths(
+      String repositoryName, String hostPlatform, ApiLevel apiLevel, Integer majorRevision) {
     this.repositoryName = repositoryName;
     this.hostPlatform = hostPlatform;
     this.apiLevel = apiLevel;
+    this.majorRevision = majorRevision;
   }
 
   public ImmutableList<ToolPath> createToolpaths(String toolchainName, String targetPlatform,
@@ -207,10 +210,21 @@ public class NdkPaths {
     String prefix =
         "external/%repositoryName%/ndk/sources/".replace("%repositoryName%", repositoryName);
 
-    return ImmutableList.of(
-        prefix + "cxx-stl/llvm-libc++/libcxx/include",
-        prefix + "cxx-stl/llvm-libc++abi/libcxxabi/include",
-        prefix + "android/support/include");
+    ImmutableList.Builder<String> includePaths = ImmutableList.builder();
+
+    includePaths.add(prefix + "android/support/include");
+
+    if (majorRevision <= 12) {
+      includePaths.add(prefix + "cxx-stl/llvm-libc++/libcxx/include");
+      includePaths.add(prefix + "cxx-stl/llvm-libc++abi/libcxxabi/include");
+    } else {
+      // libcxx/include was moved one level up from r13 onwards.
+      // See https://github.com/bazelbuild/bazel/issues/3641
+      includePaths.add(prefix + "cxx-stl/llvm-libc++/include");
+      includePaths.add(prefix + "cxx-stl/llvm-libc++abi/include");
+    }
+
+    return includePaths.build();
   }
 
   /**
@@ -236,4 +250,14 @@ public class NdkPaths {
         .replace("%targetCpu%", targetCpu)
         .replace("%fileExtension%", fileExtension);
   }
+
+  /**
+   * @param targetCpu Target CPU
+   * @return the directory of the target CPU's runtime .a files for linking
+   */
+  public String createLibcppLinkerPath(String targetCpu) {
+    return "external/%repositoryName%/ndk/sources/cxx-stl/llvm-libc++/libs/%targetCpu%"
+        .replace("%repositoryName%", repositoryName)
+        .replace("%targetCpu%", targetCpu);
+  }
 }
diff --git a/src/main/java/com/google/devtools/build/lib/bazel/rules/android/ndkcrosstools/NdkRelease.java b/src/main/java/com/google/devtools/build/lib/bazel/rules/android/ndkcrosstools/NdkRelease.java
index 31540b3d6d..7b393ce166 100644
--- a/src/main/java/com/google/devtools/build/lib/bazel/rules/android/ndkcrosstools/NdkRelease.java
+++ b/src/main/java/com/google/devtools/build/lib/bazel/rules/android/ndkcrosstools/NdkRelease.java
@@ -52,17 +52,19 @@ public class NdkRelease {
     String revision = properties.getProperty(REVISION_PROPERTY);
     String[] revisionParsed = revision.split("\\.");
     if (revisionParsed.length < 2) {
-      // Unable to parse Pkg.Revision. Return invalid NdkRelease.
-      return new NdkRelease(revision, false, null, null, null, false);
+      // Unable to parse Pkg.Revision. Return invalid NdkRelease that is assumed to be the
+      // latest NDK revision.
+      return new NdkRelease(
+          revision, false, AndroidNdkCrosstools.LATEST_KNOWN_REVISION.getKey(), null, null, false);
     } else {
       return new NdkRelease(
           revision, // raw revision
           true, // isValid
-          revisionParsed[0], // major revision
+          Integer.parseInt(revisionParsed[0]), // major revision
           revisionParsed[1], // minor revision
           null, // release candidate
           true // is64 bit. 32-bit NDKs are provided for only windows.
-      );
+          );
     }
   }
 
@@ -86,19 +88,25 @@ public class NdkRelease {
       return new NdkRelease(
           revisionString,
           isValid,
-          matcher.group("Mrev"), /* major revision */
+          Integer.parseInt(matcher.group("Mrev")), /* major revision */
           matcher.group("mrev"), /* minor revision */
           matcher.group("rc"), /* releaseCandidate */
           matcher.group("s4") != null /* is64Bit */);
     } else {
-      return new NdkRelease(revisionString, false, null, null, null, false);
+      return new NdkRelease(
+          revisionString,
+          false,
+          AndroidNdkCrosstools.LATEST_KNOWN_REVISION.getKey(), // assume latest NDK revision
+          null,
+          null,
+          false);
     }
   }
 
   public final String rawRelease;
   public final boolean isValid;
 
-  public final String majorRevision;
+  public final Integer majorRevision;
   public final String minorRevision;
   public final String releaseCandidate;
   public final boolean is64Bit;
@@ -106,7 +114,7 @@ public class NdkRelease {
   private NdkRelease(
       String rawRelease,
       boolean isValid,
-      String majorRevision,
+      Integer majorRevision,
       String minorRevision,
       String releaseCandidate,
       boolean is64Bit) {
diff --git a/src/main/java/com/google/devtools/build/lib/bazel/rules/android/ndkcrosstools/StlImpls.java b/src/main/java/com/google/devtools/build/lib/bazel/rules/android/ndkcrosstools/StlImpls.java
index 6c57bfe358..0508b7c7c5 100644
--- a/src/main/java/com/google/devtools/build/lib/bazel/rules/android/ndkcrosstools/StlImpls.java
+++ b/src/main/java/com/google/devtools/build/lib/bazel/rules/android/ndkcrosstools/StlImpls.java
@@ -52,6 +52,7 @@ public final class StlImpls {
     public void addStlImpl(Builder toolchain, String gccVersion) {
       addBaseStlImpl(toolchain, null);
       toolchain.addAllUnfilteredCxxFlag(createIncludeFlags(ndkPaths.createLibcxxIncludePaths()));
+      toolchain.addLinkerFlag("-L" + ndkPaths.createLibcppLinkerPath(toolchain.getTargetCpu()));
     }
   }
 
diff --git a/src/test/java/com/google/devtools/build/lib/bazel/rules/android/ndkcrosstools/AndroidNdkCrosstoolsTest.java b/src/test/java/com/google/devtools/build/lib/bazel/rules/android/ndkcrosstools/AndroidNdkCrosstoolsTest.java
index 00719ffbfd..a4f24c6d45 100644
--- a/src/test/java/com/google/devtools/build/lib/bazel/rules/android/ndkcrosstools/AndroidNdkCrosstoolsTest.java
+++ b/src/test/java/com/google/devtools/build/lib/bazel/rules/android/ndkcrosstools/AndroidNdkCrosstoolsTest.java
@@ -131,7 +131,9 @@ public class AndroidNdkCrosstoolsTest {
 
   public AndroidNdkCrosstoolsTest(AndroidNdkCrosstoolsTestParams params) throws IOException {
     // NDK test data is based on the x86 64-bit Linux Android NDK.
-    NdkPaths ndkPaths = new NdkPaths(REPOSITORY_NAME, HOST_PLATFORM, params.apiLevel);
+    NdkPaths ndkPaths =
+        new NdkPaths(
+            REPOSITORY_NAME, HOST_PLATFORM, params.apiLevel, params.ndkRelease.majorRevision);
 
     ImmutableList.Builder<CrosstoolRelease> crosstools = ImmutableList.builder();
     ImmutableMap.Builder<String, String> stlFilegroupsBuilder = ImmutableMap.builder();
diff --git a/src/test/java/com/google/devtools/build/lib/bazel/rules/android/ndkcrosstools/NdkReleaseTest.java b/src/test/java/com/google/devtools/build/lib/bazel/rules/android/ndkcrosstools/NdkReleaseTest.java
index c3f9c256ef..08733895c5 100644
--- a/src/test/java/com/google/devtools/build/lib/bazel/rules/android/ndkcrosstools/NdkReleaseTest.java
+++ b/src/test/java/com/google/devtools/build/lib/bazel/rules/android/ndkcrosstools/NdkReleaseTest.java
@@ -28,20 +28,20 @@ public class NdkReleaseTest {
 
   @Test
   public void testReleaseParsing() {
-    testNdkRelease("r8",                "8",  null, null, false);
-    testNdkRelease("r8 (64-bit)",       "8",  null, null, true);
-    testNdkRelease("r10",               "10", null, null, false);
-    testNdkRelease("r10 (64-bit)",      "10", null, null, true);
-    testNdkRelease("r10-rc4",           "10", null, "rc4",  false);
-    testNdkRelease("r10-rc4 (64-bit)",  "10", null, "rc4",  true);
-    testNdkRelease("r10e",              "10", "e",  null,  false);
-    testNdkRelease("r10e (64-bit)",     "10", "e",  null,  true);
-    testNdkRelease("r10e-rc4",          "10", "e",  "rc4", false);
-    testNdkRelease("r10e-rc4 (64-bit)", "10", "e",  "rc4", true);
+    testNdkRelease("r8", 8, null, null, false);
+    testNdkRelease("r8 (64-bit)", 8, null, null, true);
+    testNdkRelease("r10", 10, null, null, false);
+    testNdkRelease("r10 (64-bit)", 10, null, null, true);
+    testNdkRelease("r10-rc4", 10, null, "rc4", false);
+    testNdkRelease("r10-rc4 (64-bit)", 10, null, "rc4", true);
+    testNdkRelease("r10e", 10, "e", null, false);
+    testNdkRelease("r10e (64-bit)", 10, "e", null, true);
+    testNdkRelease("r10e-rc4", 10, "e", "rc4", false);
+    testNdkRelease("r10e-rc4 (64-bit)", 10, "e", "rc4", true);
 
     try {
       // this is actually invalid
-      testNdkRelease("r10e-rc4 (abc)", "10", "e", "rc4", false);
+      testNdkRelease("r10e-rc4 (abc)", 10, "e", "rc4", false);
       throw new Error();
     } catch (AssertionError e) {
       // expected
@@ -55,7 +55,7 @@ public class NdkReleaseTest {
     NdkRelease ndkRelease = NdkRelease.create(releaseString);
     assertThat(ndkRelease.isValid).isTrue();
     assertThat(ndkRelease.rawRelease).isEqualTo("11.2.2725575");
-    assertThat(ndkRelease.majorRevision).isEqualTo("11");
+    assertThat(ndkRelease.majorRevision).isEqualTo(11);
     assertThat(ndkRelease.minorRevision).isEqualTo("2");
     assertThat(ndkRelease.releaseCandidate).isNull();
     assertThat(ndkRelease.is64Bit).isTrue();
@@ -68,15 +68,15 @@ public class NdkReleaseTest {
     NdkRelease ndkRelease = NdkRelease.create(releaseString);
     assertThat(ndkRelease.isValid).isTrue();
     assertThat(ndkRelease.rawRelease).isEqualTo("12.1.297705");
-    assertThat(ndkRelease.majorRevision).isEqualTo("12");
+    assertThat(ndkRelease.majorRevision).isEqualTo(12);
     assertThat(ndkRelease.minorRevision).isEqualTo("1");
     assertThat(ndkRelease.releaseCandidate).isNull();
     assertThat(ndkRelease.is64Bit).isTrue();
   }
-  
+
   private static void testNdkRelease(
       String ndkReleaseString,
-      String majorRelease,
+      Integer majorRelease,
       String minorRelease,
       String releaseCandidate,
       boolean is64Bit) {
@@ -115,7 +115,8 @@ public class NdkReleaseTest {
     NdkRelease ndkRelease = NdkRelease.create(ndkReleaseString);
     assertThat(ndkRelease.isValid).isFalse();
     assertThat(ndkRelease.rawRelease).isEqualTo(ndkReleaseString);
-    assertThat(ndkRelease.majorRevision).isNull();
+    assertThat(ndkRelease.majorRevision)
+        .isEqualTo(AndroidNdkCrosstools.LATEST_KNOWN_REVISION.getKey());
     assertThat(ndkRelease.minorRevision).isNull();
     assertThat(ndkRelease.releaseCandidate).isNull();
     assertThat(ndkRelease.is64Bit).isFalse();
diff --git a/src/test/shell/bazel/android/android_ndk_integration_test.sh b/src/test/shell/bazel/android/android_ndk_integration_test.sh
index d87cac31ca..b40103afbc 100755
--- a/src/test/shell/bazel/android/android_ndk_integration_test.sh
+++ b/src/test/shell/bazel/android/android_ndk_integration_test.sh
@@ -323,4 +323,112 @@ EOF
     || fail "build failed"
 }
 
+function test_crosstool_stlport() {
+  create_new_workspace
+  setup_android_ndk_support
+  cat > BUILD <<EOF
+cc_binary(
+    name = "foo",
+    srcs = ["foo.cc"],
+    linkopts = ["-ldl"],
+)
+EOF
+  cat > foo.cc <<EOF
+#include <string>
+#include <jni.h>
+#include <android/log.h>
+#include <cstdio>
+#include <iostream>
+
+using namespace std;
+int main(){
+  string foo = "foo";
+  string bar = "bar";
+  string foobar = foo + bar;
+  return 0;
+}
+EOF
+  assert_build //:foo \
+    --cpu=armeabi-v7a \
+    --crosstool_top=@androidndk//:toolchain-stlport \
+    --host_crosstool_top=@bazel_tools//tools/cpp:toolchain
+}
+
+function test_crosstool_libcpp() {
+  create_new_workspace
+  setup_android_ndk_support
+  cat > BUILD <<EOF
+cc_binary(
+    name = "foo",
+    srcs = ["foo.cc"],
+    linkopts = ["-ldl", "-lm"],
+)
+EOF
+  cat > foo.cc <<EOF
+#include <string>
+#include <jni.h>
+#include <android/log.h>
+#include <cstdio>
+#include <iostream>
+
+using namespace std;
+int main(){
+  string foo = "foo";
+  string bar = "bar";
+  string foobar = foo + bar;
+  return 0;
+}
+EOF
+  assert_build //:foo \
+    --cpu=armeabi-v7a \
+    --crosstool_top=@androidndk//:toolchain-libcpp \
+    --host_crosstool_top=@bazel_tools//tools/cpp:toolchain
+}
+
+function test_crosstool_gnu_libstdcpp() {
+  create_new_workspace
+  setup_android_ndk_support
+  cat > BUILD <<EOF
+cc_binary(
+    name = "foo",
+    srcs = ["foo.cc"],
+)
+EOF
+  cat > foo.cc <<EOF
+#include <string>
+#include <jni.h>
+#include <android/log.h>
+#include <cstdio>
+#include <iostream>
+
+using namespace std;
+int main(){
+  string foo = "foo";
+  string bar = "bar";
+  string foobar = foo + bar;
+  return 0;
+}
+EOF
+  assert_build //:foo \
+    --cpu=armeabi-v7a \
+    --crosstool_top=@androidndk//:toolchain-gnu-libstdcpp \
+    --host_crosstool_top=@bazel_tools//tools/cpp:toolchain
+}
+
+function test_crosstool_libcpp_with_multiarch() {
+  create_new_workspace
+  setup_android_sdk_support
+  setup_android_ndk_support
+  create_android_binary
+
+  cpus="armeabi,armeabi-v7a,arm64-v8a,x86,x86_64"
+
+  assert_build //java/bazel:bin \
+    --fat_apk_cpu="$cpus" \
+    --android_crosstool_top=@androidndk//:toolchain-libcpp \
+    --host_crosstool_top=@bazel_tools//tools/cpp:toolchain
+  check_num_sos
+  check_soname
+}
+
 run_suite "Android NDK integration tests"