diff options
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()) { |