Roll forward of unknown commit: Move grep-includes to the cc_toolchain

NEW:
  * (Credits to mai93@) Fix a bug with Exec format error - Added a cpp toolchain as an attribute on J2ObjcAspect and changed CppHelper.
  * Sync and simple changes were needed because of Starlarkification of CcToolchainProviderHelper.

Automated rollback of commit 13ea972.

*** Reason for rollback ***

Exec format error fixed.

*** Original change description ***

Automated rollback of commit 770c50b.

*** Reason for rollback ***

This CL raises "Exec format error" in some targets.

*** Original change description ***

Move grep-includes to the cc_toolchain

Motivation for this change was an error with the wrong exec platform for grep-includes when Automatic Exec Groups are enabled.

To solve this, grep_includes is moved inside the cc_toolchain, which automatically chooses macos as an exec platform when needed.

PiperOrigin-RevId: 529700320
Change-Id: Ia805e05eb01d4687cbf56ee94a4ca05bcf13115e

src/main/java/com/google/devtools/build/lib/includescanning/IncludeParser.java

@@ -867,9 +867,6 @@ Collection<Inclusion> extractInclusions(
       boolean isOutputFile)
       throws IOException, ExecException, InterruptedException {
     Collection<Inclusion> inclusions;
-
-    // TODO(ulfjack): grepIncludes may be null if the corresponding attribute on the rule is missing
-    //  (see CppHelper.getGrepIncludes) or misspelled. It would be better to disallow this case.
     if (remoteIncludeScanner != null
         && grepIncludes != null
         && remoteIncludeScanner.shouldParseRemotely(file)) {

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

@@ -300,14 +300,12 @@ public CcCompilationContext getCcCompilationContext() {
   private final ActionRegistry actionRegistry;
   private final ActionConstructionContext actionConstructionContext;
   private final Label label;
-  @Nullable private final Artifact grepIncludes;
 
   /** Creates a CcCompilationHelper that outputs artifacts in a given configuration. */
   public CcCompilationHelper(
       ActionRegistry actionRegistry,
       ActionConstructionContext actionConstructionContext,
       Label label,
-      @Nullable Artifact grepIncludes,
       CppSemantics semantics,
       FeatureConfiguration featureConfiguration,
       SourceCategory sourceCategory,
@@ -333,7 +331,6 @@ public CcCompilationHelper(
     this.ruleErrorConsumer = actionConstructionContext.getRuleErrorConsumer();
     this.actionRegistry = Preconditions.checkNotNull(actionRegistry);
     this.label = Preconditions.checkNotNull(label);
-    this.grepIncludes = grepIncludes;
     this.executionInfo = Preconditions.checkNotNull(executionInfo);
     this.shouldProcessHeaders = shouldProcessHeaders;
   }
@@ -343,7 +340,6 @@ public CcCompilationHelper(
       ActionRegistry actionRegistry,
       ActionConstructionContext actionConstructionContext,
       Label label,
-      @Nullable Artifact grepIncludes,
       CppSemantics semantics,
       FeatureConfiguration featureConfiguration,
       CcToolchainProvider ccToolchain,
@@ -354,7 +350,6 @@ public CcCompilationHelper(
         actionRegistry,
         actionConstructionContext,
         label,
-        grepIncludes,
         semantics,
         featureConfiguration,
         SourceCategory.CC,
@@ -1674,7 +1669,7 @@ private static String toPathString(Artifact a) {
    */
   private CppCompileActionBuilder initializeCompileAction(Artifact sourceArtifact) {
     return new CppCompileActionBuilder(
-            actionConstructionContext, grepIncludes, ccToolchain, configuration, semantics)
+            actionConstructionContext, ccToolchain, configuration, semantics)
         .setSourceFile(sourceArtifact)
         .setCcCompilationContext(ccCompilationContext)
         .setCoptsFilter(coptsFilter)

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

@@ -127,7 +127,6 @@ public CcLinkingOutputs getCcLinkingOutputs() {
   private final SymbolGenerator<?> symbolGenerator;
   private final ImmutableMap<String, String> executionInfo;
 
-  private Artifact grepIncludes;
   private boolean isStampingEnabled;
   private boolean isTestOrTestOnlyTarget;
 
@@ -508,15 +507,6 @@ public CcLinkingHelper setTestOrTestOnlyTarget(boolean isTestOrTestOnlyTarget) {
     return this;
   }
 
-  /**
-   * Used to test the grep-includes tool. This is needing during linking because of linkstamping.
-   */
-  @CanIgnoreReturnValue
-  public CcLinkingHelper setGrepIncludes(Artifact grepIncludes) {
-    this.grepIncludes = grepIncludes;
-    return this;
-  }
-
   /** Whether linkstamping is enabled. */
   @CanIgnoreReturnValue
   public CcLinkingHelper setIsStampingEnabled(boolean isStampingEnabled) {
@@ -876,7 +866,6 @@ private CppLinkActionBuilder newLinkActionBuilder(
                 fdoContext,
                 featureConfiguration,
                 semantics)
-            .setGrepIncludes(grepIncludes)
             .setMnemonic(mnemonic)
             .setIsStampingEnabled(isStampingEnabled)
             .setTestOrTestOnlyTarget(isTestOrTestOnlyTarget)

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

@@ -1913,7 +1913,6 @@ public Tuple createLinkingContextFromCompilationOutputs(
                 TargetUtils.getExecutionInfo(
                     actions.getRuleContext().getRule(),
                     actions.getRuleContext().isAllowTagsPropagation()))
-            .setGrepIncludes(convertFromNoneable(grepIncludes, /* defaultValue= */ null))
             .addNonCodeLinkerInputs(
                 Sequence.cast(additionalInputs, Artifact.class, "additional_inputs"))
             .setShouldCreateStaticLibraries(!disallowStaticLibraries)
@@ -2432,7 +2431,6 @@ public Tuple compile(
       StarlarkThread thread)
       throws EvalException, InterruptedException {
     isCalledFromStarlarkCcCommon(thread);
-    Artifact grepIncludes = convertFromNoneable(grepIncludesObject, /* defaultValue= */ null);
     getSemantics()
         .validateStarlarkCompileApiCall(
             starlarkActionFactoryApi,
@@ -2518,7 +2516,6 @@ public Tuple compile(
             actions.asActionRegistry(actions),
             actions.getActionConstructionContext(),
             label,
-            grepIncludes,
             getSemantics(language),
             featureConfiguration.getFeatureConfiguration(),
             sourceCategory,
@@ -2763,7 +2760,6 @@ public CcLinkingOutputs link(
                 TargetUtils.getExecutionInfo(
                     actions.getRuleContext().getRule(),
                     actions.getRuleContext().isAllowTagsPropagation()))
-            .setGrepIncludes(convertFromNoneable(grepIncludes, /* defaultValue= */ null))
             .setLinkingMode(linkDepsStatically ? LinkingMode.STATIC : LinkingMode.DYNAMIC)
             .setIsStampingEnabled(isStampingEnabled)
             .addTransitiveAdditionalLinkerInputs(additionalInputsSet)
@@ -2922,7 +2918,6 @@ public void registerLinkstampCompileAction(
             CppLinkstampCompileHelper.createLinkstampCompileAction(
                 ruleContext,
                 ruleContext,
-                grepIncludes,
                 ruleContext.getConfiguration(),
                 sourceFile,
                 outputFile,

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

@@ -224,6 +224,7 @@ public CcToolchainProvider getCcToolchainProvider(
                 targetBuiltinIncludeFiles, Artifact.class, "target_builtin_include_files")
             .getImmutableList(),
         /* linkDynamicLibraryTool= */ attributes.getLinkDynamicLibraryTool(),
+        /* grepIncludes= */ attributes.getGrepIncludes(),
         /* builtInIncludeDirectories= */ builtInIncludeDirectories,
         /* sysroot= */ sysroot,
         /* targetSysroot= */ targetSysroot,

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

@@ -79,6 +79,7 @@ public class CcToolchainAttributesProvider extends NativeInfo implements HasCcTo
   private final String cpu;
   private final Artifact ifsoBuilder;
   private final Artifact linkDynamicLibraryTool;
+  @Nullable private final Artifact grepIncludes;
   private final TransitiveInfoCollection fdoOptimize;
   private final ImmutableList<Artifact> fdoOptimizeArtifacts;
   private final FdoPrefetchHintsProvider fdoPrefetch;
@@ -118,6 +119,11 @@ public CcToolchainAttributesProvider(
               + "https://github.com/bazelbuild/bazel/issues/7075 for details.");
     }
 
+    // grep_includes is not supported by Bazel.
+    String toolsRepository = ruleContext.getRuleClassProvider().getToolsRepository().getName();
+    this.grepIncludes =
+        toolsRepository.isEmpty() ? ruleContext.getPrerequisiteArtifact("$grep_includes") : null;
+
     this.cpu = ruleContext.attributes().get("cpu", Type.STRING);
     this.compiler = ruleContext.attributes().get("compiler", Type.STRING);
     this.supportsParamFiles = ruleContext.attributes().get("supports_param_files", BOOLEAN);
@@ -344,6 +350,11 @@ public Artifact getLinkDynamicLibraryTool() {
     return linkDynamicLibraryTool;
   }
 
+  @Nullable
+  public Artifact getGrepIncludes() {
+    return grepIncludes;
+  }
+
   @StarlarkMethod(
       name = "module_map",
       documented = false,

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

@@ -91,6 +91,7 @@ public final class CcToolchainProvider extends NativeInfo
   private final ImmutableList<Artifact> builtinIncludeFiles;
   private final ImmutableList<Artifact> targetBuiltinIncludeFiles;
   @Nullable private final Artifact linkDynamicLibraryTool;
+  @Nullable private final Artifact grepIncludes;
   private final ImmutableList<PathFragment> builtInIncludeDirectories;
   @Nullable private final PathFragment sysroot;
   private final PathFragment targetSysroot;
@@ -163,6 +164,7 @@ public CcToolchainProvider(
       ImmutableList<Artifact> builtinIncludeFiles,
       ImmutableList<Artifact> targetBuiltinIncludeFiles,
       Artifact linkDynamicLibraryTool,
+      @Nullable Artifact grepIncludes,
       ImmutableList<PathFragment> builtInIncludeDirectories,
       @Nullable PathFragment sysroot,
       @Nullable PathFragment targetSysroot,
@@ -260,6 +262,7 @@ public CcToolchainProvider(
     this.stripExecutable = stripExecutable;
     this.ldExecutable = ldExecutable;
     this.gcovExecutable = gcovExecutable;
+    this.grepIncludes = grepIncludes;
   }
 
   @Override
@@ -791,6 +794,12 @@ public Artifact getLinkDynamicLibraryTool() {
     return linkDynamicLibraryTool;
   }
 
+  /** Returns the grep-includes tool which is needing during linking because of linkstamping. */
+  @Nullable
+  public Artifact getGrepIncludes() {
+    return grepIncludes;
+  }
+
   /** Returns the tool that builds interface libraries from dynamic libraries. */
   public Artifact getInterfaceSoBuilder() {
     return interfaceSoBuilder;

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

@@ -304,6 +304,11 @@ instead of having no transition (i.e. target platform by default).
                 .cfg(ExecutionTransitionFactory.createFactory())
                 .singleArtifact()
                 .value(env.getToolsLabel("//tools/cpp:link_dynamic_library")))
+        .add(
+            attr("$grep_includes", LABEL)
+                .exec()
+                .cfg(ExecutionTransitionFactory.createFactory())
+                .value(env.getToolsLabel("//tools/cpp:grep-includes")))
         .add(
             attr(CcToolchain.CC_TOOLCHAIN_TYPE_ATTRIBUTE_NAME, NODEP_LABEL)
                 .value(CppRuleClasses.ccToolchainTypeAttribute(env)))

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

@@ -73,7 +73,6 @@ public class CppCompileActionBuilder {
   private Map<String, String> executionInfo = new LinkedHashMap<>();
   private CppSemantics cppSemantics;
   private final CcToolchainProvider ccToolchain;
-  @Nullable private final Artifact grepIncludes;
   private ActionEnvironment env;
   private final boolean codeCoverageEnabled;
   @Nullable private String actionName;
@@ -89,7 +88,6 @@ public class CppCompileActionBuilder {
   /** Creates a builder from a rule and configuration. */
   public CppCompileActionBuilder(
       ActionConstructionContext actionConstructionContext,
-      @Nullable Artifact grepIncludes,
       CcToolchainProvider ccToolchain,
       BuildConfigurationValue configuration,
       CppSemantics cppSemantics) {
@@ -110,7 +108,6 @@ public CppCompileActionBuilder(
     this.codeCoverageEnabled = configuration.isCodeCoverageEnabled();
     this.ccToolchain = ccToolchain;
     this.builtinIncludeDirectories = ccToolchain.getBuiltInIncludeDirectories();
-    this.grepIncludes = grepIncludes;
     this.cppSemantics = cppSemantics;
   }
 
@@ -147,7 +144,6 @@ public CppCompileActionBuilder(CppCompileActionBuilder other) {
     this.cppSemantics = other.cppSemantics;
     this.ccToolchain = other.ccToolchain;
     this.actionName = other.actionName;
-    this.grepIncludes = other.grepIncludes;
     this.builtinIncludeDirectories = other.builtinIncludeDirectories;
     this.additionalOutputs = ImmutableList.copyOf(other.additionalOutputs);
   }
@@ -339,7 +335,7 @@ public CppCompileAction buildAndVerify() throws UnconfiguredActionConfigExceptio
             actionName,
             cppSemantics,
             builtinIncludeDirectories,
-            grepIncludes,
+            ccToolchain.getGrepIncludes(),
             additionalOutputs);
     return action;
   }
@@ -369,8 +365,8 @@ NestedSet<Artifact> buildMandatoryInputs() {
     }
     ccCompilationContext.addAdditionalInputs(realMandatoryInputsBuilder);
     realMandatoryInputsBuilder.add(Preconditions.checkNotNull(sourceFile));
-    if (grepIncludes != null) {
-      realMandatoryInputsBuilder.add(grepIncludes);
+    if (ccToolchain.getGrepIncludes() != null) {
+      realMandatoryInputsBuilder.add(ccToolchain.getGrepIncludes());
     }
     if (!shouldScanIncludes && dotdFile == null && !shouldParseShowIncludes()) {
       realMandatoryInputsBuilder.addTransitive(ccCompilationContext.getDeclaredIncludeSrcs());
@@ -680,9 +676,4 @@ public boolean shouldCompileHeaders() {
     Preconditions.checkNotNull(featureConfiguration);
     return ccToolchain.shouldProcessHeaders(featureConfiguration, cppConfiguration);
   }
-
-  @Nullable
-  public Artifact getGrepIncludes() {
-    return grepIncludes;
-  }
 }

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

@@ -245,7 +245,7 @@ public static Label getToolchainTypeFromRuleClass(RuleContext ruleContext) {
           ruleContext.attributes().get(CcToolchain.CC_TOOLCHAIN_TYPE_ATTRIBUTE_NAME, NODEP_LABEL);
     } else if (ruleContext.attributes().has(CcToolchain.CC_TOOLCHAIN_TYPE_ATTRIBUTE_NAME, LABEL)) {
       toolchainType =
-          ruleContext.attributes().get(CcToolchain.CC_TOOLCHAIN_TYPE_ATTRIBUTE_NAME, LABEL);
+          ruleContext.getPrerequisite(CcToolchain.CC_TOOLCHAIN_TYPE_ATTRIBUTE_NAME).getLabel();
     } else {
       toolchainType = null;
     }
@@ -660,11 +660,4 @@ static boolean useToolchainResolution(RuleContext ruleContext) {
 
     return cppOptions.enableCcToolchainResolution;
   }
-
-  @Nullable
-  public static Artifact getGrepIncludes(RuleContext ruleContext) {
-    return ruleContext.attributes().has("$grep_includes")
-        ? ruleContext.getPrerequisiteArtifact("$grep_includes")
-        : null;
-  }
 }

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

@@ -141,7 +141,6 @@ public Artifact create(
   private final RuleErrorConsumer ruleErrorConsumer;
   private final RepositoryName repositoryName;
 
-  private Artifact grepIncludes;
   // TODO(plf): This is not exactly the same as `useTestOnlyFlags` but perhaps we can remove one
   //  of them.
   private boolean isTestOrTestOnlyTarget;
@@ -988,7 +987,6 @@ public CppLinkAction build() throws InterruptedException, RuleErrorException {
             CppLinkstampCompileHelper.createLinkstampCompileAction(
                 ruleErrorConsumer,
                 actionConstructionContext,
-                grepIncludes,
                 configuration,
                 source,
                 linkstampEntry.getValue(),
@@ -1492,15 +1490,6 @@ public CppLinkActionBuilder setTestOrTestOnlyTarget(boolean isTestOrTestOnlyTarg
     return this;
   }
 
-  /**
-   * Used to test the grep-includes tool. This is needing during linking because of linkstamping.
-   */
-  @CanIgnoreReturnValue
-  public CppLinkActionBuilder setGrepIncludes(Artifact grepIncludes) {
-    this.grepIncludes = grepIncludes;
-    return this;
-  }
-
   /** Whether linkstamping is enabled. */
   @CanIgnoreReturnValue
   public CppLinkActionBuilder setIsStampingEnabled(boolean isStampingEnabled) {

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

@@ -29,7 +29,6 @@
 import com.google.devtools.build.lib.rules.cpp.CcToolchainFeatures.FeatureConfiguration;
 import com.google.devtools.build.lib.vfs.PathFragment;
 import java.util.regex.Pattern;
-import javax.annotation.Nullable;
 
 /** Handles creation of CppCompileAction used to compile linkstamp sources. */
 public class CppLinkstampCompileHelper {
@@ -42,7 +41,6 @@ public class CppLinkstampCompileHelper {
   public static CppCompileAction createLinkstampCompileAction(
       RuleErrorConsumer ruleErrorConsumer,
       ActionConstructionContext actionConstructionContext,
-      @Nullable Artifact grepIncludes,
       BuildConfigurationValue configuration,
       Artifact sourceFile,
       Artifact outputFile,
@@ -62,11 +60,7 @@ public static CppCompileAction createLinkstampCompileAction(
       CppSemantics semantics) {
     CppCompileActionBuilder builder =
         new CppCompileActionBuilder(
-                actionConstructionContext,
-                grepIncludes,
-                ccToolchainProvider,
-                configuration,
-                semantics)
+                actionConstructionContext, ccToolchainProvider, configuration, semantics)
             .addMandatoryInputs(compilationInputs)
             .setVariables(
                 getVariables(

src/main/java/com/google/devtools/build/lib/rules/nativedeps/NativeDepsHelper.java

@@ -279,7 +279,6 @@ public static NativeDepsRunfiles createNativeDepsAction(
             ruleContext.getSymbolGenerator(),
             TargetUtils.getExecutionInfo(
                 ruleContext.getRule(), ruleContext.isAllowTagsPropagation()))
-        .setGrepIncludes(CppHelper.getGrepIncludes(ruleContext))
         .setIsStampingEnabled(AnalysisUtils.isStampingEnabled(ruleContext))
         .setTestOrTestOnlyTarget(ruleContext.isTestTarget() || ruleContext.isTestOnlyTarget())
         .setLinkerOutputArtifact(sharedLibrary)

src/main/java/com/google/devtools/build/lib/rules/objc/CompilationSupport.java

@@ -649,7 +649,6 @@ public CompilationSupport registerLinkActions(
                 ruleContext.getSymbolGenerator(),
                 TargetUtils.getExecutionInfo(
                     ruleContext.getRule(), ruleContext.isAllowTagsPropagation()))
-            .setGrepIncludes(CppHelper.getGrepIncludes(ruleContext))
             .setIsStampingEnabled(isStampingEnabled)
             .setTestOrTestOnlyTarget(ruleContext.isTestOnlyTarget() || ruleContext.isTestTarget())
             .addNonCodeLinkerInputs(asNeededLibraryList)