From dec4e17a98477162ef42ab610b1e94b7cdaaf438 Mon Sep 17 00:00:00 2001 From: Googler Date: Thu, 9 Aug 2018 11:04:02 -0700 Subject: Stop generating .d files when they won't be looked at later. While at it remove CppCompilationActionBuilder.setAllowUsingHeaderModules, which isn't used anymore and would make the logic here (even) more complicated. RELNOTES: None. PiperOrigin-RevId: 208077525 --- .../build/lib/rules/cpp/CcCompilationHelper.java | 17 +++-------------- .../build/lib/rules/cpp/CppCompileActionBuilder.java | 16 ++++------------ .../build/lib/rules/cpp/FakeCppCompileAction.java | 11 ++++++++--- 3 files changed, 15 insertions(+), 29 deletions(-) (limited to 'src') diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcCompilationHelper.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcCompilationHelper.java index 27409c97e3..ea5ee63f04 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcCompilationHelper.java +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcCompilationHelper.java @@ -1458,8 +1458,7 @@ public final class CcCompilationHelper { userCompileFlags.addAll(collectPerFileCopts(sourceFile, sourceLabel)); } String dotdFileExecPath = null; - if (isGenerateDotdFile(builder.getSourceFile())) { - Preconditions.checkNotNull(builder.getDotdFile()); + if (builder.getDotdFile() != null) { dotdFileExecPath = builder.getDotdFile().getSafeExecPath().getPathString(); } ImmutableMap.Builder allAdditionalBuildVariables = ImmutableMap.builder(); @@ -1510,22 +1509,12 @@ public final class CcCompilationHelper { * initialized. */ private CppCompileActionBuilder initializeCompileAction(Artifact sourceArtifact) { - CppCompileActionBuilder builder = createCompileActionBuilder(sourceArtifact); - builder.setFeatureConfiguration(featureConfiguration); - - return builder; - } - - /** - * Creates a basic cpp compile action builder for source file. Configures options, crosstool - * inputs, output and dotd file names, {@code CcCompilationContext} and copts. - */ - private CppCompileActionBuilder createCompileActionBuilder(Artifact source) { CppCompileActionBuilder builder = new CppCompileActionBuilder(ruleContext, ccToolchain, configuration); - builder.setSourceFile(source); + builder.setSourceFile(sourceArtifact); builder.setCcCompilationContext(ccCompilationContext); builder.setCoptsFilter(coptsFilter); + builder.setFeatureConfiguration(featureConfiguration); return builder; } 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 f119f5de22..4a1f9fb529 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 @@ -66,7 +66,6 @@ public class CppCompileActionBuilder { private CoptsFilter coptsFilter = CoptsFilter.alwaysPasses(); private ImmutableList extraSystemIncludePrefixes = ImmutableList.of(); private boolean usePic; - private boolean allowUsingHeaderModules; private UUID actionClassId = GUID; private CppConfiguration cppConfiguration; private final ArrayList additionalIncludeScanningRoots; @@ -117,7 +116,6 @@ public class CppCompileActionBuilder { this.cppConfiguration = configuration.getFragment(CppConfiguration.class); this.mandatoryInputsBuilder = NestedSetBuilder.stableOrder(); this.additionalIncludeScanningRoots = new ArrayList<>(); - this.allowUsingHeaderModules = true; this.env = configuration.getActionEnvironment(); this.codeCoverageEnabled = configuration.isCodeCoverageEnabled(); this.ccToolchain = ccToolchain; @@ -151,7 +149,6 @@ public class CppCompileActionBuilder { this.cppConfiguration = other.cppConfiguration; this.configuration = other.configuration; this.usePic = other.usePic; - this.allowUsingHeaderModules = other.allowUsingHeaderModules; this.shouldScanIncludes = other.shouldScanIncludes; this.executionInfo = new LinkedHashMap<>(other.executionInfo); this.env = other.env; @@ -398,8 +395,9 @@ public class CppCompileActionBuilder { } private boolean useHeaderModules() { - return allowUsingHeaderModules - && featureConfiguration.isEnabled(CppRuleClasses.USE_HEADER_MODULES) + Preconditions.checkNotNull(featureConfiguration); + Preconditions.checkNotNull(sourceFile); + return featureConfiguration.isEnabled(CppRuleClasses.USE_HEADER_MODULES) && (sourceFile.isFileType(CppFileTypes.CPP_SOURCE) || sourceFile.isFileType(CppFileTypes.CPP_HEADER) || sourceFile.isFileType(CppFileTypes.CPP_MODULE_MAP)); @@ -517,7 +515,7 @@ public class CppCompileActionBuilder { ruleContext, CppHelper.getArtifactNameForCategory(ruleContext, ccToolchain, outputCategory, outputName), configuration); - if (generateDotd) { + if (generateDotd && !(cppConfiguration.getNoDotdScanningWithModules() && useHeaderModules())) { String dotdFileName = CppHelper.getDotdFileName(ruleContext, ccToolchain, outputCategory, outputName); if (cppConfiguration.getInmemoryDotdFiles()) { @@ -590,12 +588,6 @@ public class CppCompileActionBuilder { return this; } - /** Sets whether the CompileAction should use header modules. */ - public CppCompileActionBuilder setAllowUsingHeaderModules(boolean allowUsingHeaderModules) { - this.allowUsingHeaderModules = allowUsingHeaderModules; - return this; - } - /** Sets the CppSemantics for this compile. */ public CppCompileActionBuilder setSemantics(CppSemantics semantics) { this.cppSemantics = semantics; 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 f281e1bf7e..362ada1878 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 @@ -218,7 +218,8 @@ public class FakeCppCompileAction extends CppCompileAction { outputPrefix + ShellEscaper.escapeString(outputFile.getExecPathString()); } if (input.equals(outputFile.getExecPathString()) - || input.equals(getDotdFile().getSafeExecPath().getPathString())) { + || (getDotdFile() != null + && input.equals(getDotdFile().getSafeExecPath().getPathString()))) { result = outputPrefix + ShellEscaper.escapeString(input); } return result; @@ -231,8 +232,12 @@ public class FakeCppCompileAction extends CppCompileAction { try { // Ensure that the .d file and .o file are siblings, so that the "mkdir" below works for // both. - Preconditions.checkState(outputFile.getExecPath().getParentDirectory().equals( - getDotdFile().getSafeExecPath().getParentDirectory())); + Preconditions.checkState( + getDotdFile() == null + || outputFile + .getExecPath() + .getParentDirectory() + .equals(getDotdFile().getSafeExecPath().getParentDirectory())); FileSystemUtils.writeContent( actionExecutionContext.getInputPath(outputFile), ISO_8859_1, -- cgit v1.2.3