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 981152cc1380d9..b64778d76dbc59 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 @@ -1517,16 +1517,16 @@ private CcToolchainVariables setupCompileBuildVariables( ruleContext, featureConfiguration, ccToolchain, - toPathString(sourceFile), - toPathString(builder.getOutputFile()), - toPathString(gcnoFile), - toPathString(dwoFile), - toPathString(ltoIndexingFile), + sourceFile, + builder.getOutputFile(), + gcnoFile, + dwoFile, + ltoIndexingFile, ImmutableList.of(), userCompileFlags.build(), cppModuleMap, usePic, - builder.getTempOutputFile(), + builder.getRealOutputFilePath(), CppHelper.getFdoBuildStamp(ruleContext, fdoSupport.getFdoSupport()), dotdFileExecPath, ImmutableList.copyOf(variablesExtensions), @@ -1538,10 +1538,6 @@ private CcToolchainVariables setupCompileBuildVariables( ccCompilationContext.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 3aa90dd83ed4ed..2c246f7a8c424d 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 @@ -27,6 +27,7 @@ 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 { @@ -107,24 +108,24 @@ public static CcToolchainVariables setupVariablesOrReportRuleError( RuleContext ruleContext, FeatureConfiguration featureConfiguration, CcToolchainProvider ccToolchainProvider, - String sourceFile, - String outputFile, - String gcnoFile, - String dwoFile, - String ltoIndexingFile, + Artifact sourceFile, + Artifact outputFile, + Artifact gcnoFile, + Artifact dwoFile, + Artifact ltoIndexingFile, ImmutableList includes, - Iterable userCompileFlags, + ImmutableList userCompileFlags, CppModuleMap cppModuleMap, boolean usePic, - PathFragment fakeOutputFile, + PathFragment realOutputFilePath, String fdoStamp, String dotdFileExecPath, ImmutableList variablesExtensions, ImmutableMap additionalBuildVariables, Iterable directModuleMaps, - Iterable includeDirs, - Iterable quoteIncludeDirs, - Iterable systemIncludeDirs, + Collection includeDirs, + Collection quoteIncludeDirs, + Collection systemIncludeDirs, Iterable defines) { try { return setupVariablesOrThrowEvalException( @@ -139,20 +140,16 @@ public static CcToolchainVariables setupVariablesOrReportRuleError( userCompileFlags, cppModuleMap, usePic, - toPathString(fakeOutputFile), + realOutputFilePath, fdoStamp, dotdFileExecPath, variablesExtensions, additionalBuildVariables, directModuleMaps, - 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)); + includeDirs, + quoteIncludeDirs, + systemIncludeDirs, + defines); } catch (EvalException e) { ruleContext.ruleError(e.getMessage()); return CcToolchainVariables.EMPTY; @@ -162,28 +159,25 @@ public static CcToolchainVariables setupVariablesOrReportRuleError( public static CcToolchainVariables setupVariablesOrThrowEvalException( FeatureConfiguration featureConfiguration, CcToolchainProvider ccToolchainProvider, - 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, + Artifact sourceFile, + Artifact outputFile, + Artifact gcnoFile, + Artifact dwoFile, + Artifact ltoIndexingFile, ImmutableList includes, - Iterable userCompileFlags, + ImmutableList userCompileFlags, CppModuleMap cppModuleMap, boolean usePic, - String fakeOutputFile, + PathFragment realOutputFilePath, String fdoStamp, String dotdFileExecPath, ImmutableList variablesExtensions, ImmutableMap additionalBuildVariables, Iterable directModuleMaps, - Iterable includeDirs, - Iterable quoteIncludeDirs, - Iterable systemIncludeDirs, - Iterable defines, - boolean addLegacyCxxOptions) + Iterable includeDirs, + Iterable quoteIncludeDirs, + Iterable systemIncludeDirs, + Iterable defines) throws EvalException { Preconditions.checkNotNull(directModuleMaps); Preconditions.checkNotNull(includeDirs); @@ -196,42 +190,34 @@ public static CcToolchainVariables 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, addLegacyCxxOptions)); + getLegacyCompileFlagsSupplier(ccToolchainProvider, sourceFilename)); - if (sourceFile != null) { - buildVariables.addStringVariable(SOURCE_FILE.getVariableName(), sourceFile); - } - - if (sourceFile == null - || (!CppFileTypes.OBJC_SOURCE.matches(sourceFile) - && !CppFileTypes.OBJCPP_SOURCE.matches(sourceFile))) { + if (!CppFileTypes.OBJC_SOURCE.matches(sourceFilename) + && !CppFileTypes.OBJCPP_SOURCE.matches(sourceFilename)) { buildVariables.addLazyStringSequenceVariable( UNFILTERED_COMPILE_FLAGS.getVariableName(), getUnfilteredCompileFlagsSupplier(ccToolchainProvider)); } - 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); - } - + // 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)) { buildVariables.addStringVariable( - OUTPUT_FILE.getVariableName(), fakeOutputFileOrRealOutputFile); + OUTPUT_OBJECT_FILE.getVariableName(), realOutputFilePath.getSafePathString()); } + buildVariables.addStringVariable( + OUTPUT_FILE.getVariableName(), realOutputFilePath.getSafePathString()); // Set dependency_file to enable .d file generation. if (dotdFileExecPath != null) { @@ -255,11 +241,12 @@ public static CcToolchainVariables setupVariablesOrThrowEvalException( buildVariables.addStringSequenceVariable(MODULE_FILES.getVariableName(), ImmutableSet.of()); } if (featureConfiguration.isEnabled(CppRuleClasses.INCLUDE_PATHS)) { - buildVariables.addStringSequenceVariable(INCLUDE_PATHS.getVariableName(), includeDirs); buildVariables.addStringSequenceVariable( - QUOTE_INCLUDE_PATHS.getVariableName(), quoteIncludeDirs); + INCLUDE_PATHS.getVariableName(), getSafePathStrings(includeDirs)); + buildVariables.addStringSequenceVariable( + QUOTE_INCLUDE_PATHS.getVariableName(), getSafePathStrings(quoteIncludeDirs)); buildVariables.addStringSequenceVariable( - SYSTEM_INCLUDE_PATHS.getVariableName(), systemIncludeDirs); + SYSTEM_INCLUDE_PATHS.getVariableName(), getSafePathStrings(systemIncludeDirs)); } if (!includes.isEmpty()) { @@ -290,16 +277,18 @@ public static CcToolchainVariables setupVariablesOrThrowEvalException( } if (gcnoFile != null) { - buildVariables.addStringVariable(GCOV_GCNO_FILE.getVariableName(), gcnoFile); + buildVariables.addStringVariable( + GCOV_GCNO_FILE.getVariableName(), gcnoFile.getExecPathString()); } if (dwoFile != null) { - buildVariables.addStringVariable(PER_OBJECT_DEBUG_INFO_FILE.getVariableName(), dwoFile); + buildVariables.addStringVariable( + PER_OBJECT_DEBUG_INFO_FILE.getVariableName(), dwoFile.getExecPathString()); } if (ltoIndexingFile != null) { buildVariables.addStringVariable( - LTO_INDEXING_BITCODE_FILE.getVariableName(), ltoIndexingFile); + LTO_INDEXING_BITCODE_FILE.getVariableName(), ltoIndexingFile.getExecPathString()); } buildVariables.addAllStringVariables(additionalBuildVariables); @@ -326,11 +315,14 @@ 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, boolean addLegacyCxxOptions) { + CcToolchainProvider toolchain, String sourceFilename) { return () -> { ImmutableList.Builder legacyCompileFlags = ImmutableList.builder(); legacyCompileFlags.addAll(toolchain.getLegacyCompileOptions()); - if (addLegacyCxxOptions) { + if (CppFileTypes.CPP_SOURCE.matches(sourceFilename) + || CppFileTypes.CPP_HEADER.matches(sourceFilename) + || CppFileTypes.CPP_MODULE_MAP.matches(sourceFilename) + || CppFileTypes.CLIF_INPUT_PROTO.matches(sourceFilename)) { legacyCompileFlags.addAll(toolchain.getLegacyCxxOptions()); } return legacyCompileFlags.build(); @@ -348,14 +340,6 @@ 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 626f3430154bc6..0339e138770395 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,7 +254,6 @@ 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 67d020f8ef5e93..7f92471e3024ef 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 @@ -146,8 +146,8 @@ private static CcToolchainVariables getVariables( ruleContext, featureConfiguration, ccToolchainProvider, - sourceFile.getExecPathString(), - outputFile.getExecPathString(), + sourceFile, + outputFile, /* gcnoFile= */ null, /* dwoFile= */ null, /* ltoIndexingFile= */ null, @@ -158,7 +158,7 @@ private static CcToolchainVariables getVariables( CcCompilationHelper.getCoptsFromOptions(cppConfiguration, sourceFile.getExecPathString()), /* cppModuleMap= */ null, needsPic, - /* fakeOutputFile= */ null, + outputFile.getExecPath(), fdoBuildStamp, /* dotdFileExecPath= */ null, /* variablesExtensions= */ ImmutableList.of(),