From a85bf4b19c680a6db11f21758847dc88ec0aa658 Mon Sep 17 00:00:00 2001 From: Cal Peyser Date: Thu, 8 Sep 2016 13:47:10 +0000 Subject: Implement input pruning using .d files in objc. -- MOS_MIGRATED_REVID=132550233 --- .../build/lib/rules/cpp/CppCompileAction.java | 221 ++++++--------- .../lib/rules/cpp/CppCompileActionBuilder.java | 1 - .../devtools/build/lib/rules/cpp/CppModel.java | 2 +- .../build/lib/rules/cpp/FakeCppCompileAction.java | 34 ++- .../build/lib/rules/cpp/HeaderDiscovery.java | 216 +++++++++++++++ .../build/lib/rules/objc/CompilationSupport.java | 48 ++-- .../lib/rules/objc/IntermediateArtifacts.java | 9 +- .../build/lib/rules/objc/ObjcCompileAction.java | 304 +++++++++++++++++++++ 8 files changed, 667 insertions(+), 168 deletions(-) create mode 100644 src/main/java/com/google/devtools/build/lib/rules/cpp/HeaderDiscovery.java create mode 100644 src/main/java/com/google/devtools/build/lib/rules/objc/ObjcCompileAction.java (limited to 'src/main/java/com/google/devtools/build/lib/rules') diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppCompileAction.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppCompileAction.java index 0ad000e569..d257656e8d 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppCompileAction.java +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppCompileAction.java @@ -164,7 +164,6 @@ public class CppCompileAction extends AbstractAction private final Artifact optionalSourceFile; private final NestedSet mandatoryInputs; private final boolean shouldScanIncludes; - private final boolean shouldPruneModules; private final boolean usePic; private final CppCompilationContext context; private final Iterable lipoScannables; @@ -176,8 +175,8 @@ public class CppCompileAction extends AbstractAction @VisibleForTesting final CppConfiguration cppConfiguration; private final FeatureConfiguration featureConfiguration; protected final Class actionContext; - private final SpecialInputsHandler specialInputsHandler; - private final CppSemantics semantics; + protected final SpecialInputsHandler specialInputsHandler; + protected final CppSemantics cppSemantics; /** * Identifier for the actual execution time behavior of the action. @@ -204,29 +203,32 @@ public class CppCompileAction extends AbstractAction /** * Creates a new action to compile C/C++ source files. * - * @param owner the owner of the action, usually the configured target that emitted it - * @param sourceFile the source file that should be compiled. {@code mandatoryInputs} must contain - * this file - * @param shouldScanIncludes a boolean indicating whether scanning of {@code sourceFile} is to be - * performed looking for inclusions. + * @param owner the owner of the action, usually the configured target that + * emitted it + * @param sourceFile the source file that should be compiled. {@code mandatoryInputs} must + * contain this file + * @param shouldScanIncludes a boolean indicating whether scanning of {@code sourceFile} + * is to be performed looking for inclusions. * @param sourceLabel the label of the rule the source file is generated by - * @param mandatoryInputs any additional files that need to be present for the compilation to - * succeed, can be empty but not null, for example, extra sources for FDO. - * @param outputFile the object file that is written as result of the compilation, or the fake - * object for {@link FakeCppCompileAction}s - * @param dotdFile the .d file that is generated as a side-effect of compilation - * @param gcnoFile the coverage notes that are written in coverage mode, can be null - * @param dwoFile the .dwo output file where debug information is stored for Fission builds (null - * if Fission mode is disabled) + * @param mandatoryInputs any additional files that need to be present for the + * compilation to succeed, can be empty but not null, for example, extra sources for FDO. + * @param outputFile the object file that is written as result of the + * compilation, or the fake object for {@link FakeCppCompileAction}s + * @param dotdFile the .d file that is generated as a side-effect of + * compilation + * @param gcnoFile the coverage notes that are written in coverage mode, can + * be null + * @param dwoFile the .dwo output file where debug information is stored for Fission + * builds (null if Fission mode is disabled) * @param optionalSourceFile an additional optional source file (null if unneeded) * @param configuration the build configurations * @param context the compilation context * @param copts options for the compiler * @param coptsFilter regular expression to remove options from {@code copts} * @param executionRequirements out-of-band hints to be passed to the execution backend to signal - * platform requirements + * platform requirements * @param actionName a string giving the name of this action for the purpose of toolchain - * evaluation + * evaluation */ protected CppCompileAction( ActionOwner owner, @@ -237,7 +239,6 @@ public class CppCompileAction extends AbstractAction CcToolchainFeatures.Variables variables, Artifact sourceFile, boolean shouldScanIncludes, - boolean shouldPruneModules, boolean usePic, Label sourceLabel, NestedSet mandatoryInputs, @@ -259,7 +260,7 @@ public class CppCompileAction extends AbstractAction ImmutableMap environment, String actionName, RuleContext ruleContext, - CppSemantics semantics) { + CppSemantics cppSemantics) { super( owner, createInputs( @@ -283,7 +284,6 @@ public class CppCompileAction extends AbstractAction // known after inclusion scanning. When *not* scanning includes, // the inputs are as declared, hence known, and remain so. this.shouldScanIncludes = shouldScanIncludes; - this.shouldPruneModules = shouldPruneModules; this.usePic = usePic; this.inputsKnown = !shouldScanIncludes; this.cppCompileCommandLine = @@ -299,8 +299,8 @@ public class CppCompileAction extends AbstractAction // artifact and will definitely exist prior to this action execution. this.mandatoryInputs = mandatoryInputs; this.builtinIncludeFiles = CppHelper.getToolchain(ruleContext).getBuiltinIncludeFiles(); - this.semantics = semantics; - if (semantics.needsIncludeValidation()) { + this.cppSemantics = cppSemantics; + if (cppSemantics.needsIncludeValidation()) { verifyIncludePaths(ruleContext); } } @@ -445,7 +445,7 @@ public class CppCompileAction extends AbstractAction return null; } - if (shouldPruneModules) { + if (featureConfiguration.isEnabled(CppRuleClasses.PRUNE_HEADER_MODULES)) { Set initialResultSet = Sets.newLinkedHashSet(initialResult); List usedModulePaths = Lists.newArrayList(); for (Artifact usedModule : context.getUsedModules(usePic, initialResultSet)) { @@ -911,105 +911,6 @@ public class CppCompileAction extends AbstractAction } } - private DependencySet processDepset(Path execRoot, CppCompileActionContext.Reply reply) - throws IOException { - DotdFile dotdFile = getDotdFile(); - Preconditions.checkNotNull(dotdFile); - DependencySet depSet = new DependencySet(execRoot); - // artifact() is null if we are using in-memory .d files. We also want to prepare for the - // case where we expected an in-memory .d file, but we did not get an appropriate response. - // Perhaps we produced the file locally. - if (dotdFile.artifact() != null || reply == null) { - return depSet.read(dotdFile.getPath()); - } else { - // This is an in-memory .d file. - return depSet.process(reply.getContents()); - } - } - - /** - * Returns a collection with additional input artifacts relevant to the action by reading the - * dynamically-discovered dependency information from the .d file after the action has run. - * - *

Artifacts are considered inputs but not "mandatory" inputs. - * - * @param reply the reply from the compilation. - * @throws ActionExecutionException iff the .d is missing (when required), malformed, or has - * unresolvable included artifacts. - */ - @VisibleForTesting - @ThreadCompatible - public NestedSet discoverInputsFromDotdFiles( - Path execRoot, ArtifactResolver artifactResolver, Reply reply) - throws ActionExecutionException { - NestedSetBuilder inputs = NestedSetBuilder.stableOrder(); - if (getDotdFile() == null) { - return inputs.build(); - } - try { - // Read .d file. - DependencySet depSet = processDepset(execRoot, reply); - - // Determine prefixes of allowed absolute inclusions. - CppConfiguration toolchain = cppConfiguration; - List systemIncludePrefixes = new ArrayList<>(); - for (PathFragment includePath : toolchain.getBuiltInIncludeDirectories()) { - if (includePath.isAbsolute()) { - systemIncludePrefixes.add(execRoot.getFileSystem().getPath(includePath)); - } - } - - // Check inclusions. - IncludeProblems problems = new IncludeProblems(); - Map allowedDerivedInputsMap = getAllowedDerivedInputsMap(); - for (Path execPath : depSet.getDependencies()) { - // Module .pcm files are generated and thus aren't declared inputs. - if (execPath.getBaseName().endsWith(".pcm")) { - continue; - } - PathFragment execPathFragment = execPath.asFragment(); - if (execPathFragment.isAbsolute()) { - // Absolute includes from system paths are ignored. - if (FileSystemUtils.startsWithAny(execPath, systemIncludePrefixes)) { - continue; - } - // Since gcc is given only relative paths on the command line, - // non-system include paths here should never be absolute. If they - // are, it's probably due to a non-hermetic #include, & we should stop - // the build with an error. - if (execPath.startsWith(execRoot)) { - execPathFragment = execPath.relativeTo(execRoot); // funky but tolerable path - } else { - problems.add(execPathFragment.getPathString()); - continue; - } - } - Artifact artifact = allowedDerivedInputsMap.get(execPathFragment); - if (artifact == null) { - artifact = artifactResolver.resolveSourceArtifact(execPathFragment); - } - if (artifact != null) { - inputs.add(artifact); - // In some cases, execution backends need extra files for each included file. Add them - // to the set of actual inputs. - inputs.addAll(specialInputsHandler.getInputsForIncludedFile(artifact, artifactResolver)); - } else { - // Abort if we see files that we can't resolve, likely caused by - // undeclared includes or illegal include constructs. - problems.add(execPathFragment.getPathString()); - } - } - //TODO(b/22551695): Remove in favor of seperate implementations. - if (semantics == null || semantics.needsIncludeValidation()) { - problems.assertProblemFree(this, getSourceFile()); - } - } catch (IOException e) { - // Some kind of IO or parse exception--wrap & rethrow it to stop the build. - throw new ActionExecutionException("error while parsing .d file", e, this, false); - } - return inputs.build(); - } - @Override public Iterable resolveInputsFromCache( ArtifactResolver artifactResolver, @@ -1061,7 +962,7 @@ public class CppCompileAction extends AbstractAction } } - private Map getAllowedDerivedInputsMap() { + protected Map getAllowedDerivedInputsMap() { Map allowedDerivedInputMap = new HashMap<>(); addToMap(allowedDerivedInputMap, mandatoryInputs); addToMap(allowedDerivedInputMap, getDeclaredIncludeSrcs()); @@ -1184,9 +1085,10 @@ public class CppCompileAction extends AbstractAction // This is the .d file scanning part. IncludeScanningContext scanningContext = executor.getContext(IncludeScanningContext.class); + Path execRoot = executor.getExecRoot(); + NestedSet discoveredInputs = - discoverInputsFromDotdFiles( - executor.getExecRoot(), scanningContext.getArtifactResolver(), reply); + discoverInputsFromDotdFiles(execRoot, scanningContext.getArtifactResolver(), reply); reply = null; // Clear in-memory .d files early. // Post-execute "include scanning", which modifies the action inputs to match what the compile @@ -1204,14 +1106,67 @@ public class CppCompileAction extends AbstractAction updateActionInputs(discoveredInputs); } - // hdrs_check: Turning this off opens the door to incorrect builds. However, we allow it - // to accommodate the current behavior in the objc rules. - if (semantics == null || semantics.needsIncludeValidation()) { - validateInclusions( - discoveredInputs, - actionExecutionContext.getArtifactExpander(), - executor.getEventHandler()); + // hdrs_check: This cannot be switched off, because doing so would allow for incorrect builds. + validateInclusions( + discoveredInputs, + actionExecutionContext.getArtifactExpander(), + executor.getEventHandler()); + } + + @VisibleForTesting + public NestedSet discoverInputsFromDotdFiles( + Path execRoot, ArtifactResolver artifactResolver, Reply reply) + throws ActionExecutionException { + if (getDotdFile() == null) { + return NestedSetBuilder.stableOrder().build(); + } + HeaderDiscovery.Builder discoveryBuilder = new HeaderDiscovery.Builder() + .setAction(this) + .setDotdFile(getDotdFile()) + .setSourceFile(getSourceFile()) + .setSpecialInputsHandler(specialInputsHandler) + .setDependencySet(processDepset(execRoot, reply)) + .setPermittedSystemIncludePrefixes(getPermittedSystemIncludePrefixes(execRoot)) + .setAllowedDerivedinputsMap(getAllowedDerivedInputsMap()); + + if (cppSemantics.needsIncludeValidation()) { + discoveryBuilder.shouldValidateInclusions(); + } + + return discoveryBuilder + .build() + .discoverInputsFromDotdFiles(execRoot, artifactResolver); + } + + public DependencySet processDepset(Path execRoot, Reply reply) throws ActionExecutionException { + try { + DotdFile dotdFile = getDotdFile(); + Preconditions.checkNotNull(dotdFile); + DependencySet depSet = new DependencySet(execRoot); + // artifact() is null if we are using in-memory .d files. We also want to prepare for the + // case where we expected an in-memory .d file, but we did not get an appropriate response. + // Perhaps we produced the file locally. + if (dotdFile.artifact() != null || reply == null) { + return depSet.read(dotdFile.getPath()); + } else { + // This is an in-memory .d file. + return depSet.process(reply.getContents()); + } + } catch (IOException e) { + // Some kind of IO or parse exception--wrap & rethrow it to stop the build. + throw new ActionExecutionException("error while parsing .d file", e, this, false); + } + } + + public List getPermittedSystemIncludePrefixes(Path execRoot) { + CppConfiguration toolchain = cppConfiguration; + List systemIncludePrefixes = new ArrayList<>(); + for (PathFragment includePath : toolchain.getBuiltInIncludeDirectories()) { + if (includePath.isAbsolute()) { + systemIncludePrefixes.add(execRoot.getFileSystem().getPath(includePath)); + } } + return systemIncludePrefixes; } /** diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppCompileActionBuilder.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppCompileActionBuilder.java index 00a26ce192..3b146269a0 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppCompileActionBuilder.java +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppCompileActionBuilder.java @@ -318,7 +318,6 @@ public class CppCompileActionBuilder { sourceFile, shouldScanIncludes, shouldPruneModules, - usePic, sourceLabel, realMandatoryInputs, outputFile, diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppModel.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppModel.java index 2386724ba3..9eae435422 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppModel.java +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppModel.java @@ -495,7 +495,7 @@ public final class CppModel { initializeCompileAction(moduleMapArtifact, moduleMapLabel, /*forInterface=*/ true); builder.setSemantics(semantics); - + // A header module compile action is just like a normal compile action, but: // - the compiled source file is the module map // - it creates a header module (.pcm file). diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/FakeCppCompileAction.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/FakeCppCompileAction.java index 631e697331..97fe0825fe 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/FakeCppCompileAction.java +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/FakeCppCompileAction.java @@ -34,6 +34,7 @@ import com.google.devtools.build.lib.analysis.RuleContext; import com.google.devtools.build.lib.analysis.config.BuildConfiguration; import com.google.devtools.build.lib.cmdline.Label; import com.google.devtools.build.lib.collect.nestedset.NestedSet; +import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder; import com.google.devtools.build.lib.concurrent.ThreadSafety.ThreadCompatible; import com.google.devtools.build.lib.events.Event; import com.google.devtools.build.lib.rules.cpp.CcToolchainFeatures.FeatureConfiguration; @@ -41,6 +42,7 @@ import com.google.devtools.build.lib.util.Preconditions; import com.google.devtools.build.lib.util.ShellEscaper; import com.google.devtools.build.lib.util.io.FileOutErr; import com.google.devtools.build.lib.vfs.FileSystemUtils; +import com.google.devtools.build.lib.vfs.Path; import com.google.devtools.build.lib.vfs.PathFragment; import java.io.IOException; import java.util.UUID; @@ -79,7 +81,7 @@ public class FakeCppCompileAction extends CppCompileAction { ImmutableList copts, Predicate nocopts, RuleContext ruleContext, - CppSemantics semantics) { + CppSemantics cppSemantics) { super( owner, features, @@ -88,7 +90,6 @@ public class FakeCppCompileAction extends CppCompileAction { sourceFile, shouldScanIncludes, shouldPruneModules, - usePic, sourceLabel, mandatoryInputs, outputFile, @@ -116,7 +117,7 @@ public class FakeCppCompileAction extends CppCompileAction { ImmutableMap.of(), CppCompileAction.CPP_COMPILE, ruleContext, - semantics); + cppSemantics); this.tempOutputFile = Preconditions.checkNotNull(tempOutputFile); } @@ -147,9 +148,30 @@ public class FakeCppCompileAction extends CppCompileAction { } } IncludeScanningContext scanningContext = executor.getContext(IncludeScanningContext.class); - NestedSet discoveredInputs = - discoverInputsFromDotdFiles( - executor.getExecRoot(), scanningContext.getArtifactResolver(), reply); + Path execRoot = executor.getExecRoot(); + + NestedSet discoveredInputs; + if (getDotdFile() == null) { + discoveredInputs = NestedSetBuilder.stableOrder().build(); + } else { + HeaderDiscovery.Builder discoveryBuilder = new HeaderDiscovery.Builder() + .setAction(this) + .setDotdFile(getDotdFile()) + .setSourceFile(getSourceFile()) + .setSpecialInputsHandler(specialInputsHandler) + .setDependencySet(processDepset(execRoot, reply)) + .setPermittedSystemIncludePrefixes(getPermittedSystemIncludePrefixes(execRoot)) + .setAllowedDerivedinputsMap(getAllowedDerivedInputsMap()); + + if (cppSemantics.needsIncludeValidation()) { + discoveryBuilder.shouldValidateInclusions(); + } + + discoveredInputs = discoveryBuilder + .build() + .discoverInputsFromDotdFiles(execRoot, scanningContext.getArtifactResolver()); + } + reply = null; // Clear in-memory .d files early. // Even cc_fake_binary rules need to properly declare their dependencies... diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/HeaderDiscovery.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/HeaderDiscovery.java new file mode 100644 index 0000000000..96c7b816b5 --- /dev/null +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/HeaderDiscovery.java @@ -0,0 +1,216 @@ +// Copyright 2016 The Bazel Authors. All rights reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package com.google.devtools.build.lib.rules.cpp; + +import com.google.common.annotations.VisibleForTesting; +import com.google.common.base.Preconditions; +import com.google.devtools.build.lib.actions.Action; +import com.google.devtools.build.lib.actions.ActionExecutionException; +import com.google.devtools.build.lib.actions.Artifact; +import com.google.devtools.build.lib.actions.ArtifactResolver; +import com.google.devtools.build.lib.collect.nestedset.NestedSet; +import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder; +import com.google.devtools.build.lib.concurrent.ThreadSafety.ThreadCompatible; +import com.google.devtools.build.lib.rules.cpp.CppCompileAction.DotdFile; +import com.google.devtools.build.lib.rules.cpp.CppCompileAction.SpecialInputsHandler; +import com.google.devtools.build.lib.util.DependencySet; +import com.google.devtools.build.lib.vfs.FileSystemUtils; +import com.google.devtools.build.lib.vfs.Path; +import com.google.devtools.build.lib.vfs.PathFragment; +import java.util.List; +import java.util.Map; + +/** Manages the process of obtaining inputs used in a compilation from .d files. */ +public class HeaderDiscovery { + + private final Action action; + private final Artifact sourceFile; + private final DotdFile dotdFile; + + private final SpecialInputsHandler specialInputsHandler; + private final boolean shouldValidateInclusions; + + private final DependencySet depSet; + private final List permittedSystemIncludePrefixes; + private final Map allowedDerivedInputsMap; + + /** + * Creates a HeaderDiscover instance + * + * @param action the action instance requiring header discovery + * @param sourceFile the source file for the compile + * @param dotdFile the .d file used for header discovery + * @param specialInputsHandler the SpecialInputsHandler for the build + * @param shouldValidateInclusions true if include validation should be performed + */ + public HeaderDiscovery( + Action action, + Artifact sourceFile, + DotdFile dotdFile, + SpecialInputsHandler specialInputsHandler, + boolean shouldValidateInclusions, + DependencySet depSet, + List permittedSystemIncludePrefixes, + Map allowedDerivedInputsMap) { + this.action = Preconditions.checkNotNull(action); + this.sourceFile = Preconditions.checkNotNull(sourceFile); + this.dotdFile = Preconditions.checkNotNull(dotdFile); + this.specialInputsHandler = specialInputsHandler; + this.shouldValidateInclusions = shouldValidateInclusions; + this.depSet = depSet; + this.permittedSystemIncludePrefixes = permittedSystemIncludePrefixes; + this.allowedDerivedInputsMap = allowedDerivedInputsMap; + } + + /** + * Returns a collection with additional input artifacts relevant to the action by reading the + * dynamically-discovered dependency information from the .d file after the action has run. + * + *

Artifacts are considered inputs but not "mandatory" inputs. + * + * @throws ActionExecutionException iff the .d is missing (when required), malformed, or has + * unresolvable included artifacts. + */ + @VisibleForTesting + @ThreadCompatible + public NestedSet discoverInputsFromDotdFiles( + Path execRoot, ArtifactResolver artifactResolver) throws ActionExecutionException { + NestedSetBuilder inputs = NestedSetBuilder.stableOrder(); + if (dotdFile == null) { + return inputs.build(); + } + List systemIncludePrefixes = permittedSystemIncludePrefixes; + + // Check inclusions. + IncludeProblems problems = new IncludeProblems(); + for (Path execPath : depSet.getDependencies()) { + // Module .pcm files are generated and thus aren't declared inputs. + if (execPath.getBaseName().endsWith(".pcm")) { + continue; + } + PathFragment execPathFragment = execPath.asFragment(); + if (execPathFragment.isAbsolute()) { + // Absolute includes from system paths are ignored. + if (FileSystemUtils.startsWithAny(execPath, systemIncludePrefixes)) { + continue; + } + // Since gcc is given only relative paths on the command line, + // non-system include paths here should never be absolute. If they + // are, it's probably due to a non-hermetic #include, & we should stop + // the build with an error. + if (execPath.startsWith(execRoot)) { + execPathFragment = execPath.relativeTo(execRoot); // funky but tolerable path + } else { + problems.add(execPathFragment.getPathString()); + continue; + } + } + Artifact artifact = allowedDerivedInputsMap.get(execPathFragment); + if (artifact == null) { + artifact = artifactResolver.resolveSourceArtifact(execPathFragment); + } + if (artifact != null) { + inputs.add(artifact); + // In some cases, execution backends need extra files for each included file. Add them + // to the set of actual inputs. + if (specialInputsHandler != null) { + inputs.addAll(specialInputsHandler.getInputsForIncludedFile(artifact, artifactResolver)); + } + } else { + // Abort if we see files that we can't resolve, likely caused by + // undeclared includes or illegal include constructs. + problems.add(execPathFragment.getPathString()); + } + } + if (shouldValidateInclusions) { + problems.assertProblemFree(action, sourceFile); + } + return inputs.build(); + } + + /** A Builder for HeaderDiscovery */ + public static class Builder { + private Action action; + private Artifact sourceFile; + private DotdFile dotdFile; + private SpecialInputsHandler specialInputsHandler; + private boolean shouldValidateInclusions = false; + + private DependencySet depSet; + private List permittedSystemIncludePrefixes; + private Map allowedDerivedInputsMap; + + /** Sets the action for which to discover inputs. */ + public Builder setAction(Action action) { + this.action = action; + return this; + } + + /** Sets the source file for which to discover inputs. */ + public Builder setSourceFile(Artifact sourceFile) { + this.sourceFile = sourceFile; + return this; + } + + /** Sets the dotd file to be used to discover inputs. */ + public Builder setDotdFile(DotdFile dotdFile) { + this.dotdFile = dotdFile; + return this; + } + + /** Sets the SpecialInputsHandler for inputs to this build. */ + public Builder setSpecialInputsHandler(SpecialInputsHandler specialInputsHandler) { + this.specialInputsHandler = specialInputsHandler; + return this; + } + + /** Sets that this compile should validate inclusions against the dotd file. */ + public Builder shouldValidateInclusions() { + this.shouldValidateInclusions = true; + return this; + } + + /** Sets the DependencySet capturing used headers by this compile. */ + public Builder setDependencySet(DependencySet depSet) { + this.depSet = depSet; + return this; + } + + /** Sets prefixes of allowed absolute inclusions */ + public Builder setPermittedSystemIncludePrefixes(List systemIncludePrefixes) { + this.permittedSystemIncludePrefixes = systemIncludePrefixes; + return this; + } + + /** Sets permitted inputs to the build */ + public Builder setAllowedDerivedinputsMap(Map allowedDerivedInputsMap) { + this.allowedDerivedInputsMap = allowedDerivedInputsMap; + return this; + } + + /** Creates a CppHeaderDiscovery instance. */ + public HeaderDiscovery build() { + return new HeaderDiscovery( + action, + sourceFile, + dotdFile, + specialInputsHandler, + shouldValidateInclusions, + depSet, + permittedSystemIncludePrefixes, + allowedDerivedInputsMap); + } + } +} 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 3a5a370491..1e45cd5d52 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 @@ -83,6 +83,7 @@ import com.google.devtools.build.lib.rules.apple.AppleConfiguration; import com.google.devtools.build.lib.rules.apple.AppleToolchain; import com.google.devtools.build.lib.rules.apple.Platform; import com.google.devtools.build.lib.rules.apple.Platform.PlatformType; +import com.google.devtools.build.lib.rules.cpp.CppCompileAction.DotdFile; 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.objc.XcodeProvider.Builder; @@ -655,19 +656,21 @@ public final class CompilationSupport { boolean runCodeCoverage = buildConfiguration.isCodeCoverageEnabled() && ObjcRuleClasses.isInstrumentable(sourceFile); boolean hasSwiftSources = compilationArtifacts.hasSwiftSources(); + DotdFile dotdFile = intermediateArtifacts.dotdFile(sourceFile); - CustomCommandLine commandLine = compileActionCommandLine( - sourceFile, - objFile, - objcProvider, - priorityHeaders, - moduleMap, - compilationArtifacts.getPchFile(), - Optional.of(intermediateArtifacts.dotdFile(sourceFile)), - otherFlags, - runCodeCoverage, - isCPlusPlusSource, - hasSwiftSources); + CustomCommandLine commandLine = + compileActionCommandLine( + sourceFile, + objFile, + objcProvider, + priorityHeaders, + moduleMap, + compilationArtifacts.getPchFile(), + Optional.of(dotdFile.artifact()), + otherFlags, + runCodeCoverage, + isCPlusPlusSource, + hasSwiftSources); Optional gcnoFile = Optional.absent(); if (runCodeCoverage && !buildConfiguration.isLLVMCoverageMapFormatEnabled()) { @@ -684,24 +687,25 @@ public final class CompilationSupport { moduleMapInputs = objcProvider.get(MODULE_MAP); } - // TODO(bazel-team): Remote private headers from inputs once they're added to the provider. + // TODO(bazel-team): Remove private headers from inputs once they're added to the provider. ruleContext.registerAction( - ObjcRuleClasses.spawnAppleEnvActionBuilder( + ObjcCompileAction.Builder.createObjcCompileActionBuilderWithAppleEnv( appleConfiguration, appleConfiguration.getSingleArchPlatform()) + .setSourceFile(sourceFile) + .addMandatoryInputs(swiftHeader.asSet()) + .addTransitiveMandatoryInputs(moduleMapInputs) + .addTransitiveMandatoryInputs(objcProvider.get(STATIC_FRAMEWORK_FILE)) + .addTransitiveMandatoryInputs(objcProvider.get(DYNAMIC_FRAMEWORK_FILE)) + .setDotdFile(dotdFile) + .addInputs(compilationArtifacts.getPrivateHdrs()) + .addInputs(compilationArtifacts.getPchFile().asSet()) .setMnemonic("ObjcCompile") .setExecutable(xcrunwrapper(ruleContext)) .setCommandLine(commandLine) - .addInput(sourceFile) - .addInputs(swiftHeader.asSet()) - .addTransitiveInputs(moduleMapInputs) .addOutput(objFile) .addOutputs(gcnoFile.asSet()) - .addOutput(intermediateArtifacts.dotdFile(sourceFile)) + .addOutput(dotdFile.artifact()) .addTransitiveInputs(objcProvider.get(HEADER)) - .addInputs(compilationArtifacts.getPrivateHdrs()) - .addTransitiveInputs(objcProvider.get(STATIC_FRAMEWORK_FILE)) - .addTransitiveInputs(objcProvider.get(DYNAMIC_FRAMEWORK_FILE)) - .addInputs(compilationArtifacts.getPchFile().asSet()) .build(ruleContext)); } 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 9b8ced2dfd..811e698ff7 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 @@ -20,6 +20,7 @@ import com.google.devtools.build.lib.analysis.RuleConfiguredTarget.Mode; import com.google.devtools.build.lib.analysis.RuleContext; import com.google.devtools.build.lib.analysis.config.BuildConfiguration; import com.google.devtools.build.lib.cmdline.Label; +import com.google.devtools.build.lib.rules.cpp.CppCompileAction.DotdFile; import com.google.devtools.build.lib.rules.cpp.CppModuleMap; import com.google.devtools.build.lib.util.Preconditions; import com.google.devtools.build.lib.vfs.FileSystemUtils; @@ -397,11 +398,9 @@ public final class IntermediateArtifacts { return appendExtension("_runner.sh"); } - /** - * Dependency file that is generated when compiling the {@code source} artifact. - */ - public Artifact dotdFile(Artifact source) { - return inUniqueObjsDir(source, ".d"); + /** Dependency file that is generated when compiling the {@code source} artifact. */ + public DotdFile dotdFile(Artifact source) { + return new DotdFile(inUniqueObjsDir(source, ".d")); } /** diff --git a/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcCompileAction.java b/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcCompileAction.java new file mode 100644 index 0000000000..459ca157cb --- /dev/null +++ b/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcCompileAction.java @@ -0,0 +1,304 @@ +// Copyright 2016 The Bazel Authors. All rights reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package com.google.devtools.build.lib.rules.objc; + +import static com.google.devtools.build.lib.collect.nestedset.Order.STABLE_ORDER; + +import com.google.common.annotations.VisibleForTesting; +import com.google.common.base.Preconditions; +import com.google.common.collect.ImmutableList; +import com.google.common.collect.ImmutableMap; +import com.google.common.collect.ImmutableSet; +import com.google.devtools.build.lib.actions.ActionExecutionContext; +import com.google.devtools.build.lib.actions.ActionExecutionException; +import com.google.devtools.build.lib.actions.ActionOwner; +import com.google.devtools.build.lib.actions.Artifact; +import com.google.devtools.build.lib.actions.ArtifactResolver; +import com.google.devtools.build.lib.actions.Executor; +import com.google.devtools.build.lib.actions.ResourceSet; +import com.google.devtools.build.lib.analysis.actions.CommandLine; +import com.google.devtools.build.lib.analysis.actions.SpawnAction; +import com.google.devtools.build.lib.collect.nestedset.NestedSet; +import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder; +import com.google.devtools.build.lib.concurrent.ThreadSafety.ThreadCompatible; +import com.google.devtools.build.lib.profiler.Profiler; +import com.google.devtools.build.lib.profiler.ProfilerTask; +import com.google.devtools.build.lib.rules.apple.AppleConfiguration; +import com.google.devtools.build.lib.rules.apple.Platform; +import com.google.devtools.build.lib.rules.cpp.CppCompileAction.DotdFile; +import com.google.devtools.build.lib.rules.cpp.HeaderDiscovery; +import com.google.devtools.build.lib.rules.cpp.IncludeScanningContext; +import com.google.devtools.build.lib.util.DependencySet; +import com.google.devtools.build.lib.util.Fingerprint; +import com.google.devtools.build.lib.vfs.Path; +import com.google.devtools.build.lib.vfs.PathFragment; +import java.io.IOException; +import java.util.HashMap; +import java.util.Map; + +/** + * An action that compiles objc or objc++ source. + * + *

We don't use a plain SpawnAction here because we implement .d input pruning, which requires + * post-execution filtering of input artifacts. + * + *

We don't use a CppCompileAction because the ObjcCompileAction uses custom logic instead of the + * CROSSTOOL to construct its command line. + */ +public class ObjcCompileAction extends SpawnAction { + + private final DotdFile dotdFile; + private final Artifact sourceFile; + private final NestedSet mandatoryInputs; + + + private static final String GUID = "a00d5bac-a72c-4f0f-99a7-d5fdc6072137"; + + private ObjcCompileAction( + ActionOwner owner, + Iterable tools, + Iterable inputs, + Iterable outputs, + ResourceSet resourceSet, + CommandLine argv, + ImmutableMap environment, + ImmutableMap executionInfo, + String progressMessage, + ImmutableMap inputManifests, + String mnemonic, + boolean executeUnconditionally, + ExtraActionInfoSupplier extraActionInfoSupplier, + DotdFile dotdFile, + Artifact sourceFile, + NestedSet mandatoryInputs) { + super( + owner, + tools, + inputs, + outputs, + resourceSet, + argv, + environment, + ImmutableSet.of(), + executionInfo, + progressMessage, + inputManifests, + mnemonic, + executeUnconditionally, + extraActionInfoSupplier); + + this.dotdFile = dotdFile; + this.sourceFile = sourceFile; + this.mandatoryInputs = mandatoryInputs; + } + + @Override + public boolean discoversInputs() { + return true; + } + + @Override + public Iterable discoverInputs(ActionExecutionContext actionExecutionContext) { + // We do not use include scanning for objc + return null; + } + + @Override + public void execute(ActionExecutionContext actionExecutionContext) + throws ActionExecutionException, InterruptedException { + super.execute(actionExecutionContext); + + Executor executor = actionExecutionContext.getExecutor(); + IncludeScanningContext scanningContext = executor.getContext(IncludeScanningContext.class); + NestedSet discoveredInputs = + discoverInputsFromDotdFiles(executor.getExecRoot(), scanningContext.getArtifactResolver()); + + updateActionInputs(discoveredInputs); + } + + @VisibleForTesting + public NestedSet discoverInputsFromDotdFiles( + Path execRoot, ArtifactResolver artifactResolver) throws ActionExecutionException { + if (dotdFile == null) { + return NestedSetBuilder.stableOrder().build(); + } + return new HeaderDiscovery.Builder() + .setAction(this) + .setSourceFile(sourceFile) + .setDotdFile(dotdFile) + .setDependencySet(processDepset(execRoot)) + .setPermittedSystemIncludePrefixes(ImmutableList.of()) + .setAllowedDerivedinputsMap(getAllowedDerivedInputsMap()) + .build() + .discoverInputsFromDotdFiles(execRoot, artifactResolver); + } + + private DependencySet processDepset(Path execRoot) throws ActionExecutionException { + try { + DependencySet depSet = new DependencySet(execRoot); + return depSet.read(dotdFile.getPath()); + } catch (IOException e) { + // Some kind of IO or parse exception--wrap & rethrow it to stop the build. + throw new ActionExecutionException("error while parsing .d file", e, this, false); + } + } + + /** Utility function that adds artifacts to an input map, but only if they are sources. */ + private void addToMapIfSource(Map map, Iterable artifacts) { + for (Artifact artifact : artifacts) { + if (!artifact.isSourceArtifact()) { + map.put(artifact.getExecPath(), artifact); + } + } + } + + private Map getAllowedDerivedInputsMap() { + Map allowedDerivedInputMap = new HashMap<>(); + addToMapIfSource(allowedDerivedInputMap, getInputs()); + allowedDerivedInputMap.put(sourceFile.getExecPath(), sourceFile); + return allowedDerivedInputMap; + } + + /** + * Recalculates this action's live input collection, including sources, middlemen. + * + * @throws ActionExecutionException iff any errors happen during update. + */ + @VisibleForTesting + @ThreadCompatible + public final synchronized void updateActionInputs(NestedSet discoveredInputs) + throws ActionExecutionException { + NestedSetBuilder inputs = NestedSetBuilder.stableOrder(); + Profiler.instance().startTask(ProfilerTask.ACTION_UPDATE, this); + try { + inputs.addTransitive(mandatoryInputs); + inputs.addTransitive(discoveredInputs); + } finally { + Profiler.instance().completeTask(ProfilerTask.ACTION_UPDATE); + setInputs(inputs.build()); + } + } + + @Override + public String computeKey() { + Fingerprint f = new Fingerprint(); + f.addString(GUID); + f.addString(super.computeKey()); + f.addBoolean(dotdFile.artifact() == null); + f.addPath(dotdFile.getSafeExecPath()); + return f.hexDigestAndReset(); + } + + /** A Builder for ObjcCompileAction */ + public static class Builder extends SpawnAction.Builder { + + private DotdFile dotdFile; + private Artifact sourceFile; + private final NestedSetBuilder mandatoryInputs = new NestedSetBuilder<>(STABLE_ORDER); + + /** + * Creates a new compile action builder with apple environment variables set that are typically + * needed by the apple toolchain. + */ + public static ObjcCompileAction.Builder createObjcCompileActionBuilderWithAppleEnv( + AppleConfiguration appleConfiguration, Platform targetPlatform) { + return (Builder) + new ObjcCompileAction.Builder() + .setExecutionInfo(ObjcRuleClasses.darwinActionExecutionRequirement()) + .setEnvironment( + ObjcRuleClasses.appleToolchainEnvironment(appleConfiguration, targetPlatform)); + } + + @Override + public Builder addTools(Iterable artifacts) { + super.addTools(artifacts); + mandatoryInputs.addAll(artifacts); + return this; + } + + /** Sets a .d file that will used to prune input headers */ + public Builder setDotdFile(DotdFile dotdFile) { + Preconditions.checkNotNull(dotdFile); + this.dotdFile = dotdFile; + return this; + } + + /** Sets the source file that is being compiled in this action */ + public Builder setSourceFile(Artifact sourceFile) { + Preconditions.checkNotNull(sourceFile); + this.sourceFile = sourceFile; + this.mandatoryInputs.add(sourceFile); + this.addInput(sourceFile); + return this; + } + + /** Add an input that cannot be pruned */ + public Builder addMandatoryInput(Artifact input) { + Preconditions.checkNotNull(input); + this.mandatoryInputs.add(input); + this.addInput(input); + return this; + } + + /** Add inputs that cannot be pruned */ + public Builder addMandatoryInputs(Iterable input) { + Preconditions.checkNotNull(input); + this.mandatoryInputs.addAll(input); + this.addInputs(input); + return this; + } + + /** Add inputs that cannot be pruned */ + public Builder addTransitiveMandatoryInputs(NestedSet input) { + Preconditions.checkNotNull(input); + this.mandatoryInputs.addTransitive(input); + this.addTransitiveInputs(input); + return this; + } + + @Override + protected SpawnAction createSpawnAction( + ActionOwner owner, + NestedSet tools, + NestedSet inputsAndTools, + ImmutableList outputs, + ResourceSet resourceSet, + CommandLine actualCommandLine, + ImmutableMap env, + ImmutableSet clientEnvironmentVariables, + ImmutableMap executionInfo, + String progressMessage, + ImmutableMap inputAndToolManifests, + String mnemonic) { + return new ObjcCompileAction( + owner, + tools, + inputsAndTools, + outputs, + resourceSet, + actualCommandLine, + env, + executionInfo, + progressMessage, + inputAndToolManifests, + mnemonic, + executeUnconditionally, + extraActionInfoSupplier, + dotdFile, + sourceFile, + mandatoryInputs.build()); + } + } +} -- cgit v1.2.3