Skip to content

Commit

Permalink
Add support for loading .scl files
Browse files Browse the repository at this point in the history
The Starlark Configuration Language (SCL) is a dialect of Starlark that resembles pure Starlark with just a few Bazel symbols (namely `visibility()` and `struct`), and a few restrictions on `load()` syntax and non-ASCII string data.

This CL adds support for loading .scl files anywhere a .bzl could be used. The restrictions on `load()` are implemented in this CL, but follow-up CLs will handle differentiating the top-level environment from .bzl, restricting non-ASCII data, and adding user documentation.

The dialect is gated on the flag `--experimental_enable_scl_dialect`.

- `BzlLoadValue` gains a new bool field on its abstract Key type to indicate whether it is for .scl. This was chosen instead of creating a new Key subclass so that .scl can work equally well in different contexts (loaded by BUILD, WORKSPACE, MODULE.bazel, or even @_builtins).

- `BzlLoadFunction.checkValidLoadLabel` and `.getLoadLabels` are both split into a public and private method. The public methods assume the load is coming from a .bzl file for validation purposes, and take a `StarlarkSemantics` to check whether the experimental flag is enabled.

- Eliminate `BzlLoadFunction.getBUILDLabel()` helper, which is obsolete.

- Modify existing tests to incidentally check that bzlmod .bzls can load .scl files and that .scl files can appear in transitive loads as reported by the Package (for `loadfiles()`).

RELNOTES: None
PiperOrigin-RevId: 528563228
Change-Id: I8493d1f33d35e1af8003dc61e5fdb626676d7e53
  • Loading branch information
brandjon authored and copybara-github committed May 1, 2023
1 parent 51249ec commit a0cd355
Show file tree
Hide file tree
Showing 9 changed files with 274 additions and 38 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -111,8 +111,7 @@ public SkyValue compute(SkyKey skyKey, Environment env)

// Check that the .bzl label isn't crazy.
try {
BzlLoadFunction.checkValidLoadLabel(
extensionId.getBzlFileLabel(), /*fromBuiltinsRepo=*/ false);
BzlLoadFunction.checkValidLoadLabel(extensionId.getBzlFileLabel(), starlarkSemantics);
} catch (LabelSyntaxException e) {
throw new SingleExtensionEvalFunctionException(
ExternalDepsException.withCauseAndMessage(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -196,6 +196,15 @@ public final class BuildLanguageOptions extends OptionsBase {
help = "If set to true, enables the APIs required to support the Android Starlark migration.")
public boolean experimentalEnableAndroidMigrationApis;

@Option(
name = "experimental_enable_scl_dialect",
defaultValue = "false",
documentationCategory = OptionDocumentationCategory.STARLARK_SEMANTICS,
effectTags = OptionEffectTag.BUILD_FILE_SEMANTICS,
// TODO(brandjon): point to more extensive user documentation somewhere
help = "If set to true, .scl files may be used in load() statements.")
public boolean experimentalEnableSclDialect;

@Option(
name = "enable_bzlmod",
oldName = "experimental_enable_bzlmod",
Expand Down Expand Up @@ -677,6 +686,7 @@ public StarlarkSemantics toStarlarkSemantics() {
.setBool(CHECK_BZL_VISIBILITY, checkBzlVisibility)
.setBool(
EXPERIMENTAL_ENABLE_ANDROID_MIGRATION_APIS, experimentalEnableAndroidMigrationApis)
.setBool(EXPERIMENTAL_ENABLE_SCL_DIALECT, experimentalEnableSclDialect)
.setBool(ENABLE_BZLMOD, enableBzlmod)
.setBool(
EXPERIMENTAL_JAVA_PROTO_LIBRARY_DEFAULT_HAS_SERVICES,
Expand Down Expand Up @@ -770,6 +780,7 @@ public StarlarkSemantics toStarlarkSemantics() {
"-experimental_disable_external_package";
public static final String EXPERIMENTAL_ENABLE_ANDROID_MIGRATION_APIS =
"-experimental_enable_android_migration_apis";
public static final String EXPERIMENTAL_ENABLE_SCL_DIALECT = "-experimental_enable_scl_dialect";
public static final String ENABLE_BZLMOD = "-enable_bzlmod";
public static final String EXPERIMENTAL_JAVA_PROTO_LIBRARY_DEFAULT_HAS_SERVICES =
"+experimental_java_proto_library_default_has_services";
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,11 @@
import net.starlark.java.syntax.StringLiteral;

/**
* A Skyframe function to look up and load a single .bzl module.
* A Skyframe function to look up and load a single .bzl (or .scl) module.
*
* <p>Note: Historically, all modules had the .bzl suffix, but this is no longer true now that Bazel
* supports the .scl dialect. In identifiers, code comments, and documentation, you should generally
* assume any "bzl" term could mean a .scl file as well.
*
* <p>Given a {@link Label} referencing a .bzl file, attempts to locate the file and load it. The
* Label must be absolute, and must not reference the special {@code external} package. If loading
Expand Down Expand Up @@ -736,6 +740,14 @@ private BzlLoadValue computeInternalWithCompiledBzl(
Label label = key.getLabel();
PackageIdentifier pkg = label.getPackageIdentifier();

boolean isSclFlagEnabled =
builtins.starlarkSemantics.getBool(BuildLanguageOptions.EXPERIMENTAL_ENABLE_SCL_DIALECT);
if (key.isSclDialect() && !isSclFlagEnabled) {
throw new BzlLoadFailedException(
"loading .scl files requires setting --experimental_enable_scl_dialect",
Code.PARSE_ERROR);
}

// Determine dependency BzlLoadValue keys for the load statements in this bzl.
// Labels are resolved relative to the current repo mapping.
RepositoryMapping repoMapping = getRepositoryMapping(key, builtins.starlarkSemantics, env);
Expand All @@ -744,7 +756,13 @@ private BzlLoadValue computeInternalWithCompiledBzl(
}
ImmutableList<Pair<String, Location>> programLoads = getLoadsFromProgram(prog);
ImmutableList<Label> loadLabels =
getLoadLabels(env.getListener(), programLoads, pkg, repoMapping);
getLoadLabels(
env.getListener(),
programLoads,
pkg,
repoMapping,
key.isSclDialect(),
isSclFlagEnabled);
if (loadLabels == null) {
throw new BzlLoadFailedException(
String.format(
Expand Down Expand Up @@ -933,11 +951,59 @@ private static RepositoryMapping getRepositoryMapping(
return repositoryMappingValue.getRepositoryMapping();
}

public static void checkValidLoadLabel(Label label, boolean fromBuiltinsRepo)
/**
* Validates a label appearing in a {@code load()} statement, throwing {@link
* LabelSyntaxException} on failure.
*
* <p>Different restrictions apply depending on what type of source file the load appears in. For
* all kinds of files, {@code label}:
*
* <ul>
* <li>may not be within {@code @//external}.
* <li>must end with either {@code .bzl} or {@code .scl}.
* </ul>
*
* <p>For source files appearing within {@code @_builtins}, {@code label} must also be within
* {@code @_builtins}. (The reverse, that those files may not be loaded by user-defined files, is
* enforced by the fact that the {@code @_builtins} pseudorepo cannot be resolved as an ordinary
* repo.)
*
* <p>For .scl files only, {@code label} must end with {@code .scl} (not {@code .bzl}). (Loads in
* .scl also should always begin with {@code //}, but that's syntactic and can't be enforced in
* this method.)
*
* @param label the label to validate
* @param fromBuiltinsRepo true if the file containing the load is within {@code @_builtins}
* @param withinSclDialect true if the file containing the load is a .scl file
* @param mentionSclInErrorMessage true if ".scl" should be advertised as a possible extension in
* error messaging
*/
private static void checkValidLoadLabel(
Label label,
boolean fromBuiltinsRepo,
boolean withinSclDialect,
boolean mentionSclInErrorMessage)
throws LabelSyntaxException {
if (!label.getName().endsWith(".bzl")) {
throw new LabelSyntaxException("The label must reference a file with extension '.bzl'");
// Check file extension.
String baseName = label.getName();
if (withinSclDialect) {
if (!baseName.endsWith(".scl")) {
String msg = "The label must reference a file with extension \".scl\"";
if (baseName.endsWith(".bzl")) {
msg += " (.scl files cannot load .bzl files)";
}
throw new LabelSyntaxException(msg);
}
} else {
if (!(baseName.endsWith(".scl") || baseName.endsWith(".bzl"))) {
String msg = "The label must reference a file with extension \".bzl\"";
if (mentionSclInErrorMessage) {
msg += " or \".scl\"";
}
throw new LabelSyntaxException(msg);
}
}

if (label.getPackageIdentifier().equals(LabelConstants.EXTERNAL_PACKAGE_IDENTIFIER)) {
throw new LabelSyntaxException(
"Starlark files may not be loaded from the //external package");
Expand All @@ -948,35 +1014,57 @@ public static void checkValidLoadLabel(Label label, boolean fromBuiltinsRepo)
}
}

/**
* Validates a label appearing in a {@code load()} statement, throwing {@link
* LabelSyntaxException} on failure.
*/
public static void checkValidLoadLabel(Label label, StarlarkSemantics starlarkSemantics)
throws LabelSyntaxException {
checkValidLoadLabel(
label,
/* fromBuiltinsRepo= */ false,
/* withinSclDialect= */ false,
/* mentionSclInErrorMessage= */ starlarkSemantics.getBool(
BuildLanguageOptions.EXPERIMENTAL_ENABLE_SCL_DIALECT));
}

/**
* Given a list of {@code load("module")} strings and their locations, in source order, returns a
* corresponding list of Labels they each resolve to. Labels are resolved relative to {@code
* base}, the file's package. If any label is malformed, the function reports one or more errors
* to the handler and returns null.
*
* <p>If {@code withinSclDialect} is true, the labels are validated according to the rules of the
* .scl dialect: Only strings beginning with {@code //} are allowed (no repo syntax, no relative
* labels), and only .scl files may be loaded (not .bzl). If {@code isSclFlagEnabled} is true,
* then ".scl" is mentioned as a possible file extension in error messages.
*/
@Nullable
static ImmutableList<Label> getLoadLabels(
private static ImmutableList<Label> getLoadLabels(
EventHandler handler,
ImmutableList<Pair<String, Location>> loads,
PackageIdentifier base,
RepositoryMapping repoMapping) {
// It's redundant that getRelativeWithRemapping needs a Label;
// a PackageIdentifier should suffice. Make one here.
Label buildLabel = getBUILDLabel(base);

RepositoryMapping repoMapping,
boolean withinSclDialect,
boolean isSclFlagEnabled) {
boolean ok = true;

ImmutableList.Builder<Label> loadLabels = ImmutableList.builderWithExpectedSize(loads.size());
for (Pair<String, Location> load : loads) {
// Parse the load statement's module string as a label.
// It must end in .bzl and not be in package "//external".
// Parse the load statement's module string as a label. Validate the unparsed string for
// syntax and the parsed label for structure.
String unparsedLabel = load.first;
try {
if (withinSclDialect && !unparsedLabel.startsWith("//")) {
throw new LabelSyntaxException("in .scl files, load labels must begin with \"//\"");
}
Label label =
Label.parseWithPackageContext(
load.first, PackageContext.of(buildLabel.getPackageIdentifier(), repoMapping));
Label.parseWithPackageContext(unparsedLabel, PackageContext.of(base, repoMapping));
checkValidLoadLabel(
label,
/* fromBuiltinsRepo= */ StarlarkBuiltinsValue.isBuiltinsRepo(base.getRepository()));
/* fromBuiltinsRepo= */ StarlarkBuiltinsValue.isBuiltinsRepo(base.getRepository()),
/* withinSclDialect= */ withinSclDialect,
/* mentionSclInErrorMessage= */ isSclFlagEnabled);
loadLabels.add(label);
} catch (LabelSyntaxException ex) {
handler.handle(Event.error(load.second, "in load statement: " + ex.getMessage()));
Expand All @@ -986,6 +1074,29 @@ static ImmutableList<Label> getLoadLabels(
return ok ? loadLabels.build() : null;
}

/**
* Given a list of {@code load("module")} strings and their locations, in source order, returns a
* corresponding list of Labels they each resolve to. Labels are resolved relative to {@code
* base}, the file's package. If any label is malformed, the function reports one or more errors
* to the handler and returns null.
*/
@Nullable
static ImmutableList<Label> getLoadLabels(
EventHandler handler,
ImmutableList<Pair<String, Location>> loads,
PackageIdentifier base,
RepositoryMapping repoMapping,
StarlarkSemantics starlarkSemantics) {
return getLoadLabels(
handler,
loads,
base,
repoMapping,
/* withinSclDialect= */ false,
/* isSclFlagEnabled= */ starlarkSemantics.getBool(
BuildLanguageOptions.EXPERIMENTAL_ENABLE_SCL_DIALECT));
}

/** Extracts load statements from compiled program (see {@link #getLoadLabels}). */
static ImmutableList<Pair<String, Location>> getLoadsFromProgram(Program prog) {
int n = prog.getLoads().size();
Expand All @@ -1010,15 +1121,6 @@ static ImmutableList<Pair<String, Location>> getLoadsFromStarlarkFiles(List<Star
return loads.build();
}

private static Label getBUILDLabel(PackageIdentifier pkgid) {
try {
return Label.create(pkgid, "BUILD");
} catch (LabelSyntaxException e) {
// Shouldn't happen; the Label is well-formed by construction.
throw new IllegalStateException(e);
}
}

/**
* Computes the BzlLoadValue for all given .bzl load keys using ordinary Skyframe evaluation,
* returning {@code null} if Skyframe deps were missing and have been requested. {@code
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,11 @@
import net.starlark.java.eval.Module;

/**
* A value that represents the .bzl module loaded by a Starlark {@code load()} statement.
* A value that represents the .bzl (or .scl) module loaded by a Starlark {@code load()} statement.
*
* <p>Note: Historically, all modules had the .bzl suffix, but this is no longer true now that Bazel
* supports the .scl dialect. In identifiers, code comments, and documentation, you should generally
* assume any "bzl" term could mean a .scl file as well.
*
* <p>The key consists of an absolute {@link Label} and the context in which the load occurs. The
* Label should not reference the special {@code external} package.
Expand Down Expand Up @@ -95,6 +99,22 @@ boolean isBuiltins() {
return false;
}

/** Returns true if the requested file follows the .scl dialect. */
// Note: Just as with .bzl, the same .scl file can be referred to from multiple key types, for
// instance if a BUILD file and a module rule both load foo.scl. Conceptually, .scl files
// shouldn't depend on what kind of top-level file caused them to load, but in practice, this
// implementation quirk means that the .scl file will be loaded twice as separate copies.
//
// This shouldn't matter except in rare edge cases, such as if a Starlark function is loaded
// from both copies and compared for equality. Performance wise, it also means that all
// transitive .scl files will be double-loaded, but we don't expect that to be significant.
//
// The alternative is to use a separate key type just for .scl, but that complicates repo logic;
// see BzlLoadFunction#getRepositoryMapping.
final boolean isSclDialect() {
return getLabel().getName().endsWith(".scl");
}

/**
* Constructs a new key suitable for evaluating a {@code load()} dependency of this key's .bzl
* file.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -220,7 +220,8 @@ private ImmutableMap<String, Module> loadBzlModules(
env.getListener(),
programLoads,
PackageIdentifier.EMPTY_PACKAGE_ID,
EMPTY_MAIN_REPO_MAPPING);
EMPTY_MAIN_REPO_MAPPING,
starlarkSemantics);
if (loadLabels == null) {
NoSuchPackageException e =
PackageFunction.PackageFunctionException.builder()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1288,7 +1288,11 @@ private LoadedPackage loadPackage(
BzlLoadFunction.getLoadsFromProgram(compiled.prog);
ImmutableList<Label> loadLabels =
BzlLoadFunction.getLoadLabels(
env.getListener(), programLoads, packageId, repositoryMapping);
env.getListener(),
programLoads,
packageId,
repositoryMapping,
starlarkBuiltinsValue.starlarkSemantics);
if (loadLabels == null) {
throw PackageFunctionException.builder()
.setType(PackageFunctionException.Type.BUILD_FILE_CONTAINS_ERRORS)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -297,7 +297,8 @@ public SkyValue compute(SkyKey skyKey, Environment env)
ImmutableList<Pair<String, Location>> programLoads =
BzlLoadFunction.getLoadsFromStarlarkFiles(chunk);
ImmutableList<Label> loadLabels =
BzlLoadFunction.getLoadLabels(env.getListener(), programLoads, rootPackage, repoMapping);
BzlLoadFunction.getLoadLabels(
env.getListener(), programLoads, rootPackage, repoMapping, starlarkSemantics);
if (loadLabels == null) {
NoSuchPackageException e =
PackageFunction.PackageFunctionException.builder()
Expand Down
Loading

0 comments on commit a0cd355

Please sign in to comment.