Rollback of commit 162d5b276e1539b1acc44afcd4a4ccadcf99023a.

*** Reason for rollback ***

Breaks targets that have the same .m file in both srcs and hdrs.

*** Original change description ***

Allow private header files in the srcs attribute of objc_* rules.

This uses CppModuleMap and CppModuleMapAction to generate clang module maps for the target and its transitive dependencies. clang enforces private header usage through these maps. Right now module maps are interpreted but modules aren't enabled.

RELNOTES: Allow private header files in the srcs attribute of objc_* rules.

--
MOS_MIGRATED_REVID=95916531

11 files changed, 45 insertions, 303 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/rules/objc/BinaryLinkingTargetFactory.java b/src/main/java/com/google/devtools/build/lib/rules/objc/BinaryLinkingTargetFactory.java
index 9d327a87bb..b59bbe30c9 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/objc/BinaryLinkingTargetFactory.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/objc/BinaryLinkingTargetFactory.java
@@ -181,7 +181,6 @@ abstract class BinaryLinkingTargetFactory implements RuleConfiguredTargetFactory
             ruleContext.getPrerequisites("non_propagated_deps", Mode.TARGET, ObjcProvider.class))
         .setIntermediateArtifacts(intermediateArtifacts)
         .setAlwayslink(false)
-        .setGeneratesModuleMap()
         .addExtraImportLibraries(ObjcRuleClasses.j2ObjcLibraries(ruleContext))
         .setLinkedBinary(intermediateArtifacts.singleArchitectureBinary());
 
diff --git a/src/main/java/com/google/devtools/build/lib/rules/objc/CompilationArtifacts.java b/src/main/java/com/google/devtools/build/lib/rules/objc/CompilationArtifacts.java
index c566bcb31f..5aac139e57 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/objc/CompilationArtifacts.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/objc/CompilationArtifacts.java
@@ -28,8 +28,6 @@ final class CompilationArtifacts {
   static class Builder {
     private Iterable<Artifact> srcs = ImmutableList.of();
     private Iterable<Artifact> nonArcSrcs = ImmutableList.of();
-    private Iterable<Artifact> additionalHdrs = ImmutableList.of();
-    private Iterable<Artifact> privateHdrs = ImmutableList.of();
     private Optional<Artifact> pchFile;
     private IntermediateArtifacts intermediateArtifacts;
 
@@ -43,23 +41,6 @@ final class CompilationArtifacts {
       return this;
     }
 
-    /**
-     * Adds header artifacts that should be directly accessible to dependers, but aren't specified
-     * in the hdrs attribute.
-     */
-    Builder addAdditionalHdrs(Iterable<Artifact> additionalHdrs) {
-      this.additionalHdrs = Iterables.concat(this.additionalHdrs, additionalHdrs);
-      return this;
-    }
-
-    /**
-     * Adds header artifacts that should not be directly accessible to dependers.
-     */
-    Builder addPrivateHdrs(Iterable<Artifact> privateHdrs) {
-      this.privateHdrs = Iterables.concat(this.privateHdrs, privateHdrs);
-      return this;
-    }
-
     Builder setPchFile(Optional<Artifact> pchFile) {
       Preconditions.checkState(this.pchFile == null,
           "pchFile is already set to: %s", this.pchFile);
@@ -79,29 +60,22 @@ final class CompilationArtifacts {
       if (!Iterables.isEmpty(srcs) || !Iterables.isEmpty(nonArcSrcs)) {
         archive = Optional.of(intermediateArtifacts.archive());
       }
-      return new CompilationArtifacts(
-          srcs, nonArcSrcs, additionalHdrs, privateHdrs, archive, pchFile);
+      return new CompilationArtifacts(srcs, nonArcSrcs, archive, pchFile);
     }
   }
 
   private final Iterable<Artifact> srcs;
   private final Iterable<Artifact> nonArcSrcs;
   private final Optional<Artifact> archive;
-  private final Iterable<Artifact> additionalHdrs;
-  private final Iterable<Artifact> privateHdrs;
   private final Optional<Artifact> pchFile;
 
   private CompilationArtifacts(
       Iterable<Artifact> srcs,
       Iterable<Artifact> nonArcSrcs,
-      Iterable<Artifact> additionalHdrs,
-      Iterable<Artifact> privateHdrs,
       Optional<Artifact> archive,
       Optional<Artifact> pchFile) {
     this.srcs = Preconditions.checkNotNull(srcs);
     this.nonArcSrcs = Preconditions.checkNotNull(nonArcSrcs);
-    this.additionalHdrs = Preconditions.checkNotNull(additionalHdrs);
-    this.privateHdrs = Preconditions.checkNotNull(privateHdrs);
     this.archive = Preconditions.checkNotNull(archive);
     this.pchFile = Preconditions.checkNotNull(pchFile);
   }
@@ -114,20 +88,6 @@ final class CompilationArtifacts {
     return nonArcSrcs;
   }
 
-  /**
-   * Returns an {@link Iterable} of public headers that aren't included in the hdrs attribute.
-   */
-  public Iterable<Artifact> getAdditionalHdrs() {
-    return additionalHdrs;
-  }
-
-  /**
-   * Returns an {@link Iterable} of private headers.
-   */
-  public Iterable<Artifact> getPrivateHdrs() {
-    return privateHdrs;
-  }
-
   public Optional<Artifact> getArchive() {
     return archive;
   }
diff --git a/src/main/java/com/google/devtools/build/lib/rules/objc/CompilationSupport.java b/src/main/java/com/google/devtools/build/lib/rules/objc/CompilationSupport.java
index 98048e437b..bfd4b1819c 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/objc/CompilationSupport.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/objc/CompilationSupport.java
@@ -24,15 +24,12 @@ 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;
 import static com.google.devtools.build.lib.rules.objc.ObjcProvider.LIBRARY;
-import static com.google.devtools.build.lib.rules.objc.ObjcProvider.MODULE_MAP;
 import static com.google.devtools.build.lib.rules.objc.ObjcProvider.SDK_DYLIB;
 import static com.google.devtools.build.lib.rules.objc.ObjcProvider.SDK_FRAMEWORK;
 import static com.google.devtools.build.lib.rules.objc.ObjcProvider.WEAK_SDK_FRAMEWORK;
 import static com.google.devtools.build.lib.rules.objc.ObjcRuleClasses.CLANG;
 import static com.google.devtools.build.lib.rules.objc.ObjcRuleClasses.CLANG_PLUSPLUS;
-import static com.google.devtools.build.lib.rules.objc.ObjcRuleClasses.COMPILABLE_SRCS_TYPE;
 import static com.google.devtools.build.lib.rules.objc.ObjcRuleClasses.DSYMUTIL;
-import static com.google.devtools.build.lib.rules.objc.ObjcRuleClasses.HEADERS;
 import static com.google.devtools.build.lib.rules.objc.ObjcRuleClasses.NON_ARC_SRCS_TYPE;
 import static com.google.devtools.build.lib.rules.objc.ObjcRuleClasses.SRCS_TYPE;
 
@@ -42,10 +39,8 @@ import com.google.common.base.Optional;
 import com.google.common.base.Preconditions;
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.Iterables;
-import com.google.common.collect.Sets;
 import com.google.devtools.build.lib.actions.Action;
 import com.google.devtools.build.lib.actions.Artifact;
-import com.google.devtools.build.lib.analysis.PrerequisiteArtifacts;
 import com.google.devtools.build.lib.analysis.RuleConfiguredTarget.Mode;
 import com.google.devtools.build.lib.analysis.RuleContext;
 import com.google.devtools.build.lib.analysis.actions.ActionConstructionContext;
@@ -54,9 +49,6 @@ import com.google.devtools.build.lib.analysis.actions.CustomCommandLine;
 import com.google.devtools.build.lib.analysis.actions.FileWriteAction;
 import com.google.devtools.build.lib.analysis.actions.SpawnAction;
 import com.google.devtools.build.lib.collect.nestedset.NestedSet;
-import com.google.devtools.build.lib.packages.Type;
-import com.google.devtools.build.lib.rules.cpp.CppModuleMap;
-import com.google.devtools.build.lib.rules.cpp.CppModuleMapAction;
 import com.google.devtools.build.lib.rules.cpp.LinkerInputs;
 import com.google.devtools.build.lib.rules.objc.ObjcCommon.CompilationAttributes;
 import com.google.devtools.build.lib.rules.objc.XcodeProvider.Builder;
@@ -64,9 +56,7 @@ import com.google.devtools.build.lib.shell.ShellUtils;
 import com.google.devtools.build.lib.vfs.PathFragment;
 
 import java.util.ArrayList;
-import java.util.HashSet;
 import java.util.List;
-import java.util.Set;
 
 /**
  * Support for rules that compile sources. Provides ways to determine files that should be output,
@@ -75,7 +65,7 @@ import java.util.Set;
  *
  * <p>Methods on this class can be called in any order without impacting the result.
  */
-public final class CompilationSupport {
+final class CompilationSupport {
 
   @VisibleForTesting
   static final String ABSOLUTE_INCLUDES_PATH_FORMAT =
@@ -100,23 +90,18 @@ public final class CompilationSupport {
     }
   }
 
-  @VisibleForTesting
-  static final String FILE_IN_SRCS_AND_HDRS_ERROR_FORMAT =
-      "File '%s' is in both srcs and hdrs.";
-
   /**
    * Returns information about the given rule's compilation artifacts.
    */
   // TODO(bazel-team): Remove this information from ObjcCommon and move it internal to this class.
   static CompilationArtifacts compilationArtifacts(RuleContext ruleContext) {
-    PrerequisiteArtifacts srcs = ruleContext.getPrerequisiteArtifacts("srcs", Mode.TARGET)
-        .errorsForNonMatching(SRCS_TYPE);
     return new CompilationArtifacts.Builder()
-        .addSrcs(srcs.filter(COMPILABLE_SRCS_TYPE).list())
+        .addSrcs(ruleContext.getPrerequisiteArtifacts("srcs", Mode.TARGET)
+            .errorsForNonMatching(SRCS_TYPE)
+            .list())
         .addNonArcSrcs(ruleContext.getPrerequisiteArtifacts("non_arc_srcs", Mode.TARGET)
             .errorsForNonMatching(NON_ARC_SRCS_TYPE)
             .list())
-        .addPrivateHdrs(srcs.filter(HEADERS).list())
         .setIntermediateArtifacts(ObjcRuleClasses.intermediateArtifacts(ruleContext))
         .setPchFile(Optional.fromNullable(ruleContext.getPrerequisiteArtifact("pch", Mode.TARGET)))
         .build();
@@ -128,7 +113,7 @@ public final class CompilationSupport {
   /**
    * Creates a new compilation support for the given rule.
    */
-  public CompilationSupport(RuleContext ruleContext) {
+  CompilationSupport(RuleContext ruleContext) {
     this.ruleContext = ruleContext;
     this.attributes = new CompilationAttributes(ruleContext);
   }
@@ -141,14 +126,10 @@ public final class CompilationSupport {
    */
   CompilationSupport registerCompileAndArchiveActions(ObjcCommon common) {
     if (common.getCompilationArtifacts().isPresent()) {
-      registerGenerateModuleMapAction(common.getCompilationArtifacts());
-      IntermediateArtifacts intermediateArtifacts =
-          ObjcRuleClasses.intermediateArtifacts(ruleContext);
       registerCompileAndArchiveActions(
           common.getCompilationArtifacts().get(),
-          intermediateArtifacts,
+          ObjcRuleClasses.intermediateArtifacts(ruleContext),
           common.getObjcProvider(),
-          intermediateArtifacts.moduleMap(),
           ruleContext.getConfiguration().isCodeCoverageEnabled());
     }
     return this;
@@ -160,20 +141,20 @@ public final class CompilationSupport {
    */
   private void registerCompileAndArchiveActions(CompilationArtifacts compilationArtifacts,
       IntermediateArtifacts intermediateArtifacts, ObjcProvider objcProvider,
-      CppModuleMap moduleMap, boolean isCodeCoverageEnabled) {
+      boolean isCodeCoverageEnabled) {
     ImmutableList.Builder<Artifact> objFiles = new ImmutableList.Builder<>();
     for (Artifact sourceFile : compilationArtifacts.getSrcs()) {
       Artifact objFile = intermediateArtifacts.objFile(sourceFile);
       objFiles.add(objFile);
       registerCompileAction(sourceFile, objFile, compilationArtifacts.getPchFile(),
-          objcProvider, moduleMap, intermediateArtifacts, ImmutableList.of("-fobjc-arc"),
+          objcProvider, intermediateArtifacts, ImmutableList.of("-fobjc-arc"),
           isCodeCoverageEnabled);
     }
     for (Artifact nonArcSourceFile : compilationArtifacts.getNonArcSrcs()) {
       Artifact objFile = intermediateArtifacts.objFile(nonArcSourceFile);
       objFiles.add(objFile);
       registerCompileAction(nonArcSourceFile, objFile, compilationArtifacts.getPchFile(),
-          objcProvider, moduleMap, intermediateArtifacts, ImmutableList.of("-fno-objc-arc"),
+          objcProvider, intermediateArtifacts, ImmutableList.of("-fno-objc-arc"),
           isCodeCoverageEnabled);
     }
     for (Artifact archive : compilationArtifacts.getArchive().asSet()) {
@@ -186,7 +167,6 @@ public final class CompilationSupport {
       Artifact objFile,
       Optional<Artifact> pchFile,
       ObjcProvider objcProvider,
-      CppModuleMap moduleMap,
       IntermediateArtifacts intermediateArtifacts,
       Iterable<String> otherFlags,
       boolean isCodeCoverageEnabled) {
@@ -203,10 +183,8 @@ public final class CompilationSupport {
     }
     commandLine
         .add(IosSdkCommands.compileFlagsForClang(objcConfiguration))
-        .add("-fmodule-maps")
-        .add("-fmodule-map-file=" + moduleMap.getArtifact().getExecPath())
-        .add("-fmodule-name=" + moduleMap.getName())
-        .add(IosSdkCommands.commonLinkAndCompileFlagsForClang(objcProvider, objcConfiguration))
+        .add(IosSdkCommands.commonLinkAndCompileFlagsForClang(
+            objcProvider, objcConfiguration))
         .add(objcConfiguration.getCoptsForCompilationMode())
         .addBeforeEachPath(
             "-iquote", ObjcCommon.userHeaderSearchPaths(ruleContext.getConfiguration()))
@@ -230,7 +208,6 @@ public final class CompilationSupport {
         .addOutputs(gcnoFiles.build())
         .addTransitiveInputs(objcProvider.get(HEADER))
         .addTransitiveInputs(objcProvider.get(FRAMEWORK_FILE))
-        .addTransitiveInputs(objcProvider.get(MODULE_MAP))
         .addInputs(pchFile.asSet())
         .build(ruleContext));
   }
@@ -303,54 +280,6 @@ public final class CompilationSupport {
     return this;
   }
 
-  /**
-   * Registers an action that will generate a clang module map for this target, using the hdrs
-   * attribute of this rule and the module maps of direct dependencies.
-   *
-   * @param moduleMap the module map to generate
-   * @param compilationArtifacts an optional {@link CompilationArtifacts} object that contains
-   *     additional headers to be included in the module map
-   */
-  public CompilationSupport registerGenerateModuleMapAction(
-      Optional<CompilationArtifacts> compilationArtifacts) {
-    Iterable<Artifact> publicHeaders = attributes.hdrs();
-    Iterable<Artifact> privateHeaders = ImmutableList.<Artifact>of();
-    if (compilationArtifacts.isPresent()) {
-      CompilationArtifacts artifacts = compilationArtifacts.get();
-      publicHeaders = Iterables.concat(publicHeaders, artifacts.getAdditionalHdrs());
-      privateHeaders = Iterables.concat(privateHeaders, artifacts.getPrivateHdrs());
-    }
-    return registerGenerateModuleMapAction(
-        ObjcRuleClasses.intermediateArtifacts(ruleContext).moduleMap(),
-        publicHeaders, privateHeaders, attributes.directDepModuleMaps());
-  }
-
-  /**
-   * Registers an action that will generate a clang module map.
-   *
-   * @param moduleMap the module map to generate
-   * @param publicHeaders the headers that should be directly accessible by dependers
-   * @param privateHeaders the headers that should only be directly accessible by this module
-   * @param moduleMapDeps the module maps to depend on
-   */
-  private CompilationSupport registerGenerateModuleMapAction(
-      CppModuleMap moduleMap, Iterable<Artifact> publicHeaders, Iterable<Artifact> privateHeaders,
-      List<CppModuleMap> moduleMapDeps) {
-    // TODO(bazel-team): The current clang (clang-600.0.57) on Darwin doesn't seem to support
-    // 'textual'. In order to enable -fmodules, we will need to wait until it does and change
-    // compiledModule to false, or ensure that no headers have any conditional #error directives.
-    ruleContext.registerAction(new CppModuleMapAction(ruleContext.getActionOwner(),
-        moduleMap,
-        privateHeaders,
-        publicHeaders,
-        moduleMapDeps,
-        ImmutableList.<PathFragment>of(),
-        /*compiledModule=*/true,
-        /*moduleMapHomeIsCwd=*/false,
-        /*generateSubModules=*/false));
-    return this;
-  }
-
   private void registerLinkAction(ObjcProvider objcProvider, ExtraLinkArgs extraLinkArgs,
       Iterable<Artifact> extraLinkInputs, Optional<Artifact> dsymBundle) {
     Artifact linkedBinary =
@@ -511,41 +440,18 @@ public final class CompilationSupport {
               : j2ObjcSource;
       IntermediateArtifacts intermediateArtifacts =
           ObjcRuleClasses.j2objcIntermediateArtifacts(ruleContext, sourceToCompile);
-      // The module map that the above intermediateArtifacts would point to is a combination of
-      // the current target and the java target. Instead, we want the one generated for that source,
-      // but there is no easy way to get that. So for now, compile the source with the module map
-      // (and therefore module name) for this rule. If private headers are introduced for J2Objc
-      // sources, this will have to be reworked.
-      // TODO(bazel-team): Generate module maps with J2Objc sources, and use that here.
-      CppModuleMap moduleMap = ObjcRuleClasses.intermediateArtifacts(ruleContext).moduleMap();
       CompilationArtifacts compilationArtifact = new CompilationArtifacts.Builder()
           .addNonArcSrcs(sourceToCompile.getObjcSrcs())
           .setIntermediateArtifacts(intermediateArtifacts)
           .setPchFile(Optional.<Artifact>absent())
           .build();
       registerCompileAndArchiveActions(compilationArtifact, intermediateArtifacts, objcProvider,
-          moduleMap, ruleContext.getConfiguration().isCodeCoverageEnabled());
+          ruleContext.getConfiguration().isCodeCoverageEnabled());
     }
 
     return this;
   }
 
-  /**
-   * Registers actions that generates a module map for all {@link J2ObjcSource}s in
-   * {@link J2ObjcSrcsProvider}.
-   *
-   * @return this compilation support
-   */
-  public CompilationSupport registerJ2ObjcGenerateModuleMapAction(J2ObjcSrcsProvider provider) {
-    ArrayList<Artifact> headers = new ArrayList<>();
-    for (J2ObjcSource j2ObjcSource : provider.getSrcs()) {
-      headers.addAll(ImmutableList.copyOf(j2ObjcSource.getObjcHdrs()));
-    }
-    return registerGenerateModuleMapAction(
-        ObjcRuleClasses.intermediateArtifacts(ruleContext).moduleMap(),
-        headers, ImmutableList.<Artifact>of(), ImmutableList.<CppModuleMap>of());
-  }
-
   private void registerJ2ObjcDeadCodeRemovalActions(Iterable<J2ObjcSource> j2ObjcSources,
       Iterable<String> entryClasses) {
     Artifact pruner = ruleContext.getPrerequisiteArtifact("$j2objc_dead_code_pruner", Mode.HOST);
@@ -615,16 +521,6 @@ public final class CompilationSupport {
           "includes", String.format(ABSOLUTE_INCLUDES_PATH_FORMAT, absoluteInclude));
     }
 
-    // Check for overlap between srcs and hdrs.
-    if (ruleContext.attributes().has("srcs", Type.LABEL_LIST)) {
-      Set<Artifact> hdrsSet = new HashSet<>(attributes.hdrs());
-      Set<Artifact> srcsSet =
-          new HashSet<>(ruleContext.getPrerequisiteArtifacts("srcs", Mode.TARGET).list());
-      for (Artifact header : Sets.intersection(hdrsSet, srcsSet)) {
-        String path = header.getRootRelativePath().toString();
-        ruleContext.attributeError("srcs", String.format(FILE_IN_SRCS_AND_HDRS_ERROR_FORMAT, path));
-      }
-    }
     return this;
   }
 
diff --git a/src/main/java/com/google/devtools/build/lib/rules/objc/IntermediateArtifacts.java b/src/main/java/com/google/devtools/build/lib/rules/objc/IntermediateArtifacts.java
index 6cc286dcbc..9e1372e8d4 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/objc/IntermediateArtifacts.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/objc/IntermediateArtifacts.java
@@ -16,13 +16,10 @@ package com.google.devtools.build.lib.rules.objc;
 
 import com.google.common.base.Optional;
 import com.google.common.base.Preconditions;
-import com.google.common.collect.Iterables;
 import com.google.devtools.build.lib.actions.Artifact;
 import com.google.devtools.build.lib.actions.Root;
 import com.google.devtools.build.lib.analysis.AnalysisEnvironment;
 import com.google.devtools.build.lib.analysis.AnalysisUtils;
-import com.google.devtools.build.lib.rules.cpp.CppFileTypes;
-import com.google.devtools.build.lib.rules.cpp.CppModuleMap;
 import com.google.devtools.build.lib.syntax.Label;
 import com.google.devtools.build.lib.vfs.FileSystemUtils;
 import com.google.devtools.build.lib.vfs.PathFragment;
@@ -32,7 +29,7 @@ import com.google.devtools.build.lib.vfs.PathFragment;
  */
 // TODO(bazel-team): This should really be named DerivedArtifacts as it contains methods for
 // final as well as intermediate artifacts.
-public final class IntermediateArtifacts {
+final class IntermediateArtifacts {
 
   /**
    * Extension used on the temporary dsym bundle location. Must end in {@code .dSYM} for dsymutil
@@ -273,13 +270,4 @@ public final class IntermediateArtifacts {
   public Artifact runnerScript() {
     return appendExtension("_runner.sh");
   }
-
-  /**
-   * {@link CppModuleMap} used to enforce proper usage of private headers.
-   */
-  public CppModuleMap moduleMap() {
-    Artifact mapFile =
-        appendExtension(Iterables.getOnlyElement(CppFileTypes.CPP_MODULE_MAP.getExtensions()));
-    return new CppModuleMap(mapFile, ownerLabel.toString());
-  }
 }
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
index bf6acfd396..e311bc3473 100644
--- 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
@@ -34,13 +34,11 @@ import static com.google.devtools.build.lib.rules.objc.ObjcProvider.INCLUDE;
 import static com.google.devtools.build.lib.rules.objc.ObjcProvider.INSTRUMENTED_SOURCE;
 import static com.google.devtools.build.lib.rules.objc.ObjcProvider.LIBRARY;
 import static com.google.devtools.build.lib.rules.objc.ObjcProvider.LINKED_BINARY;
-import static com.google.devtools.build.lib.rules.objc.ObjcProvider.MODULE_MAP;
 import static com.google.devtools.build.lib.rules.objc.ObjcProvider.SDK_DYLIB;
 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.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.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;
@@ -62,12 +60,10 @@ import com.google.devtools.build.lib.packages.Type;
 import com.google.devtools.build.lib.rules.cpp.CcCommon;
 import com.google.devtools.build.lib.rules.cpp.CcLinkParamsProvider;
 import com.google.devtools.build.lib.rules.cpp.CppCompilationContext;
-import com.google.devtools.build.lib.rules.cpp.CppModuleMap;
 import com.google.devtools.build.lib.util.FileType;
 import com.google.devtools.build.lib.util.RegexFilter;
 import com.google.devtools.build.lib.vfs.PathFragment;
 
-import java.util.ArrayList;
 import java.util.HashSet;
 import java.util.List;
 import java.util.Set;
@@ -95,10 +91,6 @@ public final class ObjcCommon {
     }
 
     ImmutableList<Artifact> hdrs() {
-      // Some rules may compile but not have the "hdrs" attribute".
-      if (!ruleContext.attributes().has("hdrs", Type.LABEL_LIST)) {
-        return ImmutableList.of();
-      }
       return ImmutableList.copyOf(CcCommon.getHeaders(ruleContext));
     }
 
@@ -184,37 +176,6 @@ public final class ObjcCommon {
       }
       return optionsProvider.getCopts();
     }
-
-    /**
-     * The clang module maps of direct dependencies of this rule. These are needed to generate
-     * this rule's module map.
-     */
-    public List<CppModuleMap> directDepModuleMaps() {
-      // Make sure all dependencies that have headers are included here. If a module map is missing,
-      // its private headers will be treated as public!
-      ArrayList<CppModuleMap> moduleMaps = new ArrayList<>();
-      collectModuleMapsFromAttributeIfExists(moduleMaps, "deps");
-      collectModuleMapsFromAttributeIfExists(moduleMaps, "non_propagated_deps");
-      return moduleMaps;
-    }
-
-    /**
-     * Collects all module maps from the targets in a certain attribute and adds them into
-     * {@code moduleMaps}.
-     *
-     * @param moduleMaps an {@link ArrayList} to collect the module maps into
-     * @param attribute the name of a label list attribute to collect module maps from
-     */
-    private void collectModuleMapsFromAttributeIfExists(ArrayList<CppModuleMap> moduleMaps,
-        String attribute) {
-      if (ruleContext.attributes().has(attribute, Type.LABEL_LIST)) {
-        Iterable<ObjcProvider> providers =
-            ruleContext.getPrerequisites(attribute, Mode.TARGET, ObjcProvider.class);
-        for (ObjcProvider provider : providers) {
-          moduleMaps.addAll(provider.get(TOP_LEVEL_MODULE_MAP).toCollection());
-        }
-      }
-    }
   }
 
   /**
@@ -273,9 +234,9 @@ public final class ObjcCommon {
     private Iterable<ObjcProvider> directDepObjcProviders = ImmutableList.of();
     private Iterable<String> defines = ImmutableList.of();
     private Iterable<PathFragment> userHeaderSearchPaths = ImmutableList.of();
+    private Iterable<Artifact> headers = ImmutableList.of();
     private IntermediateArtifacts intermediateArtifacts;
     private boolean alwayslink;
-    private boolean generatesModuleMap;
     private Iterable<Artifact> extraImportLibraries = ImmutableList.of();
     private Optional<Artifact> linkedBinary = Optional.absent();
     private Optional<Artifact> breakpadFile = Optional.absent();
@@ -358,6 +319,11 @@ public final class ObjcCommon {
       return this;
     }
 
+    public Builder addHeaders(Iterable<Artifact> headers) {
+      this.headers = Iterables.concat(this.headers, headers);
+      return this;
+    }
+
     Builder setIntermediateArtifacts(IntermediateArtifacts intermediateArtifacts) {
       this.intermediateArtifacts = intermediateArtifacts;
       return this;
@@ -369,15 +335,6 @@ public final class ObjcCommon {
     }
 
     /**
-     * Specifies that this target has a clang module map. This should be called if this target
-     * compiles sources or exposes headers for other targets to use.
-     */
-    Builder setGeneratesModuleMap() {
-      this.generatesModuleMap = true;
-      return this;
-    }
-
-    /**
      * Adds additional static libraries to be linked into the final ObjC application bundle.
      */
     Builder addExtraImportLibraries(Iterable<Artifact> extraImportLibraries) {
@@ -433,6 +390,7 @@ public final class ObjcCommon {
           .addAll(FRAMEWORK_DIR, uniqueContainers(frameworkImports, FRAMEWORK_CONTAINER_TYPE))
           .addAll(INCLUDE, userHeaderSearchPaths)
           .addAll(DEFINE, defines)
+          .addAll(HEADER, headers)
           .addTransitiveAndPropagate(depObjcProviders)
           .addTransitiveWithoutPropagating(directDepObjcProviders);
 
@@ -484,11 +442,8 @@ public final class ObjcCommon {
       for (CompilationArtifacts artifacts : compilationArtifacts.asSet()) {
         Iterable<Artifact> allSources =
             Iterables.concat(artifacts.getSrcs(), artifacts.getNonArcSrcs());
-        objcProvider
-            .addAll(HEADER, artifacts.getAdditionalHdrs())
-            .addAll(HEADER, artifacts.getPrivateHdrs())
-            .addAll(LIBRARY, artifacts.getArchive().asSet())
-            .addAll(SOURCE, allSources);
+        objcProvider.addAll(LIBRARY, artifacts.getArchive().asSet());
+        objcProvider.addAll(SOURCE, allSources);
         BuildConfiguration configuration = context.getConfiguration();
         RegexFilter filter = configuration.getInstrumentationFilter();
         if (configuration.isCodeCoverageEnabled()
@@ -525,12 +480,6 @@ public final class ObjcCommon {
         }
       }
 
-      if (this.generatesModuleMap) {
-        CppModuleMap moduleMap = intermediateArtifacts.moduleMap();
-        objcProvider.addWithoutPropagating(TOP_LEVEL_MODULE_MAP, moduleMap);
-        objcProvider.add(MODULE_MAP, moduleMap.getArtifact());
-      }
-
       objcProvider.addAll(LINKED_BINARY, linkedBinary.asSet())
           .addAll(BREAKPAD_FILE, breakpadFile.asSet());
 
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
index 38e140afad..c3b35c22cc 100644
--- 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
@@ -16,7 +16,6 @@ package com.google.devtools.build.lib.rules.objc;
 
 import static com.google.devtools.build.lib.rules.objc.XcodeProductType.LIBRARY_STATIC;
 
-import com.google.common.base.Optional;
 import com.google.devtools.build.lib.actions.Artifact;
 import com.google.devtools.build.lib.analysis.ConfiguredTarget;
 import com.google.devtools.build.lib.analysis.RuleConfiguredTarget.Mode;
@@ -38,7 +37,6 @@ public class ObjcImport implements RuleConfiguredTargetFactory {
         .setResourceAttributes(new ResourceAttributes(ruleContext))
         .setIntermediateArtifacts(ObjcRuleClasses.intermediateArtifacts(ruleContext))
         .setAlwayslink(ruleContext.attributes().get("alwayslink", Type.BOOLEAN))
-        .setGeneratesModuleMap()
         .addExtraImportLibraries(
             ruleContext.getPrerequisiteArtifacts("archives", Mode.TARGET).list())
         .addDepObjcProviders(
@@ -49,7 +47,6 @@ public class ObjcImport implements RuleConfiguredTargetFactory {
     NestedSetBuilder<Artifact> filesToBuild = NestedSetBuilder.stableOrder();
 
     new CompilationSupport(ruleContext)
-        .registerGenerateModuleMapAction(Optional.<CompilationArtifacts>absent())
         .addXcodeSettings(xcodeProviderBuilder, common)
         .validateAttributes();
 
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
index 643b75294c..98f4fe9597 100644
--- 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
@@ -76,7 +76,6 @@ public class ObjcLibrary implements RuleConfiguredTargetFactory {
             ruleContext.getPrerequisites("deps", Mode.TARGET, CcLinkParamsProvider.class))
         .setIntermediateArtifacts(ObjcRuleClasses.intermediateArtifacts(ruleContext))
         .setAlwayslink(alwayslink)
-        .setGeneratesModuleMap()
         .addExtraImportLibraries(extraImportLibraries)
         .addDepObjcProviders(extraDepObjcProviders)
         .build();
diff --git a/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcProtoLibrary.java b/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcProtoLibrary.java
index 7708e10499..7e1504721e 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcProtoLibrary.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcProtoLibrary.java
@@ -156,18 +156,16 @@ public class ObjcProtoLibrary implements RuleConfiguredTargetFactory {
         .addNonArcSrcs(protoGeneratedSources)
         .setIntermediateArtifacts(intermediateArtifacts)
         .setPchFile(Optional.<Artifact>absent())
-        .addAdditionalHdrs(protoGeneratedHeaders)
-        .addAdditionalHdrs(protoGeneratedSources)
         .build();
 
     ImmutableSet.Builder<PathFragment> searchPathEntriesBuilder =
         new ImmutableSet.Builder<PathFragment>()
             .add(workspaceRelativeOutputDir);
-    if (ruleContext.getConfiguration().getFragment(ObjcConfiguration.class).perProtoIncludes()) {
+     if (ruleContext.getConfiguration().getFragment(ObjcConfiguration.class).perProtoIncludes()) {
       searchPathEntriesBuilder
           .add(generatedProtoDir)
           .addAll(Iterables.transform(protoGeneratedHeaders, PARENT_PATHFRAGMENT));
-    }
+     }    
     ImmutableSet<PathFragment> searchPathEntries = searchPathEntriesBuilder.build();
 
     ObjcCommon common = new ObjcCommon.Builder(ruleContext)
@@ -176,7 +174,8 @@ public class ObjcProtoLibrary implements RuleConfiguredTargetFactory {
         .addDepObjcProviders(ruleContext.getPrerequisites(
             ObjcProtoLibraryRule.LIBPROTOBUF_ATTR, Mode.TARGET, ObjcProvider.class))
         .setIntermediateArtifacts(intermediateArtifacts)
-        .setGeneratesModuleMap()
+        .addHeaders(protoGeneratedHeaders)
+        .addHeaders(protoGeneratedSources)
         .build();
 
     NestedSetBuilder<Artifact> filesToBuild = NestedSetBuilder.<Artifact>stableOrder()
diff --git a/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcProvider.java b/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcProvider.java
index f21cf0b9e7..e2fc6e9363 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcProvider.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcProvider.java
@@ -27,7 +27,6 @@ import com.google.devtools.build.lib.collect.nestedset.NestedSet;
 import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder;
 import com.google.devtools.build.lib.collect.nestedset.Order;
 import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable;
-import com.google.devtools.build.lib.rules.cpp.CppModuleMap;
 import com.google.devtools.build.lib.rules.cpp.LinkerInputs;
 import com.google.devtools.build.lib.vfs.PathFragment;
 import com.google.devtools.build.xcode.xcodegen.proto.XcodeGenProtos.TargetControl;
@@ -77,9 +76,6 @@ public final class ObjcProvider implements TransitiveInfoProvider {
    */
   public static final Key<String> FORCE_LOAD_FOR_XCODEGEN = new Key<>(LINK_ORDER);
 
-  /**
-   * Contains all header files. These may be either public or private headers.
-   */
   public static final Key<Artifact> HEADER = new Key<>(STABLE_ORDER);
 
   /**
@@ -94,7 +90,7 @@ public final class ObjcProvider implements TransitiveInfoProvider {
 
   /**
    * Contains all .gcno files one for every source file if in coverage mode.
-   * It contains information to reconstruct the basic block graphs and assign source line numbers
+   * It contains information to reconstruct the basic block graphs and assign source line numbers 
    * to blocks.
    */
   public static final Key<Artifact> GCNO = new Key<>(STABLE_ORDER);
@@ -139,20 +135,6 @@ public final class ObjcProvider implements TransitiveInfoProvider {
   public static final Key<Flag> FLAG = new Key<>(STABLE_ORDER);
 
   /**
-   * Clang module maps, used to enforce proper use of private header files.
-   */
-  public static final Key<Artifact> MODULE_MAP = new Key<>(STABLE_ORDER);
-
-  /**
-   * Information about this provider's module map, in the form of a {@link CppModuleMap}. This
-   * is intransitive, and can be used to get just the target's module map to pass to clang or to
-   * get the module maps for direct but not transitive dependencies. You should only module maps for
-   * this key using {@link #addWithoutPropagating}.
-   *
-   */
-  public static final Key<CppModuleMap> TOP_LEVEL_MODULE_MAP = new Key<>(STABLE_ORDER);
-
-  /**
    * Merge zips to include in the bundle. The entries of these zip files are included in the final
    * bundle with the same path. The entries in the merge zips should not include the bundle root
    * path (e.g. {@code Foo.app}).
@@ -278,10 +260,9 @@ public final class ObjcProvider implements TransitiveInfoProvider {
     }
 
     @SuppressWarnings({"rawtypes", "unchecked"})
-    private void uncheckedAddAll(Key key, Iterable toAdd, boolean propagate) {
-      Map<Key<?>, NestedSetBuilder<?>> set = propagate ? items : nonPropagatedItems;
-      maybeAddEmptyBuilder(set, key);
-      set.get(key).addAll(toAdd);
+    private void uncheckedAddAll(Key key, Iterable toAdd) {
+      maybeAddEmptyBuilder(items, key);
+      items.get(key).addAll(toAdd);
     }
 
     @SuppressWarnings({"rawtypes", "unchecked"})
@@ -334,7 +315,7 @@ public final class ObjcProvider implements TransitiveInfoProvider {
     /**
      * Add elements from providers, but don't propagate them to any dependers on this ObjcProvider.
      * These elements will be exposed to {@link #get(Key)} calls, but not to any ObjcProviders
-     * which add this provider to themselves.
+     * which add this provider to themself.
      */
     public Builder addTransitiveWithoutPropagating(Iterable<ObjcProvider> providers) {
       for (ObjcProvider provider : providers) {
@@ -349,7 +330,7 @@ public final class ObjcProvider implements TransitiveInfoProvider {
      * Add element, and propagate it to any (transitive) dependers on this ObjcProvider.
      */
     public <E> Builder add(Key<E> key, E toAdd) {
-      uncheckedAddAll(key, ImmutableList.of(toAdd), true);
+      uncheckedAddAll(key, ImmutableList.of(toAdd));
       return this;
     }
 
@@ -357,17 +338,7 @@ public final class ObjcProvider implements TransitiveInfoProvider {
      * Add elements in toAdd, and propagate them to any (transitive) dependers on this ObjcProvider.
      */
     public <E> Builder addAll(Key<E> key, Iterable<? extends E> toAdd) {
-      uncheckedAddAll(key, toAdd, true);
-      return this;
-    }
-
-    /**
-     * Add element, but don't propagate dependers on this ObjcProvider. These elements will be
-     * exposed to {@link #get(Key)} calls, but not to any ObjcProviders which add this provider to
-     * themselves.
-     */
-    public <E> Builder addWithoutPropagating(Key<E> key, E toAdd) {
-      uncheckedAddAll(key, ImmutableList.of(toAdd), false);
+      uncheckedAddAll(key, toAdd);
       return this;
     }
 
diff --git a/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcRuleClasses.java b/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcRuleClasses.java
index 32716a65d6..dec8cc582c 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcRuleClasses.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcRuleClasses.java
@@ -83,7 +83,7 @@ public class ObjcRuleClasses {
         ruleContext.getBinOrGenfilesDirectory());
   }
 
-  public static IntermediateArtifacts intermediateArtifacts(RuleContext ruleContext) {
+  static IntermediateArtifacts intermediateArtifacts(RuleContext ruleContext) {
     return new IntermediateArtifacts(
         ruleContext.getAnalysisEnvironment(), ruleContext.getBinOrGenfilesDirectory(),
         ruleContext.getLabel(), /*archiveFileNameSuffix=*/"");
@@ -314,20 +314,7 @@ public class ObjcRuleClasses {
 
   private static final FileType NON_CPP_SOURCES = FileType.of(".m", ".c");
 
-  /**
-   * Header files, which are not compiled directly, but may be included/imported from source files.
-   */
-  static final FileType HEADERS = FileType.of(".h");
-
-  /**
-   * Files allowed in the srcs attribute. This includes private headers.
-   */
-  static final FileTypeSet SRCS_TYPE = FileTypeSet.of(NON_CPP_SOURCES, CPP_SOURCES, HEADERS);
-
-  /**
-   * Files that should actually be compiled.
-   */
-  static final FileTypeSet COMPILABLE_SRCS_TYPE = FileTypeSet.of(NON_CPP_SOURCES, CPP_SOURCES);
+  static final FileTypeSet SRCS_TYPE = FileTypeSet.of(NON_CPP_SOURCES, CPP_SOURCES);
 
   static final FileTypeSet NON_ARC_SRCS_TYPE = FileTypeSet.of(FileType.of(".m", ".mm"));
 
@@ -507,8 +494,8 @@ public class ObjcRuleClasses {
     public RuleClass build(Builder builder, RuleDefinitionEnvironment env) {
       return builder
           /* <!-- #BLAZE_RULE($objc_compile_dependency_rule).ATTRIBUTE(hdrs) -->
-          The list of C, C++, Objective-C, and Objective-C++ files that are
-          included as headers by source files in this rule or by users of this library.
+          The list of Objective-C files that are included as headers by source
+          files in this rule or by users of this library.
           ${SYNOPSIS}
           <!-- #END_BLAZE_RULE.ATTRIBUTE -->*/
           .add(attr("hdrs", LABEL_LIST)
@@ -564,13 +551,12 @@ public class ObjcRuleClasses {
     public RuleClass build(Builder builder, RuleDefinitionEnvironment env) {
       return builder
           /* <!-- #BLAZE_RULE($objc_compiling_rule).ATTRIBUTE(srcs) -->
-          The list of C, C++, Objective-C, and Objective-C++ source and header
-          files that are processed to create the library target.
+          The list of C, C++, Objective-C, and Objective-C++ files that are
+          processed to create the library target.
           ${SYNOPSIS}
-          These are your checked-in files, plus any generated files.
-          Source files are compiled into .o files with Clang. Header files
-          may be included/imported by any source or header in this target,
-          but not by any targets that depend on this rule.
+          These are your checked-in source files, plus any generated files.
+          These are compiled into .o files with Clang, so headers should not go
+          here (see the hdrs attribute).
           <!-- #END_BLAZE_RULE.ATTRIBUTE -->*/
           .add(attr("srcs", LABEL_LIST)
               .direct_compile_time_input()
diff --git a/src/main/java/com/google/devtools/build/lib/rules/objc/XcodeProvider.java b/src/main/java/com/google/devtools/build/lib/rules/objc/XcodeProvider.java
index b6941716a9..30176c75d4 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/objc/XcodeProvider.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/objc/XcodeProvider.java
@@ -531,7 +531,6 @@ public final class XcodeProvider implements TransitiveInfoProvider {
         .setName(label.getName())
         .setLabel(xcodeTargetName(label))
         .setProductType(productType.getIdentifier())
-        .addSupportFile(buildFilePath)
         .addAllImportedLibrary(Artifact.toExecPaths(objcProvider.get(IMPORTED_LIBRARY)))
         .addAllUserHeaderSearchPath(userHeaderSearchPaths)
         .addAllHeaderSearchPath(headerSearchPaths)
@@ -553,7 +552,8 @@ public final class XcodeProvider implements TransitiveInfoProvider {
         .addAllXcdatamodel(PathFragment.safePathStrings(datamodelDirs))
         .addAllBundleImport(PathFragment.safePathStrings(objcProvider.get(BUNDLE_IMPORT_DIR)))
         .addAllSdkDylib(objcProvider.get(SDK_DYLIB))
-        .addAllGeneralResourceFile(Artifact.toExecPaths(objcProvider.get(GENERAL_RESOURCE_FILE)));
+        .addAllGeneralResourceFile(Artifact.toExecPaths(objcProvider.get(GENERAL_RESOURCE_FILE)))
+        .addSupportFile(buildFilePath);
 
     if (CAN_LINK_PRODUCT_TYPES.contains(productType)) {
       for (XcodeProvider dependency : propagatedDependencies) {
@@ -596,8 +596,6 @@ public final class XcodeProvider implements TransitiveInfoProvider {
     for (CompilationArtifacts artifacts : compilationArtifacts.asSet()) {
       targetControl
           .addAllSourceFile(Artifact.toExecPaths(artifacts.getSrcs()))
-          .addAllSupportFile(Artifact.toExecPaths(artifacts.getAdditionalHdrs()))
-          .addAllSupportFile(Artifact.toExecPaths(artifacts.getPrivateHdrs()))
           .addAllNonArcSourceFile(Artifact.toExecPaths(artifacts.getNonArcSrcs()));
 
       for (Artifact pchFile : artifacts.getPchFile().asSet()) {