Skip to content

Commit

Permalink
Add --experimental_output_directory_naming_scheme with two modes.
Browse files Browse the repository at this point in the history
The `legacy` mode retains pre-existing behavior for determining the ST hash value by tracking an `affected by starlark transition` list and consuming it.

The new `diff_against_baseline` mode determines the ST hash value by diffing the current configuration against the build's baseline configuration (currently, the configuration used to prime all top-level targets). Note that this mode specifically turns off tracking the `affected by starlark transition` for performance reasons (see linked bug for details).

Related github issue: bazelbuild#14023

PiperOrigin-RevId: 433882310
  • Loading branch information
twigg authored and copybara-github committed Mar 11, 2022
1 parent 50dc446 commit 52d1d4a
Show file tree
Hide file tree
Showing 15 changed files with 727 additions and 183 deletions.
14 changes: 12 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 @@ -1536,7 +1536,6 @@ java_library(
":config/invalid_configuration_exception",
":config/run_under",
":platform_options",
":starlark/function_transition_util",
"//src/main/java/com/google/devtools/build/lib/actions",
"//src/main/java/com/google/devtools/build/lib/actions:artifacts",
"//src/main/java/com/google/devtools/build/lib/buildeventstream",
Expand Down Expand Up @@ -1867,6 +1866,17 @@ java_library(
],
)

java_library(
name = "config/optioninfo",
srcs = ["config/OptionInfo.java"],
deps = [
":config/build_options",
":config/fragment_options",
"//src/main/java/com/google/devtools/common/options",
"//third_party:guava",
],
)

# TODO(b/144899336): This should be config/transitions/BUILD
java_library(
name = "config/transitions/composing_transition",
Expand Down Expand Up @@ -2172,12 +2182,12 @@ java_library(
":config/build_options",
":config/core_options",
":config/fragment_options",
":config/optioninfo",
":config/starlark_defined_config_transition",
":config/transitions/configuration_transition",
"//src/main/java/com/google/devtools/build/lib/cmdline",
"//src/main/java/com/google/devtools/build/lib/events",
"//src/main/java/com/google/devtools/build/lib/packages",
"//src/main/java/com/google/devtools/build/lib/util",
"//src/main/java/com/google/devtools/common/options",
"//src/main/java/net/starlark/java/eval",
"//third_party:guava",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,6 @@
import com.google.devtools.build.lib.analysis.BlazeDirectories;
import com.google.devtools.build.lib.analysis.PlatformOptions;
import com.google.devtools.build.lib.analysis.config.OutputDirectories.InvalidMnemonicException;
import com.google.devtools.build.lib.analysis.starlark.FunctionTransitionUtil;
import com.google.devtools.build.lib.buildeventstream.BuildEventIdUtil;
import com.google.devtools.build.lib.buildeventstream.BuildEventStreamProtos;
import com.google.devtools.build.lib.buildeventstream.BuildEventStreamProtos.BuildEventId;
Expand Down Expand Up @@ -161,6 +160,7 @@ public static BuildConfigurationValue create(
BuildOptions buildOptions,
RepositoryName mainRepositoryName,
boolean siblingRepositoryLayout,
String transitionDirectoryNameFragment,
// Arguments below this are server-global.
BlazeDirectories directories,
GlobalStateProvider globalProvider,
Expand All @@ -175,6 +175,7 @@ public static BuildConfigurationValue create(
buildOptions,
mainRepositoryName,
siblingRepositoryLayout,
transitionDirectoryNameFragment,
directories,
fragments,
globalProvider.getReservedActionMnemonics(),
Expand All @@ -200,6 +201,7 @@ private static ImmutableSortedMap<Class<? extends Fragment>, Fragment> getConfig
BuildOptions buildOptions,
RepositoryName mainRepositoryName,
boolean siblingRepositoryLayout,
String transitionDirectoryNameFragment,
// Arguments below this are either server-global and constant or completely dependent values.
BlazeDirectories directories,
ImmutableMap<Class<? extends Fragment>, Fragment> fragments,
Expand All @@ -216,8 +218,7 @@ private static ImmutableSortedMap<Class<? extends Fragment>, Fragment> getConfig
if (buildOptions.contains(PlatformOptions.class)) {
platformOptions = buildOptions.get(PlatformOptions.class);
}
this.transitionDirectoryNameFragment =
FunctionTransitionUtil.computeOutputDirectoryNameFragment(buildOptions);
this.transitionDirectoryNameFragment = transitionDirectoryNameFragment;
this.outputDirectories =
new OutputDirectories(
directories,
Expand Down Expand Up @@ -272,12 +273,14 @@ public boolean equals(Object other) {
BuildConfigurationValue otherVal = (BuildConfigurationValue) other;
return this.buildOptions.equals(otherVal.buildOptions)
&& this.mainRepositoryName.equals(otherVal.mainRepositoryName)
&& this.siblingRepositoryLayout == otherVal.siblingRepositoryLayout;
&& this.siblingRepositoryLayout == otherVal.siblingRepositoryLayout
&& this.transitionDirectoryNameFragment.equals(otherVal.transitionDirectoryNameFragment);
}

@Override
public int hashCode() {
return Objects.hash(buildOptions, mainRepositoryName, siblingRepositoryLayout);
return Objects.hash(
buildOptions, mainRepositoryName, siblingRepositoryLayout, transitionDirectoryNameFragment);
}

private ImmutableMap<String, Class<? extends Fragment>> buildIndexOfStarlarkVisibleFragments() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -536,11 +536,11 @@ public ExecConfigurationDistinguisherSchemeConverter() {
+ " existing build actions.")
public List<Label> actionListeners;

/** Values for the --experimental_output_directory_naming_scheme options * */
/** Values for the --experimental_output_directory_naming_scheme options */
public enum OutputDirectoryNamingScheme {
/** Use `affected by starlark transition` to track configuration changes * */
/** Use `affected by starlark transition` to track configuration changes */
LEGACY,
/** Produce name based on diff from some baseline BuildOptions (usually top-level) * */
/** Produce name based on diff from some baseline BuildOptions (usually top-level) */
DIFF_AGAINST_BASELINE
}

Expand All @@ -559,7 +559,12 @@ public OutputDirectoryNamingSchemeConverter() {
documentationCategory = OptionDocumentationCategory.UNDOCUMENTED,
effectTags = {OptionEffectTag.AFFECTS_OUTPUTS},
metadataTags = {OptionMetadataTag.EXPERIMENTAL},
help = "Please only use this flag as part of a suggested migration or testing strategy.")
help =
"Please only use this flag as part of a suggested migration or testing strategy. In"
+ " legacy mode, transitions (generally only Starlark) set and use `affected by"
+ " Starlark transition` to determine the ST hash. In diff_against_baseline mode,"
+ " `affected by Starlark transition` is ignored and instead ST hash is determined,"
+ " for all configuration, by diffing against the top-level configuration.")
public OutputDirectoryNamingScheme outputDirectoryNamingScheme;

@Option(
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,69 @@
// Copyright 2022 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.config;

import static com.google.common.collect.ImmutableSet.toImmutableSet;
import static java.util.Arrays.stream;

import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.ImmutableSet;
import com.google.devtools.common.options.OptionDefinition;
import com.google.devtools.common.options.OptionMetadataTag;
import com.google.devtools.common.options.OptionsParser;

/** Stores information about a build option gathered via reflection. */
public final class OptionInfo {
private final Class<? extends FragmentOptions> optionClass;
private final OptionDefinition definition;

private OptionInfo(Class<? extends FragmentOptions> optionClass, OptionDefinition definition) {
this.optionClass = optionClass;
this.definition = definition;
}

public Class<? extends FragmentOptions> getOptionClass() {
return optionClass;
}

public OptionDefinition getDefinition() {
return definition;
}

public boolean hasOptionMetadataTag(OptionMetadataTag tag) {
return stream(getDefinition().getOptionMetadataTags()).anyMatch(tag::equals);
}

/** For all the options in the BuildOptions, build a map from option name to its information. */
public static ImmutableMap<String, OptionInfo> buildMapFrom(BuildOptions buildOptions) {
ImmutableMap.Builder<String, OptionInfo> builder = new ImmutableMap.Builder<>();

ImmutableSet<Class<? extends FragmentOptions>> optionClasses =
buildOptions.getNativeOptions().stream()
.map(FragmentOptions::getClass)
.collect(toImmutableSet());

for (Class<? extends FragmentOptions> optionClass : optionClasses) {
ImmutableList<OptionDefinition> optionDefinitions =
OptionsParser.getOptionDefinitions(optionClass);
for (OptionDefinition def : optionDefinitions) {
String optionName = def.getOptionName();
builder.put(optionName, new OptionInfo(optionClass, def));
}
}

return builder.build();
}
}
Loading

0 comments on commit 52d1d4a

Please sign in to comment.