diff options
Diffstat (limited to 'src/main/java/com/google/devtools/build/lib')
4 files changed, 29 insertions, 56 deletions
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/CommandHelper.java b/src/main/java/com/google/devtools/build/lib/analysis/CommandHelper.java index 74d05a7edf..6f1c16b3a6 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/CommandHelper.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/CommandHelper.java @@ -159,23 +159,10 @@ public final class CommandHelper { } /** - * Resolves a command, and expands known locations for $(location) variables. This method supports - * legacy heuristic label expansion, which replaces strings that look like labels with their - * corresponding file names. Use {@link #resolveCommandAndExpandLabels} instead. - */ - public String resolveCommandAndHeuristicallyExpandLabels( - String command, @Nullable String attribute, boolean enableLegacyHeuristicLabelExpansion) { - command = resolveCommandAndExpandLabels(command, attribute, false); - if (enableLegacyHeuristicLabelExpansion) { - command = expandLabels(command, labelMap); - } - return command; - } - - /** * Resolves a command, and expands known locations for $(location) * variables. */ + @Deprecated // Only exists to support a legacy Skylark API. public String resolveCommandAndExpandLabels( String command, @Nullable String attribute, boolean allowDataInLabel) { LocationExpander expander; @@ -201,7 +188,7 @@ public final class CommandHelper { * <p>If the expansion fails, an attribute error is reported and the original * expression is returned. */ - private <T extends Iterable<Artifact>> String expandLabels(String expr, Map<Label, T> labelMap) { + public String expandLabelsHeuristically(String expr) { try { return LabelExpander.expand(expr, labelMap, ruleContext.getLabel()); } catch (LabelExpander.NotUniqueExpansionException nuee) { diff --git a/src/main/java/com/google/devtools/build/lib/analysis/Expander.java b/src/main/java/com/google/devtools/build/lib/analysis/Expander.java index 9dc224a29c..e22ac2c447 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/Expander.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/Expander.java @@ -73,6 +73,14 @@ public final class Expander { } /** + * Returns a new instance that also expands locations, passing {@link Options#EXEC_PATHS} to the + * underlying {@link LocationTemplateContext}. + */ + public Expander withExecLocations() { + return withLocations(Options.EXEC_PATHS); + } + + /** * Returns a new instance that also expands locations, passing the given location map, as well as * {@link Options#EXEC_PATHS} to the underlying {@link LocationTemplateContext}. */ diff --git a/src/main/java/com/google/devtools/build/lib/rules/extra/ExtraActionFactory.java b/src/main/java/com/google/devtools/build/lib/rules/extra/ExtraActionFactory.java index 47411e1a95..1eb434e0f7 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/extra/ExtraActionFactory.java +++ b/src/main/java/com/google/devtools/build/lib/rules/extra/ExtraActionFactory.java @@ -54,8 +54,7 @@ public final class ExtraActionFactory implements RuleConfiguredTargetFactory { List<String>outputTemplates = context.attributes().get("out_templates", Type.STRING_LIST); - String command = commandHelper.resolveCommandAndExpandLabels( - context.attributes().get("cmd", Type.STRING), "cmd", /*allowDataInLabel=*/true); + String command = context.attributes().get("cmd", Type.STRING); // This is a bit of a hack. We want to run the MakeVariableExpander first, so we expand $ on // variables that are expanded below with $$, which gets reverted to $ by the // MakeVariableExpander. This allows us to expand package-specific make variables in the @@ -67,7 +66,10 @@ public final class ExtraActionFactory implements RuleConfiguredTargetFactory { command = command.replace("$(output ", "$$(output "); ConfigurationMakeVariableContext makeVariableContext = new ConfigurationMakeVariableContext( context, context.getTarget().getPackage(), context.getConfiguration()); - command = context.getExpander(makeVariableContext).expand("cmd", command); + command = context + .getExpander(makeVariableContext) + .withDataExecLocations() + .expand("cmd", command); boolean requiresActionOutput = context.attributes().get("requires_action_output", Type.BOOLEAN); diff --git a/src/main/java/com/google/devtools/build/lib/rules/genrule/GenRuleBase.java b/src/main/java/com/google/devtools/build/lib/rules/genrule/GenRuleBase.java index d4aabb4bc9..d9104d36e6 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/genrule/GenRuleBase.java +++ b/src/main/java/com/google/devtools/build/lib/rules/genrule/GenRuleBase.java @@ -82,18 +82,6 @@ public abstract class GenRuleBase implements RuleConfiguredTargetFactory { } /** - * Returns an {@link Iterable} of {@link NestedSet}s, which will be added to the genrule's inputs - * using the {@link NestedSetBuilder#addTransitive} method. - * - * <p>GenRule implementations can override this method to better control what inputs are needed - * for specific command inputs. - */ - protected Iterable<NestedSet<Artifact>> getExtraInputArtifacts( - RuleContext ruleContext, String command) { - return ImmutableList.of(); - } - - /** * Returns {@code true} if the rule should be stamped. * * <p>Genrule implementations can set this based on the rule context, including by defining their @@ -153,17 +141,22 @@ public abstract class GenRuleBase implements RuleConfiguredTargetFactory { return null; } - String baseCommand = commandHelper.resolveCommandAndHeuristicallyExpandLabels( - ruleContext.attributes().get("cmd", Type.STRING), - "cmd", - ruleContext.attributes().get("heuristic_label_expansion", Type.BOOLEAN)); + String baseCommand = ruleContext.attributes().get("cmd", Type.STRING); + // Expand template variables and functions. + String command = ruleContext + .getExpander(new CommandResolverContext(ruleContext, resolvedSrcs, filesToBuild)) + .withExecLocations() + .expand("cmd", baseCommand); - // Adds the genrule environment setup script before the actual shell command - String command = String.format("source %s; %s", - ruleContext.getPrerequisiteArtifact("$genrule_setup", Mode.HOST).getExecPath(), - baseCommand); + // Heuristically expand things that look like labels. + if (ruleContext.attributes().get("heuristic_label_expansion", Type.BOOLEAN)) { + command = commandHelper.expandLabelsHeuristically(command); + } - command = resolveCommand(command, ruleContext, resolvedSrcs, filesToBuild); + // Add the genrule environment setup script before the actual shell command. + command = String.format("source %s; %s", + ruleContext.getPrerequisiteArtifact("$genrule_setup", Mode.HOST).getExecPath(), + command); String messageAttr = ruleContext.attributes().get("message", Type.STRING); String message = messageAttr.isEmpty() ? "Executing genrule" : messageAttr; @@ -207,10 +200,6 @@ public abstract class GenRuleBase implements RuleConfiguredTargetFactory { inputs.addTransitive(JavaHelper.getHostJavabaseInputs(ruleContext)); } - for (NestedSet<Artifact> extraInputs : getExtraInputArtifacts(ruleContext, baseCommand)) { - inputs.addTransitive(extraInputs); - } - if (isStampingEnabled(ruleContext)) { inputs.add(ruleContext.getAnalysisEnvironment().getStableWorkspaceStatusArtifact()); inputs.add(ruleContext.getAnalysisEnvironment().getVolatileWorkspaceStatusArtifact()); @@ -265,19 +254,6 @@ public abstract class GenRuleBase implements RuleConfiguredTargetFactory { } /** - * Resolves any variables, including make and genrule-specific variables, in the command and - * returns the expanded command. - * - * <p>GenRule implementations may override this method to perform additional expansions. - */ - protected String resolveCommand(String command, final RuleContext ruleContext, - final NestedSet<Artifact> resolvedSrcs, final NestedSet<Artifact> filesToBuild) { - return ruleContext - .getExpander(new CommandResolverContext(ruleContext, resolvedSrcs, filesToBuild)) - .expand("cmd", command); - } - - /** * Implementation of {@link ConfigurationMakeVariableContext} used to expand variables in a * genrule command string. */ |