Skip to content

Commit

Permalink
cquery --show_config_fragments: capture Make variables.
Browse files Browse the repository at this point in the history
Any Make variable "$(foo)" is equivalent to requiring
"--define foo".

This works for native rule logic and Starlark rules routing
through https://docs.bazel.build/versions/master/skylark/lib/ctx.html#expand_make_variables,
but not Starlark rules routing through
https://docs.bazel.build/versions/master/skylark/lib/ctx.html#var.

Supports #10613.
Added refactoring TODO in #11221.

PiperOrigin-RevId: 308632503
  • Loading branch information
gregestren authored and Copybara-Service committed Apr 27, 2020
1 parent e971d85 commit 31e4479
Show file tree
Hide file tree
Showing 9 changed files with 310 additions and 107 deletions.
33 changes: 30 additions & 3 deletions src/main/java/com/google/devtools/build/lib/analysis/Expander.java
Expand Up @@ -16,7 +16,9 @@
import com.google.common.collect.ImmutableCollection;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.ImmutableSortedSet;
import com.google.devtools.build.lib.actions.Artifact;
import com.google.devtools.build.lib.analysis.stringtemplate.Expansion;
import com.google.devtools.build.lib.analysis.stringtemplate.ExpansionException;
import com.google.devtools.build.lib.analysis.stringtemplate.TemplateContext;
import com.google.devtools.build.lib.analysis.stringtemplate.TemplateExpander;
Expand All @@ -25,6 +27,7 @@
import com.google.devtools.build.lib.shell.ShellUtils;
import java.util.ArrayList;
import java.util.List;
import java.util.TreeSet;
import javax.annotation.Nullable;

/**
Expand All @@ -35,6 +38,8 @@ public final class Expander {
private final RuleContext ruleContext;
private final TemplateContext templateContext;
@Nullable ImmutableMap<Label, ImmutableCollection<Artifact>> labelMap;
/* Which variables were looked up over this instance's lifetime? */
private final TreeSet<String> lookedUpVariables;

Expander(RuleContext ruleContext, TemplateContext templateContext) {
this(ruleContext, templateContext, /* labelMap= */ null);
Expand All @@ -44,9 +49,20 @@ public final class Expander {
RuleContext ruleContext,
TemplateContext templateContext,
@Nullable ImmutableMap<Label, ImmutableCollection<Artifact>> labelMap) {
this(ruleContext, templateContext, labelMap, /*lookedUpVariables=*/ null);
}

Expander(
RuleContext ruleContext,
TemplateContext templateContext,
@Nullable ImmutableMap<Label, ImmutableCollection<Artifact>> labelMap,
@Nullable TreeSet<String> lookedUpVariables) {
this.ruleContext = ruleContext;
this.templateContext = templateContext;
this.labelMap = labelMap;
// TODO(https://github.com/bazelbuild/bazel/issues/11221): Eliminate all methods that construct
// an Expander from an existing Expander. These make it hard to keep lookeduUpVariables correct.
this.lookedUpVariables = lookedUpVariables == null ? new TreeSet<>() : lookedUpVariables;
}

/**
Expand All @@ -57,7 +73,7 @@ private Expander withLocations(boolean execPaths, boolean allowData) {
TemplateContext newTemplateContext =
new LocationTemplateContext(
templateContext, ruleContext, labelMap, execPaths, allowData, false);
return new Expander(ruleContext, newTemplateContext, labelMap);
return new Expander(ruleContext, newTemplateContext, labelMap, lookedUpVariables);
}

/**
Expand Down Expand Up @@ -85,7 +101,7 @@ public Expander withExecLocations(
TemplateContext newTemplateContext =
new LocationTemplateContext(
templateContext, ruleContext, locations, true, false, windowsPath);
return new Expander(ruleContext, newTemplateContext);
return new Expander(ruleContext, newTemplateContext, labelMap, lookedUpVariables);
}

public Expander withExecLocations(ImmutableMap<Label, ImmutableCollection<Artifact>> locations) {
Expand Down Expand Up @@ -142,7 +158,9 @@ public String expand(String attributeName) {
*/
public String expand(@Nullable String attributeName, String expression) {
try {
return TemplateExpander.expand(expression, templateContext);
Expansion expansion = TemplateExpander.expand(expression, templateContext);
lookedUpVariables.addAll(expansion.lookedUpVariables());
return expansion.expansion();
} catch (ExpansionException e) {
if (attributeName == null) {
ruleContext.ruleError(e.getMessage());
Expand Down Expand Up @@ -216,4 +234,13 @@ public String expandSingleMakeVariable(String attrName, String expression) {
return expression;
}
}

/**
* Which variables were looked up over this {@link Expander}'s lifetime?
*
* <p>The returned set is guaranteed alphabetically ordered.
*/
public ImmutableSortedSet<String> lookedUpVariables() {
return ImmutableSortedSet.copyOf(lookedUpVariables);
}
}
Expand Up @@ -99,6 +99,7 @@
import com.google.devtools.build.lib.util.StringUtil;
import com.google.devtools.build.lib.vfs.FileSystemUtils;
import com.google.devtools.build.lib.vfs.PathFragment;
import java.util.ArrayList;
import java.util.Collection;
import java.util.HashMap;
import java.util.HashSet;
Expand Down Expand Up @@ -201,6 +202,7 @@ public ImmutableList<TransitiveInfoCollection> getFiles() {
@Nullable private final ToolchainCollection<ResolvedToolchainContext> toolchainContexts;
private final ConstraintSemantics<RuleContext> constraintSemantics;
private final ImmutableSet<String> requiredConfigFragments;
private final List<Expander> makeVariableExpanders = new ArrayList<>();

private ActionOwner actionOwner;
private final SymbolGenerator<ActionLookupValue.ActionLookupKey> actionOwnerSymbolGenerator;
Expand Down Expand Up @@ -1200,15 +1202,21 @@ public void initConfigurationMakeVariableContext(MakeVariableSupplier... makeVar
}

public Expander getExpander(TemplateContext templateContext) {
return new Expander(this, templateContext);
Expander expander = new Expander(this, templateContext);
makeVariableExpanders.add(expander);
return expander;
}

public Expander getExpander() {
return new Expander(this, getConfigurationMakeVariableContext());
Expander expander = new Expander(this, getConfigurationMakeVariableContext());
makeVariableExpanders.add(expander);
return expander;
}

public Expander getExpander(ImmutableMap<Label, ImmutableCollection<Artifact>> labelMap) {
return new Expander(this, getConfigurationMakeVariableContext(), labelMap);
Expander expander = new Expander(this, getConfigurationMakeVariableContext(), labelMap);
makeVariableExpanders.add(expander);
return expander;
}

/**
Expand Down Expand Up @@ -1250,8 +1258,26 @@ public ConstraintSemantics<RuleContext> getConstraintSemantics() {
return constraintSemantics;
}

public ImmutableSet<String> getRequiredConfigFragments() {
return requiredConfigFragments;
/**
* Returns the configuration fragments this rule uses.
*
* <p>Returned results are alphabetically ordered.
*/
public ImmutableSortedSet<String> getRequiredConfigFragments() {
ImmutableSortedSet.Builder<String> ans = ImmutableSortedSet.naturalOrder();
ans.addAll(requiredConfigFragments);
for (Expander makeVariableExpander : makeVariableExpanders) {
for (String makeVariable : makeVariableExpander.lookedUpVariables()) {
// User-defined make values may be set either in "--define foo=bar" or in a vardef in the
// rule's package. Both are equivalent for these purposes, since in both cases setting
// "--define foo=bar" impacts the rule's output.
if (getRule().getPackage().getMakeEnvironment().containsKey(makeVariable)
|| getConfiguration().getCommandLineBuildVariables().containsKey(makeVariable)) {
ans.add("--define:" + makeVariable);
}
}
}
return ans.build();
}

public Map<String, String> getTargetExecProperties() {
Expand Down
Expand Up @@ -11,4 +11,8 @@ filegroup(
java_library(
name = "stringtemplate",
srcs = glob(["*.java"]),
deps = [
"//third_party:auto_value",
"//third_party:guava",
],
)
@@ -0,0 +1,32 @@
// Copyright 2020 The Bazel Authors. All rights reserved.
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.
package com.google.devtools.build.lib.analysis.stringtemplate;

import com.google.auto.value.AutoValue;
import com.google.common.collect.ImmutableSet;

/**
* Holds the result of expanding a string with make variables: both the new (expanded) string and
* the set of variables that were expanded.
*/
@AutoValue
public abstract class Expansion {
public static Expansion create(String expansion, ImmutableSet<String> lookedUpVariables) {
return new AutoValue_Expansion(expansion, lookedUpVariables);
}

public abstract String expansion();

public abstract ImmutableSet<String> lookedUpVariables();
}
Expand Up @@ -13,6 +13,8 @@
// limitations under the License.
package com.google.devtools.build.lib.analysis.stringtemplate;

import com.google.common.collect.ImmutableSet;

/**
* Simple string template expansion. String templates consist of text interspersed with
* <code>$(variable)</code> or <code>$(function value)</code> references, which are replaced by
Expand All @@ -29,6 +31,16 @@ private TemplateExpander(String expression) {
offset = 0;
}

/**
* If the string contains a single variable, return the expansion of that variable. Otherwise,
* return null.
*/
public static String expandSingleVariable(String expression, TemplateContext context)
throws ExpansionException {
String var = new TemplateExpander(expression).getSingleVariable();
return (var != null) ? context.lookupVariable(var) : null;
}

/**
* Expands all references to template variables embedded within string "expr", using the provided
* {@link TemplateContext} instance to expand individual variables.
Expand All @@ -38,26 +50,16 @@ private TemplateExpander(String expression) {
* @return the expansion of "expr"
* @throws ExpansionException if "expr" contained undefined or ill-formed variables references
*/
public static String expand(String expression, TemplateContext context)
public static Expansion expand(String expression, TemplateContext context)
throws ExpansionException {
if (expression.indexOf('$') < 0) {
return expression;
return Expansion.create(expression, ImmutableSet.of());
}
return expand(expression, context, 0);
}

/**
* If the string contains a single variable, return the expansion of that variable.
* Otherwise, return null.
*/
public static String expandSingleVariable(String expression, TemplateContext context)
throws ExpansionException {
String var = new TemplateExpander(expression).getSingleVariable();
return (var != null) ? context.lookupVariable(var) : null;
}

// Helper method for counting recursion depth.
private static String expand(String expression, TemplateContext context, int depth)
private static Expansion expand(String expression, TemplateContext context, int depth)
throws ExpansionException {
if (depth > 10) { // plenty!
throw new ExpansionException(
Expand All @@ -66,8 +68,9 @@ private static String expand(String expression, TemplateContext context, int dep
return new TemplateExpander(expression).expand(context, depth);
}

private String expand(TemplateContext context, int depth) throws ExpansionException {
private Expansion expand(TemplateContext context, int depth) throws ExpansionException {
StringBuilder result = new StringBuilder();
ImmutableSet.Builder<String> lookedUpVariables = ImmutableSet.builder();
while (offset < length) {
char c = buffer[offset];
if (c == '$') { // variable
Expand All @@ -82,17 +85,21 @@ private String expand(TemplateContext context, int depth) throws ExpansionExcept
int spaceIndex = var.indexOf(' ');
if (spaceIndex < 0) {
String value = context.lookupVariable(var);
lookedUpVariables.add(var);
// To prevent infinite recursion for the ignored shell variables
if (!value.equals(var)) {
// recursively expand using Make's ":=" semantics:
value = expand(value, context, depth + 1);
Expansion expansion = expand(value, context, depth + 1);
value = expansion.expansion();
lookedUpVariables.addAll(expansion.lookedUpVariables());
}
result.append(value);
} else {
String name = var.substring(0, spaceIndex);
// Trim the string to remove leading and trailing whitespace.
String param = var.substring(spaceIndex + 1).trim();
String value = context.lookupFunction(name, param);
lookedUpVariables.add(name);
result.append(value);
}
}
Expand All @@ -101,7 +108,7 @@ private String expand(TemplateContext context, int depth) throws ExpansionExcept
}
offset++;
}
return result.toString();
return Expansion.create(result.toString(), lookedUpVariables.build());
}

/**
Expand Down
Expand Up @@ -165,23 +165,24 @@ static class LazyCommandLineExpansion extends LazyString {
public String toString() {
try {
return TemplateExpander.expand(
template,
new TemplateContext() {
@Override
public String lookupVariable(String name)
throws ExpansionException {
CharSequence value = variableValues.get(name);
if (value == null) {
throw new ExpansionException(String.format("$(%s) not defined", name));
}
return value.toString();
}

@Override
public String lookupFunction(String name, String param) throws ExpansionException {
throw new ExpansionException(String.format("$(%s) not defined", name));
}
});
template,
new TemplateContext() {
@Override
public String lookupVariable(String name) throws ExpansionException {
CharSequence value = variableValues.get(name);
if (value == null) {
throw new ExpansionException(String.format("$(%s) not defined", name));
}
return value.toString();
}

@Override
public String lookupFunction(String name, String param)
throws ExpansionException {
throw new ExpansionException(String.format("$(%s) not defined", name));
}
})
.expansion();
} catch (ExpansionException e) {
// Squeelch. We don't throw this exception in the lookupMakeVariable implementation above,
// and we can't report it here anyway, because this code will typically execute in the
Expand Down

0 comments on commit 31e4479

Please sign in to comment.