Skip to content

Commit

Permalink
Add optional tool_path_origin to Tools in C++ crosstool
Browse files Browse the repository at this point in the history 
This change adds an optional field to the C++ crosstool proto that
allows configuring the origin tool_path is relative to.

For now, the origin can be one of the following:
  - CROSSTOOL_PACKAGE: If tool_path is a relative path, it's assumed to
    be relative to the package of the crosstool top.
  - FILESYSTEM_ROOT: tool_path is an absolute path. This is checked by
    Bazel.
  - WORKSPACE_ROOT: tool_path must be a relative path and is assumed to
    be relative to the exec-root of the workspace (similar to other
    executables).

Updates #8438
  • Loading branch information
@Yannic
Yannic committed Mar 15, 2020
1 parent 7475b77 commit 92ffa4c
Show file tree
Hide file tree
Showing 6 changed files with 348 additions and 85 deletions.
108 changes: 78 additions & 30 deletions src/main/java/com/google/devtools/build/lib/rules/cpp/CcModule.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@

import com.google.common.annotations.VisibleForTesting;
import com.google.common.base.Joiner;
import com.google.common.base.Preconditions;
import com.google.common.base.Strings;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
Expand Down Expand Up @@ -997,8 +998,8 @@ private static Object getValueOrNull(ClassObject x, String name) {
@VisibleForTesting
static Feature featureFromSkylark(SkylarkInfo featureStruct) throws EvalException {
checkRightProviderType(featureStruct, "feature");
String name = getFieldFromSkylarkProvider(featureStruct, "name", String.class);
Boolean enabled = getFieldFromSkylarkProvider(featureStruct, "enabled", Boolean.class);
String name = getMandatoryFieldFromSkylarkProvider(featureStruct, "name", String.class);
Boolean enabled = getMandatoryFieldFromSkylarkProvider(featureStruct, "enabled", Boolean.class);
if (name == null || (name.isEmpty() && !enabled)) {
throw new EvalException(
featureStruct.getCreationLoc(),
Expand Down Expand Up @@ -1072,8 +1073,8 @@ static Feature featureFromSkylark(SkylarkInfo featureStruct) throws EvalExceptio
static Pair<String, String> makeVariableFromSkylark(SkylarkInfo makeVariableStruct)
throws EvalException {
checkRightProviderType(makeVariableStruct, "make_variable");
String name = getFieldFromSkylarkProvider(makeVariableStruct, "name", String.class);
String value = getFieldFromSkylarkProvider(makeVariableStruct, "value", String.class);
String name = getMandatoryFieldFromSkylarkProvider(makeVariableStruct, "name", String.class);
String value = getMandatoryFieldFromSkylarkProvider(makeVariableStruct, "value", String.class);
if (name == null || name.isEmpty()) {
throw new EvalException(
makeVariableStruct.getCreationLoc(),
Expand All @@ -1095,8 +1096,8 @@ static Pair<String, String> makeVariableFromSkylark(SkylarkInfo makeVariableStru
@VisibleForTesting
static Pair<String, String> toolPathFromSkylark(SkylarkInfo toolPathStruct) throws EvalException {
checkRightProviderType(toolPathStruct, "tool_path");
String name = getFieldFromSkylarkProvider(toolPathStruct, "name", String.class);
String path = getFieldFromSkylarkProvider(toolPathStruct, "path", String.class);
String name = getMandatoryFieldFromSkylarkProvider(toolPathStruct, "name", String.class);
String path = getMandatoryFieldFromSkylarkProvider(toolPathStruct, "path", String.class);
if (name == null || name.isEmpty()) {
throw new EvalException(
toolPathStruct.getCreationLoc(),
Expand All @@ -1115,8 +1116,10 @@ static Pair<String, String> toolPathFromSkylark(SkylarkInfo toolPathStruct) thro
static VariableWithValue variableWithValueFromSkylark(SkylarkInfo variableWithValueStruct)
throws EvalException {
checkRightProviderType(variableWithValueStruct, "variable_with_value");
String name = getFieldFromSkylarkProvider(variableWithValueStruct, "name", String.class);
String value = getFieldFromSkylarkProvider(variableWithValueStruct, "value", String.class);
String name =
getMandatoryFieldFromSkylarkProvider(variableWithValueStruct, "name", String.class);
String value =
getMandatoryFieldFromSkylarkProvider(variableWithValueStruct, "value", String.class);
if (name == null || name.isEmpty()) {
throw new EvalException(
variableWithValueStruct.getCreationLoc(),
Expand All @@ -1134,8 +1137,8 @@ static VariableWithValue variableWithValueFromSkylark(SkylarkInfo variableWithVa
@VisibleForTesting
static EnvEntry envEntryFromSkylark(SkylarkInfo envEntryStruct) throws EvalException {
checkRightProviderType(envEntryStruct, "env_entry");
String key = getFieldFromSkylarkProvider(envEntryStruct, "key", String.class);
String value = getFieldFromSkylarkProvider(envEntryStruct, "value", String.class);
String key = getMandatoryFieldFromSkylarkProvider(envEntryStruct, "key", String.class);
String value = getMandatoryFieldFromSkylarkProvider(envEntryStruct, "value", String.class);
if (key == null || key.isEmpty()) {
throw new EvalException(
envEntryStruct.getCreationLoc(),
Expand Down Expand Up @@ -1216,17 +1219,19 @@ static FlagGroup flagGroupFromSkylark(SkylarkInfo flagGroupStruct) throws EvalEx
flagGroupStruct.getCreationLoc(), "Both 'flags' and 'flag_groups' are empty.");
}

String iterateOver = getFieldFromSkylarkProvider(flagGroupStruct, "iterate_over", String.class);
String iterateOver =
getMandatoryFieldFromSkylarkProvider(flagGroupStruct, "iterate_over", String.class);
String expandIfAvailable =
getFieldFromSkylarkProvider(flagGroupStruct, "expand_if_available", String.class);
getMandatoryFieldFromSkylarkProvider(flagGroupStruct, "expand_if_available", String.class);
String expandIfNotAvailable =
getFieldFromSkylarkProvider(flagGroupStruct, "expand_if_not_available", String.class);
getMandatoryFieldFromSkylarkProvider(
flagGroupStruct, "expand_if_not_available", String.class);
String expandIfTrue =
getFieldFromSkylarkProvider(flagGroupStruct, "expand_if_true", String.class);
getMandatoryFieldFromSkylarkProvider(flagGroupStruct, "expand_if_true", String.class);
String expandIfFalse =
getFieldFromSkylarkProvider(flagGroupStruct, "expand_if_false", String.class);
getMandatoryFieldFromSkylarkProvider(flagGroupStruct, "expand_if_false", String.class);
SkylarkInfo expandIfEqualStruct =
getFieldFromSkylarkProvider(flagGroupStruct, "expand_if_equal", SkylarkInfo.class);
getMandatoryFieldFromSkylarkProvider(flagGroupStruct, "expand_if_equal", SkylarkInfo.class);
VariableWithValue expandIfEqual =
expandIfEqualStruct == null ? null : variableWithValueFromSkylark(expandIfEqualStruct);

Expand Down Expand Up @@ -1280,12 +1285,37 @@ static FlagSet flagSetFromSkylark(SkylarkInfo flagSetStruct, String actionName)
@VisibleForTesting
static CcToolchainFeatures.Tool toolFromSkylark(SkylarkInfo toolStruct) throws EvalException {
checkRightProviderType(toolStruct, "tool");
String toolPathString = getFieldFromSkylarkProvider(toolStruct, "path", String.class);
PathFragment toolPath = toolPathString == null ? null : PathFragment.create(toolPathString);
if (toolPath != null && toolPath.isEmpty()) {
throw new EvalException(
toolStruct.getCreationLoc(), "The 'path' field of tool must be a nonempty string.");

String toolPathString = getOptionalFieldFromSkylarkProvider(toolStruct, "path", String.class);
Artifact toolArtifact = getOptionalFieldFromSkylarkProvider(toolStruct, "tool", Artifact.class);

PathFragment toolPath;
CToolchain.Tool.PathOrigin toolPathOrigin;
if (toolPathString != null) {
if (toolArtifact != null) {
throw new EvalException(
toolStruct.getCreationLoc(), "\"tool\" and \"path\" cannot be set at the same time.");
}

toolPath = PathFragment.create(toolPathString);
if (toolPath.isEmpty()) {
throw new EvalException(
toolStruct.getCreationLoc(), "The 'path' field of tool must be a nonempty string.");
}

if (toolPath.isAbsolute()) {
toolPathOrigin = CToolchain.Tool.PathOrigin.FILESYSTEM_ROOT;
} else {
toolPathOrigin = CToolchain.Tool.PathOrigin.CROSSTOOL_PACKAGE;
}
} else if (toolArtifact != null) {
toolPath = toolArtifact.getExecPath();
toolPathOrigin = CToolchain.Tool.PathOrigin.WORKSPACE_ROOT;
} else {
throw Starlark.errorf("Exactly one of \"tool\" and \"path\" must be set.");
}
Preconditions.checkState(toolPath != null && toolPathOrigin != null);

ImmutableSet.Builder<WithFeatureSet> withFeatureSetBuilder = ImmutableSet.builder();
ImmutableList<SkylarkInfo> withFeatureSetStructs =
getSkylarkProviderListFromSkylarkField(toolStruct, "with_features");
Expand All @@ -1296,15 +1326,15 @@ static CcToolchainFeatures.Tool toolFromSkylark(SkylarkInfo toolStruct) throws E
ImmutableSet<String> executionRequirements =
getStringSetFromSkylarkProviderField(toolStruct, "execution_requirements");
return new CcToolchainFeatures.Tool(
toolPath, executionRequirements, withFeatureSetBuilder.build());
toolPath, toolPathOrigin, executionRequirements, withFeatureSetBuilder.build());
}

/** Creates an {@link ActionConfig} from a {@link SkylarkInfo}. */
@VisibleForTesting
static ActionConfig actionConfigFromSkylark(SkylarkInfo actionConfigStruct) throws EvalException {
checkRightProviderType(actionConfigStruct, "action_config");
String actionName =
getFieldFromSkylarkProvider(actionConfigStruct, "action_name", String.class);
getMandatoryFieldFromSkylarkProvider(actionConfigStruct, "action_name", String.class);
if (actionName == null || actionName.isEmpty()) {
throw new EvalException(
actionConfigStruct.getCreationLoc(),
Expand All @@ -1319,7 +1349,8 @@ static ActionConfig actionConfigFromSkylark(SkylarkInfo actionConfigStruct) thro
actionName));
}

Boolean enabled = getFieldFromSkylarkProvider(actionConfigStruct, "enabled", Boolean.class);
Boolean enabled =
getMandatoryFieldFromSkylarkProvider(actionConfigStruct, "enabled", Boolean.class);

ImmutableList.Builder<CcToolchainFeatures.Tool> toolBuilder = ImmutableList.builder();
ImmutableList<SkylarkInfo> toolStructs =
Expand Down Expand Up @@ -1348,7 +1379,8 @@ static ArtifactNamePattern artifactNamePatternFromSkylark(SkylarkInfo artifactNa
throws EvalException {
checkRightProviderType(artifactNamePatternStruct, "artifact_name_pattern");
String categoryName =
getFieldFromSkylarkProvider(artifactNamePatternStruct, "category_name", String.class);
getMandatoryFieldFromSkylarkProvider(
artifactNamePatternStruct, "category_name", String.class);
if (categoryName == null || categoryName.isEmpty()) {
throw new EvalException(
artifactNamePatternStruct.getCreationLoc(),
Expand All @@ -1369,7 +1401,8 @@ static ArtifactNamePattern artifactNamePatternFromSkylark(SkylarkInfo artifactNa

String extension =
Strings.nullToEmpty(
getFieldFromSkylarkProvider(artifactNamePatternStruct, "extension", String.class));
getMandatoryFieldFromSkylarkProvider(
artifactNamePatternStruct, "extension", String.class));
if (!foundCategory.getAllowedExtensions().contains(extension)) {
throw new EvalException(
artifactNamePatternStruct.getCreationLoc(),
Expand All @@ -1383,16 +1416,31 @@ static ArtifactNamePattern artifactNamePatternFromSkylark(SkylarkInfo artifactNa

String prefix =
Strings.nullToEmpty(
getFieldFromSkylarkProvider(artifactNamePatternStruct, "prefix", String.class));
getMandatoryFieldFromSkylarkProvider(
artifactNamePatternStruct, "prefix", String.class));
return new ArtifactNamePattern(foundCategory, prefix, extension);
}

private static <T> T getFieldFromSkylarkProvider(
private static <T> T getOptionalFieldFromSkylarkProvider(
SkylarkInfo provider, String fieldName, Class<T> clazz) throws EvalException {
return getFieldFromSkylarkProvider(provider, fieldName, clazz, false);
}

private static <T> T getMandatoryFieldFromSkylarkProvider(
SkylarkInfo provider, String fieldName, Class<T> clazz) throws EvalException {
return getFieldFromSkylarkProvider(provider, fieldName, clazz, true);
}

private static <T> T getFieldFromSkylarkProvider(
SkylarkInfo provider, String fieldName, Class<T> clazz, boolean mandatory)
throws EvalException {
Object obj = provider.getValue(fieldName);
if (obj == null) {
throw new EvalException(
provider.getCreationLoc(), String.format("Missing mandatory field '%s'", fieldName));
if (mandatory) {
throw new EvalException(
provider.getCreationLoc(), String.format("Missing mandatory field '%s'", fieldName));
}
return null;
}
if (clazz.isInstance(obj)) {
return clazz.cast(obj);
Expand Down
1 change: 1 addition & 0 deletions src/main/java/com/google/devtools/build/lib/rules/cpp/CcToolchainConfigInfo.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -385,6 +385,7 @@ private static CToolchain.FeatureSet featureSetToProto(ImmutableSet<String> feat
private static CToolchain.Tool toolToProto(Tool tool) {
return CToolchain.Tool.newBuilder()
.setToolPath(tool.getToolPathFragment().toString())
.setToolPathOrigin(tool.getToolPathOrigin())
.addAllWithFeature(
tool.getWithFeatureSetSets().stream()
.map(withFeatureSet -> withFeatureSetToProto(withFeatureSet))
Expand Down
103 changes: 82 additions & 21 deletions src/main/java/com/google/devtools/build/lib/rules/cpp/CcToolchainFeatures.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@
import com.google.devtools.build.lib.skyframe.serialization.autocodec.AutoCodec;
import com.google.devtools.build.lib.skyframe.serialization.autocodec.AutoCodec.VisibleForSerialization;
import com.google.devtools.build.lib.syntax.EvalException;
import com.google.devtools.build.lib.syntax.Starlark;
import com.google.devtools.build.lib.util.Pair;
import com.google.devtools.build.lib.util.StringUtil;
import com.google.devtools.build.lib.vfs.PathFragment;
Expand Down Expand Up @@ -791,7 +792,7 @@ public static class Feature implements Serializable, CrosstoolSelectable {
flagSetBuilder.add(new FlagSet(flagSet));
}
this.flagSets = flagSetBuilder.build();

ImmutableList.Builder<EnvSet> envSetBuilder = ImmutableList.builder();
for (CToolchain.EnvSet flagSet : feature.getEnvSetList()) {
envSetBuilder.add(new EnvSet(flagSet));
Expand Down Expand Up @@ -925,30 +926,88 @@ public ImmutableList<String> getProvides() {
@Immutable
public static class Tool {
private final PathFragment toolPathFragment;
private final CToolchain.Tool.PathOrigin toolPathOrigin;
private final ImmutableSet<String> executionRequirements;
private final ImmutableSet<WithFeatureSet> withFeatureSetSets;

private Tool(
CToolchain.Tool tool,
ImmutableSet<WithFeatureSet> withFeatureSetSets) {
this.withFeatureSetSets = withFeatureSetSets;
this.toolPathFragment = PathFragment.create(tool.getToolPath());
executionRequirements = ImmutableSet.copyOf(tool.getExecutionRequirementList());
ImmutableSet<WithFeatureSet> withFeatureSetSets) throws EvalException {
this(
PathFragment.create(tool.getToolPath()),
tool.getToolPathOrigin(),
ImmutableSet.copyOf(tool.getExecutionRequirementList()),
withFeatureSetSets);
}

@VisibleForTesting
public Tool(
PathFragment toolPathFragment,
CToolchain.Tool.PathOrigin toolPathOrigin,
ImmutableSet<String> executionRequirements,
ImmutableSet<WithFeatureSet> withFeatureSetSets) {
ImmutableSet<WithFeatureSet> withFeatureSetSets) throws EvalException {
checkToolPath(toolPathFragment, toolPathOrigin);
this.toolPathFragment = toolPathFragment;
this.toolPathOrigin = toolPathOrigin;
this.executionRequirements = executionRequirements;
this.withFeatureSetSets = withFeatureSetSets;
}

@Deprecated
@VisibleForTesting
public Tool(
PathFragment toolPathFragment,
ImmutableSet<String> executionRequirements,
ImmutableSet<WithFeatureSet> withFeatureSetSets) throws EvalException {
this(
toolPathFragment,
CToolchain.Tool.PathOrigin.CROSSTOOL_PACKAGE,
executionRequirements,
withFeatureSetSets);
}

private static void checkToolPath(
PathFragment toolPath, CToolchain.Tool.PathOrigin origin) throws EvalException {
switch (origin) {
case CROSSTOOL_PACKAGE:
// For legacy reasons, we allow absolute and relative paths here.
return;

case FILESYSTEM_ROOT:
if (!toolPath.isAbsolute()) {
throw Starlark.errorf(
"Tool-path with origin FILESYSTEM_ROOT must be absolute, got '%s'.",
toolPath.getPathString());
}
return;

case WORKSPACE_ROOT:
if (toolPath.isAbsolute()) {
throw Starlark.errorf(
"Tool-path with origin WORKSPACE_ROOT must be relative, got '%s'.",
toolPath.getPathString());
}
return;
}

// Unreached.
throw new IllegalStateException();
}

/** Returns the path to this action's tool relative to the provided crosstool path. */
String getToolPathString(PathFragment ccToolchainPath) {
return ccToolchainPath.getRelative(toolPathFragment).getSafePathString();
switch (toolPathOrigin) {
case CROSSTOOL_PACKAGE:
// Legacy behavior.
return ccToolchainPath.getRelative(toolPathFragment).getSafePathString();

case FILESYSTEM_ROOT: // fallthrough.
case WORKSPACE_ROOT:
return toolPathFragment.getSafePathString();
}

// Unreached.
throw new IllegalStateException();
}

/**
Expand All @@ -969,6 +1028,10 @@ ImmutableSet<WithFeatureSet> getWithFeatureSetSets() {
PathFragment getToolPathFragment() {
return toolPathFragment;
}

CToolchain.Tool.PathOrigin getToolPathOrigin() {
return toolPathOrigin;
}
}

/**
Expand Down Expand Up @@ -1011,19 +1074,17 @@ public static class ActionConfig implements Serializable, CrosstoolSelectable {
ActionConfig(CToolchain.ActionConfig actionConfig) throws EvalException {
this.configName = actionConfig.getConfigName();
this.actionName = actionConfig.getActionName();
this.tools =
actionConfig
.getToolList()
.stream()
.map(
t ->
new Tool(
t,
t.getWithFeatureList()
.stream()
.map(f -> new WithFeatureSet(f))
.collect(ImmutableSet.toImmutableSet())))
.collect(ImmutableList.toImmutableList());

ImmutableList.Builder<Tool> tools = ImmutableList.builder();
for (CToolchain.Tool tool : actionConfig.getToolList()) {
ImmutableSet<WithFeatureSet> withFeatureSetSets =
tool.getWithFeatureList()
.stream()
.map(f -> new WithFeatureSet(f))
.collect(ImmutableSet.toImmutableSet());
tools.add(new Tool(tool, withFeatureSetSets));
}
this.tools = tools.build();

ImmutableList.Builder<FlagSet> flagSetBuilder = ImmutableList.builder();
for (CToolchain.FlagSet flagSet : actionConfig.getFlagSetList()) {
Expand Down Expand Up @@ -1525,7 +1586,7 @@ public CcToolchainFeatures(
}
}
this.defaultSelectables = defaultSelectablesBuilder.build();

this.selectables = selectablesBuilder.build();
this.selectablesByName = ImmutableMap.copyOf(selectablesByName);

Expand Down
Loading

0 comments on commit 92ffa4c

Please sign in to comment.