diff options
Diffstat (limited to 'src/main/java/com/google/devtools')
9 files changed, 107 insertions, 82 deletions
diff --git a/src/main/java/com/google/devtools/build/lib/actions/CommandAction.java b/src/main/java/com/google/devtools/build/lib/actions/CommandAction.java index e8d5a93d1c..1ac7da832d 100644 --- a/src/main/java/com/google/devtools/build/lib/actions/CommandAction.java +++ b/src/main/java/com/google/devtools/build/lib/actions/CommandAction.java @@ -14,6 +14,7 @@ package com.google.devtools.build.lib.actions; +import com.google.common.annotations.VisibleForTesting; import com.google.common.collect.ImmutableMap; import java.util.List; @@ -33,5 +34,6 @@ public interface CommandAction extends Action, ExecutionInfoSpecifier { ImmutableMap<String, String> getEnvironment(); /** Returns inputs to this action, including inputs that may be pruned. */ + @VisibleForTesting // productionVisibility = Visibility.PRIVATE Iterable<Artifact> getPossibleInputsForTesting(); } diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcCommon.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcCommon.java index 47aa40a0ec..ff3af5dc16 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcCommon.java +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcCommon.java @@ -90,7 +90,6 @@ public final class CcCommon { private static final ImmutableSet<String> DEFAULT_FEATURES = ImmutableSet.of( CppRuleClasses.DEPENDENCY_FILE, - CppRuleClasses.COMPILE_ACTION_FLAGS_IN_FLAG_SET, CppRuleClasses.RANDOM_SEED, CppRuleClasses.MODULE_MAPS, CppRuleClasses.MODULE_MAP_HOME_CWD, diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CompileCommandLine.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CompileCommandLine.java index 3215cbb36e..73bf3e85bb 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CompileCommandLine.java +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CompileCommandLine.java @@ -20,10 +20,8 @@ import com.google.devtools.build.lib.actions.Artifact; import com.google.devtools.build.lib.rules.cpp.CcToolchainFeatures.FeatureConfiguration; import com.google.devtools.build.lib.rules.cpp.CcToolchainFeatures.Variables; import com.google.devtools.build.lib.rules.cpp.CppCompileAction.DotdFile; -import com.google.devtools.build.lib.util.FileType; import com.google.devtools.build.lib.util.Pair; import com.google.devtools.build.lib.util.Preconditions; -import com.google.devtools.build.lib.vfs.PathFragment; import java.util.ArrayList; import java.util.List; import java.util.Map; @@ -33,7 +31,6 @@ import javax.annotation.Nullable; public final class CompileCommandLine { private final Artifact sourceFile; - private final Artifact outputFile; private final Predicate<String> coptsFilter; private final FeatureConfiguration featureConfiguration; private final CcToolchainFeatures.Variables variables; @@ -43,7 +40,6 @@ public final class CompileCommandLine { private CompileCommandLine( Artifact sourceFile, - Artifact outputFile, Predicate<String> coptsFilter, FeatureConfiguration featureConfiguration, CppConfiguration cppConfiguration, @@ -51,7 +47,6 @@ public final class CompileCommandLine { String actionName, DotdFile dotdFile) { this.sourceFile = Preconditions.checkNotNull(sourceFile); - this.outputFile = Preconditions.checkNotNull(outputFile); this.coptsFilter = coptsFilter; this.featureConfiguration = Preconditions.checkNotNull(featureConfiguration); this.cppConfiguration = Preconditions.checkNotNull(cppConfiguration); @@ -71,8 +66,12 @@ public final class CompileCommandLine { return featureConfiguration.getEnvironmentVariables(actionName, variables); } - protected List<String> getArgv( - PathFragment outputFile, CcToolchainFeatures.Variables overwrittenVariables) { + /** + * @param overwrittenVariables: Variables that will overwrite original build variables. When null, + * unmodified original variables are used. + */ + protected List<String> getArguments( + @Nullable CcToolchainFeatures.Variables overwrittenVariables) { List<String> commandLine = new ArrayList<>(); // first: The command name. @@ -87,17 +86,6 @@ public final class CompileCommandLine { // second: The compiler options. commandLine.addAll(getCompilerOptions(overwrittenVariables)); - - if (!featureConfiguration.isEnabled("compile_action_flags_in_flag_set")) { - // third: The file to compile! - commandLine.add("-c"); - commandLine.add(sourceFile.getExecPathString()); - - // finally: The output file. (Prefixed with -o). - commandLine.add("-o"); - commandLine.add(outputFile.getPathString()); - } - return commandLine; } @@ -116,19 +104,6 @@ public final class CompileCommandLine { addFilteredOptions( options, featureConfiguration.getPerFeatureExpansions(actionName, updatedVariables)); - if (!featureConfiguration.isEnabled("compile_action_flags_in_flag_set")) { - if (FileType.contains(outputFile, CppFileTypes.ASSEMBLER, CppFileTypes.PIC_ASSEMBLER)) { - options.add("-S"); - } else if (FileType.contains( - outputFile, - CppFileTypes.PREPROCESSED_C, - CppFileTypes.PREPROCESSED_CPP, - CppFileTypes.PIC_PREPROCESSED_C, - CppFileTypes.PIC_PREPROCESSED_CPP)) { - options.add("-E"); - } - } - return options; } @@ -174,19 +149,16 @@ public final class CompileCommandLine { public static Builder builder( Artifact sourceFile, - Artifact outputFile, Predicate<String> coptsFilter, String actionName, CppConfiguration cppConfiguration, DotdFile dotdFile) { - return new Builder( - sourceFile, outputFile, coptsFilter, actionName, cppConfiguration, dotdFile); + return new Builder(sourceFile, coptsFilter, actionName, cppConfiguration, dotdFile); } /** A builder for a {@link CompileCommandLine}. */ public static final class Builder { private final Artifact sourceFile; - private final Artifact outputFile; private Predicate<String> coptsFilter; private FeatureConfiguration featureConfiguration; private CcToolchainFeatures.Variables variables = Variables.EMPTY; @@ -197,7 +169,6 @@ public final class CompileCommandLine { public CompileCommandLine build() { return new CompileCommandLine( Preconditions.checkNotNull(sourceFile), - Preconditions.checkNotNull(outputFile), Preconditions.checkNotNull(coptsFilter), Preconditions.checkNotNull(featureConfiguration), Preconditions.checkNotNull(cppConfiguration), @@ -208,13 +179,11 @@ public final class CompileCommandLine { private Builder( Artifact sourceFile, - Artifact outputFile, Predicate<String> coptsFilter, String actionName, CppConfiguration cppConfiguration, DotdFile dotdFile) { this.sourceFile = sourceFile; - this.outputFile = outputFile; this.coptsFilter = coptsFilter; this.actionName = actionName; this.cppConfiguration = cppConfiguration; 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 e965d4a82f..29a00d0142 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 @@ -70,6 +70,8 @@ public class CppActionConfigs { " implies: 'user_compile_flags'", " implies: 'sysroot'", " implies: 'unfiltered_compile_flags'", + " implies: 'compiler_input_flags'", + " implies: 'compiler_output_flags'", "}", "action_config {", " config_name: 'preprocess-assemble'", @@ -81,6 +83,8 @@ public class CppActionConfigs { " implies: 'user_compile_flags'", " implies: 'sysroot'", " implies: 'unfiltered_compile_flags'", + " implies: 'compiler_input_flags'", + " implies: 'compiler_output_flags'", "}", "action_config {", " config_name: 'c-compile'", @@ -92,6 +96,8 @@ public class CppActionConfigs { " implies: 'user_compile_flags'", " implies: 'sysroot'", " implies: 'unfiltered_compile_flags'", + " implies: 'compiler_input_flags'", + " implies: 'compiler_output_flags'", "}", "action_config {", " config_name: 'c++-compile'", @@ -103,6 +109,8 @@ public class CppActionConfigs { " implies: 'user_compile_flags'", " implies: 'sysroot'", " implies: 'unfiltered_compile_flags'", + " implies: 'compiler_input_flags'", + " implies: 'compiler_output_flags'", "}", "action_config {", " config_name: 'c++-header-parsing'", @@ -114,6 +122,8 @@ public class CppActionConfigs { " implies: 'user_compile_flags'", " implies: 'sysroot'", " implies: 'unfiltered_compile_flags'", + " implies: 'compiler_input_flags'", + " implies: 'compiler_output_flags'", "}", "action_config {", " config_name: 'c++-header-preprocessing'", @@ -125,6 +135,8 @@ public class CppActionConfigs { " implies: 'user_compile_flags'", " implies: 'sysroot'", " implies: 'unfiltered_compile_flags'", + " implies: 'compiler_input_flags'", + " implies: 'compiler_output_flags'", "}", "action_config {", " config_name: 'c++-module-compile'", @@ -136,6 +148,8 @@ public class CppActionConfigs { " implies: 'user_compile_flags'", " implies: 'sysroot'", " implies: 'unfiltered_compile_flags'", + " implies: 'compiler_input_flags'", + " implies: 'compiler_output_flags'", "}", "action_config {", " config_name: 'c++-module-codegen'", @@ -147,6 +161,8 @@ public class CppActionConfigs { " implies: 'user_compile_flags'", " implies: 'sysroot'", " implies: 'unfiltered_compile_flags'", + " implies: 'compiler_input_flags'", + " implies: 'compiler_output_flags'", "}", ifTrue( !existingFeatureNames.contains(CppRuleClasses.LEGACY_COMPILE_FLAGS), @@ -1027,6 +1043,66 @@ public class CppActionConfigs { " flag: '@%{linker_param_file}'", " }", " }", + "}"), + ifTrue( + !existingFeatureNames.contains("compiler_input_flags"), + "feature {", + " name: 'compiler_input_flags'", + " enabled: true", + " flag_set {", + " action: 'assemble'", + " action: 'preprocess-assemble'", + " action: 'c-compile'", + " action: 'c++-compile'", + " action: 'c++-module-compile'", + " action: 'c++-module-codegen'", + " action: 'objc-compile'", + " action: 'objc++-compile'", + " action: 'c++-header-preprocessing'", + " action: 'c++-header-parsing'", + " action: 'lto-backend'", + " expand_if_all_available: 'source_file'", + " flag_group {", + " flag: '-c'", + " flag: '%{source_file}'", + " }", + " }", + "}"), + ifTrue( + !existingFeatureNames.contains("compiler_output_flags"), + "feature {", + " name: 'compiler_output_flags'", + " enabled: true", + " flag_set {", + " action: 'assemble'", + " action: 'preprocess-assemble'", + " action: 'c-compile'", + " action: 'c++-compile'", + " action: 'c++-module-compile'", + " action: 'c++-module-codegen'", + " action: 'objc-compile'", + " action: 'objc++-compile'", + " action: 'c++-header-preprocessing'", + " action: 'c++-header-parsing'", + " action: 'lto-backend'", + " flag_group {", + " expand_if_all_available: 'output_object_file'", + " flag: '-o'", + " flag: '%{output_object_file}'", + " }", + " flag_group {", + " expand_if_all_available: 'output_assembly_file'", + " flag: '-o'", + " flag: '%{output_assembly_file}'", + " flag: '-S'", + " }", + " flag_group {", + " expand_if_all_available: 'output_preprocess_file'", + " flag: '-o'", + " flag: '%{output_preprocess_file}'", + " flag: '-E'", + " }", + " }", "}"))); } 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 0de697be74..735e24ecb8 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 @@ -341,7 +341,6 @@ public class CppCompileAction extends AbstractAction this.compileCommandLine = CompileCommandLine.builder( sourceFile, - outputFile, coptsFilter, actionName, cppConfiguration, @@ -663,7 +662,7 @@ public class CppCompileAction extends AbstractAction @Override public List<String> getCmdlineIncludes() { ImmutableList.Builder<String> cmdlineIncludes = ImmutableList.builder(); - List<String> args = getArgv(); + List<String> args = getArguments(); for (Iterator<String> argi = args.iterator(); argi.hasNext();) { String arg = argi.next(); if (arg.equals("-include") && argi.hasNext()) { @@ -725,21 +724,9 @@ public class CppCompileAction extends AbstractAction return ImmutableMap.copyOf(environment); } - /** - * Returns a new, mutable list of command and arguments (argv) to be passed - * to the gcc subprocess. - */ - public final List<String> getArgv() { - return getArgv(getInternalOutputFile()); - } - @Override public List<String> getArguments() { - return getArgv(); - } - - protected final List<String> getArgv(PathFragment outputFile) { - return compileCommandLine.getArgv(outputFile, overwrittenVariables); + return compileCommandLine.getArguments(overwrittenVariables); } @Override @@ -1100,7 +1087,8 @@ public class CppCompileAction extends AbstractAction public ResourceSet estimateResourceConsumptionLocal() { // We use a local compile, so much of the time is spent waiting for IO, // but there is still significant CPU; hence we estimate 50% cpu usage. - return ResourceSet.createWithRamCpuIo(/*memoryMb=*/200, /*cpuUsage=*/0.5, /*ioUsage=*/0.0); + return ResourceSet.createWithRamCpuIo( + /* memoryMb= */ 200, /* cpuUsage= */ 0.5, /* ioUsage= */ 0.0); } @Override @@ -1117,10 +1105,10 @@ public class CppCompileAction extends AbstractAction // itself is fully determined by the input source files and module maps. // A better long-term solution would be to make the compiler to find them automatically and // never hand in the .pcm files explicitly on the command line in the first place. - f.addStrings(compileCommandLine.getArgv(getInternalOutputFile(), null)); + f.addStrings(compileCommandLine.getArguments(/* overwrittenVariables= */ null)); /* - * getArgv() above captures all changes which affect the compilation + * getArguments() above captures all changes which affect the compilation * command and hence the contents of the object file. But we need to * also make sure that we reexecute the action if any of the fields * that affect whether validateIncludes() will report an error or warning @@ -1348,9 +1336,9 @@ public class CppCompileAction extends AbstractAction message.append(getProgressMessage()); message.append('\n'); // Outputting one argument per line makes it easier to diff the results. - // The first element in getArgv() is actually the command to execute. + // The first element in getArguments() is actually the command to execute. String legend = " Command: "; - for (String argument : ShellEscaper.escapeAll(getArgv())) { + for (String argument : ShellEscaper.escapeAll(getArguments())) { message.append(legend); message.append(argument); message.append('\n'); diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppRuleClasses.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppRuleClasses.java index 390aa32254..275c717353 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppRuleClasses.java +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppRuleClasses.java @@ -190,13 +190,6 @@ public class CppRuleClasses { public static final String RANDOM_SEED = "random_seed"; /** - * A string constant for the compile_action_flags_in_flag_set feature. This feature is just a - * transitional feature which helps telling whether -c and -o options are already in flag_set of - * action_config in CROSSTOOL file. Once the transition is done, it should be removed. - */ - public static final String COMPILE_ACTION_FLAGS_IN_FLAG_SET = "compile_action_flags_in_flag_set"; - - /** * A string constant for the dependency_file feature. This feature generates the .d file. */ public static final String DEPENDENCY_FILE = "dependency_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 1daec4300b..24afe83de2 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 @@ -199,14 +199,12 @@ public class FakeCppCompileAction extends CppCompileAction { // line to write to $TEST_TMPDIR instead. final String outputPrefix = "$TEST_TMPDIR/"; String argv = - getArgv(outputFile.getExecPath()) + getArguments() .stream() .map( input -> { String result = ShellEscaper.escapeString(input); - // Once -c and -o options are added into action_config, the argument of - // getArgv(outputFile.getExecPath()) won't be used anymore. There will always be - // -c <tempOutputFile>, but here it has to be outputFile, so we replace it. + // Replace -c <tempOutputFile> so it's -c <outputFile>. if (input.equals(tempOutputFile.getPathString())) { result = outputPrefix + ShellEscaper.escapeString(outputFile.getExecPathString()); diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/SpawnGccStrategy.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/SpawnGccStrategy.java index f24be92101..cf8612a89e 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/SpawnGccStrategy.java +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/SpawnGccStrategy.java @@ -53,17 +53,18 @@ public class SpawnGccStrategy implements CppCompileActionContext { + action.getPrimaryInput().getExecPathString()); } Iterable<Artifact> inputs = Iterables.concat(action.getInputs(), action.getAdditionalInputs()); - Spawn spawn = new SimpleSpawn( - action, - ImmutableList.copyOf(action.getArgv()), - ImmutableMap.copyOf(action.getEnvironment()), - ImmutableMap.copyOf(action.getExecutionInfo()), - EmptyRunfilesSupplier.INSTANCE, - ImmutableList.<Artifact>copyOf(inputs), - /*tools=*/ImmutableList.<Artifact>of(), - /*filesetManifests=*/ImmutableList.<Artifact>of(), - action.getOutputs().asList(), - action.estimateResourceConsumptionLocal()); + Spawn spawn = + new SimpleSpawn( + action, + ImmutableList.copyOf(action.getArguments()), + ImmutableMap.copyOf(action.getEnvironment()), + ImmutableMap.copyOf(action.getExecutionInfo()), + EmptyRunfilesSupplier.INSTANCE, + ImmutableList.copyOf(inputs), + /* tools= */ ImmutableList.of(), + /* filesetManifests= */ ImmutableList.of(), + action.getOutputs().asList(), + action.estimateResourceConsumptionLocal()); actionExecutionContext .getSpawnActionContext(action.getMnemonic()) diff --git a/src/main/java/com/google/devtools/build/lib/rules/objc/CrosstoolCompilationSupport.java b/src/main/java/com/google/devtools/build/lib/rules/objc/CrosstoolCompilationSupport.java index 8241b68cfb..69d071eeee 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/objc/CrosstoolCompilationSupport.java +++ b/src/main/java/com/google/devtools/build/lib/rules/objc/CrosstoolCompilationSupport.java @@ -502,7 +502,6 @@ public class CrosstoolCompilationSupport extends CompilationSupport { .add(CppRuleClasses.COMPILE_ALL_MODULES) .add(CppRuleClasses.EXCLUDE_PRIVATE_HEADERS_IN_MODULE_MAPS) .add(CppRuleClasses.ONLY_DOTH_HEADERS_IN_MODULE_MAPS) - .add(CppRuleClasses.COMPILE_ACTION_FLAGS_IN_FLAG_SET) .add(CppRuleClasses.DEPENDENCY_FILE) .add(CppRuleClasses.INCLUDE_PATHS) .add(isHost ? "host" : "nonhost") |