Automated rollback of commit 7c2174fea2b50af8fbf10367835b820af29700f6.
*** Reason for rollback ***
Breaks the nightly
*** Original change description ***
bazel syntax: delete BuiltinFunction.Factory
The mechanism existed so that functions like glob could
be closures over the PackageContext, which is now accessible
as a thread-local value.
Also:
- replace FuncallExpression with Location in various calls
All of these are for EvalException and could thus be deleted.
- document element types for StarlarkList<?> parameters
PiperOrigin-RevId: 276738614
diff --git a/src/main/java/com/google/devtools/build/lib/packages/PackageFactory.java b/src/main/java/com/google/devtools/build/lib/packages/PackageFactory.java
index 89d4f1e..82bcc56 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/PackageFactory.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/PackageFactory.java
@@ -540,43 +540,47 @@
+ " excluded).")
},
documented = false,
- useLocation = true,
+ useAst = true,
useStarlarkThread = true)
- private static final BuiltinFunction globFunction =
- new BuiltinFunction("glob") {
- public SkylarkList<String> invoke(
- SkylarkList<?> include, // <String>
- SkylarkList<?> exclude, // <String>
- Integer excludeDirectories,
- Object allowEmpty,
- Location loc,
- StarlarkThread thread)
- throws EvalException, InterruptedException {
- return callGlob(
- getContext(thread, loc),
- include,
- exclude,
- excludeDirectories != 0,
- allowEmpty,
- loc,
- thread);
+ private static final BuiltinFunction.Factory newGlobFunction =
+ new BuiltinFunction.Factory("glob") {
+ public BuiltinFunction create(final PackageContext originalContext) {
+ return new BuiltinFunction("glob", this) {
+ public SkylarkList invoke(
+ SkylarkList include,
+ SkylarkList exclude,
+ Integer excludeDirectories,
+ Object allowEmpty,
+ FuncallExpression ast,
+ StarlarkThread thread)
+ throws EvalException, ConversionException, InterruptedException {
+ return callGlob(
+ originalContext,
+ include,
+ exclude,
+ excludeDirectories != 0,
+ allowEmpty,
+ ast,
+ thread);
+ }
+ };
}
};
- static SkylarkList<String> callGlob(
+ static SkylarkList<Object> callGlob(
@Nullable PackageContext originalContext,
- SkylarkList<?> include, // <String>
- SkylarkList<?> exclude, // <String>
+ Object include,
+ Object exclude,
boolean excludeDirs,
Object allowEmptyArgument,
- Location loc,
+ FuncallExpression ast,
StarlarkThread thread)
throws EvalException, ConversionException, InterruptedException {
// Skylark build extensions need to get the PackageContext from the StarlarkThread;
// async glob functions cannot do the same because the StarlarkThread is not thread safe.
PackageContext context;
if (originalContext == null) {
- context = getContext(thread, loc);
+ context = getContext(thread, ast.getLocation());
} else {
context = originalContext;
}
@@ -592,7 +596,8 @@
allowEmpty = (Boolean) allowEmptyArgument;
} else {
throw new EvalException(
- loc, "expected boolean for argument `allow_empty`, got `" + allowEmptyArgument + "`");
+ ast.getLocation(),
+ "expected boolean for argument `allow_empty`, got `" + allowEmptyArgument + "`");
}
try {
@@ -605,11 +610,11 @@
Joiner.on(", ").join(includes),
excludes.isEmpty() ? "" : " - [" + Joiner.on(", ").join(excludes) + "]",
e.getMessage());
- context.eventHandler.handle(Event.error(loc, errorMessage));
+ context.eventHandler.handle(Event.error(ast.getLocation(), errorMessage));
context.pkgBuilder.setIOExceptionAndMessage(e, errorMessage);
matches = ImmutableList.of();
} catch (BadGlobException e) {
- throw new EvalException(loc, e.getMessage());
+ throw new EvalException(ast.getLocation(), e.getMessage());
}
return MutableList.copyOf(thread, matches);
@@ -633,22 +638,32 @@
@Param(name = "name", type = String.class, doc = "The name of the target."),
},
documented = false,
- useLocation = true,
+ useAst = true,
useStarlarkThread = true)
- private static final BuiltinFunction existingRuleFunction =
- new BuiltinFunction("existing_rule") {
- public Object invoke(String name, Location loc, StarlarkThread thread)
- throws EvalException {
- return callExistingRule(name, loc, thread);
+ private static final BuiltinFunction.Factory newExistingRuleFunction =
+ new BuiltinFunction.Factory("existing_rule") {
+ public BuiltinFunction create(final PackageContext originalContext) {
+ return new BuiltinFunction("existing_rule", this) {
+ public Object invoke(String name, FuncallExpression ast, StarlarkThread thread)
+ throws EvalException {
+ return callExistingRule(name, ast, thread);
+ }
+ };
}
};
- static Object callExistingRule(String name, Location loc, StarlarkThread thread)
+ static Object callExistingRule(String name, FuncallExpression ast, StarlarkThread thread)
throws EvalException {
- PackageContext context = getContext(thread, loc);
+
+ PackageContext context = getContext(thread, ast.getLocation());
Target target = context.pkgBuilder.getTarget(name);
- SkylarkDict<String, Object> rule = targetDict(target, loc, thread);
- return rule != null ? rule : Runtime.NONE;
+ SkylarkDict<String, Object> rule = targetDict(target, ast.getLocation(), thread);
+
+ if (rule != null) {
+ return rule;
+ }
+
+ return Runtime.NONE;
}
/**
@@ -657,7 +672,6 @@
*/
@SkylarkSignature(
name = "existing_rules",
- returnType = SkylarkDict.class,
doc =
"Returns a dictionary containing all the targets instantiated so far. The map key is the "
+ "name of the target. The map value is equivalent to the <code>existing_rule</code> "
@@ -665,20 +679,26 @@
+ ""
+ "<p><i>Note: If possible, avoid using this function. It makes BUILD files brittle "
+ "and order-dependent.</i>",
- useLocation = true,
+ useAst = true,
useStarlarkThread = true)
- private static final BuiltinFunction existingRulesFunction =
- new BuiltinFunction("existing_rules") {
- public SkylarkDict<String, SkylarkDict<String, Object>> invoke(
- Location loc, StarlarkThread thread) throws EvalException {
- return callExistingRules(loc, thread);
+ private static final BuiltinFunction.Factory newExistingRulesFunction =
+ new BuiltinFunction.Factory("existing_rules") {
+ public BuiltinFunction create(final PackageContext originalContext) {
+ return new BuiltinFunction("existing_rules", this) {
+ public SkylarkDict<String, SkylarkDict<String, Object>> invoke(
+ FuncallExpression ast, StarlarkThread thread) throws EvalException {
+ return callExistingRules(ast, thread);
+ }
+ };
}
};
static SkylarkDict<String, SkylarkDict<String, Object>> callExistingRules(
- Location loc, StarlarkThread thread) throws EvalException {
- PackageContext context = getContext(thread, loc);
+ FuncallExpression ast, StarlarkThread thread) throws EvalException {
+ PackageContext context = getContext(thread, ast.getLocation());
Collection<Target> targets = context.pkgBuilder.getTargets();
+ Location loc = ast.getLocation();
+
SkylarkDict<String, SkylarkDict<String, Object>> rules = SkylarkDict.of(thread);
for (Target t : targets) {
if (t instanceof Rule) {
@@ -692,8 +712,8 @@
}
/**
- * Function value implementing "environment_group" in the specified package context. Syntax is as
- * follows:
+ * Returns a function value implementing "environment_group" in the specified package context.
+ * Syntax is as follows:
*
* <pre>{@code
* environment_group(
@@ -736,43 +756,42 @@
doc = "A list of Labels.")
}, // TODO(bazel-team): document what that is
documented = false,
- useLocation = true,
- useStarlarkThread = true)
- private static final BuiltinFunction environmentGroupFunction =
- new BuiltinFunction("environment_group") {
- public Runtime.NoneType invoke(
- String name,
- SkylarkList<?> environmentsList, // <Label>
- SkylarkList<?> defaultsList, // <Label>
- Location loc,
- StarlarkThread thread)
- throws EvalException {
- PackageContext context = getContext(thread, loc);
- List<Label> environments =
- BuildType.LABEL_LIST.convert(
- environmentsList,
- "'environment_group argument'",
- context.pkgBuilder.getBuildFileLabel());
- List<Label> defaults =
- BuildType.LABEL_LIST.convert(
- defaultsList,
- "'environment_group argument'",
- context.pkgBuilder.getBuildFileLabel());
+ useLocation = true)
+ private static final BuiltinFunction.Factory newEnvironmentGroupFunction =
+ new BuiltinFunction.Factory("environment_group") {
+ public BuiltinFunction create(final PackageContext context) {
+ return new BuiltinFunction("environment_group", this) {
+ public Runtime.NoneType invoke(
+ String name, SkylarkList environmentsList, SkylarkList defaultsList, Location loc)
+ throws EvalException, ConversionException {
+ List<Label> environments =
+ BuildType.LABEL_LIST.convert(
+ environmentsList,
+ "'environment_group argument'",
+ context.pkgBuilder.getBuildFileLabel());
+ List<Label> defaults =
+ BuildType.LABEL_LIST.convert(
+ defaultsList,
+ "'environment_group argument'",
+ context.pkgBuilder.getBuildFileLabel());
- if (environments.isEmpty()) {
- throw new EvalException(
- location, "environment group " + name + " must contain at least one environment");
- }
- try {
- context.pkgBuilder.addEnvironmentGroup(
- name, environments, defaults, context.eventHandler, loc);
- return Runtime.NONE;
- } catch (LabelSyntaxException e) {
- throw new EvalException(
- loc, "environment group has invalid name: " + name + ": " + e.getMessage());
- } catch (Package.NameConflictException e) {
- throw new EvalException(loc, e.getMessage());
- }
+ if (environments.isEmpty()) {
+ throw new EvalException(
+ location,
+ "environment group " + name + " must contain at least one environment");
+ }
+ try {
+ context.pkgBuilder.addEnvironmentGroup(
+ name, environments, defaults, context.eventHandler, loc);
+ return Runtime.NONE;
+ } catch (LabelSyntaxException e) {
+ throw new EvalException(
+ loc, "environment group has invalid name: " + name + ": " + e.getMessage());
+ } catch (Package.NameConflictException e) {
+ throw new EvalException(loc, e.getMessage());
+ }
+ }
+ };
}
};
@@ -806,25 +825,33 @@
doc = "A list of strings specifying the licenses used in the exported code.")
},
documented = false,
- useLocation = true,
+ useAst = true,
useStarlarkThread = true)
- private static final BuiltinFunction exportsFilesFunction =
- new BuiltinFunction("exports_files") {
- public Runtime.NoneType invoke(
- SkylarkList<?> srcs, // <String>
- Object visibility,
- Object licenses,
- Location loc,
- StarlarkThread thread)
- throws EvalException {
- return callExportsFiles(srcs, visibility, licenses, loc, thread);
+ private static final BuiltinFunction.Factory newExportsFilesFunction =
+ new BuiltinFunction.Factory("exports_files") {
+ public BuiltinFunction create() {
+ return new BuiltinFunction("exports_files", this) {
+ public Runtime.NoneType invoke(
+ SkylarkList srcs,
+ Object visibility,
+ Object licenses,
+ FuncallExpression ast,
+ StarlarkThread thread)
+ throws EvalException, ConversionException {
+ return callExportsFiles(srcs, visibility, licenses, ast, thread);
+ }
+ };
}
};
static Runtime.NoneType callExportsFiles(
- Object srcs, Object visibilityO, Object licensesO, Location loc, StarlarkThread thread)
+ Object srcs,
+ Object visibilityO,
+ Object licensesO,
+ FuncallExpression ast,
+ StarlarkThread thread)
throws EvalException, ConversionException {
- Package.Builder pkgBuilder = getContext(thread, loc).pkgBuilder;
+ Package.Builder pkgBuilder = getContext(thread, ast.getLocation()).pkgBuilder;
List<String> files = Type.STRING_LIST.convert(srcs, "'exports_files' operand");
RuleVisibility visibility;
@@ -836,7 +863,7 @@
"'exports_files' operand",
pkgBuilder.getBuildFileLabel()));
} catch (EvalException e) {
- throw new EvalException(loc, e.getMessage());
+ throw new EvalException(ast.getLocation(), e.getMessage());
}
// TODO(bazel-team): is licenses plural or singular?
License license = BuildType.LICENSE.convertOptional(licensesO, "'exports_files' operand");
@@ -844,21 +871,20 @@
for (String file : files) {
String errorMessage = LabelValidator.validateTargetName(file);
if (errorMessage != null) {
- throw new EvalException(loc, errorMessage);
+ throw new EvalException(ast.getLocation(), errorMessage);
}
try {
- InputFile inputFile = pkgBuilder.createInputFile(file, loc);
+ InputFile inputFile = pkgBuilder.createInputFile(file, ast.getLocation());
if (inputFile.isVisibilitySpecified()
&& inputFile.getVisibility() != visibility) {
- throw new EvalException(
- loc,
- String.format(
- "visibility for exported file '%s' declared twice", inputFile.getName()));
+ throw new EvalException(ast.getLocation(),
+ String.format("visibility for exported file '%s' declared twice",
+ inputFile.getName()));
}
if (license != null && inputFile.isLicenseSpecified()) {
- throw new EvalException(
- loc,
- String.format("licenses for exported file '%s' declared twice", inputFile.getName()));
+ throw new EvalException(ast.getLocation(),
+ String.format("licenses for exported file '%s' declared twice",
+ inputFile.getName()));
}
// See if we should check third-party licenses: first checking for any hard-coded policy,
@@ -878,63 +904,54 @@
&& license == null
&& !pkgBuilder.getDefaultLicense().isSpecified()
&& RuleClass.isThirdPartyPackage(pkgBuilder.getPackageIdentifier())) {
- throw new EvalException(
- loc,
- "third-party file '"
- + inputFile.getName()
- + "' lacks a license declaration "
- + "with one of the following types: notice, reciprocal, permissive, "
- + "restricted, unencumbered, by_exception_only");
+ throw new EvalException(ast.getLocation(),
+ "third-party file '" + inputFile.getName() + "' lacks a license declaration "
+ + "with one of the following types: notice, reciprocal, permissive, "
+ + "restricted, unencumbered, by_exception_only");
}
pkgBuilder.setVisibilityAndLicense(inputFile, visibility, license);
} catch (Package.Builder.GeneratedLabelConflict e) {
- throw new EvalException(loc, e.getMessage());
+ throw new EvalException(ast.getLocation(), e.getMessage());
}
}
return Runtime.NONE;
}
/**
- * Returns a function-value implementing "licenses" in the specified package context.
+ * Returns a function-value implementing "licenses" in the specified package
+ * context.
* TODO(bazel-team): Remove in favor of package.licenses.
*/
- @SkylarkSignature(
- name = "licenses",
- returnType = Runtime.NoneType.class,
+ @SkylarkSignature(name = "licenses", returnType = Runtime.NoneType.class,
doc = "Declare the license(s) for the code in the current package.",
parameters = {
- @Param(
- name = "license_strings",
- type = SkylarkList.class,
- generic1 = String.class,
- doc = "A list of strings, the names of the licenses used.")
- },
- documented = false,
- useStarlarkThread = true,
- useLocation = true)
- private static final BuiltinFunction licensesFunction =
- new BuiltinFunction("licenses") {
- public Runtime.NoneType invoke(
- SkylarkList<?> licensesList, // list of license strings
- Location loc,
- StarlarkThread thread)
- throws EvalException {
- PackageContext context = getContext(thread, loc);
- try {
- License license = BuildType.LICENSE.convert(licensesList, "'licenses' operand");
- context.pkgBuilder.setDefaultLicense(license);
- } catch (ConversionException e) {
- context.eventHandler.handle(Event.error(loc, e.getMessage()));
- context.pkgBuilder.setContainsErrors();
- }
- return Runtime.NONE;
+ @Param(name = "license_strings", type = SkylarkList.class, generic1 = String.class,
+ doc = "A list of strings, the names of the licenses used.")},
+ documented = false, useLocation = true)
+ private static final BuiltinFunction.Factory newLicensesFunction =
+ new BuiltinFunction.Factory("licenses") {
+ public BuiltinFunction create(final PackageContext context) {
+ return new BuiltinFunction("licenses", this) {
+ public Runtime.NoneType invoke(SkylarkList licensesList, Location loc) {
+ try {
+ License license = BuildType.LICENSE.convert(licensesList, "'licenses' operand");
+ context.pkgBuilder.setDefaultLicense(license);
+ } catch (ConversionException e) {
+ context.eventHandler.handle(Event.error(loc, e.getMessage()));
+ context.pkgBuilder.setContainsErrors();
+ }
+ return Runtime.NONE;
+ }
+ };
}
};
/** Returns a function-value implementing "distribs" in the specified package context. */
// TODO(bazel-team): Remove in favor of package.distribs.
- // TODO(bazel-team): share functions with the native package. Requires unifying the List types.
+ // TODO(bazel-team): Remove all these new*Function-s and/or have static functions
+ // that consult the context dynamically via getContext(thread, loc) since we have that,
+ // and share the functions with the native package... which requires unifying the List types.
@SkylarkSignature(
name = "distribs",
returnType = Runtime.NoneType.class,
@@ -943,23 +960,23 @@
@Param(name = "distribution_strings", type = Object.class, doc = "The distributions.")
},
documented = false,
- useStarlarkThread = true,
useLocation = true)
- private static final BuiltinFunction distribsFunction =
- new BuiltinFunction("distribs") {
- public Runtime.NoneType invoke(Object object, Location loc, StarlarkThread thread)
- throws EvalException {
- PackageContext context = getContext(thread, loc);
-
- try {
- Set<DistributionType> distribs =
- BuildType.DISTRIBUTIONS.convert(object, "'distribs' operand");
- context.pkgBuilder.setDefaultDistribs(distribs);
- } catch (ConversionException e) {
- context.eventHandler.handle(Event.error(loc, e.getMessage()));
- context.pkgBuilder.setContainsErrors();
- }
- return Runtime.NONE;
+ private static final BuiltinFunction.Factory newDistribsFunction =
+ new BuiltinFunction.Factory("distribs") {
+ public BuiltinFunction create(final PackageContext context) {
+ return new BuiltinFunction("distribs", this) {
+ public Runtime.NoneType invoke(Object object, Location loc) {
+ try {
+ Set<DistributionType> distribs =
+ BuildType.DISTRIBUTIONS.convert(object, "'distribs' operand");
+ context.pkgBuilder.setDefaultDistribs(distribs);
+ } catch (ConversionException e) {
+ context.eventHandler.handle(Event.error(loc, e.getMessage()));
+ context.pkgBuilder.setContainsErrors();
+ }
+ return Runtime.NONE;
+ }
+ };
}
};
@@ -993,18 +1010,22 @@
doc = "A list of Label specifiers for the files to include.")
},
documented = false,
- useLocation = true,
+ useAst = true,
useStarlarkThread = true)
- private static final BuiltinFunction packageGroupFunction =
- new BuiltinFunction("package_group") {
- public Runtime.NoneType invoke(
- String name,
- SkylarkList<?> packages, // <String>
- SkylarkList<?> includes, // <Label>
- Location loc,
- StarlarkThread thread)
- throws EvalException {
- return callPackageFunction(name, packages, includes, loc, thread);
+ private static final BuiltinFunction.Factory newPackageGroupFunction =
+ new BuiltinFunction.Factory("package_group") {
+ public BuiltinFunction create() {
+ return new BuiltinFunction("package_group", this) {
+ public Runtime.NoneType invoke(
+ String name,
+ SkylarkList packages,
+ SkylarkList includes,
+ FuncallExpression ast,
+ StarlarkThread thread)
+ throws EvalException, ConversionException {
+ return callPackageFunction(name, packages, includes, ast, thread);
+ }
+ };
}
};
@@ -1165,9 +1186,9 @@
}
static Runtime.NoneType callPackageFunction(
- String name, Object packagesO, Object includesO, Location loc, StarlarkThread thread)
+ String name, Object packagesO, Object includesO, FuncallExpression ast, StarlarkThread thread)
throws EvalException, ConversionException {
- PackageContext context = getContext(thread, loc);
+ PackageContext context = getContext(thread, ast.getLocation());
List<String> packages = Type.STRING_LIST.convert(
packagesO, "'package_group.packages argument'");
@@ -1175,13 +1196,14 @@
"'package_group.includes argument'", context.pkgBuilder.getBuildFileLabel());
try {
- context.pkgBuilder.addPackageGroup(name, packages, includes, context.eventHandler, loc);
+ context.pkgBuilder.addPackageGroup(name, packages, includes, context.eventHandler,
+ ast.getLocation());
return Runtime.NONE;
} catch (LabelSyntaxException e) {
- throw new EvalException(
- loc, "package group has invalid name: " + name + ": " + e.getMessage());
+ throw new EvalException(ast.getLocation(),
+ "package group has invalid name: " + name + ": " + e.getMessage());
} catch (Package.NameConflictException e) {
- throw new EvalException(loc, e.getMessage());
+ throw new EvalException(ast.getLocation(), e.getMessage());
}
}
@@ -1221,13 +1243,13 @@
@Override
public Object call(Object[] arguments, FuncallExpression ast, StarlarkThread thread)
throws EvalException {
- Location loc = ast.getLocation();
- Package.Builder pkgBuilder = getContext(thread, loc).pkgBuilder;
+ Package.Builder pkgBuilder = getContext(thread, ast.getLocation()).pkgBuilder;
// Validate parameter list
if (pkgBuilder.isPackageFunctionUsed()) {
- throw new EvalException(loc, "'package' can only be used once per BUILD file");
+ throw new EvalException(
+ ast.getLocation(), "'package' can only be used once per BUILD file");
}
pkgBuilder.setPackageFunctionUsed();
@@ -1238,13 +1260,13 @@
Object value = arguments[i];
if (value != null) {
foundParameter = true;
- argumentSpecifiers[i].convertAndProcess(pkgBuilder, loc, value);
+ argumentSpecifiers[i].convertAndProcess(pkgBuilder, ast.getLocation(), value);
}
}
if (!foundParameter) {
throw new EvalException(
- loc, "at least one argument must be given to the 'package' function");
+ ast.getLocation(), "at least one argument must be given to the 'package' function");
}
return Runtime.NONE;
@@ -1618,7 +1640,8 @@
return StructProvider.STRUCT.create(builder.build(), "no native function or rule '%s'");
}
- private void populateEnvironment(ImmutableMap.Builder<String, Object> env) {
+ private void populateEnvironment(
+ ImmutableMap.Builder<String, Object> env, PackageContext context) {
// TODO(bazel-team): remove the naked functions that are redundant with the nativeModule,
// or if not possible, at least make them straight copies from the native module variant.
// or better, use a common StarlarkThread.Frame for these common bindings
@@ -1636,17 +1659,17 @@
}
env.putAll(BazelLibrary.GLOBALS.getBindings());
- env.put("distribs", distribsFunction);
- env.put("glob", globFunction);
- env.put("licenses", licensesFunction);
- env.put("exports_files", exportsFilesFunction);
- env.put("package_group", packageGroupFunction);
+ env.put("distribs", newDistribsFunction.apply(context));
+ env.put("glob", newGlobFunction.apply(context));
+ env.put("licenses", newLicensesFunction.apply(context));
+ env.put("exports_files", newExportsFilesFunction.apply());
+ env.put("package_group", newPackageGroupFunction.apply());
env.put("package", newPackageFunction(packageArguments));
env.put("package_name", packageNameFunction);
env.put("repository_name", repositoryNameFunction);
- env.put("environment_group", environmentGroupFunction);
- env.put("existing_rule", existingRuleFunction);
- env.put("existing_rules", existingRulesFunction);
+ env.put("environment_group", newEnvironmentGroupFunction.apply(context));
+ env.put("existing_rule", newExistingRuleFunction.apply(context));
+ env.put("existing_rules", newExistingRulesFunction.apply(context));
env.putAll(ruleFunctions);
for (EnvironmentExtension ext : environmentExtensions) {
@@ -1707,9 +1730,12 @@
.setWorkspaceName(workspaceName)
.setRepositoryMapping(repositoryMapping);
+ // Stuff that closes over the package context:
+ PackageContext context = new PackageContext(pkgBuilder, globber, eventHandler);
+
// environment
ImmutableMap.Builder<String, Object> env = ImmutableMap.builder();
- populateEnvironment(env);
+ populateEnvironment(env, context);
try (Mutability mutability = Mutability.create("package %s", packageId)) {
StarlarkThread thread =
@@ -1723,8 +1749,7 @@
// TODO(adonovan): save this as a field in BazelSkylarkContext.
// It needn't be a third thread-local.
- thread.setThreadLocal(
- PackageContext.class, new PackageContext(pkgBuilder, globber, eventHandler));
+ thread.setThreadLocal(PackageContext.class, context);
new BazelStarlarkContext(
ruleClassProvider.getToolsRepository(),
diff --git a/src/main/java/com/google/devtools/build/lib/packages/SkylarkNativeModule.java b/src/main/java/com/google/devtools/build/lib/packages/SkylarkNativeModule.java
index ad5d8f4..f818957 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/SkylarkNativeModule.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/SkylarkNativeModule.java
@@ -19,28 +19,17 @@
import com.google.devtools.build.lib.packages.Type.ConversionException;
import com.google.devtools.build.lib.skylarkbuildapi.SkylarkNativeModuleApi;
import com.google.devtools.build.lib.syntax.EvalException;
+import com.google.devtools.build.lib.syntax.FuncallExpression;
import com.google.devtools.build.lib.syntax.Runtime;
import com.google.devtools.build.lib.syntax.SkylarkDict;
import com.google.devtools.build.lib.syntax.SkylarkList;
import com.google.devtools.build.lib.syntax.SkylarkUtils;
import com.google.devtools.build.lib.syntax.StarlarkThread;
-/** The Skylark native module. */
-// TODO(laurentlb): Some definitions are duplicated from PackageFactory.
-// This class defines:
-// native.existing_rule
-// native.existing_rules
-// native.exports_files -- also global
-// native.glob -- also global
-// native.package_group -- also global
-// native.package_name
-// native.repository_name
-//
-// PackageFactory also defines:
-// distribs -- hidden?
-// licenses -- hidden?
-// package -- global
-// environment_group -- hidden?
+/**
+ * A class for the Skylark native module. TODO(laurentlb): Some definitions are duplicated from
+ * PackageFactory.
+ */
public class SkylarkNativeModule implements SkylarkNativeModuleApi {
@Override
@@ -49,23 +38,23 @@
SkylarkList<?> exclude,
Integer excludeDirectories,
Object allowEmpty,
- Location loc,
+ FuncallExpression ast,
StarlarkThread thread)
throws EvalException, ConversionException, InterruptedException {
- SkylarkUtils.checkLoadingPhase(thread, "native.glob", loc);
+ SkylarkUtils.checkLoadingPhase(thread, "native.glob", ast.getLocation());
try {
return PackageFactory.callGlob(
- null, include, exclude, excludeDirectories != 0, allowEmpty, loc, thread);
+ null, include, exclude, excludeDirectories != 0, allowEmpty, ast, thread);
} catch (IllegalArgumentException e) {
- throw new EvalException(loc, "illegal argument in call to glob", e);
+ throw new EvalException(ast.getLocation(), "illegal argument in call to glob", e);
}
}
@Override
- public Object existingRule(String name, Location loc, StarlarkThread thread)
+ public Object existingRule(String name, FuncallExpression ast, StarlarkThread thread)
throws EvalException, InterruptedException {
- SkylarkUtils.checkLoadingOrWorkspacePhase(thread, "native.existing_rule", loc);
- return PackageFactory.callExistingRule(name, loc, thread);
+ SkylarkUtils.checkLoadingOrWorkspacePhase(thread, "native.existing_rule", ast.getLocation());
+ return PackageFactory.callExistingRule(name, ast, thread);
}
/*
@@ -74,9 +63,9 @@
*/
@Override
public SkylarkDict<String, SkylarkDict<String, Object>> existingRules(
- Location loc, StarlarkThread thread) throws EvalException, InterruptedException {
- SkylarkUtils.checkLoadingOrWorkspacePhase(thread, "native.existing_rules", loc);
- return PackageFactory.callExistingRules(loc, thread);
+ FuncallExpression ast, StarlarkThread thread) throws EvalException, InterruptedException {
+ SkylarkUtils.checkLoadingOrWorkspacePhase(thread, "native.existing_rules", ast.getLocation());
+ return PackageFactory.callExistingRules(ast, thread);
}
@Override
@@ -84,26 +73,30 @@
String name,
SkylarkList<?> packages,
SkylarkList<?> includes,
- Location loc,
+ FuncallExpression ast,
StarlarkThread thread)
throws EvalException {
- SkylarkUtils.checkLoadingPhase(thread, "native.package_group", loc);
- return PackageFactory.callPackageFunction(name, packages, includes, loc, thread);
+ SkylarkUtils.checkLoadingPhase(thread, "native.package_group", ast.getLocation());
+ return PackageFactory.callPackageFunction(name, packages, includes, ast, thread);
}
@Override
public Runtime.NoneType exportsFiles(
- SkylarkList<?> srcs, Object visibility, Object licenses, Location loc, StarlarkThread thread)
+ SkylarkList<?> srcs,
+ Object visibility,
+ Object licenses,
+ FuncallExpression ast,
+ StarlarkThread thread)
throws EvalException {
- SkylarkUtils.checkLoadingPhase(thread, "native.exports_files", loc);
- return PackageFactory.callExportsFiles(srcs, visibility, licenses, loc, thread);
+ SkylarkUtils.checkLoadingPhase(thread, "native.exports_files", ast.getLocation());
+ return PackageFactory.callExportsFiles(srcs, visibility, licenses, ast, thread);
}
@Override
- public String packageName(Location loc, StarlarkThread thread) throws EvalException {
- SkylarkUtils.checkLoadingPhase(thread, "native.package_name", loc);
+ public String packageName(FuncallExpression ast, StarlarkThread thread) throws EvalException {
+ SkylarkUtils.checkLoadingPhase(thread, "native.package_name", ast.getLocation());
PackageIdentifier packageId =
- PackageFactory.getContext(thread, loc).getBuilder().getPackageIdentifier();
+ PackageFactory.getContext(thread, ast.getLocation()).getBuilder().getPackageIdentifier();
return packageId.getPackageFragment().getPathString();
}
diff --git a/src/main/java/com/google/devtools/build/lib/skylarkbuildapi/SkylarkNativeModuleApi.java b/src/main/java/com/google/devtools/build/lib/skylarkbuildapi/SkylarkNativeModuleApi.java
index 228b838..c5e6091 100644
--- a/src/main/java/com/google/devtools/build/lib/skylarkbuildapi/SkylarkNativeModuleApi.java
+++ b/src/main/java/com/google/devtools/build/lib/skylarkbuildapi/SkylarkNativeModuleApi.java
@@ -20,6 +20,7 @@
import com.google.devtools.build.lib.skylarkinterface.SkylarkModule;
import com.google.devtools.build.lib.skylarkinterface.SkylarkModuleCategory;
import com.google.devtools.build.lib.syntax.EvalException;
+import com.google.devtools.build.lib.syntax.FuncallExpression;
import com.google.devtools.build.lib.syntax.Runtime;
import com.google.devtools.build.lib.syntax.SkylarkDict;
import com.google.devtools.build.lib.syntax.SkylarkList;
@@ -85,14 +86,14 @@
+ " result must be non-empty (after the matches of the `exclude` patterns are"
+ " excluded).")
},
- useLocation = true,
+ useAst = true,
useStarlarkThread = true)
public SkylarkList<?> glob(
SkylarkList<?> include,
SkylarkList<?> exclude,
Integer excludeDirectories,
Object allowEmpty,
- Location loc,
+ FuncallExpression ast,
StarlarkThread thread)
throws EvalException, InterruptedException;
@@ -112,9 +113,9 @@
legacyNamed = true,
doc = "The name of the target.")
},
- useLocation = true,
+ useAst = true,
useStarlarkThread = true)
- public Object existingRule(String name, Location loc, StarlarkThread thread)
+ public Object existingRule(String name, FuncallExpression ast, StarlarkThread thread)
throws EvalException, InterruptedException;
@SkylarkCallable(
@@ -126,10 +127,10 @@
+ ""
+ "<p><i>Note: If possible, avoid using this function. It makes BUILD files brittle "
+ "and order-dependent.</i>",
- useLocation = true,
+ useAst = true,
useStarlarkThread = true)
public SkylarkDict<String, SkylarkDict<String, Object>> existingRules(
- Location loc, StarlarkThread thread) throws EvalException, InterruptedException;
+ FuncallExpression ast, StarlarkThread thread) throws EvalException, InterruptedException;
@SkylarkCallable(
name = "package_group",
@@ -160,13 +161,13 @@
positional = false,
doc = "Other package groups that are included in this one.")
},
- useLocation = true,
+ useAst = true,
useStarlarkThread = true)
public Runtime.NoneType packageGroup(
String name,
SkylarkList<?> packages,
SkylarkList<?> includes,
- Location loc,
+ FuncallExpression ast,
StarlarkThread thread)
throws EvalException;
@@ -202,10 +203,14 @@
defaultValue = "None",
doc = "Licenses to be specified.")
},
- useLocation = true,
+ useAst = true,
useStarlarkThread = true)
public Runtime.NoneType exportsFiles(
- SkylarkList<?> srcs, Object visibility, Object licenses, Location loc, StarlarkThread thread)
+ SkylarkList<?> srcs,
+ Object visibility,
+ Object licenses,
+ FuncallExpression ast,
+ StarlarkThread thread)
throws EvalException;
@SkylarkCallable(
@@ -218,9 +223,9 @@
+ "<code>package_name()</code> will match the caller BUILD file package. "
+ "This function is equivalent to the deprecated variable <code>PACKAGE_NAME</code>.",
parameters = {},
- useLocation = true,
+ useAst = true,
useStarlarkThread = true)
- public String packageName(Location loc, StarlarkThread thread) throws EvalException;
+ public String packageName(FuncallExpression ast, StarlarkThread thread) throws EvalException;
@SkylarkCallable(
name = "repository_name",
diff --git a/src/main/java/com/google/devtools/build/lib/skylarkinterface/SkylarkSignature.java b/src/main/java/com/google/devtools/build/lib/skylarkinterface/SkylarkSignature.java
index 5942b68..e0fbeaa2 100644
--- a/src/main/java/com/google/devtools/build/lib/skylarkinterface/SkylarkSignature.java
+++ b/src/main/java/com/google/devtools/build/lib/skylarkinterface/SkylarkSignature.java
@@ -21,10 +21,10 @@
/**
* An annotation to mark built-in keyword argument methods accessible from Skylark.
*
- * <p>Use this annotation around a {@link com.google.devtools.build.lib.syntax.BuiltinFunction}. The
- * annotated function should expect the arguments described by {@link #parameters()}, {@link
- * #extraPositionals()}, and {@link #extraKeywords()}. It should also expect the following
- * extraneous arguments:
+ * <p>Use this annotation around a {@link com.google.devtools.build.lib.syntax.BuiltinFunction} or a
+ * {@link com.google.devtools.build.lib.syntax.BuiltinFunction.Factory}. The annotated function
+ * should expect the arguments described by {@link #parameters()}, {@link #extraPositionals()}, and
+ * {@link #extraKeywords()}. It should also expect the following extraneous arguments:
*
* <ul>
* <li>{@link com.google.devtools.build.lib.events.Location} if {@link #useLocation()} is true.
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/BaseFunction.java b/src/main/java/com/google/devtools/build/lib/syntax/BaseFunction.java
index 6f139ae..af8d758 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/BaseFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/BaseFunction.java
@@ -55,7 +55,7 @@
// TODO(adonovan): this class has too many fields and relies too heavily on side effects and the
// class hierarchy (the configure methods are the worse offenders). Turn fields into abstract
// methods. Make processArguments a static function with multiple parameters, instead of a
- // "mix-in" that accesses instance fields.
+ // "mix-in" that accesses instance fields. And get rid of BuiltinFunction.Factory.
/**
* The name of the function.
@@ -92,9 +92,12 @@
// Some functions are also Namespaces or other Skylark entities.
@Nullable protected Class<?> objectType;
+ // Documentation for variables, if any
+ @Nullable protected List<String> paramDoc;
+
// The types actually enforced by the Skylark runtime, as opposed to those enforced by the JVM,
// or those displayed to the user in the documentation.
- @Nullable List<SkylarkType> enforcedArgumentTypes;
+ @Nullable protected List<SkylarkType> enforcedArgumentTypes;
/**
* Returns the name of this function.
@@ -176,6 +179,11 @@
this(name, signature, /*defaultValues=*/ null, /*location=*/ null);
}
+ /** Get parameter documentation as a list corresponding to each parameter */
+ List<String> getParamDoc() {
+ return paramDoc;
+ }
+
/**
* The size of the array required by the callee.
*/
@@ -488,10 +496,12 @@
public void configure(SkylarkSignature annotation) {
Preconditions.checkState(!isConfigured()); // must not be configured yet
+ this.paramDoc = new ArrayList<>();
+
// side effect: appends to getEnforcedArgumentTypes()
SkylarkSignatureProcessor.SignatureInfo info =
SkylarkSignatureProcessor.getSignatureForCallable(
- getName(), annotation, /*paramDoc=*/ new ArrayList<>(), getEnforcedArgumentTypes());
+ getName(), annotation, paramDoc, getEnforcedArgumentTypes());
this.signature = info.signature;
this.paramTypes = info.types;
this.defaultValues = info.defaultValues;
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/BuiltinFunction.java b/src/main/java/com/google/devtools/build/lib/syntax/BuiltinFunction.java
index 652bc24..af176ba 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/BuiltinFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/BuiltinFunction.java
@@ -86,11 +86,21 @@
configure();
}
+ /** Creates a BuiltinFunction from the given name and a Factory */
+ protected BuiltinFunction(String name, Factory factory) {
+ super(name);
+ configure(factory);
+ }
+
@Override
protected int getArgArraySize() {
return innerArgumentCount;
}
+ ExtraArgKind[] getExtraArgs() {
+ return extraArgs;
+ }
+
@Override
@Nullable
public Object call(Object[] args, @Nullable FuncallExpression ast, StarlarkThread thread)
@@ -188,7 +198,7 @@
stacktraceToString(e.getStackTrace()),
this,
Arrays.asList(invokeMethod.getParameterTypes()),
- getEnforcedArgumentTypes()),
+ enforcedArgumentTypes),
e);
}
@@ -234,9 +244,7 @@
parameterType);
if (enforcedType instanceof SkylarkType.Simple
|| enforcedType instanceof SkylarkFunctionType) {
- Preconditions.checkArgument(
- parameterType.isAssignableFrom(enforcedType.getType()), msg);
-
+ Preconditions.checkArgument(enforcedType.getType() == parameterType, msg);
// No need to enforce Simple types on the Skylark side, the JVM will do it for us.
enforcedArgumentTypes.set(i, null);
} else if (enforcedType instanceof SkylarkType.Combination) {
@@ -263,6 +271,24 @@
}
}
+ /** Configure by copying another function's configuration */
+ // Alternatively, we could have an extension BuiltinFunctionSignature of FunctionSignature,
+ // and use *that* instead of a Factory.
+ private void configure(BuiltinFunction.Factory factory) {
+ // this function must not be configured yet, but the factory must be
+ Preconditions.checkState(!isConfigured());
+ Preconditions.checkState(
+ factory.isConfigured(), "function factory is not configured for %s", getName());
+
+ this.paramDoc = factory.getParamDoc();
+ this.signature = factory.getSignature();
+ this.defaultValues = factory.getDefaultValues();
+ this.paramTypes = factory.getEnforcedArgumentTypes();
+ this.extraArgs = factory.getExtraArgs();
+ this.objectType = factory.getObjectType();
+ configure();
+ }
+
/** Returns list, or null if all its elements are null. */
@Nullable
private static <E> List<E> valueListOrNull(List<E> list) {
@@ -296,6 +322,50 @@
return found;
}
+ /**
+ * A Factory allows for a @SkylarkSignature annotation to be provided and processed in advance
+ * for a function that will be defined later as a closure (see e.g. in PackageFactory).
+ *
+ * <p>Each instance of this class must define a method create that closes over some (final)
+ * variables and returns a BuiltinFunction.
+ */
+ public abstract static class Factory extends BuiltinFunction {
+ @Nullable private Method createMethod;
+
+ /** Create unconfigured function Factory from its name */
+ public Factory(String name) {
+ super(name);
+ }
+
+ @Override
+ public void configure() {
+ if (createMethod != null) {
+ return;
+ }
+ createMethod = findMethod("create");
+ }
+
+ @Override
+ public Object call(Object[] args, @Nullable FuncallExpression ast, StarlarkThread thread)
+ throws EvalException {
+ throw new EvalException(null, "tried to invoke a Factory for function " + this);
+ }
+
+ /** Instantiate the Factory
+ * @param args arguments to pass to the create method
+ * @return a new BuiltinFunction that closes over the arguments
+ */
+ public BuiltinFunction apply(Object... args) {
+ try {
+ return (BuiltinFunction) createMethod.invoke(this, args);
+ } catch (InvocationTargetException | IllegalArgumentException | IllegalAccessException e) {
+ throw new RuntimeException(String.format(
+ "Exception while applying BuiltinFunction.Factory %s: %s",
+ this, e.getMessage()), e);
+ }
+ }
+ }
+
@Override
public void repr(SkylarkPrinter printer) {
printer.append("<built-in function " + getName() + ">");
diff --git a/src/main/java/com/google/devtools/build/skydoc/fakebuildapi/FakeSkylarkNativeModuleApi.java b/src/main/java/com/google/devtools/build/skydoc/fakebuildapi/FakeSkylarkNativeModuleApi.java
index 0928823..4376ac4 100644
--- a/src/main/java/com/google/devtools/build/skydoc/fakebuildapi/FakeSkylarkNativeModuleApi.java
+++ b/src/main/java/com/google/devtools/build/skydoc/fakebuildapi/FakeSkylarkNativeModuleApi.java
@@ -20,6 +20,7 @@
import com.google.devtools.build.lib.skylarkbuildapi.SkylarkNativeModuleApi;
import com.google.devtools.build.lib.syntax.ClassObject;
import com.google.devtools.build.lib.syntax.EvalException;
+import com.google.devtools.build.lib.syntax.FuncallExpression;
import com.google.devtools.build.lib.syntax.Runtime.NoneType;
import com.google.devtools.build.lib.syntax.SkylarkDict;
import com.google.devtools.build.lib.syntax.SkylarkList;
@@ -36,21 +37,21 @@
SkylarkList<?> exclude,
Integer excludeDirectories,
Object allowEmpty,
- Location loc,
+ FuncallExpression ast,
StarlarkThread thread)
throws EvalException, InterruptedException {
return MutableList.of(thread);
}
@Override
- public Object existingRule(String name, Location loc, StarlarkThread thread)
+ public Object existingRule(String name, FuncallExpression ast, StarlarkThread thread)
throws EvalException, InterruptedException {
return null;
}
@Override
public SkylarkDict<String, SkylarkDict<String, Object>> existingRules(
- Location loc, StarlarkThread thread) throws EvalException, InterruptedException {
+ FuncallExpression ast, StarlarkThread thread) throws EvalException, InterruptedException {
return SkylarkDict.of(thread);
}
@@ -59,7 +60,7 @@
String name,
SkylarkList<?> packages,
SkylarkList<?> includes,
- Location loc,
+ FuncallExpression ast,
StarlarkThread thread)
throws EvalException {
return null;
@@ -67,13 +68,17 @@
@Override
public NoneType exportsFiles(
- SkylarkList<?> srcs, Object visibility, Object licenses, Location loc, StarlarkThread thread)
+ SkylarkList<?> srcs,
+ Object visibility,
+ Object licenses,
+ FuncallExpression ast,
+ StarlarkThread thread)
throws EvalException {
return null;
}
@Override
- public String packageName(Location loc, StarlarkThread thread) throws EvalException {
+ public String packageName(FuncallExpression ast, StarlarkThread thread) throws EvalException {
return "";
}
diff --git a/src/test/java/com/google/devtools/build/lib/packages/PackageFactoryTest.java b/src/test/java/com/google/devtools/build/lib/packages/PackageFactoryTest.java
index 821ad38..dc91321 100644
--- a/src/test/java/com/google/devtools/build/lib/packages/PackageFactoryTest.java
+++ b/src/test/java/com/google/devtools/build/lib/packages/PackageFactoryTest.java
@@ -661,7 +661,9 @@
events.setFailFast(false);
assertGlobFails(
"glob(1, exclude=2)",
- "expected sequence of strings for 'include' while calling glob but got int instead: 1");
+ "argument 'include' has type 'int', but should be 'sequence'\n"
+ + "in call to builtin function glob(include, *, exclude, exclude_directories, "
+ + "allow_empty)");
}
@Test
