diff options
author | 2017-08-03 14:14:48 +0200 | |
---|---|---|
committer | 2017-08-03 15:43:16 +0200 | |
commit | b3be32e7135895737b64e3ad8ddb09176bfc8dd5 (patch) | |
tree | 98cb5d34f25a4be1c22accfb7f0ada15abff3a10 /src/main/java/com | |
parent | ca9290380880ecc0370cea53a26e647cd527c29f (diff) |
Use feature configuration to construct command line for 'strip' action
This cl introduces new action_config named 'strip' for the strip action. While
at it, it fixes support for executionRequirements.
Fixed #209
RELNOTES: 'strip' action is now configured via feature configuration
PiperOrigin-RevId: 164110478
Diffstat (limited to 'src/main/java/com')
7 files changed, 102 insertions, 23 deletions
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcBinary.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcBinary.java index 12e660fe47..b0c1a42921 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcBinary.java +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcBinary.java @@ -332,7 +332,7 @@ public abstract class CcBinary implements RuleConfiguredTargetFactory { Artifact strippedFile = ruleContext.getImplicitOutputArtifact( CppRuleClasses.CC_BINARY_STRIPPED); CppHelper.createStripAction( - ruleContext, ccToolchain, cppConfiguration, executable, strippedFile); + ruleContext, ccToolchain, cppConfiguration, executable, strippedFile, featureConfiguration); DwoArtifactsCollector dwoArtifacts = collectTransitiveDwoArtifacts( diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcLibraryHelper.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcLibraryHelper.java index dfc3e7200c..690de23037 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcLibraryHelper.java +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcLibraryHelper.java @@ -104,6 +104,7 @@ public final class CcLibraryHelper { CppFileTypes.ASSEMBLER_WITH_C_PREPROCESSOR), ImmutableSet.<String>of( CppCompileAction.CC_FLAGS_MAKE_VARIABLE_ACTION_NAME, + CppCompileAction.STRIP_ACTION_NAME, CppCompileAction.C_COMPILE, CppCompileAction.CPP_COMPILE, CppCompileAction.CPP_HEADER_PARSING, 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 e1669ddca7..0922c08800 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 @@ -37,6 +37,7 @@ public class CppActionConfigs { String gccToolPath, String cppLinkDynamicLibraryToolPath, String arToolPath, + String stripToolPath, boolean supportsEmbeddedRuntimes, boolean supportsInterfaceSharedLibraries) { String cppDynamicLibraryLinkerTool = ""; @@ -843,6 +844,46 @@ public class CppActionConfigs { " flag: '%{copts}'", " }", " }", + "}", + "action_config {", + " config_name: 'strip'", + " action_name: 'strip'", + " tool {", + " tool_path: '" + stripToolPath + "'", + " }", + " flag_set {", + " flag_group {", + " flag: '-S'", + ifLinux(platform, "flag: '-p'"), + " flag: '-o'", + " flag: '%{output_file}'", + " }", + " flag_group {", + " flag: '-R'", + " flag: '.gnu.switches.text.quote_paths'", + " flag: '-R'", + " flag: '.gnu.switches.text.bracket_paths'", + " flag: '-R'", + " flag: '.gnu.switches.text.system_paths'", + " flag: '-R'", + " flag: '.gnu.switches.text.cpp_defines'", + " flag: '-R'", + " flag: '.gnu.switches.text.cpp_includes'", + " flag: '-R'", + " flag: '.gnu.switches.text.cl_args'", + " flag: '-R'", + " flag: '.gnu.switches.text.lipo_info'", + " flag: '-R'", + " flag: '.gnu.switches.text.annotation'", + " }", + " flag_group {", + " iterate_over: 'stripopts'", + " flag: '%{stripopts}'", + " }", + " flag_group {", + " flag: '%{input_file}'", + " }", + " }", "}")); } 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 b4cc77960c..2862ade4e6 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 @@ -121,6 +121,9 @@ public class CppCompileAction extends AbstractAction public static final java.lang.String CC_FLAGS_MAKE_VARIABLE_ACTION_NAME = "cc-flags-make-variable"; + /** A string constant for the strip action name. */ + public static final String STRIP_ACTION_NAME = "strip"; + /** * A string constant for the c compilation action. */ @@ -438,7 +441,8 @@ public class CppCompileAction extends AbstractAction return discoversInputs; } - @VisibleForTesting // productionVisibility = Visibility.PRIVATE + @Override + @VisibleForTesting // productionVisibility = Visibility.PRIVATE public Iterable<Artifact> getPossibleInputsForTesting() { return Iterables.concat(getInputs(), prunableInputs); } diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppConfiguration.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppConfiguration.java index 11f05ac28f..5fb4589bb1 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppConfiguration.java +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppConfiguration.java @@ -641,6 +641,7 @@ public class CppConfiguration extends BuildConfiguration.Fragment { String gccToolPath = "DUMMY_GCC_TOOL"; String linkerToolPath = "DUMMY_LINKER_TOOL"; String arToolPath = "DUMMY_AR_TOOL"; + String stripToolPath = "DUMMY_STRIP_TOOL"; for (ToolPath tool : toolchain.getToolPathList()) { if (tool.getName().equals(Tool.GCC.getNamePart())) { gccToolPath = tool.getPath(); @@ -652,6 +653,9 @@ public class CppConfiguration extends BuildConfiguration.Fragment { if (tool.getName().equals(Tool.AR.getNamePart())) { arToolPath = tool.getPath(); } + if (tool.getName().equals(Tool.STRIP.getNamePart())) { + stripToolPath = tool.getPath(); + } } TextFormat.merge( CppActionConfigs.getCppActionConfigs( @@ -660,9 +664,10 @@ public class CppConfiguration extends BuildConfiguration.Fragment { gccToolPath, linkerToolPath, arToolPath, + stripToolPath, supportsEmbeddedRuntimes, toolchain.getSupportsInterfaceSharedObjects()), - toolchainBuilder); + toolchainBuilder); } catch (ParseException e) { // Can only happen if we change the proto definition without changing our // configuration above. diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppHelper.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppHelper.java index 47d0964d2b..8e2b7ed94c 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppHelper.java +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppHelper.java @@ -18,7 +18,9 @@ import static com.google.devtools.build.lib.packages.BuildType.LABEL; import com.google.common.annotations.VisibleForTesting; import com.google.common.collect.ImmutableList; +import com.google.common.collect.ImmutableMap; import com.google.common.collect.Iterables; +import com.google.devtools.build.lib.actions.Action; import com.google.devtools.build.lib.actions.ActionOwner; import com.google.devtools.build.lib.actions.Artifact; import com.google.devtools.build.lib.actions.MiddlemanFactory; @@ -40,6 +42,9 @@ import com.google.devtools.build.lib.collect.nestedset.Order; import com.google.devtools.build.lib.packages.RuleErrorConsumer; import com.google.devtools.build.lib.rules.cpp.CcCommon.CcFlagsSupplier; import com.google.devtools.build.lib.rules.cpp.CcLinkParams.Linkstamp; +import com.google.devtools.build.lib.rules.cpp.CcToolchainFeatures.FeatureConfiguration; +import com.google.devtools.build.lib.rules.cpp.CcToolchainFeatures.Tool; +import com.google.devtools.build.lib.rules.cpp.CcToolchainFeatures.Variables; import com.google.devtools.build.lib.rules.cpp.CppCompilationContext.Builder; import com.google.devtools.build.lib.rules.cpp.Link.LinkTargetType; import com.google.devtools.build.lib.shell.ShellUtils; @@ -601,32 +606,49 @@ public class CppHelper { .getChild(configuration.getGenfilesFragment().getBaseName()); } - /** - * Creates an action to strip an executable. - */ - public static void createStripAction(RuleContext context, CcToolchainProvider toolchain, - CppConfiguration cppConfiguration, Artifact input, Artifact output) { - context.registerAction( + /** Creates an action to strip an executable. */ + public static void createStripAction( + RuleContext context, + CcToolchainProvider toolchain, + CppConfiguration cppConfiguration, + Artifact input, + Artifact output, + FeatureConfiguration featureConfiguration) { + if (!featureConfiguration.actionIsConfigured(CppCompileAction.STRIP_ACTION_NAME)) { + context.ruleError("Expected action_config for 'strip' to be configured."); + return; + } + + Tool stripTool = + Preconditions.checkNotNull( + featureConfiguration.getToolForAction(CppCompileAction.STRIP_ACTION_NAME)); + Variables variables = + new Variables.Builder() + .addStringVariable(CppModel.OUTPUT_FILE_VARIABLE_NAME, output.getExecPathString()) + .addStringSequenceVariable( + CppModel.STRIPOPTS_VARIABLE_NAME, cppConfiguration.getStripOpts()) + .addStringVariable(CppModel.INPUT_FILE_VARIABLE_NAME, input.getExecPathString()) + .build(); + ImmutableList<String> commandLine = + ImmutableList.copyOf( + featureConfiguration.getCommandLine(CppCompileAction.STRIP_ACTION_NAME, variables)); + ImmutableMap.Builder<String, String> executionInfoBuilder = ImmutableMap.builder(); + for (String executionRequirement : stripTool.getExecutionRequirements()) { + executionInfoBuilder.put(executionRequirement, ""); + } + Action[] stripAction = new SpawnAction.Builder() .addInput(input) .addTransitiveInputs(toolchain.getStrip()) .addOutput(output) .useDefaultShellEnvironment() - .setExecutable(cppConfiguration.getStripExecutable()) - .addArguments("-S", "-p", "-o", output.getExecPathString()) - .addArguments("-R", ".gnu.switches.text.quote_paths") - .addArguments("-R", ".gnu.switches.text.bracket_paths") - .addArguments("-R", ".gnu.switches.text.system_paths") - .addArguments("-R", ".gnu.switches.text.cpp_defines") - .addArguments("-R", ".gnu.switches.text.cpp_includes") - .addArguments("-R", ".gnu.switches.text.cl_args") - .addArguments("-R", ".gnu.switches.text.lipo_info") - .addArguments("-R", ".gnu.switches.text.annotation") - .addArguments(cppConfiguration.getStripOpts()) - .addArgument(input.getExecPathString()) - .setProgressMessage("Stripping %s for %s", output.prettyPrint(), context.getLabel()) + .setExecutable(stripTool.getToolPath(cppConfiguration.getCrosstoolTopPathFragment())) + .addArguments(commandLine) + .setExecutionInfo(executionInfoBuilder.build()) + .setProgressMessage("Stripping " + output.prettyPrint() + " for " + context.getLabel()) .setMnemonic("CcStrip") - .build(context)); + .build(context); + context.registerAction(stripAction); } public static void maybeAddStaticLinkMarkerProvider(RuleConfiguredTargetBuilder builder, diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppModel.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppModel.java index 1ca69d798e..7bd76742af 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppModel.java +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppModel.java @@ -66,6 +66,9 @@ public final class CppModel { /** Name of the build variable for the path to the source file being compiled. */ public static final String SOURCE_FILE_VARIABLE_NAME = "source_file"; + /** Name of the build variable for the path to the input file being processed. */ + public static final String INPUT_FILE_VARIABLE_NAME = "input_file"; + /** Name of the build variable for the path to the compilation output file. */ public static final String OUTPUT_FILE_VARIABLE_NAME = "output_file"; @@ -137,6 +140,9 @@ public final class CppModel { /** Build variable for all user provided copt flags. */ public static final String COPTS_VARIABLE_VALUE = "copts"; + /** Name of the build variable for stripopts for the strip action. */ + public static final String STRIPOPTS_VARIABLE_NAME = "stripopts"; + private final CppSemantics semantics; private final RuleContext ruleContext; private final BuildConfiguration configuration; |