From c4fc6201fdfa71993e2e9e295a946150e6990c75 Mon Sep 17 00:00:00 2001 From: hlopko Date: Mon, 14 May 2018 05:31:56 -0700 Subject: [PATCH] Expose cc_common.create_compile_build_variables This cl enabled skylark rules to create build variables used for C++ compile actions (in a limited form, e.g. build variables for modules are not exposed etc). Working towards #4571. RELNOTES: None PiperOrigin-RevId: 196491567 --- .../lib/rules/cpp/CcCompilationHelper.java | 16 ++- .../lib/rules/cpp/CompileBuildVariables.java | 134 ++++++++++-------- .../build/lib/rules/cpp/CppActionConfigs.java | 1 + .../rules/cpp/CppLinkstampCompileHelper.java | 6 +- 4 files changed, 89 insertions(+), 68 deletions(-) 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 1d8c9528b3d329..11afab63327527 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 @@ -1521,16 +1521,16 @@ private Variables setupCompileBuildVariables( ruleContext, featureConfiguration, ccToolchain, - sourceFile, - builder.getOutputFile(), - gcnoFile, - dwoFile, - ltoIndexingFile, + toPathString(sourceFile), + toPathString(builder.getOutputFile()), + toPathString(gcnoFile), + toPathString(dwoFile), + toPathString(ltoIndexingFile), ImmutableList.of(), userCompileFlags.build(), cppModuleMap, usePic, - builder.getRealOutputFilePath(), + builder.getTempOutputFile(), CppHelper.getFdoBuildStamp(ruleContext, fdoSupport.getFdoSupport()), dotdFileExecPath, ImmutableList.copyOf(variablesExtensions), @@ -1542,6 +1542,10 @@ private Variables setupCompileBuildVariables( ccCompilationContextInfo.getDefines()); } + private static String toPathString(Artifact a) { + return a == null ? null : a.getExecPathString(); + } + /** * Returns a {@code CppCompileActionBuilder} with the common fields for a C++ compile action being * initialized. diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CompileBuildVariables.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CompileBuildVariables.java index 7c6aa8459faa16..f9c2f662af08c9 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CompileBuildVariables.java +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CompileBuildVariables.java @@ -28,7 +28,6 @@ import com.google.devtools.build.lib.syntax.EvalException; import com.google.devtools.build.lib.util.FileType; import com.google.devtools.build.lib.vfs.PathFragment; -import java.util.Collection; /** Enum covering all build variables we create for all various {@link CppCompileAction}. */ public enum CompileBuildVariables { @@ -109,24 +108,24 @@ public static Variables setupVariablesOrReportRuleError( RuleContext ruleContext, FeatureConfiguration featureConfiguration, CcToolchainProvider ccToolchainProvider, - Artifact sourceFile, - Artifact outputFile, - Artifact gcnoFile, - Artifact dwoFile, - Artifact ltoIndexingFile, + String sourceFile, + String outputFile, + String gcnoFile, + String dwoFile, + String ltoIndexingFile, ImmutableList includes, - ImmutableList userCompileFlags, + Iterable userCompileFlags, CppModuleMap cppModuleMap, boolean usePic, - PathFragment realOutputFilePath, + PathFragment fakeOutputFile, String fdoStamp, String dotdFileExecPath, ImmutableList variablesExtensions, ImmutableMap additionalBuildVariables, Iterable directModuleMaps, - Collection includeDirs, - Collection quoteIncludeDirs, - Collection systemIncludeDirs, + Iterable includeDirs, + Iterable quoteIncludeDirs, + Iterable systemIncludeDirs, Iterable defines) { try { return setupVariablesOrThrowEvalException( @@ -141,16 +140,20 @@ public static Variables setupVariablesOrReportRuleError( userCompileFlags, cppModuleMap, usePic, - realOutputFilePath, + toPathString(fakeOutputFile), fdoStamp, dotdFileExecPath, variablesExtensions, additionalBuildVariables, directModuleMaps, - includeDirs, - quoteIncludeDirs, - systemIncludeDirs, - defines); + getSafePathStrings(includeDirs), + getSafePathStrings(quoteIncludeDirs), + getSafePathStrings(systemIncludeDirs), + defines, + /* addLegacyCxxOptions= */ CppFileTypes.CPP_SOURCE.matches(sourceFile) + || CppFileTypes.CPP_HEADER.matches(sourceFile) + || CppFileTypes.CPP_MODULE_MAP.matches(sourceFile) + || CppFileTypes.CLIF_INPUT_PROTO.matches(sourceFile)); } catch (EvalException e) { ruleContext.ruleError(e.getMessage()); return Variables.EMPTY; @@ -160,25 +163,28 @@ public static Variables setupVariablesOrReportRuleError( public static Variables setupVariablesOrThrowEvalException( FeatureConfiguration featureConfiguration, CcToolchainProvider ccToolchainProvider, - Artifact sourceFile, - Artifact outputFile, - Artifact gcnoFile, - Artifact dwoFile, - Artifact ltoIndexingFile, + String sourceFile, + // TODO(b/76195763): Remove once blaze with cl/189769259 is released and crosstools are + // updated. + String outputFile, + String gcnoFile, + String dwoFile, + String ltoIndexingFile, ImmutableList includes, - ImmutableList userCompileFlags, + Iterable userCompileFlags, CppModuleMap cppModuleMap, boolean usePic, - PathFragment realOutputFilePath, + String fakeOutputFile, String fdoStamp, String dotdFileExecPath, ImmutableList variablesExtensions, ImmutableMap additionalBuildVariables, Iterable directModuleMaps, - Iterable includeDirs, - Iterable quoteIncludeDirs, - Iterable systemIncludeDirs, - Iterable defines) + Iterable includeDirs, + Iterable quoteIncludeDirs, + Iterable systemIncludeDirs, + Iterable defines, + boolean addLegacyCxxOptions) throws EvalException { Preconditions.checkNotNull(directModuleMaps); Preconditions.checkNotNull(includeDirs); @@ -191,34 +197,42 @@ public static Variables setupVariablesOrThrowEvalException( buildVariables.addStringSequenceVariable( USER_COMPILE_FLAGS.getVariableName(), userCompileFlags); - buildVariables.addStringVariable(SOURCE_FILE.getVariableName(), sourceFile.getExecPathString()); - - String sourceFilename = sourceFile.getExecPathString(); buildVariables.addLazyStringSequenceVariable( LEGACY_COMPILE_FLAGS.getVariableName(), - getLegacyCompileFlagsSupplier(ccToolchainProvider, sourceFilename)); + getLegacyCompileFlagsSupplier(ccToolchainProvider, addLegacyCxxOptions)); - if (!CppFileTypes.OBJC_SOURCE.matches(sourceFilename) - && !CppFileTypes.OBJCPP_SOURCE.matches(sourceFilename)) { + if (sourceFile != null) { + buildVariables.addStringVariable(SOURCE_FILE.getVariableName(), sourceFile); + } + + if (sourceFile == null + || (!CppFileTypes.OBJC_SOURCE.matches(sourceFile) + && !CppFileTypes.OBJCPP_SOURCE.matches(sourceFile))) { buildVariables.addLazyStringSequenceVariable( UNFILTERED_COMPILE_FLAGS.getVariableName(), getUnfilteredCompileFlagsSupplier(ccToolchainProvider)); } - // TODO(b/76195763): Remove once blaze with cl/189769259 is released and crosstools are updated. - if (!FileType.contains( - outputFile, - CppFileTypes.ASSEMBLER, - CppFileTypes.PIC_ASSEMBLER, - CppFileTypes.PREPROCESSED_C, - CppFileTypes.PREPROCESSED_CPP, - CppFileTypes.PIC_PREPROCESSED_C, - CppFileTypes.PIC_PREPROCESSED_CPP)) { + String fakeOutputFileOrRealOutputFile = fakeOutputFile != null ? fakeOutputFile : outputFile; + + if (outputFile != null) { + // TODO(b/76195763): Remove once blaze with cl/189769259 is released and crosstools are + // updated. + if (!FileType.contains( + PathFragment.create(outputFile), + CppFileTypes.ASSEMBLER, + CppFileTypes.PIC_ASSEMBLER, + CppFileTypes.PREPROCESSED_C, + CppFileTypes.PREPROCESSED_CPP, + CppFileTypes.PIC_PREPROCESSED_C, + CppFileTypes.PIC_PREPROCESSED_CPP)) { + buildVariables.addStringVariable( + OUTPUT_OBJECT_FILE.getVariableName(), fakeOutputFileOrRealOutputFile); + } + buildVariables.addStringVariable( - OUTPUT_OBJECT_FILE.getVariableName(), realOutputFilePath.getSafePathString()); + OUTPUT_FILE.getVariableName(), fakeOutputFileOrRealOutputFile); } - buildVariables.addStringVariable( - OUTPUT_FILE.getVariableName(), realOutputFilePath.getSafePathString()); // Set dependency_file to enable .d file generation. if (dotdFileExecPath != null) { @@ -242,12 +256,11 @@ public static Variables setupVariablesOrThrowEvalException( buildVariables.addStringSequenceVariable(MODULE_FILES.getVariableName(), ImmutableSet.of()); } if (featureConfiguration.isEnabled(CppRuleClasses.INCLUDE_PATHS)) { + buildVariables.addStringSequenceVariable(INCLUDE_PATHS.getVariableName(), includeDirs); buildVariables.addStringSequenceVariable( - INCLUDE_PATHS.getVariableName(), getSafePathStrings(includeDirs)); - buildVariables.addStringSequenceVariable( - QUOTE_INCLUDE_PATHS.getVariableName(), getSafePathStrings(quoteIncludeDirs)); + QUOTE_INCLUDE_PATHS.getVariableName(), quoteIncludeDirs); buildVariables.addStringSequenceVariable( - SYSTEM_INCLUDE_PATHS.getVariableName(), getSafePathStrings(systemIncludeDirs)); + SYSTEM_INCLUDE_PATHS.getVariableName(), systemIncludeDirs); } if (!includes.isEmpty()) { @@ -278,18 +291,16 @@ public static Variables setupVariablesOrThrowEvalException( } if (gcnoFile != null) { - buildVariables.addStringVariable( - GCOV_GCNO_FILE.getVariableName(), gcnoFile.getExecPathString()); + buildVariables.addStringVariable(GCOV_GCNO_FILE.getVariableName(), gcnoFile); } if (dwoFile != null) { - buildVariables.addStringVariable( - PER_OBJECT_DEBUG_INFO_FILE.getVariableName(), dwoFile.getExecPathString()); + buildVariables.addStringVariable(PER_OBJECT_DEBUG_INFO_FILE.getVariableName(), dwoFile); } if (ltoIndexingFile != null) { buildVariables.addStringVariable( - LTO_INDEXING_BITCODE_FILE.getVariableName(), ltoIndexingFile.getExecPathString()); + LTO_INDEXING_BITCODE_FILE.getVariableName(), ltoIndexingFile); } buildVariables.addAllStringVariables(additionalBuildVariables); @@ -316,14 +327,11 @@ private static ImmutableSet getSafePathStrings(Iterable pa * to arguments (to prevent accidental capture of enclosing instance which could regress memory). */ private static Supplier> getLegacyCompileFlagsSupplier( - CcToolchainProvider toolchain, String sourceFilename) { + CcToolchainProvider toolchain, boolean addLegacyCxxOptions) { return () -> { ImmutableList.Builder legacyCompileFlags = ImmutableList.builder(); legacyCompileFlags.addAll(toolchain.getLegacyCompileOptions()); - if (CppFileTypes.CPP_SOURCE.matches(sourceFilename) - || CppFileTypes.CPP_HEADER.matches(sourceFilename) - || CppFileTypes.CPP_MODULE_MAP.matches(sourceFilename) - || CppFileTypes.CLIF_INPUT_PROTO.matches(sourceFilename)) { + if (addLegacyCxxOptions) { legacyCompileFlags.addAll(toolchain.getLegacyCxxOptions()); } return legacyCompileFlags.build(); @@ -341,6 +349,14 @@ private static Supplier> getUnfilteredCompileFlagsSupplier return () -> ccToolchain.getUnfilteredCompilerOptions(); } + private static String toPathString(PathFragment a) { + if (a == null) { + return null; + } + + return a.getSafePathString(); + } + public String getVariableName() { return variableName; } diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppActionConfigs.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppActionConfigs.java index 0339e138770395..626f3430154bc6 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppActionConfigs.java +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppActionConfigs.java @@ -254,6 +254,7 @@ public static String getCppActionConfigs( " action: 'c++-module-codegen'", " action: 'c++-module-compile'", " flag_group {", + " expand_if_all_available: 'output_file'", " flag: '-frandom-seed=%{output_file}'", " }", " }", diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppLinkstampCompileHelper.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppLinkstampCompileHelper.java index 83fa13d355e321..5d23af98d76ded 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppLinkstampCompileHelper.java +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppLinkstampCompileHelper.java @@ -147,8 +147,8 @@ private static Variables getVariables( ruleContext, featureConfiguration, ccToolchainProvider, - sourceFile, - outputFile, + sourceFile.getExecPathString(), + outputFile.getExecPathString(), /* gcnoFile= */ null, /* dwoFile= */ null, /* ltoIndexingFile= */ null, @@ -159,7 +159,7 @@ private static Variables getVariables( CcCompilationHelper.getCoptsFromOptions(cppConfiguration, sourceFile.getExecPathString()), /* cppModuleMap= */ null, needsPic, - outputFile.getExecPath(), + /* fakeOutputFile= */ null, fdoBuildStamp, /* dotdFileExecPath= */ null, /* variablesExtensions= */ ImmutableList.of(),