Skip to content

Commit

Permalink
Do not trim test configuration when --nodistinct_host_configuration i…
Browse files Browse the repository at this point in the history
…s on.

Background is in b/117932061. The infrastructure avoids trimming the host configuration and thus the untrimmed target configuration is used in various places when --nodistinct_host_configuration is on. This leads to potential issues.

This may fixable more principally by allowing and ensuring the effective host configuration is trimmed when --nodistinct_host_configuration is in use; however, this requires far more intricate adjustments to configuration resolution.

PiperOrigin-RevId: 364898085
  • Loading branch information
twigg authored and copybara-github committed Mar 24, 2021
1 parent c792ed3 commit 0b51d43
Show file tree
Hide file tree
Showing 2 changed files with 9 additions and 2 deletions.
1 change: 1 addition & 0 deletions src/main/java/com/google/devtools/build/lib/analysis/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -2348,6 +2348,7 @@ java_library(
deps = [
":config/build_options",
":config/build_options_cache",
":config/core_options",
":config/fragment_options",
":config/transitions/no_transition",
":config/transitions/patch_transition",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
import com.google.devtools.build.lib.analysis.config.BuildOptions;
import com.google.devtools.build.lib.analysis.config.BuildOptionsCache;
import com.google.devtools.build.lib.analysis.config.BuildOptionsView;
import com.google.devtools.build.lib.analysis.config.CoreOptions;
import com.google.devtools.build.lib.analysis.config.FragmentOptions;
import com.google.devtools.build.lib.analysis.config.transitions.NoTransition;
import com.google.devtools.build.lib.analysis.config.transitions.PatchTransition;
Expand Down Expand Up @@ -52,7 +53,7 @@ public enum TestTrimmingTransition implements PatchTransition {

@Override
public ImmutableSet<Class<? extends FragmentOptions>> requiresOptionFragments() {
return ImmutableSet.of(TestOptions.class);
return ImmutableSet.of(TestOptions.class, CoreOptions.class);
}

@Override
Expand All @@ -61,9 +62,14 @@ public BuildOptions patch(BuildOptionsView originalOptions, EventHandler eventHa
// nothing to do, already trimmed this fragment
return originalOptions.underlying();
}
CoreOptions originalCoreOptions = originalOptions.get(CoreOptions.class);
TestOptions originalTestOptions = originalOptions.get(TestOptions.class);
if (!originalTestOptions.trimTestConfiguration) {
if (!originalTestOptions.trimTestConfiguration
|| !originalCoreOptions.useDistinctHostConfiguration) {
// nothing to do, trimming is disabled
// Due to repercussions of b/117932061, do not trim when `--nodistinct_host_configuration`
// TODO(twigg): See if can remove distinct_host_configuration read here and thus
// dependency on CoreOptions above.
return originalOptions.underlying();
}
return cache.applyTransition(
Expand Down

0 comments on commit 0b51d43

Please sign in to comment.