Smooth out differences in cc_common.compile() API

This adds a new parameter to the cc_common.compile API that is generally unsupported and should be removed in the future. The purpose of this change is to have a single API internally and in OSS. RELNOTES:none PiperOrigin-RevId: 487030674 Change-Id: I05c1b924257b3e6e653e21cdc5b28ec55aeec975
fmeum · Nov 8, 2022 · 7befa7b · 7befa7b
1 parent eaf7f8b
commit 7befa7b
Show file tree

Hide file tree

Showing 4 changed files with 38 additions and 10 deletions.
diff --git a/src/main/java/com/google/devtools/build/lib/bazel/rules/cpp/BazelCcModule.java b/src/main/java/com/google/devtools/build/lib/bazel/rules/cpp/BazelCcModule.java
@@ -14,7 +14,6 @@
 
 package com.google.devtools.build.lib.bazel.rules.cpp;
 
-import com.google.common.collect.ImmutableList;
 import com.google.devtools.build.lib.actions.Artifact;
 import com.google.devtools.build.lib.analysis.platform.ConstraintValueInfo;
 import com.google.devtools.build.lib.analysis.starlark.StarlarkActionFactory;
@@ -105,6 +104,7 @@ public Tuple compile(
       String name,
       boolean disallowPicOutputs,
       boolean disallowNopicOutputs,
+      Sequence<?> additionalIncludeScanningRoots, // <Artifact> expected
       Sequence<?> additionalInputs, // <Artifact> expected
       Object moduleMap,
       Object additionalModuleMaps,
@@ -121,7 +121,7 @@ public Tuple compile(
       Object nonCompilationAdditionalInputs,
       StarlarkThread thread)
       throws EvalException, InterruptedException {
-    return compile(
+    return super.compile(
         starlarkActionFactoryApi,
         starlarkFeatureConfiguration,
         starlarkCcToolchainProvider,
@@ -146,9 +146,8 @@ public Tuple compile(
         disallowPicOutputs,
         disallowNopicOutputs,
         /* grepIncludes= */ null,
-        /* headersForClifDoNotUseThisParam= */ ImmutableList.of(),
-        StarlarkList.immutableCopyOf(
-            Sequence.cast(additionalInputs, Artifact.class, "additional_inputs")),
+        /* additionalIncludeScanningRoots= */ StarlarkList.empty(),
+        additionalInputs,
         moduleMap,
         additionalModuleMaps,
         propagateModuleMapToCompileAction,

diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcCompilationHelper.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcCompilationHelper.java
@@ -801,7 +801,7 @@ public CcCompilationHelper addAdditionalCompilationInputs(
   // TODO(plf): This is only needed for CLIF. Investigate whether this is strictly necessary or
   // there is a way to avoid include scanning for CLIF rules.
   @CanIgnoreReturnValue
-  public CcCompilationHelper addAditionalIncludeScanningRoots(
+  public CcCompilationHelper addAdditionalIncludeScanningRoots(
       Collection<Artifact> additionalIncludeScanningRoots) {
     this.additionalIncludeScanningRoots.addAll(additionalIncludeScanningRoots);
     return this;

diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcModule.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcModule.java
@@ -139,6 +139,11 @@ public abstract class CcModule
           PackageIdentifier.createInMainRepo("rust/private"),
           PackageIdentifier.createUnchecked("rules_rust", "rust/private"));
 
+  // TODO(bazel-team): This only makes sense for the parameter in cc_common.compile()
+  //  additional_include_scanning_roots which is technical debt and should go away.
+  private static final PathFragment MATCH_CLIF_ALLOWLISTED_LOCATION =
+      PathFragment.create("tools/build_defs/clif");
+
   public abstract CppSemantics getSemantics();
 
   public abstract CppSemantics getSemantics(Language language);
@@ -2021,8 +2026,8 @@ protected Tuple compile(
       boolean disallowPicOutputs,
       boolean disallowNopicOutputs,
       Artifact grepIncludes,
-      List<Artifact> headersForClifDoNotUseThisParam,
-      Sequence<?> additionalInputs,
+      Sequence<?> additionalIncludeScanningRoots, // <Artifact> expected
+      Sequence<?> additionalInputs, // <Artifact> expected
       Object moduleMapNoneable,
       Object additionalModuleMapsNoneable,
       Object propagateModuleMapToCompileActionObject,
@@ -2054,6 +2059,9 @@ protected Tuple compile(
       CcModule.checkPrivateStarlarkificationAllowlist(thread);
     }
 
+    List<Artifact> includeScanningRoots =
+        getAdditionalIncludeScanningRoots(additionalIncludeScanningRoots, thread);
+
     StarlarkActionFactory actions = starlarkActionFactoryApi;
     CcToolchainProvider ccToolchainProvider =
         convertFromNoneable(starlarkCcToolchainProvider, null);
@@ -2181,11 +2189,10 @@ protected Tuple compile(
         .setCopts(
             ImmutableList.copyOf(
                 Sequence.cast(userCompileFlags, String.class, "user_compile_flags")))
-        .addAdditionalCompilationInputs(headersForClifDoNotUseThisParam)
         .addAdditionalCompilationInputs(
             Sequence.cast(additionalInputs, Artifact.class, "additional_inputs"))
         .addAdditionalInputs(nonCompilationAdditionalInputs)
-        .addAditionalIncludeScanningRoots(headersForClifDoNotUseThisParam)
+        .addAdditionalIncludeScanningRoots(includeScanningRoots)
         .setPurpose(common.getPurpose(getSemantics(language)))
         .addAdditionalExportedHeaders(
             additionalExportedHeaders.stream()
@@ -2251,6 +2258,21 @@ protected Tuple compile(
     }
   }
 
+  private List<Artifact> getAdditionalIncludeScanningRoots(
+      Sequence<?> additionalIncludeScanningRoots, StarlarkThread thread) throws EvalException {
+    PackageIdentifier packageIdentifier =
+        BazelModuleContext.of(Module.ofInnermostEnclosingStarlarkFunction(thread))
+            .label()
+            .getPackageIdentifier();
+    if (!additionalIncludeScanningRoots.isEmpty()
+        && !packageIdentifier.getPackageFragment().startsWith(MATCH_CLIF_ALLOWLISTED_LOCATION)) {
+      throw Starlark.errorf(
+          "This can only be used in %s", MATCH_CLIF_ALLOWLISTED_LOCATION.getPathString());
+    }
+    return Sequence.cast(
+        additionalIncludeScanningRoots, Artifact.class, "additional_include_scanning_roots");
+  }
+
   private boolean checkAllSourcesContainTuplesOrNoneOfThem(ImmutableList<Sequence<?>> files)
       throws EvalException {
     boolean nonTuple = false;

diff --git a/src/main/java/com/google/devtools/build/lib/starlarkbuildapi/cpp/BazelCcModuleApi.java b/src/main/java/com/google/devtools/build/lib/starlarkbuildapi/cpp/BazelCcModuleApi.java
@@ -260,6 +260,12 @@ public interface BazelCcModuleApi<
             positional = false,
             named = true,
             defaultValue = "False"),
+        @Param(
+            name = "additional_include_scanning_roots",
+            documented = false,
+            positional = false,
+            named = true,
+            defaultValue = "[]"),
         @Param(
             name = "additional_inputs",
             doc = "List of additional files needed for compilation of srcs",
@@ -385,6 +391,7 @@ Tuple compile(
       String name,
       boolean disallowPicOutputs,
       boolean disallowNopicOutputs,
+      Sequence<?> additionalIncludeScanningRoots, // <FileT> expected
       Sequence<?> additionalInputs, // <FileT> expected
       Object moduleMap,
       Object additionalModuleMaps,