From d6e61641e9eb232064004cc81719714dca83d3c7 Mon Sep 17 00:00:00 2001 From: twerth Date: Mon, 4 Nov 2019 00:46:41 -0800 Subject: [PATCH] Make experimental_genquery_use_graphless_query a TriState. RELNOTES: None PiperOrigin-RevId: 278320174 --- .../lib/analysis/config/CoreOptions.java | 2 +- .../build/lib/rules/genquery/GenQuery.java | 25 +++++++++++-------- 2 files changed, 16 insertions(+), 11 deletions(-) diff --git a/src/main/java/com/google/devtools/build/lib/analysis/config/CoreOptions.java b/src/main/java/com/google/devtools/build/lib/analysis/config/CoreOptions.java index 9dd32fd71fe28b..b20e03775b2f02 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/config/CoreOptions.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/config/CoreOptions.java @@ -751,7 +751,7 @@ public OutputPathsConverter() { OptionEffectTag.LOADING_AND_ANALYSIS }, help = "Whether to use graphless query and disable output ordering.") - public boolean useGraphlessQuery; + public TriState useGraphlessQuery; @Option( name = "experimental_inmemory_unused_inputs_list", diff --git a/src/main/java/com/google/devtools/build/lib/rules/genquery/GenQuery.java b/src/main/java/com/google/devtools/build/lib/rules/genquery/GenQuery.java index c833ecd8eed064..b49c9eea1500e8 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/genquery/GenQuery.java +++ b/src/main/java/com/google/devtools/build/lib/rules/genquery/GenQuery.java @@ -76,6 +76,7 @@ import com.google.devtools.build.lib.query2.query.output.QueryOptions; import com.google.devtools.build.lib.query2.query.output.QueryOptions.OrderOutput; import com.google.devtools.build.lib.query2.query.output.QueryOutputUtils; +import com.google.devtools.build.lib.query2.query.output.StreamedFormatter; import com.google.devtools.build.lib.rules.genquery.GenQueryOutputStream.GenQueryResult; import com.google.devtools.build.lib.runtime.KeepGoingOption; import com.google.devtools.build.lib.skyframe.PackageValue; @@ -164,14 +165,8 @@ public ConfiguredTarget create(RuleContext ruleContext) ruleContext.attributeError("opts", "option --experimental_graphless_query is not allowed"); return null; } - if (ruleContext.getConfiguration().getOptions().get(CoreOptions.class).useGraphlessQuery) { - queryOptions.orderOutput = OrderOutput.NO; - queryOptions.useGraphlessQuery = TriState.YES; - } else { - // Force results to be deterministic. - queryOptions.orderOutput = OrderOutput.FULL; - queryOptions.useGraphlessQuery = TriState.NO; - } + queryOptions.useGraphlessQuery = + ruleContext.getConfiguration().getOptions().get(CoreOptions.class).useGraphlessQuery; // force relative_locations to true so it has a deterministic output across machines. queryOptions.relativeLocations = true; @@ -329,7 +324,7 @@ private GenQueryResult doQuery( QueryEvalResult queryResult; OutputFormatter formatter; AggregateAllOutputFormatterCallback targets; - boolean graphlessQuery = queryOptions.useGraphlessQuery == TriState.YES; + boolean graphlessQuery = false; try { Set settings = queryOptions.toSettings(); @@ -344,6 +339,16 @@ private GenQueryResult doQuery( OutputFormatters.formatterNames(OutputFormatters.getDefaultFormatters()))); return null; } + graphlessQuery = + queryOptions.useGraphlessQuery == TriState.YES + || (queryOptions.useGraphlessQuery == TriState.AUTO + && formatter instanceof StreamedFormatter); + if (graphlessQuery) { + queryOptions.orderOutput = OrderOutput.NO; + } else { + // Force results to be deterministic. + queryOptions.orderOutput = OrderOutput.FULL; + } AbstractBlazeQueryEnvironment queryEnvironment = QUERY_ENVIRONMENT_FACTORY.create( /*transitivePackageLoader=*/ null, @@ -354,7 +359,7 @@ private GenQueryResult doQuery( PathFragment.EMPTY_FRAGMENT, /*keepGoing=*/ false, ruleContext.attributes().get("strict", Type.BOOLEAN), - /*orderedResults=*/ !QueryOutputUtils.shouldStreamResults(queryOptions, formatter), + /*orderedResults=*/ !graphlessQuery, /*universeScope=*/ ImmutableList.of(), // Use a single thread to prevent race conditions causing nondeterministic output // (b/127644784). All the packages are already loaded at this point, so there is