Skip to content

Commit

Permalink
[2] Trim input build settings after transition
Browse files Browse the repository at this point in the history
By convention, BuildOptions should not contain Starlark build settings
that are set to their default values. However, if a Starlark transition
has a build setting as an input, the value of that setting has to be
added to the input BuildOptions temporarily so that it can be read by
the transition.

Before this commit, if a build setting was an input but not an output of
a transition, its default value remained in the BuildOptions and could
cause action conflicts. With this commit, all build settings rather than
only output build settings are trimmed post-transition if they are set
to their default value.

Fixes #12888.
  • Loading branch information
fmeum committed Sep 25, 2021
1 parent fd79299 commit 10d8b83
Show file tree
Hide file tree
Showing 2 changed files with 49 additions and 0 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -517,6 +517,12 @@ private static Map<String, BuildOptions> applyStarlarkTransition(
throw new TransitionException("Errors encountered while applying Starlark transition");
}
result = StarlarkTransition.validate(transition, buildSettingPackages, result);
// Starlark build settings set to default values explicitly on the command line are not included
// in build options, but StarlarkTransition.addDefaultStarlarkOption might have added some so
// that the transition can read its inputs. Trim all these explicitly set defaults here to
// prevent action conflicts.
result = StarlarkTransition
.trimDefaultStarlarkOptions(result, transition, buildSettingPackages);
// If the transition errored (like bad Starlark code), this method already exited with an
// exception so the results won't go into the cache. We still want to collect non-error events
// like print() output.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@
import java.util.HashSet;
import java.util.List;
import java.util.Map;
import java.util.Map.Entry;
import java.util.Objects;
import java.util.Set;
import java.util.concurrent.atomic.AtomicBoolean;
Expand Down Expand Up @@ -447,6 +448,48 @@ public static BuildOptions addDefaultStarlarkOptions(
return optionsWithDefaults == null ? fromOptions : optionsWithDefaults.build();
}

/*
* Removes all build settings explicitly set to their default values from the output build options
* of a transition. This cancels the effect of addDefaultStarlarkOptions post-transition if called
* with the same {@code buildSettingPackages} and thus restores the invariant that default-valued
* Starlark build settings are not stored in {@code BuildOptions}.
*/
public static Map<String, BuildOptions> trimDefaultStarlarkOptions(
Map<String, BuildOptions> toOptions,
ConfigurationTransition transition,
Map<PackageValue.Key, PackageValue> buildSettingPackages)
throws TransitionException {
if (buildSettingPackages.isEmpty()) {
return toOptions;
}
ImmutableMap<Label, Object> buildSettingDefaults =
getDefaultValues(buildSettingPackages, transition);
ImmutableMap.Builder<String, BuildOptions> toOptionsBuilder = ImmutableMap
.builderWithExpectedSize(toOptions.size());
for (Entry<String, BuildOptions> entry : toOptions.entrySet()) {
BuildOptions options = entry.getValue();
BuildOptions.Builder optionsWithoutDefaults = null;
for (Map.Entry<Label, Object> buildSettingDefault : buildSettingDefaults.entrySet()) {
Label buildSetting = buildSettingDefault.getKey();
if (!options.getStarlarkOptions().containsKey(buildSetting)) {
continue;
}
Object value = options.getStarlarkOptions().get(buildSetting);
Object defaultValue = buildSettingDefault.getValue();
if ((value == null && defaultValue == null) ||
(value != null && value.equals(defaultValue))) {
if (optionsWithoutDefaults == null) {
optionsWithoutDefaults = options.toBuilder();
}
optionsWithoutDefaults.removeStarlarkOption(buildSetting);
}
}
toOptionsBuilder.put(entry.getKey(),
optionsWithoutDefaults == null ? entry.getValue() : optionsWithoutDefaults.build());
}
return toOptionsBuilder.build();
}

/**
* For a given transition, find all Starlark build settings that are input/output while applying
* it, then return a map of their label to their default values.
Expand Down

0 comments on commit 10d8b83

Please sign in to comment.