Skip to content

Commit

Permalink
Diff against a dedicated exec version of the baseline configuration
Browse files Browse the repository at this point in the history
When `--experimental_output_directory_naming_scheme` is set to
`diff_against_baseline`, the current configuration is diffed against a
baseline and a hash of the diff is appended to the output directory.

If a target that is built in the exec configuration transitions further,
the diff would, before this change, include the (substantial) set of
flags that differ between the top-level target and exec configuration.
This made caching less efficient across builds that change flags that
are reset by the exec configuration, e.g. `--collect_code_coverage`.

This is fixed by comparing exec configuration against a dedicated exec
version of the top-level target configuration, which is safe since
`is exec configuration` itself is explicit in the output path.
  • Loading branch information
fmeum committed Apr 6, 2023
1 parent 22bc76f commit e11022a
Show file tree
Hide file tree
Showing 3 changed files with 7 additions and 1 deletion.
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,9 @@ public SkyValue compute(SkyKey skyKey, Environment env)
if (platformMappingValue == null) {
return null;
}
BuildOptions baselineOptions = PrecomputedValue.BASELINE_CONFIGURATION.get(env);
BuildOptions baselineOptions = targetOptions.get(CoreOptions.class).isExec
? PrecomputedValue.EXEC_BASELINE_CONFIGURATION.get(env)
: PrecomputedValue.BASELINE_CONFIGURATION.get(env);
try {
BuildOptions mappedBaselineOptions =
BuildConfigurationKey.withPlatformMapping(platformMappingValue, baselineOptions)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,8 @@ public static <T> Injected injected(Precomputed<T> precomputed, T value) {
// Unsharable because of complications in deserializing BuildOptions on startup due to caching.
public static final Precomputed<BuildOptions> BASELINE_CONFIGURATION =
new Precomputed<>("baseline_configuration", /*shareable=*/ false);
public static final Precomputed<BuildOptions> EXEC_BASELINE_CONFIGURATION =
new Precomputed<>("exec_baseline_configuration", /*shareable=*/ false);

private final Object value;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1223,6 +1223,8 @@ private void setStarlarkSemantics(StarlarkSemantics starlarkSemantics) {

public void setBaselineConfiguration(BuildOptions buildOptions) {
PrecomputedValue.BASELINE_CONFIGURATION.set(injectable(), buildOptions);
PrecomputedValue.EXEC_BASELINE_CONFIGURATION.set(injectable(),
buildOptions.createExecOptions());
}

public void injectExtraPrecomputedValues(List<PrecomputedValue.Injected> extraPrecomputedValues) {
Expand Down

0 comments on commit e11022a

Please sign in to comment.