Sibling repository layout support for Android NDK builds.

PiperOrigin-RevId: 354192973

src/main/java/com/google/devtools/build/lib/bazel/rules/android/AndroidNdkRepositoryFunction.java

@@ -35,9 +35,11 @@
 import com.google.devtools.build.lib.events.Event;
 import com.google.devtools.build.lib.packages.Rule;
 import com.google.devtools.build.lib.packages.Type;
+import com.google.devtools.build.lib.packages.semantics.BuildLanguageOptions;
 import com.google.devtools.build.lib.rules.repository.RepositoryDirectoryValue;
 import com.google.devtools.build.lib.rules.repository.WorkspaceAttributeMapper;
 import com.google.devtools.build.lib.skyframe.DirectoryListingValue;
+import com.google.devtools.build.lib.skyframe.PrecomputedValue;
 import com.google.devtools.build.lib.util.ResourceFileLoader;
 import com.google.devtools.build.lib.vfs.FileSystemUtils;
 import com.google.devtools.build.lib.vfs.Path;
@@ -59,6 +61,7 @@
 import java.util.Map;
 import net.starlark.java.eval.EvalException;
 import net.starlark.java.eval.Starlark;
+import net.starlark.java.eval.StarlarkSemantics;
 
 /** Implementation of the {@code android_ndk_repository} rule. */
 public class AndroidNdkRepositoryFunction extends AndroidRepositoryFunction {
@@ -376,13 +379,22 @@ public RepositoryDirectoryValue.Builder fetch(
           AndroidNdkCrosstools.KNOWN_NDK_MAJOR_REVISIONS.get(ndkRelease.majorRevision);
     }
 
+    StarlarkSemantics starlarkSemantics = PrecomputedValue.STARLARK_SEMANTICS.get(env);
+    if (starlarkSemantics == null) {
+      return null;
+    }
+    boolean siblingRepositoryLayout =
+        starlarkSemantics.getBool(BuildLanguageOptions.EXPERIMENTAL_SIBLING_REPOSITORY_LAYOUT);
+
     ApiLevel apiLevel = ndkMajorRevision.apiLevel(env.getListener(), ruleName, apiLevelString);
 
     ImmutableList.Builder<CrosstoolStlPair> crosstoolsAndStls = ImmutableList.builder();
     try {
 
       String hostPlatform = AndroidNdkCrosstools.getHostPlatform(ndkRelease);
-      NdkPaths ndkPaths = new NdkPaths(ruleName, hostPlatform, apiLevel, ndkRelease.majorRevision);
+      NdkPaths ndkPaths =
+          new NdkPaths(
+              ruleName, hostPlatform, apiLevel, ndkRelease.majorRevision, siblingRepositoryLayout);
 
       for (StlImpl stlImpl : StlImpls.get(ndkPaths, ndkRelease.majorRevision)) {
         CrosstoolRelease crosstoolRelease =

src/main/java/com/google/devtools/build/lib/bazel/rules/android/BUILD

@@ -39,6 +39,7 @@ java_library(
         "//src/main/java/com/google/devtools/build/lib/collect/nestedset",
         "//src/main/java/com/google/devtools/build/lib/events",
         "//src/main/java/com/google/devtools/build/lib/packages",
+        "//src/main/java/com/google/devtools/build/lib/packages/semantics",
         "//src/main/java/com/google/devtools/build/lib/rules:repository/repository_directory_value",
         "//src/main/java/com/google/devtools/build/lib/rules:repository/repository_function",
         "//src/main/java/com/google/devtools/build/lib/rules:repository/workspace_attribute_mapper",
@@ -50,6 +51,7 @@ java_library(
         "//src/main/java/com/google/devtools/build/lib/rules/java:java-compilation",
         "//src/main/java/com/google/devtools/build/lib/skyframe:directory_listing_value",
         "//src/main/java/com/google/devtools/build/lib/skyframe:dirents",
+        "//src/main/java/com/google/devtools/build/lib/skyframe:precomputed_value",
         "//src/main/java/com/google/devtools/build/lib/util",
         "//src/main/java/com/google/devtools/build/lib/util:shell_escaper",
         "//src/main/java/com/google/devtools/build/lib/vfs",

src/main/java/com/google/devtools/build/lib/bazel/rules/android/ndkcrosstools/NdkPaths.java

@@ -41,13 +41,19 @@ public static String stripRepositoryPrefix(String path) {
   private final String hostPlatform;
   private final ApiLevel apiLevel;
   private final Integer majorRevision;
+  private final String externalExecPathBase;
 
   public NdkPaths(
-      String repositoryName, String hostPlatform, ApiLevel apiLevel, Integer majorRevision) {
+      String repositoryName,
+      String hostPlatform,
+      ApiLevel apiLevel,
+      Integer majorRevision,
+      boolean siblingRepositoryLayout) {
     this.repositoryName = repositoryName;
     this.hostPlatform = hostPlatform;
     this.apiLevel = apiLevel;
     this.majorRevision = majorRevision;
+    this.externalExecPathBase = siblingRepositoryLayout ? ".." : "external";
   }
 
   public ImmutableList<ToolPath> createToolpaths(String toolchainName, String targetPlatform,
@@ -112,9 +118,11 @@ public static String getToolchainDirectoryFromToolPath(String toolPath) {
   public String createGccToolchainPath(String toolchainName) {
 
     String gccToolchainPathTemplate =
-        "external/%repositoryName%/ndk/toolchains/%toolchainName%/prebuilt/%hostPlatform%";
+        "%externalExecPathBase%/%repositoryName%/ndk/toolchains/%toolchainName%/prebuilt"
+            + "/%hostPlatform%";
 
     return gccToolchainPathTemplate
+        .replace("%externalExecPathBase%", externalExecPathBase)
         .replace("%repositoryName%", repositoryName)
         .replace("%toolchainName%", toolchainName)
         .replace("%hostPlatform%", hostPlatform);
@@ -130,9 +138,10 @@ public String createGccToolchainPath(String toolchainName) {
    */
   public String createClangToolchainBuiltinIncludeDirectory(String clangVersion) {
     String clangBuiltinIncludeDirectoryPathTemplate =
-        "external/%repositoryName%/ndk/toolchains/llvm/prebuilt/%hostPlatform%/lib64/clang/"
-            + "%clangVersion%/include";
+        "%externalExecPathBase%/%repositoryName%/ndk/toolchains/llvm/prebuilt/%hostPlatform%/lib64/"
+            + "clang/%clangVersion%/include";
     return clangBuiltinIncludeDirectoryPathTemplate
+        .replace("%externalExecPathBase%", externalExecPathBase)
         .replace("%repositoryName%", repositoryName)
         .replace("%hostPlatform%", hostPlatform)
         .replace("%clangVersion%", clangVersion);
@@ -149,12 +158,13 @@ public String createClangToolchainBuiltinIncludeDirectory(String clangVersion) {
   public List<String> createGccToolchainBuiltinIncludeDirectories(
       final String toolchainName, final String targetPlatform, final String gccVersion) {
     final String toolchainIncludePathTemplate =
-        "external/%repositoryName%/ndk/toolchains/%toolchainName%/prebuilt/%hostPlatform%"
-            + "/lib/gcc/%targetPlatform%/%gccVersion%/%includeFolderName%";
+        "%externalExecPathBase%/%repositoryName%/ndk/toolchains/%toolchainName%/prebuilt/"
+            + "%hostPlatform%/lib/gcc/%targetPlatform%/%gccVersion%/%includeFolderName%";
     return Lists.transform(
         ImmutableList.of("include", "include-fixed"),
         includeFolderName ->
             toolchainIncludePathTemplate
+                .replace("%externalExecPathBase%", externalExecPathBase)
                 .replace("%repositoryName%", repositoryName)
                 .replace("%toolchainName%", toolchainName)
                 .replace("%hostPlatform%", hostPlatform)
@@ -175,9 +185,10 @@ public String createBuiltinSysroot(String targetCpu) {
     String correctedApiLevel = apiLevel.getCpuCorrectedApiLevel(targetCpu);
 
     String androidPlatformIncludePathTemplate =
-        "external/%repositoryName%/ndk/platforms/android-%apiLevel%/arch-%arch%";
+        "%externalExecPathBase%/%repositoryName%/ndk/platforms/android-%apiLevel%/arch-%arch%";
 
     return androidPlatformIncludePathTemplate
+        .replace("%externalExecPathBase%", externalExecPathBase)
         .replace("%repositoryName%", repositoryName)
         .replace("%apiLevel%", correctedApiLevel)
         .replace("%arch%", targetCpu);
@@ -192,7 +203,9 @@ public String createBuiltinSysroot() {
     // This location does not exist prior to NDK 15
     Preconditions.checkState(majorRevision >= 15);
 
-    return "external/%repositoryName%/ndk/sysroot".replace("%repositoryName%", repositoryName);
+    return "%externalExecPathBase%/%repositoryName%/ndk/sysroot"
+        .replace("%externalExecPathBase%", externalExecPathBase)
+        .replace("%repositoryName%", repositoryName);
   }
 
   public String getCorrectedApiLevel(String targetCpu) {
@@ -203,7 +216,8 @@ ImmutableList<String> createGnuLibstdcIncludePaths(String gccVersion, String tar
 
     String cpuNoThumb = targetCpu.replaceAll("-thumb$", "");
 
-    String prefix = "external/%repositoryName%/ndk/sources/cxx-stl/gnu-libstdc++/%gccVersion%/";
+    String prefix =
+        "%externalExecPathBase%/%repositoryName%/ndk/sources/cxx-stl/gnu-libstdc++/%gccVersion%/";
     List<String> includePathTemplates = Arrays.asList(
         prefix + "include",
         prefix + "libs/%targetCpu%/include",
@@ -213,17 +227,19 @@ ImmutableList<String> createGnuLibstdcIncludePaths(String gccVersion, String tar
     for (String template : includePathTemplates) {
       includePaths.add(
           template
-            .replace("%repositoryName%", repositoryName)
-            .replace("%gccVersion%", gccVersion)
-            .replace("%targetCpu%", cpuNoThumb));
+              .replace("%externalExecPathBase%", externalExecPathBase)
+              .replace("%repositoryName%", repositoryName)
+              .replace("%gccVersion%", gccVersion)
+              .replace("%targetCpu%", cpuNoThumb));
     }
     return includePaths.build();
   }
 
   ImmutableList<String> createStlportIncludePaths() {
 
     String prefix =
-        "external/%repositoryName%/ndk/sources/cxx-stl/"
+        "%externalExecPathBase%/%repositoryName%/ndk/sources/cxx-stl/"
+            .replace("%externalExecPathBase%", externalExecPathBase)
             .replace("%repositoryName%", repositoryName);
 
     return ImmutableList.of(prefix + "stlport/stlport", prefix + "gabi++/include");
@@ -232,7 +248,9 @@ ImmutableList<String> createStlportIncludePaths() {
   ImmutableList<String> createLibcxxIncludePaths() {
 
     String prefix =
-        "external/%repositoryName%/ndk/sources/".replace("%repositoryName%", repositoryName);
+        "%externalExecPathBase%/%repositoryName%/ndk/sources/"
+            .replace("%externalExecPathBase%", externalExecPathBase)
+            .replace("%repositoryName%", repositoryName);
 
     ImmutableList.Builder<String> includePaths = ImmutableList.builder();
 
@@ -280,7 +298,8 @@ static String createStlRuntimeLibsGlob(
    * @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%"
+    return "%externalExecPathBase%/%repositoryName%/ndk/sources/cxx-stl/llvm-libc++/libs/%targetCpu%"
+        .replace("%externalExecPathBase%", externalExecPathBase)
         .replace("%repositoryName%", repositoryName)
         .replace("%targetCpu%", targetCpu);
   }

src/main/java/com/google/devtools/build/lib/rules/cpp/CcToolchainProviderHelper.java

@@ -373,9 +373,6 @@ static PathFragment resolveIncludeDir(
       }
     }
 
-    if (!PathFragment.isNormalized(pathString)) {
-      throw new InvalidConfigurationException("The include path '" + s + "' is not normalized.");
-    }
     PathFragment path = PathFragment.create(pathString);
     return pathPrefix.getRelative(path);
   }

src/main/java/com/google/devtools/build/lib/rules/cpp/CppConfiguration.java

@@ -667,10 +667,6 @@ public static PathFragment computeDefaultSysroot(String builtInSysroot) {
     if (builtInSysroot.isEmpty()) {
       return null;
     }
-    if (!PathFragment.isNormalized(builtInSysroot)) {
-      throw new IllegalArgumentException(
-          "The built-in sysroot '" + builtInSysroot + "' is not normalized.");
-    }
     return PathFragment.create(builtInSysroot);
   }
 

src/test/java/com/google/devtools/build/lib/bazel/rules/android/ndkcrosstools/AndroidNdkCrosstoolsTest.java

@@ -134,7 +134,7 @@ public AndroidNdkCrosstoolsTest(AndroidNdkCrosstoolsTestParams params) throws IO
     // NDK test data is based on the x86 64-bit Linux Android NDK.
     NdkPaths ndkPaths =
         new NdkPaths(
-            REPOSITORY_NAME, HOST_PLATFORM, params.apiLevel, params.ndkRelease.majorRevision);
+            REPOSITORY_NAME, HOST_PLATFORM, params.apiLevel, params.ndkRelease.majorRevision, true);
 
     ImmutableList.Builder<CrosstoolRelease> crosstools = ImmutableList.builder();
     ImmutableMap.Builder<String, String> stlFilegroupsBuilder = ImmutableMap.builder();

src/test/java/com/google/devtools/build/lib/rules/cpp/CcToolchainTest.java

@@ -313,8 +313,6 @@ public void testInvalidIncludeDirectory() throws Exception {
         "%package(//a:@@a)%", "The package '//a:@@a' is not valid");
     assertInvalidIncludeDirectoryMessage(
         "%package(//a)%foo", "The path in the package.*is not valid");
-    assertInvalidIncludeDirectoryMessage(
-        "%package(//a)%/../bar", "The include path.*is not normalized");
   }
 
   @Test

src/test/shell/bazel/android/android_ndk_integration_test.sh

@@ -219,6 +219,28 @@ function test_android_binary() {
   check_soname
 }
 
+function test_android_binary_sibling_repository_layout() {
+  create_new_workspace
+  setup_android_sdk_support
+  setup_android_ndk_support
+  create_android_binary
+
+  # TODO(b/161709111): enable platform-based toolchain resolution when
+  # --fat_apk_cpu fully supports it. Now it sets a split transition that clears
+  # out --platforms. The mapping in android_helper.sh re-enables a test Android
+  # platform for ARM but not x86. Enabling it for x86 requires an
+  # Android-compatible cc toolchain in tools/cpp/BUILD.tools.
+  add_to_bazelrc "build --noincompatible_enable_android_toolchain_resolution"
+
+  cpus="armeabi,armeabi-v7a,arm64-v8a,x86,x86_64"
+
+  bazel build --experimental_sibling_repository_layout -s \
+      //java/bazel:bin --fat_apk_cpu="$cpus" \
+      || fail "build failed"
+  check_num_sos
+  check_soname
+}
+
 function test_android_binary_clang() {
   create_new_workspace
   setup_android_sdk_support