From 546a20a7fdc3834bb512de984cc45ebe01f07152 Mon Sep 17 00:00:00 2001 From: Fabian Meumertzheim Date: Thu, 30 Jun 2022 13:56:38 +0200 Subject: [PATCH] Address review comments --- .../cquery/FilesOutputFormatterCallback.java | 16 ++ .../devtools/build/lib/query2/cquery/BUILD | 18 ++ .../FilesOutputFormatterCallbackTest.java | 174 ++++++++++++++++++ .../integration/configured_query_test.sh | 69 ------- 4 files changed, 208 insertions(+), 69 deletions(-) create mode 100644 src/test/java/com/google/devtools/build/lib/query2/cquery/FilesOutputFormatterCallbackTest.java diff --git a/src/main/java/com/google/devtools/build/lib/query2/cquery/FilesOutputFormatterCallback.java b/src/main/java/com/google/devtools/build/lib/query2/cquery/FilesOutputFormatterCallback.java index 3e5e996c8dbb77..fc4ec81972655c 100644 --- a/src/main/java/com/google/devtools/build/lib/query2/cquery/FilesOutputFormatterCallback.java +++ b/src/main/java/com/google/devtools/build/lib/query2/cquery/FilesOutputFormatterCallback.java @@ -1,3 +1,16 @@ +// 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.query2.cquery; import com.google.devtools.build.lib.actions.Artifact; @@ -10,6 +23,9 @@ import java.io.IOException; import java.io.OutputStream; +/** + * Cquery output formatter that prints the set of output files advertised by the matched targets. + */ public class FilesOutputFormatterCallback extends CqueryThreadsafeCallback { private final TopLevelArtifactContext topLevelArtifactContext; diff --git a/src/test/java/com/google/devtools/build/lib/query2/cquery/BUILD b/src/test/java/com/google/devtools/build/lib/query2/cquery/BUILD index 53d584a4b7097f..6eae93115cd401 100644 --- a/src/test/java/com/google/devtools/build/lib/query2/cquery/BUILD +++ b/src/test/java/com/google/devtools/build/lib/query2/cquery/BUILD @@ -163,3 +163,21 @@ java_test( "//third_party:truth", ], ) + +java_test( + name = "FilesOutputFormatterCallbackTest", + srcs = ["FilesOutputFormatterCallbackTest.java"], + shard_count = 4, + deps = [ + ":configured_target_query_helper", + ":configured_target_query_test", + "//src/main/java/com/google/devtools/build/lib/analysis:analysis_cluster", + "//src/main/java/com/google/devtools/build/lib/analysis:top_level_artifact_context", + "//src/main/java/com/google/devtools/build/lib/events", + "//src/main/java/com/google/devtools/build/lib/query2", + "//src/main/java/com/google/devtools/build/lib/query2/engine", + "//third_party:guava", + "//third_party:junit4", + "//third_party:truth", + ], +) diff --git a/src/test/java/com/google/devtools/build/lib/query2/cquery/FilesOutputFormatterCallbackTest.java b/src/test/java/com/google/devtools/build/lib/query2/cquery/FilesOutputFormatterCallbackTest.java new file mode 100644 index 00000000000000..abda389c2ce11b --- /dev/null +++ b/src/test/java/com/google/devtools/build/lib/query2/cquery/FilesOutputFormatterCallbackTest.java @@ -0,0 +1,174 @@ +// 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.query2.cquery; + +import static com.google.common.truth.Truth.assertThat; + +import com.google.common.collect.ImmutableList; +import com.google.common.eventbus.EventBus; +import com.google.devtools.build.lib.analysis.OutputGroupInfo; +import com.google.devtools.build.lib.analysis.OutputGroupInfo.ValidationMode; +import com.google.devtools.build.lib.analysis.TopLevelArtifactContext; +import com.google.devtools.build.lib.events.Event; +import com.google.devtools.build.lib.events.Reporter; +import com.google.devtools.build.lib.query2.PostAnalysisQueryEnvironment; +import com.google.devtools.build.lib.query2.engine.QueryExpression; +import com.google.devtools.build.lib.query2.engine.QueryParser; +import java.io.ByteArrayOutputStream; +import java.io.PrintStream; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.LinkedHashSet; +import java.util.List; +import java.util.Set; +import org.junit.Before; +import org.junit.Test; + +/** + * Tests cquery's {@link --output=files} format. + */ +public class FilesOutputFormatterCallbackTest extends ConfiguredTargetQueryTest { + + private CqueryOptions options; + private Reporter reporter; + private final List events = new ArrayList<>(); + + @Before + public final void defineSimpleRule() throws Exception { + writeFile( + "defs/rules.bzl", + "def _r_impl(ctx):", + " default_file = ctx.actions.declare_file(ctx.attr.name + '_default_file')", + " output_group_only = ctx.actions.declare_file(ctx.attr.name + '_output_group_only')", + " runfile = ctx.actions.declare_file(ctx.attr.name + '_runfile')", + " executable_only = ctx.actions.declare_file(ctx.attr.name + '_executable')", + " files = [default_file, output_group_only, runfile, executable_only]", + " ctx.actions.run_shell(", + " outputs = files,", + " command = '\\n'.join(['touch %s' % file.path for file in files]),", + " )", + " return [", + " DefaultInfo(", + " executable = executable_only,", + " files = depset(", + " direct = [", + " default_file,", + " ctx.file._implicit_source_dep,", + " ctx.file.explicit_source_dep,", + " ],", + " transitive = [info[DefaultInfo].files for info in ctx.attr.deps]", + " ),", + " runfiles = ctx.runfiles([runfile]),", + " ),", + " OutputGroupInfo(", + " foobar = [output_group_only],", + " ),", + " ]", + "r = rule(", + " implementation = _r_impl,", + " executable = True,", + " attrs = {", + " 'deps': attr.label_list(),", + " '_implicit_source_dep': attr.label(default = 'rules.bzl', allow_single_file = True),", + " 'explicit_source_dep': attr.label(allow_single_file = True),", + " },", + ")"); + writeFile("defs/BUILD", + "exports_files(['rules.bzl'])"); + writeFile("pkg/BUILD", + "load('//defs:rules.bzl', 'r')", + "r(", + " name = 'main',", + " explicit_source_dep = 'BUILD',", + ")", + "r(", + " name = 'other',", + " deps = [':main'],", + " explicit_source_dep = 'BUILD',", + ")"); + } + + @Before + public final void setUpCqueryOptions() { + this.options = new CqueryOptions(); + this.reporter = new Reporter(new EventBus(), events::add); + } + + private List getOutput(String queryExpression, List outputGroups) + throws Exception { + QueryExpression expression = QueryParser.parse(queryExpression, getDefaultFunctions()); + Set targetPatternSet = new LinkedHashSet<>(); + expression.collectTargetPatterns(targetPatternSet); + PostAnalysisQueryEnvironment env = + ((ConfiguredTargetQueryHelper) helper).getPostAnalysisQueryEnvironment(targetPatternSet); + + ByteArrayOutputStream output = new ByteArrayOutputStream(); + FilesOutputFormatterCallback callback = + new FilesOutputFormatterCallback( + reporter, + options, + new PrintStream(output), + getHelper().getSkyframeExecutor(), + env.getAccessor(), + // Based on BuildRequest#getTopLevelArtifactContext. + new TopLevelArtifactContext(false, false, false, + OutputGroupInfo.determineOutputGroups(outputGroups, ValidationMode.OFF, false))); + env.evaluateQuery(expression, callback); + return Arrays.asList(output.toString().split(System.lineSeparator())); + } + + @Test + public void basicQuery_defaultOutputGroup() throws Exception { + List output = getOutput("//pkg:all", ImmutableList.of()); + assertContainsExactlyWithBinDirPrefix(output, + "pkg/main_default_file", + "pkg/other_default_file" + ); + } + + @Test + public void basicQuery_defaultAndCustomOutputGroup() throws Exception { + List output = getOutput("//pkg:main", ImmutableList.of("+foobar")); + assertContainsExactlyWithBinDirPrefix(output, + "pkg/main_default_file", + "pkg/main_output_group_only" + ); + } + + @Test + public void basicQuery_customOutputGroupOnly() throws Exception { + List output = getOutput("//pkg:other", ImmutableList.of("foobar")); + assertContainsExactlyWithBinDirPrefix(output, + "pkg/other_output_group_only" + ); + } + + private void assertContainsExactlyWithBinDirPrefix(List output, String... binDirRelativePaths) { + if (binDirRelativePaths.length == 0) { + assertThat(output).isEmpty(); + return; + } + + // Extract the configuration-dependent bin dir from the first output. + assertThat(output).isNotEmpty(); + String firstPath = output.get(0); + String binDir = firstPath.substring(0, firstPath.indexOf("bin/") + "bin/".length()); + + assertThat(output).containsExactly( + Arrays.stream(binDirRelativePaths) + .map(binDirRelativePath -> binDir + binDirRelativePath) + .toArray() + ); + } +} diff --git a/src/test/shell/integration/configured_query_test.sh b/src/test/shell/integration/configured_query_test.sh index 7407e0e7ea8fe6..ff34224e4aa6be 100755 --- a/src/test/shell/integration/configured_query_test.sh +++ b/src/test/shell/integration/configured_query_test.sh @@ -1416,73 +1416,4 @@ EOF expect_not_log "QueryException" } -function test_files_output_mode() { - local -r pkg=$FUNCNAME - mkdir -p $pkg - cat > $pkg/rules.bzl <<'EOF' -def _r_impl(ctx): - default_file = ctx.actions.declare_file(ctx.attr.name + "_default_file") - output_group_only = ctx.actions.declare_file(ctx.attr.name + "_output_group_only") - runfile = ctx.actions.declare_file(ctx.attr.name + "_runfile") - executable_only = ctx.actions.declare_file(ctx.attr.name + "_executable") - - files = [default_file, output_group_only, runfile, executable_only] - - ctx.actions.run_shell( - outputs = files, - command = "\n".join(["touch %s" % file.path for file in files]), - ) - - return [ - DefaultInfo( - executable = executable_only, - files = depset([default_file]), - runfiles = ctx.runfiles([runfile]), - ), - OutputGroupInfo( - foobar = [output_group_only], - ), - ] - -r = rule( - implementation = _r_impl, - executable = True, -) -EOF - cat > $pkg/BUILD <<'EOF' -load(":rules.bzl", "r") - -r(name = "main") -r(name = "other") -EOF - - bazel cquery "//$pkg:all" --output=files \ - > output 2>"$TEST_log" || fail "Expected success" - - assert_contains "$pkg/main_default_file" output - assert_contains "$pkg/other_default_file" output - - assert_not_contains "_executable" output - assert_not_contains "_runfile" output - assert_not_contains "_output_group_only" output - - bazel cquery "//$pkg:main" --output=files --output_groups=foobar \ - > output 2>"$TEST_log" || fail "Expected success" - - assert_contains "$pkg/main_output_group_only" output - - assert_not_contains "main_default_file" output - assert_not_contains "main_executable" output - assert_not_contains "main_runfile" output - - bazel cquery "//$pkg:other" --output=files --output_groups=+foobar \ - > output 2>"$TEST_log" || fail "Expected success" - - assert_contains "other_default_file" output - assert_contains "$pkg/other_output_group_only" output - - assert_not_contains "other_executable" output - assert_not_contains "other_runfile" output -} - run_suite "${PRODUCT_NAME} configured query tests"