Skip to content

Commit

Permalink
Extract a separate StarlarkToolchainContext for starlark-only operati…
Browse files Browse the repository at this point in the history
…ons.

Also make ToolchainContextApi use starlark threads.

PiperOrigin-RevId: 367515900
  • Loading branch information
katre authored and copybara-github committed Apr 8, 2021
1 parent b9519f9 commit 627c16e
Show file tree
Hide file tree
Showing 6 changed files with 151 additions and 94 deletions.
18 changes: 16 additions & 2 deletions src/main/java/com/google/devtools/build/lib/analysis/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -350,6 +350,7 @@ java_library(
":starlark/starlark_command_line",
":starlark/starlark_exec_group_collection",
":starlark/starlark_late_bound_default",
":starlark/starlark_toolchain_context",
":template_variable_info",
":test/analysis_failure",
":test/analysis_failure_info",
Expand Down Expand Up @@ -1010,8 +1011,6 @@ java_library(
"//src/main/java/com/google/devtools/build/lib/skyframe:toolchain_context_key",
"//src/main/java/com/google/devtools/build/lib/skyframe:toolchain_exception",
"//src/main/java/com/google/devtools/build/lib/skyframe:unloaded_toolchain_context",
"//src/main/java/com/google/devtools/build/lib/starlarkbuildapi/platform",
"//src/main/java/net/starlark/java/eval",
"//src/main/protobuf:failure_details_java_proto",
"//third_party:auto_value",
"//third_party:guava",
Expand Down Expand Up @@ -2191,6 +2190,7 @@ java_library(
srcs = ["starlark/StarlarkExecGroupCollection.java"],
deps = [
":resolved_toolchain_context",
":starlark/starlark_toolchain_context",
":toolchain_collection",
"//src/main/java/com/google/devtools/build/lib/starlarkbuildapi/platform",
"//src/main/java/net/starlark/java/eval",
Expand Down Expand Up @@ -2237,6 +2237,20 @@ java_library(
],
)

java_library(
name = "starlark/starlark_toolchain_context",
srcs = ["starlark/StarlarkToolchainContext.java"],
deps = [
":resolved_toolchain_context",
"//src/main/java/com/google/devtools/build/lib/analysis/platform",
"//src/main/java/com/google/devtools/build/lib/cmdline",
"//src/main/java/com/google/devtools/build/lib/starlarkbuildapi/platform",
"//src/main/java/net/starlark/java/eval",
"//third_party:auto_value",
"//third_party:jsr305",
],
)

# TODO(b/144899336): This should be lib/analysis/test/BUILD
java_library(
name = "test/analysis_failure",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,29 +14,21 @@

package com.google.devtools.build.lib.analysis;

import static java.util.stream.Collectors.joining;

import com.google.auto.value.AutoValue;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
import com.google.devtools.build.lib.analysis.platform.PlatformProviderUtils;
import com.google.devtools.build.lib.analysis.platform.ToolchainInfo;
import com.google.devtools.build.lib.analysis.platform.ToolchainTypeInfo;
import com.google.devtools.build.lib.cmdline.Label;
import com.google.devtools.build.lib.cmdline.LabelSyntaxException;
import com.google.devtools.build.lib.cmdline.RepositoryName;
import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable;
import com.google.devtools.build.lib.concurrent.ThreadSafety.ThreadSafe;
import com.google.devtools.build.lib.server.FailureDetails.Toolchain.Code;
import com.google.devtools.build.lib.skyframe.ConfiguredTargetAndData;
import com.google.devtools.build.lib.skyframe.ToolchainException;
import com.google.devtools.build.lib.skyframe.UnloadedToolchainContext;
import com.google.devtools.build.lib.starlarkbuildapi.platform.ToolchainContextApi;
import javax.annotation.Nullable;
import net.starlark.java.eval.EvalException;
import net.starlark.java.eval.Printer;
import net.starlark.java.eval.Starlark;
import net.starlark.java.eval.StarlarkSemantics;

/**
* Represents the data needed for a specific target's use of toolchains and platforms, including
Expand All @@ -45,7 +37,7 @@
@AutoValue
@Immutable
@ThreadSafe
public abstract class ResolvedToolchainContext implements ToolchainContextApi, ToolchainContext {
public abstract class ResolvedToolchainContext implements ToolchainContext {

/**
* Finishes preparing the {@link ResolvedToolchainContext} by finding the specific toolchain
Expand Down Expand Up @@ -107,15 +99,15 @@ public static ResolvedToolchainContext load(
}

/** Returns the repository mapping applied by the Starlark 'in' operator to string-form labels. */
abstract ImmutableMap<RepositoryName, RepositoryName> repoMapping();
public abstract ImmutableMap<RepositoryName, RepositoryName> repoMapping();

/** Returns a description of the target being used, for error messaging. */
public abstract String targetDescription();

/** Sets the map from requested {@link Label} to toolchain type provider. */
abstract ImmutableMap<Label, ToolchainTypeInfo> requestedToolchainTypeLabels();
public abstract ImmutableMap<Label, ToolchainTypeInfo> requestedToolchainTypeLabels();

abstract ImmutableMap<ToolchainTypeInfo, ToolchainInfo> toolchains();
public abstract ImmutableMap<ToolchainTypeInfo, ToolchainInfo> toolchains();

/** Returns the template variables that these toolchains provide. */
public abstract ImmutableList<TemplateVariableInfo> templateVariableProviders();
Expand All @@ -138,67 +130,6 @@ public ToolchainInfo forToolchainType(ToolchainTypeInfo toolchainType) {
return toolchains().get(toolchainType);
}

@Override
public boolean isImmutable() {
return true;
}

@Override
public void repr(Printer printer) {
printer.append("<toolchain_context.resolved_labels: ");
printer.append(
toolchains().keySet().stream()
.map(ToolchainTypeInfo::typeLabel)
.map(Label::toString)
.collect(joining(", ")));
printer.append(">");
}

private static Label transformKey(
Object key, ImmutableMap<RepositoryName, RepositoryName> repoMapping) throws EvalException {
if (key instanceof Label) {
return (Label) key;
} else if (key instanceof ToolchainTypeInfo) {
return ((ToolchainTypeInfo) key).typeLabel();
} else if (key instanceof String) {
try {
return Label.parseAbsolute((String) key, repoMapping);
} catch (LabelSyntaxException e) {
throw Starlark.errorf("Unable to parse toolchain label '%s': %s", key, e.getMessage());
}
} else {
throw Starlark.errorf(
"Toolchains only supports indexing by toolchain type, got %s instead",
Starlark.type(key));
}
}

@Override
public ToolchainInfo getIndex(StarlarkSemantics semantics, Object key) throws EvalException {
Label toolchainTypeLabel = transformKey(key, repoMapping());

if (!containsKey(semantics, key)) {
// TODO(bazel-configurability): The list of available toolchain types is confusing in the
// presence of aliases, since it only contains the actual label, not the alias passed to the
// rule definition.
throw Starlark.errorf(
"In %s, toolchain type %s was requested but only types [%s] are configured",
targetDescription(),
toolchainTypeLabel,
requiredToolchainTypes().stream()
.map(ToolchainTypeInfo::typeLabel)
.map(Label::toString)
.collect(joining(", ")));
}
return forToolchainType(toolchainTypeLabel);
}

@Override
public boolean containsKey(StarlarkSemantics semantics, Object key) throws EvalException {
Label toolchainTypeLabel = transformKey(key, repoMapping());
return requestedToolchainTypeLabels().containsKey(toolchainTypeLabel);
}

/**
* Exception used when a toolchain type is required but the resolved target does not have
* ToolchainInfo.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,8 @@ public StarlarkExecGroupContext getIndex(StarlarkSemantics semantics, Object key
execGroup,
String.join(", ", getScrubbedExecGroups()));
}
ToolchainContextApi toolchainContext = toolchainCollection().getToolchainContext(execGroup);
ToolchainContextApi toolchainContext =
StarlarkToolchainContext.create(toolchainCollection().getToolchainContext(execGroup));
return new AutoValue_StarlarkExecGroupCollection_StarlarkExecGroupContext(toolchainContext);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,6 @@
import com.google.devtools.build.lib.analysis.FileProvider;
import com.google.devtools.build.lib.analysis.FilesToRunProvider;
import com.google.devtools.build.lib.analysis.LocationExpander;
import com.google.devtools.build.lib.analysis.ResolvedToolchainContext;
import com.google.devtools.build.lib.analysis.RuleContext;
import com.google.devtools.build.lib.analysis.Runfiles;
import com.google.devtools.build.lib.analysis.RunfilesProvider;
Expand Down Expand Up @@ -703,22 +702,7 @@ public Dict<String, String> var() throws EvalException {
@Override
public ToolchainContextApi toolchains() throws EvalException {
checkMutable("toolchains");
ResolvedToolchainContext toolchainContext = ruleContext.getToolchainContext();
if (toolchainContext == null) {
// Starlark rules are easier if this cannot be null, so return a no-op value instead.
return new ToolchainContextApi() {
@Override
public Object getIndex(StarlarkSemantics semantics, Object key) {
return Starlark.NONE;
}

@Override
public boolean containsKey(StarlarkSemantics semantics, Object key) {
return false;
}
};
}
return toolchainContext;
return StarlarkToolchainContext.create(ruleContext.getToolchainContext());
}

@Override
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,127 @@
// Copyright 2021 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.starlark;

import static java.util.stream.Collectors.joining;

import com.google.auto.value.AutoValue;
import com.google.devtools.build.lib.analysis.ResolvedToolchainContext;
import com.google.devtools.build.lib.analysis.platform.ToolchainInfo;
import com.google.devtools.build.lib.analysis.platform.ToolchainTypeInfo;
import com.google.devtools.build.lib.cmdline.Label;
import com.google.devtools.build.lib.cmdline.LabelSyntaxException;
import com.google.devtools.build.lib.starlarkbuildapi.platform.ToolchainContextApi;
import javax.annotation.Nullable;
import net.starlark.java.eval.EvalException;
import net.starlark.java.eval.Printer;
import net.starlark.java.eval.Starlark;
import net.starlark.java.eval.StarlarkSemantics;
import net.starlark.java.eval.StarlarkThread;

/**
* An implementation of ToolchainContextApi that can better handle converting strings into Labels.
*/
@AutoValue
public abstract class StarlarkToolchainContext implements ToolchainContextApi {

private static final ToolchainContextApi NO_OP =
new ToolchainContextApi() {
@Override
public Object getIndex(
StarlarkThread starlarkThread, StarlarkSemantics semantics, Object key) {
// TODO(jcater): throw Starlark.errorf instead of returning NONE.
return Starlark.NONE;
}

@Override
public boolean containsKey(
StarlarkThread starlarkThread, StarlarkSemantics semantics, Object key) {
return false;
}
};

public static ToolchainContextApi create(@Nullable ResolvedToolchainContext toolchainContext) {
if (toolchainContext == null) {
return NO_OP;
}

return new AutoValue_StarlarkToolchainContext(toolchainContext);
}

protected abstract ResolvedToolchainContext toolchainContext();

@Override
public boolean isImmutable() {
return true;
}

@Override
public void repr(Printer printer) {
printer.append("<toolchain_context.resolved_labels: ");
printer.append(
toolchainContext().toolchains().keySet().stream()
.map(ToolchainTypeInfo::typeLabel)
.map(Label::toString)
.collect(joining(", ")));
printer.append(">");
}

private Label transformKey(StarlarkThread starlarkThread, Object key) throws EvalException {
if (key instanceof Label) {
return (Label) key;
} else if (key instanceof ToolchainTypeInfo) {
return ((ToolchainTypeInfo) key).typeLabel();
} else if (key instanceof String) {
try {
// TODO(jcater): Use LabelConversionContext and StarlarkThread to convert this instead.
// Also remove repoMapping from ResolvedToolchainContext.
return Label.parseAbsolute((String) key, toolchainContext().repoMapping());
} catch (LabelSyntaxException e) {
throw Starlark.errorf("Unable to parse toolchain label '%s': %s", key, e.getMessage());
}
} else {
throw Starlark.errorf(
"Toolchains only supports indexing by toolchain type, got %s instead",
Starlark.type(key));
}
}

@Override
public ToolchainInfo getIndex(
StarlarkThread starlarkThread, StarlarkSemantics semantics, Object key) throws EvalException {
Label toolchainTypeLabel = transformKey(starlarkThread, key);

if (!containsKey(starlarkThread, semantics, key)) {
// TODO(bazel-configurability): The list of available toolchain types is confusing in the
// presence of aliases, since it only contains the actual label, not the alias passed to the
// rule definition.
throw Starlark.errorf(
"In %s, toolchain type %s was requested but only types [%s] are configured",
toolchainContext().targetDescription(),
toolchainTypeLabel,
toolchainContext().requiredToolchainTypes().stream()
.map(ToolchainTypeInfo::typeLabel)
.map(Label::toString)
.collect(joining(", ")));
}
return toolchainContext().forToolchainType(toolchainTypeLabel);
}

@Override
public boolean containsKey(StarlarkThread starlarkThread, StarlarkSemantics semantics, Object key)
throws EvalException {
Label toolchainTypeLabel = transformKey(starlarkThread, key);
return toolchainContext().requestedToolchainTypeLabels().containsKey(toolchainTypeLabel);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -27,4 +27,4 @@
"Holds toolchains available for a particular exec group. Toolchain targets are accessed by"
+ " indexing with the toolchain type, as in"
+ " <code>context[\"//pkg:my_toolchain_type\"]</code>.")
public interface ToolchainContextApi extends StarlarkValue, StarlarkIndexable {}
public interface ToolchainContextApi extends StarlarkValue, StarlarkIndexable.Threaded {}

0 comments on commit 627c16e

Please sign in to comment.