Remove --incompatible_objc_framework_cleanup

RELNOTES: None
PiperOrigin-RevId: 273869859

src/main/java/com/google/devtools/build/lib/packages/StarlarkSemanticsOptions.java

@@ -584,22 +584,6 @@ public class StarlarkSemanticsOptions extends OptionsBase implements Serializabl
               + "See https://github.com/bazelbuild/bazel/issues/7670 for details.")
   public boolean incompatibleDoNotSplitLinkingCmdline;
 
-  @Option(
-      name = "incompatible_objc_framework_cleanup",
-      defaultValue = "true",
-      documentationCategory = OptionDocumentationCategory.STARLARK_SEMANTICS,
-      effectTags = {OptionEffectTag.BUILD_FILE_SEMANTICS},
-      metadataTags = {
-        OptionMetadataTag.INCOMPATIBLE_CHANGE,
-        OptionMetadataTag.TRIGGERED_BY_ALL_INCOMPATIBLE_CHANGES
-      },
-      help =
-          "If enabled, use the post-cleanup mode for prebuilt frameworks.  The cleanup changes "
-              + "the objc provider API pertaining to frameworks.  This change is expected to be "
-              + "transparent to most users unless they write their own Starlark rules to handle "
-              + "frameworks.  See https://github.com/bazelbuild/bazel/issues/7944 for details.")
-  public boolean incompatibleObjcFrameworkCleanup;
-
   @Option(
       name = "incompatible_restrict_named_params",
       defaultValue = "true",
@@ -688,7 +672,6 @@ public StarlarkSemantics toSkylarkSemantics() {
             .incompatibleNoSupportToolsInActionInputs(incompatibleNoSupportToolsInActionInputs)
             .incompatibleNoTargetOutputGroup(incompatibleNoTargetOutputGroup)
             .incompatibleNoTransitiveLoads(incompatibleNoTransitiveLoads)
-            .incompatibleObjcFrameworkCleanup(incompatibleObjcFrameworkCleanup)
             .incompatibleRemapMainRepo(incompatibleRemapMainRepo)
             .incompatibleRemoveNativeMavenJar(incompatibleRemoveNativeMavenJar)
             .incompatibleRestrictNamedParams(incompatibleRestrictNamedParams)

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

@@ -18,11 +18,9 @@
 import static com.google.devtools.build.lib.packages.ImplicitOutputsFunction.fromTemplates;
 import static com.google.devtools.build.lib.rules.cpp.Link.LINK_LIBRARY_FILETYPES;
 import static com.google.devtools.build.lib.rules.objc.ObjcProvider.DEFINE;
-import static com.google.devtools.build.lib.rules.objc.ObjcProvider.DYNAMIC_FRAMEWORK_DIR;
 import static com.google.devtools.build.lib.rules.objc.ObjcProvider.DYNAMIC_FRAMEWORK_FILE;
 import static com.google.devtools.build.lib.rules.objc.ObjcProvider.FORCE_LOAD_LIBRARY;
 import static com.google.devtools.build.lib.rules.objc.ObjcProvider.FRAMEWORK_SEARCH_PATHS;
-import static com.google.devtools.build.lib.rules.objc.ObjcProvider.FRAMEWORK_SUFFIX;
 import static com.google.devtools.build.lib.rules.objc.ObjcProvider.HEADER;
 import static com.google.devtools.build.lib.rules.objc.ObjcProvider.IMPORTED_LIBRARY;
 import static com.google.devtools.build.lib.rules.objc.ObjcProvider.INCLUDE;
@@ -121,7 +119,6 @@
 import com.google.devtools.build.lib.rules.cpp.UmbrellaHeaderAction;
 import com.google.devtools.build.lib.rules.objc.ObjcProvider.Flag;
 import com.google.devtools.build.lib.rules.objc.ObjcVariablesExtension.VariableCategory;
-import com.google.devtools.build.lib.syntax.StarlarkSemantics;
 import com.google.devtools.build.lib.util.FileTypeSet;
 import com.google.devtools.build.lib.util.Pair;
 import com.google.devtools.build.lib.vfs.PathFragment;
@@ -270,15 +267,8 @@ public static final FilesToRunProvider libtool(RuleContext ruleContext) {
    * ObjProvider.
    */
   private Iterable<Artifact> getExtraIncludeProcessingInputs(
-      ObjcProvider objcProvider, Collection<Artifact> privateHdrs, Artifact pchHdr) {
+      Collection<Artifact> privateHdrs, Artifact pchHdr) {
     Iterable<Artifact> extraInputs = privateHdrs;
-    if (!starlarkSemantics.incompatibleObjcFrameworkCleanup()) {
-      extraInputs =
-          Iterables.concat(
-              extraInputs,
-              objcProvider.get(STATIC_FRAMEWORK_FILE),
-              objcProvider.get(DYNAMIC_FRAMEWORK_FILE));
-    }
     if (pchHdr != null) {
       extraInputs = Iterables.concat(extraInputs, ImmutableList.of(pchHdr));
     }
@@ -545,17 +535,15 @@ private Pair<CcCompilationOutputs, ImmutableMap<String, NestedSet<Artifact>>> cc
 
   private ObjcCppSemantics createObjcCppSemantics(
       ObjcProvider objcProvider, Collection<Artifact> privateHdrs, Artifact pchHdr) {
-    Iterable<Artifact> extraInputs =
-        getExtraIncludeProcessingInputs(objcProvider, privateHdrs, pchHdr);
+    Iterable<Artifact> extraInputs = getExtraIncludeProcessingInputs(privateHdrs, pchHdr);
     return new ObjcCppSemantics(
         objcProvider,
         includeProcessingType,
         createIncludeProcessing(Iterables.concat(extraInputs, objcProvider.get(HEADER))),
         extraInputs,
         ruleContext.getFragment(ObjcConfiguration.class),
         intermediateArtifacts,
-        buildConfiguration,
-        starlarkSemantics);
+        buildConfiguration);
   }
 
   private FeatureConfiguration getFeatureConfiguration(
@@ -713,29 +701,10 @@ static CompilationArtifacts compilationArtifacts(
         .build();
   }
 
-  /** Returns a list of framework search paths for clang actions for pre-cleanup mode. */
-  static ImmutableList<PathFragment> preCleanupFrameworkSearchPathFragments(
-      ObjcProvider provider, RuleContext ruleContext, BuildConfiguration buildConfiguration) {
-
-    ImmutableList.Builder<PathFragment> searchPaths = new ImmutableList.Builder<>();
-    return searchPaths
-        // Add custom (non-SDK) framework search paths. For each framework foo/bar.framework,
-        // include "foo" as a search path.
-        .addAll(uniqueParentDirectories(provider.getStaticFrameworkDirs()))
-        .addAll(uniqueParentDirectories(provider.get(DYNAMIC_FRAMEWORK_DIR)))
-        .addAll(uniqueParentDirectories(provider.get(FRAMEWORK_SEARCH_PATHS)))
-        .build();
-  }
-
   /** Returns a list of framework header search path fragments. */
   static ImmutableList<PathFragment> frameworkHeaderSearchPathFragments(
       ObjcProvider provider, RuleContext ruleContext, BuildConfiguration buildConfiguration)
       throws InterruptedException {
-    StarlarkSemantics starlarkSemantics =
-        ruleContext.getAnalysisEnvironment().getSkylarkSemantics();
-    if (!starlarkSemantics.incompatibleObjcFrameworkCleanup()) {
-      return preCleanupFrameworkSearchPathFragments(provider, ruleContext, buildConfiguration);
-    }
     ImmutableList.Builder<PathFragment> searchPaths = new ImmutableList.Builder<>();
     return searchPaths
         .addAll(uniqueParentDirectories(provider.get(FRAMEWORK_SEARCH_PATHS)))
@@ -759,17 +728,7 @@ static ImmutableList<String> frameworkHeaderSearchPaths(
   static ImmutableList<String> frameworkLibrarySearchPaths(
       ObjcProvider provider, RuleContext ruleContext, BuildConfiguration buildConfiguration)
       throws InterruptedException {
-    StarlarkSemantics starlarkSemantics =
-        ruleContext.getAnalysisEnvironment().getSkylarkSemantics();
     ImmutableList.Builder<String> searchPaths = new ImmutableList.Builder<>();
-    if (!starlarkSemantics.incompatibleObjcFrameworkCleanup()) {
-      return searchPaths
-          .addAll(
-              Iterables.transform(
-                  preCleanupFrameworkSearchPathFragments(provider, ruleContext, buildConfiguration),
-                  PathFragment::getSafePathString))
-          .build();
-    }
     return searchPaths
         // Add library search paths corresponding to custom (non-SDK) frameworks. For each framework
         // foo/bar.framework, include "foo" as a search path.
@@ -779,7 +738,6 @@ static ImmutableList<String> frameworkLibrarySearchPaths(
   }
 
   private final RuleContext ruleContext;
-  private final StarlarkSemantics starlarkSemantics;
   private final BuildConfiguration buildConfiguration;
   private final ObjcConfiguration objcConfiguration;
   private final AppleConfiguration appleConfiguration;
@@ -817,7 +775,6 @@ private CompilationSupport(
       boolean usePch)
       throws InterruptedException {
     this.ruleContext = ruleContext;
-    this.starlarkSemantics = ruleContext.getAnalysisEnvironment().getSkylarkSemantics();
     this.buildConfiguration = buildConfiguration;
     this.objcConfiguration = buildConfiguration.getFragment(ObjcConfiguration.class);
     this.appleConfiguration = buildConfiguration.getFragment(AppleConfiguration.class);
@@ -1483,22 +1440,8 @@ CompilationSupport registerFullyLinkAction(
   private Set<String> frameworkNames(ObjcProvider provider) {
     Set<String> names = new LinkedHashSet<>();
     Iterables.addAll(names, SdkFramework.names(provider.get(SDK_FRAMEWORK)));
-    if (!starlarkSemantics.incompatibleObjcFrameworkCleanup()) {
-      for (PathFragment frameworkDir :
-          Iterables.concat(
-              provider.getStaticFrameworkDirs(), provider.get(DYNAMIC_FRAMEWORK_DIR))) {
-        String segment = frameworkDir.getBaseName();
-        Preconditions.checkState(
-            segment.endsWith(FRAMEWORK_SUFFIX),
-            "expect %s to end with %s, but it does not",
-            segment,
-            FRAMEWORK_SUFFIX);
-        names.add(segment.substring(0, segment.length() - FRAMEWORK_SUFFIX.length()));
-      }
-    } else {
-      Iterables.addAll(names, provider.staticFrameworkNames());
-      Iterables.addAll(names, provider.dynamicFrameworkNames());
-    }
+    Iterables.addAll(names, provider.staticFrameworkNames());
+    Iterables.addAll(names, provider.dynamicFrameworkNames());
     return names;
   }
 
@@ -1958,11 +1901,6 @@ private void registerHeaderScanningAction(
           "%s:%s", info.sourceFile.getExecPath(), info.headersListFile.getExecPath());
       builder.addInput(info.sourceFile).addOutput(info.headersListFile);
     }
-    if (!starlarkSemantics.incompatibleObjcFrameworkCleanup()) {
-      builder
-          .addTransitiveInputs(objcProvider.get(ObjcProvider.STATIC_FRAMEWORK_FILE))
-          .addTransitiveInputs(objcProvider.get(ObjcProvider.DYNAMIC_FRAMEWORK_FILE));
-    }
     ruleContext.registerAction(
         builder
             .addCommandLine(cmdLine.add("--").addAll(args).build())

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

@@ -16,7 +16,6 @@
 
 import static com.google.devtools.build.lib.rules.objc.ObjcProvider.CC_LIBRARY;
 import static com.google.devtools.build.lib.rules.objc.ObjcProvider.DEFINE;
-import static com.google.devtools.build.lib.rules.objc.ObjcProvider.DYNAMIC_FRAMEWORK_DIR;
 import static com.google.devtools.build.lib.rules.objc.ObjcProvider.DYNAMIC_FRAMEWORK_FILE;
 import static com.google.devtools.build.lib.rules.objc.ObjcProvider.FLAG;
 import static com.google.devtools.build.lib.rules.objc.ObjcProvider.FORCE_LOAD_LIBRARY;
@@ -357,20 +356,9 @@ ObjcCommon build() {
               .addAll(DEFINE, defines)
               .addTransitiveAndPropagate(depObjcProviders);
 
-      if (!semantics.incompatibleObjcFrameworkCleanup()) {
-        objcProvider.addAll(
-            DYNAMIC_FRAMEWORK_DIR,
-            uniqueContainers(dynamicFrameworkImports, FRAMEWORK_CONTAINER_TYPE));
-      }
-
       for (ObjcProvider provider : runtimeDepObjcProviders) {
-        if (!semantics.incompatibleObjcFrameworkCleanup()) {
-          objcProvider.addTransitiveAndPropagate(ObjcProvider.DYNAMIC_FRAMEWORK_FILE, provider);
-          objcProvider.addTransitiveAndPropagate(ObjcProvider.STATIC_FRAMEWORK_FILE, provider);
-        } else {
-          objcProvider.addTransitiveAndPropagate(ObjcProvider.FRAMEWORK_SEARCH_PATHS, provider);
-          objcProvider.addTransitiveAndPropagate(ObjcProvider.HEADER, provider);
-        }
+        objcProvider.addTransitiveAndPropagate(ObjcProvider.FRAMEWORK_SEARCH_PATHS, provider);
+        objcProvider.addTransitiveAndPropagate(ObjcProvider.HEADER, provider);
         objcProvider.addTransitiveAndPropagate(ObjcProvider.MERGE_ZIP, provider);
       }
 

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

@@ -17,9 +17,7 @@
 import static com.google.devtools.build.lib.rules.objc.CompilationSupport.IncludeProcessingType.HEADER_THINNING;
 import static com.google.devtools.build.lib.rules.objc.CompilationSupport.IncludeProcessingType.INCLUDE_SCANNING;
 import static com.google.devtools.build.lib.rules.objc.CompilationSupport.IncludeProcessingType.NO_PROCESSING;
-import static com.google.devtools.build.lib.rules.objc.ObjcProvider.DYNAMIC_FRAMEWORK_FILE;
 import static com.google.devtools.build.lib.rules.objc.ObjcProvider.HEADER;
-import static com.google.devtools.build.lib.rules.objc.ObjcProvider.STATIC_FRAMEWORK_FILE;
 
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.Iterables;
@@ -36,7 +34,6 @@
 import com.google.devtools.build.lib.rules.cpp.HeaderDiscovery.DotdPruningMode;
 import com.google.devtools.build.lib.rules.cpp.IncludeProcessing;
 import com.google.devtools.build.lib.rules.objc.CompilationSupport.IncludeProcessingType;
-import com.google.devtools.build.lib.syntax.StarlarkSemantics;
 import com.google.devtools.build.lib.util.FileTypeSet;
 
 /**
@@ -51,7 +48,6 @@ public class ObjcCppSemantics implements CppSemantics {
   private final ObjcConfiguration config;
   private final IntermediateArtifacts intermediateArtifacts;
   private final BuildConfiguration buildConfiguration;
-  private final StarlarkSemantics starlarkSemantics;
 
   /**
    * Set of {@link com.google.devtools.build.lib.util.FileType} of source artifacts that are
@@ -85,16 +81,14 @@ public ObjcCppSemantics(
       Iterable<Artifact> extraIncludeScanningInputs,
       ObjcConfiguration config,
       IntermediateArtifacts intermediateArtifacts,
-      BuildConfiguration buildConfiguration,
-      StarlarkSemantics starlarkSemantics) {
+      BuildConfiguration buildConfiguration) {
     this.objcProvider = objcProvider;
     this.includeProcessingType = includeProcessingType;
     this.includeProcessing = includeProcessing;
     this.extraIncludeScanningInputs = extraIncludeScanningInputs;
     this.config = config;
     this.intermediateArtifacts = intermediateArtifacts;
     this.buildConfiguration = buildConfiguration;
-    this.starlarkSemantics = starlarkSemantics;
   }
 
   @Override
@@ -110,12 +104,6 @@ public void finalizeCompileActionBuilder(
         .addTransitiveMandatoryInputs(actionBuilder.getToolchain().getAllFilesMiddleman())
         .setShouldScanIncludes(includeProcessingType == INCLUDE_SCANNING);
 
-    if (!starlarkSemantics.incompatibleObjcFrameworkCleanup()) {
-      actionBuilder
-          .addTransitiveMandatoryInputs(objcProvider.get(STATIC_FRAMEWORK_FILE))
-          .addTransitiveMandatoryInputs(objcProvider.get(DYNAMIC_FRAMEWORK_FILE));
-    }
-
     if (includeProcessingType == HEADER_THINNING) {
       Artifact sourceFile = actionBuilder.getSourceFile();
       if (!sourceFile.isTreeArtifact()

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

@@ -248,15 +248,6 @@ public Class<E> getType() {
   public static final Key<Artifact> STATIC_FRAMEWORK_FILE =
       new Key<>(STABLE_ORDER, "static_framework_file", Artifact.class);
 
-  /**
-   * Exec paths of {@code .framework} directories corresponding to dynamic frameworks to link. These
-   * cause -F arguments (framework search paths) to be added to each compile action, and
-   * -framework (link framework) arguments to be added to each link action. These differ from
-   * static frameworks in that they are not statically linked into the binary.
-   */
-  public static final Key<PathFragment> DYNAMIC_FRAMEWORK_DIR =
-      new Key<>(LINK_ORDER, "dynamic_framework_dir", PathFragment.class);
-
   /** The dynamic library files of user-specified dynamic frameworks. */
   public static final Key<Artifact> DYNAMIC_FRAMEWORK_FILE =
       new Key<>(STABLE_ORDER, "dynamic_framework_file", Artifact.class);
@@ -358,7 +349,6 @@ public enum Flag {
   static final ImmutableList<Key<?>> KEYS_FOR_SKYLARK =
       ImmutableList.<Key<?>>of(
           DEFINE,
-          DYNAMIC_FRAMEWORK_DIR,
           DYNAMIC_FRAMEWORK_FILE,
           EXPORTED_DEBUG_ARTIFACTS,
           FRAMEWORK_SEARCH_PATHS,
@@ -408,11 +398,6 @@ public NestedSet<String> define() {
     return get(DEFINE);
   }
 
-  @Override
-  public SkylarkNestedSet dynamicFrameworkDir() {
-    return ObjcProviderSkylarkConverters.convertPathFragmentsToSkylark(get(DYNAMIC_FRAMEWORK_DIR));
-  }
-
   @Override
   public NestedSet<Artifact> dynamicFrameworkFile() {
     return get(DYNAMIC_FRAMEWORK_FILE);
@@ -592,7 +577,6 @@ public SkylarkNestedSet weakSdkFramework() {
   private static final ImmutableSet<Key<?>> NON_SUBTRACTABLE_KEYS =
       ImmutableSet.<Key<?>>of(
           DEFINE,
-          DYNAMIC_FRAMEWORK_DIR,
           DYNAMIC_FRAMEWORK_FILE,
           FLAG,
           MERGE_ZIP,
@@ -908,24 +892,6 @@ private <T> void addTransitiveAndAvoid(ObjcProvider.Builder objcProviderBuilder,
     addTransitiveAndFilter(objcProviderBuilder, key, Predicates.not(Predicates.in(avoidItemsSet)));
   }
 
-  /**
-   * Returns all unique static framework directories (directories ending in '.framework') for all
-   * static framework files in this provider.
-   */
-  public Iterable<PathFragment> getStaticFrameworkDirs() {
-    return ObjcCommon.uniqueContainers(get(STATIC_FRAMEWORK_FILE),
-        ObjcCommon.FRAMEWORK_CONTAINER_TYPE);
-  }
-
-  /**
-   * Returns all unique static framework directories (directories ending in '.framework') for all
-   * static framework files in this provider.
-   */
-  @Override
-  public SkylarkNestedSet getStaticFrameworkDirsForSkylark() {
-    return ObjcProviderSkylarkConverters.convertPathFragmentsToSkylark(getStaticFrameworkDirs());
-  }
-
   /**
    * Check whether that a path fragment is a framework directory (i.e. ends in FRAMEWORK_SUFFIX).
    */