From 786fc5728ec9352b9ccde25d842ed07132888975 Mon Sep 17 00:00:00 2001 From: Cal Peyser Date: Thu, 15 Sep 2016 16:35:06 +0000 Subject: Rollback of commit a85bf4b19c680a6db11f21758847dc88ec0aa658. *** Reason for rollback *** Breaks Bigtop incremental build *** Original change description *** Implement input pruning using .d files in objc. -- MOS_MIGRATED_REVID=133271059 --- .../build/lib/rules/objc/CompilationSupport.java | 48 ++-- .../lib/rules/objc/IntermediateArtifacts.java | 9 +- .../build/lib/rules/objc/ObjcCompileAction.java | 304 --------------------- 3 files changed, 27 insertions(+), 334 deletions(-) delete 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/objc') 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 1e45cd5d52..3a5a370491 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,7 +83,6 @@ 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; @@ -656,21 +655,19 @@ 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(dotdFile.artifact()), - otherFlags, - runCodeCoverage, - isCPlusPlusSource, - hasSwiftSources); + CustomCommandLine commandLine = compileActionCommandLine( + sourceFile, + objFile, + objcProvider, + priorityHeaders, + moduleMap, + compilationArtifacts.getPchFile(), + Optional.of(intermediateArtifacts.dotdFile(sourceFile)), + otherFlags, + runCodeCoverage, + isCPlusPlusSource, + hasSwiftSources); Optional gcnoFile = Optional.absent(); if (runCodeCoverage && !buildConfiguration.isLLVMCoverageMapFormatEnabled()) { @@ -687,25 +684,24 @@ public final class CompilationSupport { moduleMapInputs = objcProvider.get(MODULE_MAP); } - // TODO(bazel-team): Remove private headers from inputs once they're added to the provider. + // TODO(bazel-team): Remote private headers from inputs once they're added to the provider. ruleContext.registerAction( - ObjcCompileAction.Builder.createObjcCompileActionBuilderWithAppleEnv( + ObjcRuleClasses.spawnAppleEnvActionBuilder( 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(dotdFile.artifact()) + .addOutput(intermediateArtifacts.dotdFile(sourceFile)) .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 811e698ff7..9b8ced2dfd 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,7 +20,6 @@ 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; @@ -398,9 +397,11 @@ public final class IntermediateArtifacts { return appendExtension("_runner.sh"); } - /** Dependency file that is generated when compiling the {@code source} artifact. */ - public DotdFile dotdFile(Artifact source) { - return new DotdFile(inUniqueObjsDir(source, ".d")); + /** + * Dependency file that is generated when compiling the {@code source} artifact. + */ + public Artifact dotdFile(Artifact source) { + return 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 deleted file mode 100644 index 459ca157cb..0000000000 --- a/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcCompileAction.java +++ /dev/null @@ -1,304 +0,0 @@ -// 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