Delete resource attributes from objc_library and objc_import.

The incompatible flag to disable these resources will be enabled by default in Bazel 0.25. RELNOTES: objc_library does not support resource attributes any more. Please read #7594 for more info. PiperOrigin-RevId: 244026921
bazelbuild · Apr 17, 2019 · 5456e41 · 5456e41
1 parent 5ece650
commit 5456e41
Show file tree

Hide file tree

Showing 14 changed files with 15 additions and 1,652 deletions.
diff --git a/src/main/java/com/google/devtools/build/lib/bazel/rules/ObjcRules.java b/src/main/java/com/google/devtools/build/lib/bazel/rules/ObjcRules.java
@@ -75,17 +75,14 @@ public void init(ConfiguredRuleClassProvider.Builder builder) {
     builder.addRuleDefinition(new ObjcImportRule());
     builder.addRuleDefinition(new ObjcLibraryRule());
     builder.addRuleDefinition(new ObjcRuleClasses.CoptsRule());
-    builder.addRuleDefinition(new ObjcRuleClasses.BundlingRule());
     builder.addRuleDefinition(new ObjcRuleClasses.DylibDependingRule(objcProtoAspect));
     builder.addRuleDefinition(new ObjcRuleClasses.CompilingRule());
     builder.addRuleDefinition(new ObjcRuleClasses.LinkingRule(objcProtoAspect));
     builder.addRuleDefinition(new ObjcRuleClasses.PlatformRule());
     builder.addRuleDefinition(new ObjcRuleClasses.MultiArchPlatformRule(objcProtoAspect));
-    builder.addRuleDefinition(new ObjcRuleClasses.ResourcesRule());
     builder.addRuleDefinition(new ObjcRuleClasses.AlwaysLinkRule());
     builder.addRuleDefinition(new ObjcRuleClasses.SdkFrameworksDependerRule());
     builder.addRuleDefinition(new ObjcRuleClasses.CompileDependencyRule());
-    builder.addRuleDefinition(new ObjcRuleClasses.ResourceToolsRule());
     builder.addRuleDefinition(new ObjcRuleClasses.XcrunRule());
     builder.addRuleDefinition(new ObjcRuleClasses.LibtoolRule());
     builder.addRuleDefinition(new ObjcRuleClasses.CrosstoolRule());

diff --git a/src/main/java/com/google/devtools/build/lib/rules/objc/BundleSupport.java b/src/main/java/com/google/devtools/build/lib/rules/objc/BundleSupport.java
diff --git a/src/main/java/com/google/devtools/build/lib/rules/objc/Bundling.java b/src/main/java/com/google/devtools/build/lib/rules/objc/Bundling.java
@@ -28,8 +28,6 @@
 import static com.google.devtools.build.lib.rules.objc.ObjcProvider.STRINGS;
 import static com.google.devtools.build.lib.rules.objc.ObjcProvider.XCDATAMODEL;
 import static com.google.devtools.build.lib.rules.objc.ObjcProvider.XIB;
-import static com.google.devtools.build.lib.rules.objc.ObjcRuleClasses.BundlingRule.FAMILIES_ATTR;
-import static com.google.devtools.build.lib.rules.objc.ObjcRuleClasses.BundlingRule.INFOPLIST_ATTR;
 
 import com.google.common.base.Optional;
 import com.google.common.base.Preconditions;
@@ -39,8 +37,6 @@
 import com.google.common.collect.ImmutableSet;
 import com.google.common.collect.Iterables;
 import com.google.devtools.build.lib.actions.Artifact;
-import com.google.devtools.build.lib.analysis.RuleContext;
-import com.google.devtools.build.lib.analysis.configuredtargets.RuleConfiguredTarget.Mode;
 import com.google.devtools.build.lib.collect.nestedset.NestedSet;
 import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder;
 import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable;
@@ -144,27 +140,6 @@ public Builder setAutomaticEntriesInfoplistInput(Artifact automaticEntriesInfopl
       return this;
     }
 
-    /**
-     * Adds any info plists specified in the given rule's {@code infoplist}  or {@code infoplists}
-     * attribute as well as from its {@code options} as inputs to this bundle's {@code Info.plist}
-     * (which is merged from any such added plists plus some additional information).
-     */
-    public Builder addInfoplistInputFromRule(RuleContext ruleContext) {
-      Artifact infoplist =
-          ruleContext.getPrerequisiteArtifact(INFOPLIST_ATTR, Mode.TARGET);
-      if (infoplist != null) {
-        infoplistInputs.add(infoplist);
-      }
-
-      Iterable<Artifact> infoplists =
-          ruleContext.getPrerequisiteArtifacts("infoplists", Mode.TARGET).list();
-      if (infoplists != null) {
-        infoplistInputs.addAll(infoplists);
-      }
-
-      return this;
-    }
-
     public Builder setIntermediateArtifacts(IntermediateArtifacts intermediateArtifacts) {
       this.intermediateArtifacts = intermediateArtifacts;
       return this;
@@ -321,10 +296,10 @@ private NestedSet<BundleableFile> dynamicFrameworkFiles() {
      * set of files returned.
      *
      * <p>Files can have the same bundle path for various illegal reasons and errors are raised for
-     * that separately (see {@link BundleSupport#validateResources}). There are situations though
-     * where the same file exists multiple times (for example in multi-architecture builds) and
-     * would conflict when creating the bundle. In all these cases it shouldn't matter which one is
-     * included and this class will select the first one.
+     * that separately. There are situations though where the same file exists multiple times (for
+     * example in multi-architecture builds) and would conflict when creating the bundle. In all
+     * these cases it shouldn't matter which one is included and this class will select the first
+     * one.
      */
     ImmutableList<BundleableFile> deduplicateByBundlePaths(
         ImmutableList<BundleableFile> bundleFiles) {
@@ -339,8 +314,8 @@ ImmutableList<BundleableFile> deduplicateByBundlePaths(
     }
 
     public Bundling build() {
-      Preconditions.checkNotNull(intermediateArtifacts, "intermediateArtifacts");
-      Preconditions.checkNotNull(families, FAMILIES_ATTR);
+      Preconditions.checkNotNull(intermediateArtifacts, "intermediateArtifacts should not be null");
+      Preconditions.checkNotNull(families, "families should not be null");
       NestedSet<Artifact> bundleInfoplistInputs = bundleInfoplistInputs();
       Optional<Artifact> bundleInfoplist = bundleInfoplist(bundleInfoplistInputs);
       Optional<Artifact> actoolzipOutput = actoolzipOutput();
@@ -631,8 +606,8 @@ public DottedVersion getMinimumOsVersion() {
   }
 
   /**
-   * Returns the list of {@link TargetDeviceFamily} values this bundle is targeting.
-   * If empty, the default values specified by {@link FAMILIES_ATTR} will be used.
+   * Returns the list of {@link TargetDeviceFamily} values this bundle is targeting. If empty, the
+   * default values specified by "families" will be used.
    */
   public ImmutableSet<TargetDeviceFamily> getTargetDeviceFamilies() {
     return families;

diff --git a/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcCommandLineOptions.java b/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcCommandLineOptions.java
@@ -319,7 +319,7 @@ public class ObjcCommandLineOptions extends FragmentOptions {
         OptionMetadataTag.INCOMPATIBLE_CHANGE,
         OptionMetadataTag.TRIGGERED_BY_ALL_INCOMPATIBLE_CHANGES
       },
-      help = "If enabled, objc_library resource attributes are disallowed.")
+      help = "Unused. Will be removed in future versions of Bazel.")
   public boolean disableObjcLibraryResources;
 
   @Override

diff --git a/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcCommon.java b/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcCommon.java
@@ -14,9 +14,6 @@
 
 package com.google.devtools.build.lib.rules.objc;
 
-import static com.google.devtools.build.lib.packages.BuildType.LABEL;
-import static com.google.devtools.build.lib.rules.objc.ObjcProvider.ASSET_CATALOG;
-import static com.google.devtools.build.lib.rules.objc.ObjcProvider.BUNDLE_FILE;
 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;
@@ -40,14 +37,9 @@
 import static com.google.devtools.build.lib.rules.objc.ObjcProvider.SDK_FRAMEWORK;
 import static com.google.devtools.build.lib.rules.objc.ObjcProvider.SOURCE;
 import static com.google.devtools.build.lib.rules.objc.ObjcProvider.STATIC_FRAMEWORK_FILE;
-import static com.google.devtools.build.lib.rules.objc.ObjcProvider.STORYBOARD;
-import static com.google.devtools.build.lib.rules.objc.ObjcProvider.STRINGS;
 import static com.google.devtools.build.lib.rules.objc.ObjcProvider.TOP_LEVEL_MODULE_MAP;
 import static com.google.devtools.build.lib.rules.objc.ObjcProvider.UMBRELLA_HEADER;
 import static com.google.devtools.build.lib.rules.objc.ObjcProvider.WEAK_SDK_FRAMEWORK;
-import static com.google.devtools.build.lib.rules.objc.ObjcProvider.XCASSETS_DIR;
-import static com.google.devtools.build.lib.rules.objc.ObjcProvider.XCDATAMODEL;
-import static com.google.devtools.build.lib.rules.objc.ObjcProvider.XIB;
 
 import com.google.common.annotations.VisibleForTesting;
 import com.google.common.base.Optional;
@@ -61,7 +53,6 @@
 import com.google.devtools.build.lib.analysis.RuleContext;
 import com.google.devtools.build.lib.analysis.TransitiveInfoCollection;
 import com.google.devtools.build.lib.analysis.config.BuildConfiguration;
-import com.google.devtools.build.lib.analysis.configuredtargets.RuleConfiguredTarget.Mode;
 import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder;
 import com.google.devtools.build.lib.packages.BuiltinProvider;
 import com.google.devtools.build.lib.packages.Info;
@@ -101,53 +92,11 @@ public static ImmutableList<Artifact> filterFileset(Iterable<Artifact> artifacts
     return inputs.build();
   }
 
-  /**
-   * Provides a way to access attributes that are common to all resources rules.
-   */
-  // TODO(bazel-team): Delete and move into support-specific attributes classes once ObjcCommon is
-  // gone.
-  static final class ResourceAttributes {
-    private final RuleContext ruleContext;
-
-    ResourceAttributes(RuleContext ruleContext) {
-      this.ruleContext = ruleContext;
-    }
-
-    ImmutableList<Artifact> strings() {
-      return ruleContext.getPrerequisiteArtifacts("strings", Mode.TARGET).list();
-    }
-
-    ImmutableList<Artifact> xibs() {
-      return ruleContext.getPrerequisiteArtifacts("xibs", Mode.TARGET).list();
-    }
-
-    ImmutableList<Artifact> storyboards() {
-      return ruleContext.getPrerequisiteArtifacts("storyboards", Mode.TARGET).list();
-    }
-
-    ImmutableList<Artifact> resources() {
-      return ruleContext.getPrerequisiteArtifacts("resources", Mode.TARGET).list();
-    }
-
-    ImmutableList<Artifact> structuredResources() {
-      return ruleContext.getPrerequisiteArtifacts("structured_resources", Mode.TARGET).list();
-    }
-
-    ImmutableList<Artifact> datamodels() {
-      return ruleContext.getPrerequisiteArtifacts("datamodels", Mode.TARGET).list();
-    }
-
-    ImmutableList<Artifact> assetCatalogs() {
-      return ruleContext.getPrerequisiteArtifacts("asset_catalogs", Mode.TARGET).list();
-    }
-  }
-
   static class Builder {
     private final RuleContext context;
     private final StarlarkSemantics semantics;
     private final BuildConfiguration buildConfiguration;
     private Optional<CompilationAttributes> compilationAttributes = Optional.absent();
-    private Optional<ResourceAttributes> resourceAttributes = Optional.absent();
     private Iterable<SdkFramework> extraSdkFrameworks = ImmutableList.of();
     private Iterable<SdkFramework> extraWeakSdkFrameworks = ImmutableList.of();
     private Iterable<String> extraSdkDylibs = ImmutableList.of();
@@ -199,15 +148,6 @@ public Builder setCompilationAttributes(CompilationAttributes baseCompilationAtt
       return this;
     }
 
-    public Builder setResourceAttributes(ResourceAttributes baseResourceAttributes) {
-      Preconditions.checkState(
-          !this.resourceAttributes.isPresent(),
-          "resourceAttributes is already set to: %s",
-          this.resourceAttributes);
-      this.resourceAttributes = Optional.of(baseResourceAttributes);
-      return this;
-    }
-
     Builder addExtraSdkFrameworks(Iterable<SdkFramework> extraSdkFrameworks) {
       this.extraSdkFrameworks = Iterables.concat(this.extraSdkFrameworks, extraSdkFrameworks);
       return this;
@@ -398,12 +338,9 @@ Builder setLinkmapFile(Artifact linkmapFile) {
 
     ObjcCommon build() {
 
-      Iterable<BundleableFile> bundleImports = BundleableFile.bundleImportsFromRule(context);
-
       ObjcProvider.Builder objcProvider =
           new ObjcProvider.Builder(semantics)
               .addAll(IMPORTED_LIBRARY, extraImportLibraries)
-              .addAll(BUNDLE_FILE, bundleImports)
               .addAll(SDK_FRAMEWORK, extraSdkFrameworks)
               .addAll(WEAK_SDK_FRAMEWORK, extraWeakSdkFrameworks)
               .addAll(SDK_DYLIB, extraSdkDylibs)
@@ -485,33 +422,6 @@ ObjcCommon build() {
             .addAll(SDK_DYLIB, attributes.sdkDylibs());
       }
 
-      if (resourceAttributes.isPresent()) {
-        ResourceAttributes attributes = resourceAttributes.get();
-        objcProvider
-            .addAll(BUNDLE_FILE, BundleableFile.flattenedRawResourceFiles(attributes.resources()))
-            .addAll(
-                BUNDLE_FILE,
-                BundleableFile.structuredRawResourceFiles(attributes.structuredResources()))
-            .addAll(
-                XCASSETS_DIR,
-                uniqueContainers(attributes.assetCatalogs(), ASSET_CATALOG_CONTAINER_TYPE))
-            .addAll(ASSET_CATALOG, attributes.assetCatalogs())
-            .addAll(XCDATAMODEL, attributes.datamodels())
-            .addAll(XIB, attributes.xibs())
-            .addAll(STRINGS, attributes.strings())
-            .addAll(STORYBOARD, attributes.storyboards());
-      }
-
-      if (useLaunchStoryboard(context)) {
-        Artifact launchStoryboard =
-            context.getPrerequisiteArtifact("launch_storyboard", Mode.TARGET);
-        if (ObjcRuleClasses.STORYBOARD_TYPE.matches(launchStoryboard.getPath())) {
-          objcProvider.add(STORYBOARD, launchStoryboard);
-        } else {
-          objcProvider.add(XIB, launchStoryboard);
-        }
-      }
-
       for (CompilationArtifacts artifacts : compilationArtifacts.asSet()) {
         Iterable<Artifact> allSources =
             Iterables.concat(artifacts.getSrcs(), artifacts.getNonArcSrcs());
@@ -608,18 +518,6 @@ private static boolean isCcLibrary(ConfiguredTargetAndData info) {
         return false;
       }
     }
-
-    /**
-     * Returns {@code true} if the given rule context has a launch storyboard set.
-     */
-    private static boolean useLaunchStoryboard(RuleContext ruleContext) {
-      if (!ruleContext.attributes().has("launch_storyboard", LABEL)) {
-        return false;
-      }
-      Artifact launchStoryboard =
-          ruleContext.getPrerequisiteArtifact("launch_storyboard", Mode.TARGET);
-      return launchStoryboard != null;
-    }
   }
 
   static final FileType BUNDLE_CONTAINER_TYPE = FileType.of(".bundle");

diff --git a/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcImport.java b/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcImport.java
@@ -22,7 +22,6 @@
 import com.google.devtools.build.lib.analysis.configuredtargets.RuleConfiguredTarget.Mode;
 import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder;
 import com.google.devtools.build.lib.rules.cpp.CppModuleMap;
-import com.google.devtools.build.lib.rules.objc.ObjcCommon.ResourceAttributes;
 import com.google.devtools.build.lib.syntax.Type;
 
 /**
@@ -36,15 +35,11 @@ public ConfiguredTarget create(RuleContext ruleContext)
         new ObjcCommon.Builder(ruleContext)
             .setCompilationAttributes(
                 CompilationAttributes.Builder.fromRuleContext(ruleContext).build())
-            .setResourceAttributes(new ResourceAttributes(ruleContext))
             .setIntermediateArtifacts(ObjcRuleClasses.intermediateArtifacts(ruleContext))
             .setAlwayslink(ruleContext.attributes().get("alwayslink", Type.BOOLEAN))
             .setHasModuleMap()
             .addExtraImportLibraries(
                 ruleContext.getPrerequisiteArtifacts("archives", Mode.TARGET).list())
-            .addDepObjcProviders(
-                ruleContext.getPrerequisites(
-                    "bundles", Mode.TARGET, ObjcProvider.SKYLARK_CONSTRUCTOR))
             .build();
 
     NestedSetBuilder<Artifact> filesToBuild = NestedSetBuilder.stableOrder();
@@ -63,8 +58,6 @@ public ConfiguredTarget create(RuleContext ruleContext)
         .registerGenerateModuleMapAction(moduleMap, publicHeaders)
         .validateAttributes();
 
-    new ResourceSupport(ruleContext).validateAttributes();
-
     return ObjcRuleClasses.ruleConfiguredTarget(ruleContext, filesToBuild.build())
         .addNativeDeclaredProvider(common.getObjcProvider())
         .build();

diff --git a/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcLibrary.java b/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcLibrary.java
@@ -30,7 +30,6 @@
 import com.google.devtools.build.lib.rules.cpp.LibraryToLink;
 import com.google.devtools.build.lib.rules.cpp.LibraryToLink.CcLinkingContext;
 import com.google.devtools.build.lib.rules.cpp.LibraryToLink.CcLinkingContext.LinkOptions;
-import com.google.devtools.build.lib.rules.objc.ObjcCommon.ResourceAttributes;
 import com.google.devtools.build.lib.syntax.Type;
 import com.google.devtools.build.lib.vfs.FileSystemUtils;
 import java.util.Map;
@@ -48,13 +47,10 @@ private ObjcCommon common(RuleContext ruleContext) throws InterruptedException {
     return new ObjcCommon.Builder(ruleContext)
         .setCompilationAttributes(
             CompilationAttributes.Builder.fromRuleContext(ruleContext).build())
-        .setResourceAttributes(new ResourceAttributes(ruleContext))
         .addDefines(ruleContext.getExpander().withDataLocations().tokenized("defines"))
         .setCompilationArtifacts(CompilationSupport.compilationArtifacts(ruleContext))
         .addDeps(ruleContext.getPrerequisiteConfiguredTargetAndTargets("deps", Mode.TARGET))
         .addRuntimeDeps(ruleContext.getPrerequisites("runtime_deps", Mode.TARGET))
-        .addDepObjcProviders(
-            ruleContext.getPrerequisites("bundles", Mode.TARGET, ObjcProvider.SKYLARK_CONSTRUCTOR))
         .setIntermediateArtifacts(ObjcRuleClasses.intermediateArtifacts(ruleContext))
         .setAlwayslink(ruleContext.attributes().get("alwayslink", Type.BOOLEAN))
         .setHasModuleMap()
@@ -87,8 +83,6 @@ public ConfiguredTarget create(RuleContext ruleContext)
             ruleContext.getImplicitOutputArtifact(CompilationSupport.FULLY_LINKED_LIB))
         .validateAttributes();
 
-    new ResourceSupport(ruleContext).validateAttributes();
-
     J2ObjcMappingFileProvider j2ObjcMappingFileProvider = J2ObjcMappingFileProvider.union(
             ruleContext.getPrerequisites("deps", Mode.TARGET, J2ObjcMappingFileProvider.class));
     J2ObjcEntryClassProvider j2ObjcEntryClassProvider = new J2ObjcEntryClassProvider.Builder()
@@ -177,27 +171,6 @@ private ImmutableList<LibraryToLink> convertLibrariesToStaticLibraries(
 
   /** Throws errors or warnings for bad attribute state. */
   private static void validateAttributes(RuleContext ruleContext) throws RuleErrorException {
-    if (ObjcRuleClasses.objcConfiguration(ruleContext).disableObjcLibraryResources()) {
-      ImmutableList<String> resourceAttributes =
-          ImmutableList.of(
-              "asset_catalogs",
-              "bundles",
-              "datamodels",
-              "resources",
-              "storyboards",
-              "strings",
-              "structured_resources",
-              "xibs");
-      for (String attribute : resourceAttributes) {
-        if (ruleContext.attributes().isAttributeValueExplicitlySpecified(attribute)) {
-          ruleContext.throwWithAttributeError(
-              attribute,
-              "objc_library resource attributes are not allowed. Please use the 'data' "
-                  + "attribute instead.");
-        }
-      }
-    }
-
     for (String copt : ObjcCommon.getNonCrosstoolCopts(ruleContext)) {
       if (copt.contains("-fmodules-cache-path")) {
         ruleContext.ruleWarning(CompilationSupport.MODULES_CACHE_PATH_WARNING);