diff options
Diffstat (limited to 'src/main/java')
7 files changed, 122 insertions, 31 deletions
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 7d1377b5a5..5c8c5956af 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 @@ -27,8 +27,11 @@ import com.google.devtools.build.lib.actions.Artifact; */ final class CompilationArtifacts { static class Builder { + // TODO(bazel-team): Should these be sets instead of just iterables? 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; @@ -42,6 +45,25 @@ final class CompilationArtifacts { return this; } + /** + * Adds header artifacts that should be directly accessible to dependers, but aren't specified + * in the hdrs attribute. {@code additionalHdrs} should not be a {@link NestedSet}, as it will + * be flattened when added. + */ + 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. + * {@code privateHdrs} should not be a {@link NestedSet}, as it will be flattened when added. + */ + 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); @@ -61,23 +83,30 @@ final class CompilationArtifacts { if (!Iterables.isEmpty(srcs) || !Iterables.isEmpty(nonArcSrcs)) { archive = Optional.of(intermediateArtifacts.archive()); } - return new CompilationArtifacts(srcs, nonArcSrcs, archive, pchFile); + return new CompilationArtifacts( + srcs, nonArcSrcs, additionalHdrs, privateHdrs, 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 final boolean hasSwiftSources; 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); this.hasSwiftSources = Iterables.any(this.srcs, new Predicate<Artifact>() { @@ -96,6 +125,21 @@ final class CompilationArtifacts { return nonArcSrcs; } + /** + * Returns the public headers that aren't included in the hdrs attribute. + */ + public Iterable<Artifact> getAdditionalHdrs() { + return additionalHdrs; + } + + /** + * Returns the private headers from the srcs attribute, which may by imported by any source or + * header in this target, but not by sources or headers of dependers. + */ + 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 c76f35baa2..b26a702fa5 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 @@ -31,7 +31,9 @@ import static com.google.devtools.build.lib.rules.objc.ObjcProvider.SDK_FRAMEWOR 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; import static com.google.devtools.build.lib.rules.objc.ObjcRuleClasses.STRIP; @@ -44,8 +46,10 @@ import com.google.common.base.Preconditions; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableSet; 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; @@ -55,6 +59,7 @@ 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.TargetUtils; +import com.google.devtools.build.lib.packages.Type; import com.google.devtools.build.lib.rules.cpp.LinkerInputs; import com.google.devtools.build.lib.rules.java.J2ObjcConfiguration; import com.google.devtools.build.lib.rules.objc.ObjcCommon.CompilationAttributes; @@ -63,7 +68,9 @@ 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, @@ -72,7 +79,7 @@ import java.util.List; * * <p>Methods on this class can be called in any order without impacting the result. */ -final class CompilationSupport { +public final class CompilationSupport { @VisibleForTesting static final String ABSOLUTE_INCLUDES_PATH_FORMAT = @@ -98,18 +105,23 @@ final class CompilationSupport { } } + @VisibleForTesting + static final String FILE_IN_SRCS_AND_HDRS_WARNING_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(ruleContext.getPrerequisiteArtifacts("srcs", Mode.TARGET) - .errorsForNonMatching(SRCS_TYPE) - .list()) + .addSrcs(srcs.filter(COMPILABLE_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(); @@ -121,7 +133,7 @@ final class CompilationSupport { /** * Creates a new compilation support for the given rule. */ - CompilationSupport(RuleContext ruleContext) { + public CompilationSupport(RuleContext ruleContext) { this.ruleContext = ruleContext; this.attributes = new CompilationAttributes(ruleContext); } @@ -216,8 +228,7 @@ final class CompilationSupport { commandLine .add(IosSdkCommands.compileFlagsForClang(objcConfiguration)) - .add(IosSdkCommands.commonLinkAndCompileFlagsForClang( - objcProvider, objcConfiguration)) + .add(IosSdkCommands.commonLinkAndCompileFlagsForClang(objcProvider, objcConfiguration)) .add(objcConfiguration.getCoptsForCompilationMode()) .addBeforeEachPath( "-iquote", ObjcCommon.userHeaderSearchPaths(ruleContext.getConfiguration())) @@ -232,6 +243,7 @@ final class CompilationSupport { .addExecPath("-c", sourceFile) .addExecPath("-o", objFile); + // TODO(bazel-team): Remote private headers from inputs once they're added to the provider. ruleContext.registerAction(ObjcRuleClasses.spawnOnDarwinActionBuilder() .setMnemonic("ObjcCompile") .setExecutable(CLANG) @@ -241,6 +253,7 @@ final class CompilationSupport { .addOutput(objFile) .addOutputs(gcnoFiles.build()) .addTransitiveInputs(objcProvider.get(HEADER)) + .addInputs(compilationArtifacts.getPrivateHdrs()) .addTransitiveInputs(objcProvider.get(FRAMEWORK_FILE)) .addInputs(compilationArtifacts.getPchFile().asSet()) .build(ruleContext)); @@ -732,6 +745,17 @@ 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.attributeWarning( + "srcs", String.format(FILE_IN_SRCS_AND_HDRS_WARNING_FORMAT, path)); + } + } return this; } 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 030164eaf4..3a2ce46dd0 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 @@ -93,6 +93,10 @@ 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)); } @@ -241,7 +245,6 @@ 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 Iterable<Artifact> extraImportLibraries = ImmutableList.of(); @@ -326,11 +329,6 @@ 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; @@ -397,7 +395,6 @@ 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); @@ -453,8 +450,12 @@ public final class ObjcCommon { for (CompilationArtifacts artifacts : compilationArtifacts.asSet()) { Iterable<Artifact> allSources = Iterables.concat(artifacts.getSrcs(), artifacts.getNonArcSrcs()); - objcProvider.addAll(LIBRARY, artifacts.getArchive().asSet()); - objcProvider.addAll(SOURCE, allSources); + // TODO(bazel-team): Add private headers to the provider when we have module maps to enforce + // them. + objcProvider + .addAll(HEADER, artifacts.getAdditionalHdrs()) + .addAll(LIBRARY, artifacts.getArchive().asSet()) + .addAll(SOURCE, allSources); BuildConfiguration configuration = context.getConfiguration(); RegexFilter filter = configuration.getInstrumentationFilter(); if (configuration.isCodeCoverageEnabled() 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 2510fcb80b..ce1709a5ab 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 @@ -153,6 +153,8 @@ public class ObjcProtoLibrary implements RuleConfiguredTargetFactory { .addNonArcSrcs(protoGeneratedSources) .setIntermediateArtifacts(intermediateArtifacts) .setPchFile(Optional.<Artifact>absent()) + .addAdditionalHdrs(protoGeneratedHeaders) + .addAdditionalHdrs(protoGeneratedSources) .build(); ImmutableSet.Builder<PathFragment> searchPathEntriesBuilder = @@ -175,8 +177,6 @@ public class ObjcProtoLibrary implements RuleConfiguredTargetFactory { .addDepObjcProviders(ruleContext.getPrerequisites( ObjcProtoLibraryRule.LIBPROTOBUF_ATTR, Mode.TARGET, ObjcProvider.class)) .setIntermediateArtifacts(intermediateArtifacts) - .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 413ed2d5b2..24f33e479c 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 @@ -76,6 +76,9 @@ 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); /** @@ -90,7 +93,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); @@ -334,7 +337,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 themself. + * which add this provider to themselves. */ public Builder addTransitiveWithoutPropagating(Iterable<ObjcProvider> providers) { for (ObjcProvider provider : providers) { 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 ad32d2e36b..ec3f27950b 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 @@ -317,10 +317,24 @@ public class ObjcRuleClasses { private static final FileType NON_CPP_SOURCES = FileType.of(".m", ".c", ".s", ".asm"); static final FileType PREPROCESSED_ASSEMBLY_SOURCES = FileType.of(".S"); - + static final FileType SWIFT_SOURCES = FileType.of(".swift"); + /** + * Header files, which are not compiled directly, but may be included/imported from source files. + */ + static final FileType HEADERS = FileType.of(".h", ".inc"); + + /** + * Files allowed in the srcs attribute. This includes private headers. + */ static final FileTypeSet SRCS_TYPE = FileTypeSet.of(NON_CPP_SOURCES, CPP_SOURCES, + PREPROCESSED_ASSEMBLY_SOURCES, SWIFT_SOURCES, HEADERS); + + /** + * Files that should actually be compiled. + */ + static final FileTypeSet COMPILABLE_SRCS_TYPE = FileTypeSet.of(NON_CPP_SOURCES, CPP_SOURCES, PREPROCESSED_ASSEMBLY_SOURCES, SWIFT_SOURCES); static final FileTypeSet NON_ARC_SRCS_TYPE = FileTypeSet.of(FileType.of(".m", ".mm")); @@ -514,8 +528,9 @@ public class ObjcRuleClasses { public RuleClass build(Builder builder, RuleDefinitionEnvironment env) { return builder /* <!-- #BLAZE_RULE($objc_compile_dependency_rule).ATTRIBUTE(hdrs) --> - The list of Objective-C files that are included as headers by source - files in this rule or by users of this library. + 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. ${SYNOPSIS} <!-- #END_BLAZE_RULE.ATTRIBUTE -->*/ .add(attr("hdrs", LABEL_LIST) @@ -579,12 +594,14 @@ 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++ files that are - processed to create the library target. + The list of C, C++, Objective-C, and Objective-C++ source and header + files that are processed to create the library target. ${SYNOPSIS} - 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). + 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 the srcs attribute + of this target, but not by headers in hdrs or any targets that depend + on this rule. <!-- #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 96a124c290..3e77023af2 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 @@ -526,6 +526,7 @@ 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) @@ -552,8 +553,7 @@ public final class XcodeProvider implements TransitiveInfoProvider { .addAllGeneralResourceFile( Artifact.toExecPaths(objcProvider.get(GENERAL_RESOURCE_FILE))) .addAllGeneralResourceFile( - PathFragment.safePathStrings(objcProvider.get(GENERAL_RESOURCE_DIR))) - .addSupportFile(buildFilePath); + PathFragment.safePathStrings(objcProvider.get(GENERAL_RESOURCE_DIR))); if (CAN_LINK_PRODUCT_TYPES.contains(productType)) { // For builds with --ios_multi_cpus set, we may have several copies of some XCodeProviders @@ -605,6 +605,8 @@ 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()) { |