Skip to content

Commit

Permalink
Support updated PROJECT.scl schema with single top-level project
Browse files Browse the repository at this point in the history
…variable.

Supports both the old form:

```
active_directories = [...]
configs = {...}
default_config = "..."
```

and new form:

```
project = {
  "active_directories": [...],
  "configs": {...},
  "default_config": "...",
}
```

We'll eventually remove support for the old form.

This schema is still experimental and subject to change without notice.

PiperOrigin-RevId: 700800427
Change-Id: I550efb9c85b37090524adc1310634c1185db52e7
  • Loading branch information
gregestren authored and copybara-github committed Nov 27, 2024
1 parent 37d733f commit d35695f
Show file tree
Hide file tree
Showing 6 changed files with 307 additions and 9 deletions.
4 changes: 2 additions & 2 deletions src/main/java/com/google/devtools/build/lib/skyframe/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -1214,9 +1214,9 @@ java_library(
name = "project_function",
srcs = ["ProjectFunction.java"],
deps = [
":bzl_load_failed_exception",
":bzl_load_value",
":project_value",
"//src/main/java/com/google/devtools/build/lib/skyframe:bzl_load_failed_exception",
"//src/main/java/com/google/devtools/build/lib/skyframe:bzl_load_value",
"//src/main/java/com/google/devtools/build/skyframe:skyframe-objects",
"//src/main/java/net/starlark/java/eval",
"//third_party:guava",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,9 +33,15 @@ public class ProjectFunction implements SkyFunction {

/** The top level reserved globals in the PROJECT.scl file. */
private enum ReservedGlobals {
OWNED_CODE_PATHS("owned_code_paths"),
/** Deprecated PROJECT.scl structure. See {@link #PROJECT}. */
ACTIVE_DIRECTORIES("active_directories"),

ACTIVE_DIRECTORIES("active_directories");
/**
* Forward-facing PROJECT.scl structure: a single top-level "project" variable that contains all
* project data in nested data structures.
*/
// TODO: b/345100818 - delete deprecated form when no depot instances use it.
PROJECT("project");

private final String key;

Expand Down Expand Up @@ -67,8 +73,39 @@ public SkyValue compute(SkyKey skyKey, Environment env)
return null;
}

Object projectRaw = bzlLoadValue.getModule().getGlobal(ReservedGlobals.PROJECT.getKey());
ImmutableMap<String, Object> project =
switch (projectRaw) {
case null -> ImmutableMap.of();
case Dict<?, ?> dict -> {
ImmutableMap.Builder<String, Object> projectBuilder = ImmutableMap.builder();
for (Object k : dict.keySet()) {
if (!(k instanceof String stringKey)) {
throw new ProjectFunctionException(
new TypecheckFailureException(
String.format(
"%s variable: expected string key, got element of %s",
ReservedGlobals.PROJECT.getKey(), k.getClass())));
}
projectBuilder.put(stringKey, dict.get(stringKey));
}
yield projectBuilder.buildOrThrow();
}
default ->
throw new ProjectFunctionException(
new TypecheckFailureException(
String.format(
"%s variable: expected a map of string to objects, got %s",
ReservedGlobals.PROJECT.getKey(), projectRaw.getClass())));
};

Object activeDirectoriesRaw =
bzlLoadValue.getModule().getGlobal(ReservedGlobals.ACTIVE_DIRECTORIES.getKey());
if (activeDirectoriesRaw == null && project != null) {
// TODO: b/345100818 - make this the only way to retrieve "active_directoreis" when all
// instances of the old form are gone.
activeDirectoriesRaw = project.get(ReservedGlobals.ACTIVE_DIRECTORIES.getKey());
}

// Crude typechecking to prevent server crashes.
// TODO: all of these typechecking should probably be handled by a proto spec.
Expand Down Expand Up @@ -115,6 +152,7 @@ public SkyValue compute(SkyKey skyKey, Environment env)
+ activeDirectoriesRaw.getClass()));
};

// TODO: b/345100818 - remove residual global support when everything is under "project".
ImmutableMap<String, Object> residualGlobals =
bzlLoadValue.getModule().getGlobals().entrySet().stream()
.filter(
Expand All @@ -129,7 +167,7 @@ public SkyValue compute(SkyKey skyKey, Environment env)
"non-empty active_directories must contain the 'default' key"));
}

return new ProjectValue(activeDirectories, residualGlobals);
return new ProjectValue(project, activeDirectories, residualGlobals);
}

private static final class TypecheckFailureException extends Exception {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,14 +26,17 @@

/** A SkyValue representing the parsed definitions from a PROJECT.scl file. */
public final class ProjectValue implements SkyValue {
private final ImmutableMap<String, Object> project;

private final ImmutableMap<String, Collection<String>> activeDirectories;

private final ImmutableMap<String, Object> residualGlobals;

public ProjectValue(
ImmutableMap<String, Object> project,
ImmutableMap<String, Collection<String>> activeDirectories,
ImmutableMap<String, Object> residualGlobals) {
this.project = project;
this.activeDirectories = activeDirectories;
this.residualGlobals = residualGlobals;
}
Expand Down Expand Up @@ -68,6 +71,16 @@ public ImmutableSet<String> getDefaultActiveDirectory() {
return ImmutableSet.copyOf(activeDirectories.get("default"));
}

/**
* Returns the top-level project definition. Entries are currently self-typed: it's up to the
* entry's consumer to validate and correctly read it.
*
* <p>If {@code "project"} is not defined in the file, returns an empty map.
*/
public ImmutableMap<String, Object> getProject() {
return project;
}

/**
* Returns the map of named active directories in the project. If the map is not defined in the
* file, returns an empty map.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@
import com.google.common.collect.ImmutableSet;
import com.google.common.collect.Iterables;
import com.google.devtools.build.lib.analysis.config.BuildOptions;
import com.google.devtools.build.lib.analysis.config.CoreOptions;
import com.google.devtools.build.lib.analysis.config.FragmentOptions;
import com.google.devtools.build.lib.analysis.test.TestConfiguration;
import com.google.devtools.build.lib.cmdline.Label;
Expand Down Expand Up @@ -111,18 +110,58 @@ private ImmutableList<String> getSclConfig(
ImmutableMap<String, String> userOptions)
throws FlagSetFunctionException {

var configs = (Dict<String, Collection<String>>) sclContent.getResidualGlobal(CONFIGS);
var unTypeCheckedConfigs =
sclContent.getProject().isEmpty()
? sclContent.getResidualGlobal(CONFIGS)
: sclContent.getProject().get(CONFIGS);
// This project file doesn't define configs, so it must not be used for canonical configs.
if (configs == null) {
if (unTypeCheckedConfigs == null) {
return ImmutableList.of();
}
boolean expectedConfigsType = false;
if (unTypeCheckedConfigs instanceof Dict<?, ?> configsAsDict) {
expectedConfigsType = true;
for (var entry : configsAsDict.entrySet()) {
if (!(entry.getKey() instanceof String
&& entry.getValue() instanceof Collection<?> values)) {
expectedConfigsType = false;
break;
}
for (var value : values) {
if (!(value instanceof String)) {
expectedConfigsType = false;
break;
}
}
}
}
if (!expectedConfigsType) {
throw new FlagSetFunctionException(
new InvalidProjectFileException(
String.format("%s variable must be a map of strings to lists of strings", CONFIGS)),
Transience.PERSISTENT);
}
var configs = (Dict<String, Collection<String>>) unTypeCheckedConfigs;

String sclConfigNameForMessage = sclConfigName;
Collection<String> sclConfigValue = null;
if (sclConfigName.isEmpty()) {
// If there's no --scl_config, try to use the default_config.
var defaultConfigName = (String) sclContent.getResidualGlobal(DEFAULT_CONFIG);
var defaultConfigNameRaw =
sclContent.getProject().isEmpty()
? sclContent.getResidualGlobal(DEFAULT_CONFIG)
: sclContent.getProject().get(DEFAULT_CONFIG);
try {
if (defaultConfigNameRaw != null && !(defaultConfigNameRaw instanceof String)) {
throw new FlagSetFunctionException(
new InvalidProjectFileException(
String.format(
"%s must be a string matching a %s variable definition",
DEFAULT_CONFIG, CONFIGS)),
Transience.PERSISTENT);
}

String defaultConfigName = (String) defaultConfigNameRaw;
sclConfigValue = validateDefaultConfig(defaultConfigName, configs);
sclConfigNameForMessage = defaultConfigName;
} catch (InvalidProjectFileException e) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,30 @@ public void projectFunction_returnsDefaultActiveDirectories() throws Exception {
assertThat(trie.includes(PathFragment.create("d"))).isFalse();
}

@Test
public void projectFunction_returnsDefaultActiveDirectories_topLevelProjectSchema()
throws Exception {
scratch.file(
"test/PROJECT.scl",
"""
project = {
"active_directories": { "default": ["a", "b/c"] }
}
""");
scratch.file("test/BUILD");
ProjectValue.Key key = new ProjectValue.Key(Label.parseCanonical("//test:PROJECT.scl"));

EvaluationResult<ProjectValue> result =
SkyframeExecutorTestUtils.evaluate(skyframeExecutor, key, false, reporter);
assertThat(result.hasError()).isFalse();

ProjectValue value = result.get(key);
PathFragmentPrefixTrie trie = PathFragmentPrefixTrie.of(value.getDefaultActiveDirectory());
assertThat(trie.includes(PathFragment.create("a"))).isTrue();
assertThat(trie.includes(PathFragment.create("b/c"))).isTrue();
assertThat(trie.includes(PathFragment.create("d"))).isFalse();
}

@Test
public void projectFunction_nonEmptyActiveDirectoriesMustHaveADefault() throws Exception {
scratch.file("test/PROJECT.scl", "active_directories = { 'foo': ['a', 'b/c'] }");
Expand Down Expand Up @@ -133,6 +157,44 @@ public void projectFunction_incorrectType_inList() throws Exception {
.matches("expected a list of strings, got element of .+Int32");
}

@Test
public void projectFunction_incorrectProjectType() throws Exception {
scratch.file(
"test/PROJECT.scl",
"""
project = 1
""");

scratch.file("test/BUILD");
ProjectValue.Key key = new ProjectValue.Key(Label.parseCanonical("//test:PROJECT.scl"));

EvaluationResult<ProjectValue> result =
SkyframeExecutorTestUtils.evaluate(skyframeExecutor, key, false, reporter);
assertThat(result.hasError()).isTrue();
assertThat(result.getError().getException())
.hasMessageThat()
.matches("project variable: expected a map of string to objects, got .+Int32");
}

@Test
public void projectFunction_incorrectProjectKeyType() throws Exception {
scratch.file(
"test/PROJECT.scl",
"""
project = {1: [] }
""");

scratch.file("test/BUILD");
ProjectValue.Key key = new ProjectValue.Key(Label.parseCanonical("//test:PROJECT.scl"));

EvaluationResult<ProjectValue> result =
SkyframeExecutorTestUtils.evaluate(skyframeExecutor, key, false, reporter);
assertThat(result.hasError()).isTrue();
assertThat(result.getError().getException())
.hasMessageThat()
.matches("project variable: expected string key, got element of .+Int32");
}

@Test
public void projectFunction_parsesResidualGlobals() throws Exception {
scratch.file(
Expand Down
Loading

0 comments on commit d35695f

Please sign in to comment.