From ca943735af8daeb97b901998fd8595500d45ed9b Mon Sep 17 00:00:00 2001 From: Fabian Meumertzheim Date: Fri, 28 Oct 2022 13:06:49 +0200 Subject: [PATCH 1/3] Optionally output source files with `cquery --output=files` RELNOTES: `cquery --output=files` can be configured to additionally output source files with the new `--files:include_source_files` flag. --- site/en/query/cquery.md | 3 +- .../lib/query2/cquery/CqueryOptions.java | 8 ++++ .../cquery/FilesOutputFormatterCallback.java | 7 ++- .../FilesOutputFormatterCallbackTest.java | 43 ++++++++++++++++--- .../integration/configured_query_test.sh | 19 ++++++++ 5 files changed, 72 insertions(+), 8 deletions(-) diff --git a/site/en/query/cquery.md b/site/en/query/cquery.md index e920bceb262a05..2c6aefde34c0de 100644 --- a/site/en/query/cquery.md +++ b/site/en/query/cquery.md @@ -370,7 +370,8 @@ by the query similar to the list printed at the end of a `bazel build` invocation. The output contains only the files advertised in the requested output groups as determined by the [`--output_groups`](/reference/command-line-reference#flag--output_groups) flag -and never contains source files. +and does not contain source files unless `--files:include_source_files` is +specified. Note: The output of `bazel cquery --output=files //pkg:foo` contains the output files of `//pkg:foo` in *all* configurations that occur in the build (also see diff --git a/src/main/java/com/google/devtools/build/lib/query2/cquery/CqueryOptions.java b/src/main/java/com/google/devtools/build/lib/query2/cquery/CqueryOptions.java index 0d85d0db680ea7..c2e670780f6522 100644 --- a/src/main/java/com/google/devtools/build/lib/query2/cquery/CqueryOptions.java +++ b/src/main/java/com/google/devtools/build/lib/query2/cquery/CqueryOptions.java @@ -107,4 +107,12 @@ public enum Transitions { + " error to specify both --starlark:expr and --starlark:file. See help for" + " --output=starlark for additional detail.") public String file; + + @Option( + name = "files:include_source_files", + defaultValue = "false", + documentationCategory = OptionDocumentationCategory.QUERY, + effectTags = {OptionEffectTag.TERMINAL_OUTPUT}, + help = "If true, source files are included in the output of cquery with --output=files.") + public boolean includeSourceFiles; } 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 d0ceeeaef5b088..619df99a316a27 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 @@ -17,6 +17,7 @@ import com.google.devtools.build.lib.analysis.ConfiguredTarget; import com.google.devtools.build.lib.analysis.TopLevelArtifactContext; import com.google.devtools.build.lib.analysis.TopLevelArtifactHelper; +import com.google.devtools.build.lib.analysis.configuredtargets.InputFileConfiguredTarget; import com.google.devtools.build.lib.events.ExtendedEventHandler; import com.google.devtools.build.lib.query2.engine.QueryEnvironment.TargetAccessor; import com.google.devtools.build.lib.skyframe.SkyframeExecutor; @@ -53,14 +54,16 @@ public void processOutput(Iterable partialResult) throws IOException, InterruptedException { for (KeyedConfiguredTarget keyedTarget : partialResult) { ConfiguredTarget target = keyedTarget.getConfiguredTarget(); - if (!TopLevelArtifactHelper.shouldConsiderForDisplay(target)) { + if (!TopLevelArtifactHelper.shouldConsiderForDisplay(target) && !(options.includeSourceFiles + && target instanceof InputFileConfiguredTarget)) { continue; } TopLevelArtifactHelper.getAllArtifactsToBuild(target, topLevelArtifactContext) .getImportantArtifacts() .toList() .stream() - .filter(TopLevelArtifactHelper::shouldDisplay) + .filter(artifact -> TopLevelArtifactHelper.shouldDisplay(artifact) || ( + artifact.isSourceArtifact() && options.includeSourceFiles)) .map(Artifact::getExecPathString) .forEach(this::addResult); } 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 index 5aeb5dd7d0f664..295bf048522a5c 100644 --- 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 @@ -33,6 +33,7 @@ import java.util.LinkedHashSet; import java.util.List; import java.util.Set; +import java.util.stream.Collectors; import org.junit.Before; import org.junit.Test; @@ -71,7 +72,7 @@ public final void defineSimpleRule() throws Exception { " runfiles = ctx.runfiles([runfile]),", " ),", " OutputGroupInfo(", - " foobar = [output_group_only],", + " foobar = [output_group_only, ctx.file.explicit_source_dep],", " ),", " ]", "r = rule(", @@ -105,7 +106,8 @@ public final void setUpCqueryOptions() { this.reporter = new Reporter(new EventBus(), events::add); } - private List getOutput(String queryExpression, List outputGroups) + private List getOutput(String queryExpression, List outputGroups, + boolean includeSourceFiles) throws Exception { QueryExpression expression = QueryParser.parse(queryExpression, getDefaultFunctions()); Set targetPatternSet = new LinkedHashSet<>(); @@ -113,6 +115,7 @@ private List getOutput(String queryExpression, List outputGroups PostAnalysisQueryEnvironment env = ((ConfiguredTargetQueryHelper) helper).getPostAnalysisQueryEnvironment(targetPatternSet); + options.includeSourceFiles = includeSourceFiles; ByteArrayOutputStream output = new ByteArrayOutputStream(); FilesOutputFormatterCallback callback = new FilesOutputFormatterCallback( @@ -133,24 +136,54 @@ private List getOutput(String queryExpression, List outputGroups @Test public void basicQuery_defaultOutputGroup() throws Exception { - List output = getOutput("//pkg:all", ImmutableList.of()); + List output = getOutput("//pkg:all", ImmutableList.of(), false); assertContainsExactlyWithBinDirPrefix( output, "pkg/main_default_file", "pkg/other_default_file"); } + @Test + public void basicQuery_defaultOutputGroup_includeSourceFiles() throws Exception { + List output = getOutput("//pkg:all", ImmutableList.of(), true); + var sourceAndGeneratedFiles = output.stream() + .collect(Collectors.partitioningBy(path -> path.startsWith("bazel-out/"))); + assertThat(sourceAndGeneratedFiles.get(false)).containsExactly("pkg/BUILD", "defs/rules.bzl"); + assertContainsExactlyWithBinDirPrefix(sourceAndGeneratedFiles.get(true), + "pkg/main_default_file", "pkg/other_default_file"); + } + @Test public void basicQuery_defaultAndCustomOutputGroup() throws Exception { - List output = getOutput("//pkg:main", ImmutableList.of("+foobar")); + List output = getOutput("//pkg:main", ImmutableList.of("+foobar"), false); assertContainsExactlyWithBinDirPrefix( output, "pkg/main_default_file", "pkg/main_output_group_only"); } + @Test + public void basicQuery_defaultAndCustomOutputGroup_includeSourceFiles() throws Exception { + List output = getOutput("//pkg:main", ImmutableList.of("+foobar"), true); + var sourceAndGeneratedFiles = output.stream() + .collect(Collectors.partitioningBy(path -> path.startsWith("bazel-out/"))); + assertThat(sourceAndGeneratedFiles.get(false)).containsExactly("pkg/BUILD", "defs/rules.bzl"); + assertContainsExactlyWithBinDirPrefix( + sourceAndGeneratedFiles.get(true), "pkg/main_default_file", "pkg/main_output_group_only"); + } + @Test public void basicQuery_customOutputGroupOnly() throws Exception { - List output = getOutput("//pkg:other", ImmutableList.of("foobar")); + List output = getOutput("//pkg:other", ImmutableList.of("foobar"), false); assertContainsExactlyWithBinDirPrefix(output, "pkg/other_output_group_only"); } + @Test + public void basicQuery_customOutputGroupOnly_includeSourceFiles() throws Exception { + List output = getOutput("//pkg:other", ImmutableList.of("foobar"), true); + var sourceAndGeneratedFiles = output.stream() + .collect(Collectors.partitioningBy(path -> path.startsWith("bazel-out/"))); + assertThat(sourceAndGeneratedFiles.get(false)).containsExactly("pkg/BUILD"); + assertContainsExactlyWithBinDirPrefix(sourceAndGeneratedFiles.get(true), + "pkg/other_output_group_only"); + } + private void assertContainsExactlyWithBinDirPrefix( List output, String... binDirRelativePaths) { if (binDirRelativePaths.length == 0) { diff --git a/src/test/shell/integration/configured_query_test.sh b/src/test/shell/integration/configured_query_test.sh index 677f624a5f7b7f..c77e2a3c66e764 100755 --- a/src/test/shell/integration/configured_query_test.sh +++ b/src/test/shell/integration/configured_query_test.sh @@ -1445,5 +1445,24 @@ EOF expect_log "//peach:harken" } +function test_files_include_source_files() { + local -r pkg=$FUNCNAME + mkdir -p $pkg + cat > $pkg/BUILD <<'EOF' +filegroup(name="files", srcs=["BUILD"]) +alias(name="alias", actual="single_file") +EOF + touch $pkg/single_file + + bazel cquery --output=files //$pkg:all \ + > output 2>"$TEST_log" || fail "Unexpected failure" + assert_not_contains "$pkg/BUILD" output + assert_not_contains "$pkg/single_file" output + + bazel cquery --output=files --files:include_source_files //$pkg:all \ + > output 2>"$TEST_log" || fail "Unexpected failure" + assert_contains "$pkg/BUILD" output + assert_contains "$pkg/single_file" output +} run_suite "${PRODUCT_NAME} configured query tests" From 793b295ff4ccef97ea6a7ef9e2d976267c90e7f8 Mon Sep 17 00:00:00 2001 From: Fabian Meumertzheim Date: Tue, 15 Nov 2022 21:56:46 -0500 Subject: [PATCH 2/3] Always include source files with --output=files --- site/en/query/cquery.md | 5 ++-- .../lib/query2/cquery/CqueryOptions.java | 8 ----- .../cquery/FilesOutputFormatterCallback.java | 8 ++--- .../FilesOutputFormatterCallbackTest.java | 30 +++---------------- .../integration/configured_query_test.sh | 8 +---- 5 files changed, 11 insertions(+), 48 deletions(-) diff --git a/site/en/query/cquery.md b/site/en/query/cquery.md index 2c6aefde34c0de..99ea8613585814 100644 --- a/site/en/query/cquery.md +++ b/site/en/query/cquery.md @@ -369,9 +369,8 @@ This option prints a list of the output files produced by each target matched by the query similar to the list printed at the end of a `bazel build` invocation. The output contains only the files advertised in the requested output groups as determined by the -[`--output_groups`](/reference/command-line-reference#flag--output_groups) flag -and does not contain source files unless `--files:include_source_files` is -specified. +[`--output_groups`](/reference/command-line-reference#flag--output_groups) flag. +It does include source files. Note: The output of `bazel cquery --output=files //pkg:foo` contains the output files of `//pkg:foo` in *all* configurations that occur in the build (also see diff --git a/src/main/java/com/google/devtools/build/lib/query2/cquery/CqueryOptions.java b/src/main/java/com/google/devtools/build/lib/query2/cquery/CqueryOptions.java index c2e670780f6522..0d85d0db680ea7 100644 --- a/src/main/java/com/google/devtools/build/lib/query2/cquery/CqueryOptions.java +++ b/src/main/java/com/google/devtools/build/lib/query2/cquery/CqueryOptions.java @@ -107,12 +107,4 @@ public enum Transitions { + " error to specify both --starlark:expr and --starlark:file. See help for" + " --output=starlark for additional detail.") public String file; - - @Option( - name = "files:include_source_files", - defaultValue = "false", - documentationCategory = OptionDocumentationCategory.QUERY, - effectTags = {OptionEffectTag.TERMINAL_OUTPUT}, - help = "If true, source files are included in the output of cquery with --output=files.") - public boolean includeSourceFiles; } 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 619df99a316a27..48a5065a0ff11d 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 @@ -54,16 +54,16 @@ public void processOutput(Iterable partialResult) throws IOException, InterruptedException { for (KeyedConfiguredTarget keyedTarget : partialResult) { ConfiguredTarget target = keyedTarget.getConfiguredTarget(); - if (!TopLevelArtifactHelper.shouldConsiderForDisplay(target) && !(options.includeSourceFiles - && target instanceof InputFileConfiguredTarget)) { + if (!TopLevelArtifactHelper.shouldConsiderForDisplay(target) + && !(target instanceof InputFileConfiguredTarget)) { continue; } TopLevelArtifactHelper.getAllArtifactsToBuild(target, topLevelArtifactContext) .getImportantArtifacts() .toList() .stream() - .filter(artifact -> TopLevelArtifactHelper.shouldDisplay(artifact) || ( - artifact.isSourceArtifact() && options.includeSourceFiles)) + .filter(artifact -> TopLevelArtifactHelper.shouldDisplay(artifact) + || artifact.isSourceArtifact()) .map(Artifact::getExecPathString) .forEach(this::addResult); } 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 index 295bf048522a5c..317fd1169d5279 100644 --- 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 @@ -106,8 +106,7 @@ public final void setUpCqueryOptions() { this.reporter = new Reporter(new EventBus(), events::add); } - private List getOutput(String queryExpression, List outputGroups, - boolean includeSourceFiles) + private List getOutput(String queryExpression, List outputGroups) throws Exception { QueryExpression expression = QueryParser.parse(queryExpression, getDefaultFunctions()); Set targetPatternSet = new LinkedHashSet<>(); @@ -115,7 +114,6 @@ private List getOutput(String queryExpression, List outputGroups PostAnalysisQueryEnvironment env = ((ConfiguredTargetQueryHelper) helper).getPostAnalysisQueryEnvironment(targetPatternSet); - options.includeSourceFiles = includeSourceFiles; ByteArrayOutputStream output = new ByteArrayOutputStream(); FilesOutputFormatterCallback callback = new FilesOutputFormatterCallback( @@ -136,14 +134,7 @@ private List getOutput(String queryExpression, List outputGroups @Test public void basicQuery_defaultOutputGroup() throws Exception { - List output = getOutput("//pkg:all", ImmutableList.of(), false); - assertContainsExactlyWithBinDirPrefix( - output, "pkg/main_default_file", "pkg/other_default_file"); - } - - @Test - public void basicQuery_defaultOutputGroup_includeSourceFiles() throws Exception { - List output = getOutput("//pkg:all", ImmutableList.of(), true); + List output = getOutput("//pkg:all", ImmutableList.of()); var sourceAndGeneratedFiles = output.stream() .collect(Collectors.partitioningBy(path -> path.startsWith("bazel-out/"))); assertThat(sourceAndGeneratedFiles.get(false)).containsExactly("pkg/BUILD", "defs/rules.bzl"); @@ -153,14 +144,7 @@ public void basicQuery_defaultOutputGroup_includeSourceFiles() throws Exception @Test public void basicQuery_defaultAndCustomOutputGroup() throws Exception { - List output = getOutput("//pkg:main", ImmutableList.of("+foobar"), false); - assertContainsExactlyWithBinDirPrefix( - output, "pkg/main_default_file", "pkg/main_output_group_only"); - } - - @Test - public void basicQuery_defaultAndCustomOutputGroup_includeSourceFiles() throws Exception { - List output = getOutput("//pkg:main", ImmutableList.of("+foobar"), true); + List output = getOutput("//pkg:main", ImmutableList.of("+foobar")); var sourceAndGeneratedFiles = output.stream() .collect(Collectors.partitioningBy(path -> path.startsWith("bazel-out/"))); assertThat(sourceAndGeneratedFiles.get(false)).containsExactly("pkg/BUILD", "defs/rules.bzl"); @@ -170,13 +154,7 @@ public void basicQuery_defaultAndCustomOutputGroup_includeSourceFiles() throws E @Test public void basicQuery_customOutputGroupOnly() throws Exception { - List output = getOutput("//pkg:other", ImmutableList.of("foobar"), false); - assertContainsExactlyWithBinDirPrefix(output, "pkg/other_output_group_only"); - } - - @Test - public void basicQuery_customOutputGroupOnly_includeSourceFiles() throws Exception { - List output = getOutput("//pkg:other", ImmutableList.of("foobar"), true); + List output = getOutput("//pkg:other", ImmutableList.of("foobar")); var sourceAndGeneratedFiles = output.stream() .collect(Collectors.partitioningBy(path -> path.startsWith("bazel-out/"))); assertThat(sourceAndGeneratedFiles.get(false)).containsExactly("pkg/BUILD"); diff --git a/src/test/shell/integration/configured_query_test.sh b/src/test/shell/integration/configured_query_test.sh index c77e2a3c66e764..391663d0dea460 100755 --- a/src/test/shell/integration/configured_query_test.sh +++ b/src/test/shell/integration/configured_query_test.sh @@ -1454,13 +1454,7 @@ alias(name="alias", actual="single_file") EOF touch $pkg/single_file - bazel cquery --output=files //$pkg:all \ - > output 2>"$TEST_log" || fail "Unexpected failure" - assert_not_contains "$pkg/BUILD" output - assert_not_contains "$pkg/single_file" output - - bazel cquery --output=files --files:include_source_files //$pkg:all \ - > output 2>"$TEST_log" || fail "Unexpected failure" + bazel cquery --output=files //$pkg:all > output 2>"$TEST_log" || fail "Unexpected failure" assert_contains "$pkg/BUILD" output assert_contains "$pkg/single_file" output } From f78419526dbd4d71e5e4d733454011f20ed2a115 Mon Sep 17 00:00:00 2001 From: Fabian Meumertzheim Date: Tue, 22 Nov 2022 20:41:29 +0100 Subject: [PATCH 3/3] Do not hardcode `"bazel"` in tests --- .../lib/query2/cquery/FilesOutputFormatterCallbackTest.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) 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 index 317fd1169d5279..f8b87fbd699327 100644 --- 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 @@ -136,7 +136,7 @@ private List getOutput(String queryExpression, List outputGroups public void basicQuery_defaultOutputGroup() throws Exception { List output = getOutput("//pkg:all", ImmutableList.of()); var sourceAndGeneratedFiles = output.stream() - .collect(Collectors.partitioningBy(path -> path.startsWith("bazel-out/"))); + .collect(Collectors.partitioningBy(path -> path.matches("^[^/]*-out/.*"))); assertThat(sourceAndGeneratedFiles.get(false)).containsExactly("pkg/BUILD", "defs/rules.bzl"); assertContainsExactlyWithBinDirPrefix(sourceAndGeneratedFiles.get(true), "pkg/main_default_file", "pkg/other_default_file"); @@ -146,7 +146,7 @@ public void basicQuery_defaultOutputGroup() throws Exception { public void basicQuery_defaultAndCustomOutputGroup() throws Exception { List output = getOutput("//pkg:main", ImmutableList.of("+foobar")); var sourceAndGeneratedFiles = output.stream() - .collect(Collectors.partitioningBy(path -> path.startsWith("bazel-out/"))); + .collect(Collectors.partitioningBy(path -> path.matches("^[^/]*-out/.*"))); assertThat(sourceAndGeneratedFiles.get(false)).containsExactly("pkg/BUILD", "defs/rules.bzl"); assertContainsExactlyWithBinDirPrefix( sourceAndGeneratedFiles.get(true), "pkg/main_default_file", "pkg/main_output_group_only"); @@ -156,7 +156,7 @@ public void basicQuery_defaultAndCustomOutputGroup() throws Exception { public void basicQuery_customOutputGroupOnly() throws Exception { List output = getOutput("//pkg:other", ImmutableList.of("foobar")); var sourceAndGeneratedFiles = output.stream() - .collect(Collectors.partitioningBy(path -> path.startsWith("bazel-out/"))); + .collect(Collectors.partitioningBy(path -> path.matches("^[^/]*-out/.*"))); assertThat(sourceAndGeneratedFiles.get(false)).containsExactly("pkg/BUILD"); assertContainsExactlyWithBinDirPrefix(sourceAndGeneratedFiles.get(true), "pkg/other_output_group_only");