From f00f911b7eff7057da804f7a058da605c577fb4a Mon Sep 17 00:00:00 2001 From: tanzhengwei Date: Wed, 21 Oct 2020 04:47:41 -0700 Subject: [PATCH] Add optional functionality to display source file locations in `output=location `instead of targets. Added an additional flag `[no]incompatible_display_source_file_location` to override the default behaviour (when false) to print the source file targets instead of their locations. Additionally, the `relative_locations` flag will similarly display the relative location of the source file. This https://github.com/bazelbuild/bazel/issues/12322 describes the issue and potential migration recipes. Example of new configurations for blaze query location=output with `incompatible_display_source_file_location` and `relative_locations` flags below: 1. Default behaviour: Displays absolute paths to both BUILD and source file ``` $ bazel query blazetest:main.py --output=location --incompatible_display_source_file_location /home/tanzhengwei/blazetest/BUILD:1:10: source file /home/tanzhengwei/blazetest/main.py:1:1 ``` 2. Displays relative paths to both BUILD and source file ``` $ bazel query blazetest:main.py --output=location --relative_locations --incompatible_display_source_file_location blazetest/BUILD:1:10: source file blazetest/main.py:1:1 ``` 3. Displays relative path to BUILD file and source file target ``` $ bazel query blazetest:main.py --output=location --relative_locations --noincompatible_display_source_file_location blazetest/BUILD:1:10: source file //blazetest:main.py ``` 4. Displays absolute path to BUILD file and source file target ``` $ bazel query blazetest:main.py --output=location --noincompatible_display_source_file_location /home/tanzhengwei/blazetest/BUILD:1:10: source file //blazetest:main.py ``` RELNOTES: Added flag `incompatible_display_source_file_location` for `blaze query location=output` to print the location of line 1 of the actual source files instead of the source file targets. Provides a solution to https://github.com/bazelbuild/bazel/issues/8900. PiperOrigin-RevId: 338240601 --- .../lib/query2/common/CommonQueryOptions.java | 21 ++++++++ .../lib/query2/query/output/FormatUtils.java | 27 ++++++++++ .../query/output/LocationOutputFormatter.java | 4 +- .../shell/integration/bazel_query_test.sh | 51 +++++++++++++++++++ 4 files changed, 102 insertions(+), 1 deletion(-) diff --git a/src/main/java/com/google/devtools/build/lib/query2/common/CommonQueryOptions.java b/src/main/java/com/google/devtools/build/lib/query2/common/CommonQueryOptions.java index d9e8af49c5ac2c..63111a36162eea 100644 --- a/src/main/java/com/google/devtools/build/lib/query2/common/CommonQueryOptions.java +++ b/src/main/java/com/google/devtools/build/lib/query2/common/CommonQueryOptions.java @@ -23,6 +23,7 @@ import com.google.devtools.common.options.Option; import com.google.devtools.common.options.OptionDocumentationCategory; import com.google.devtools.common.options.OptionEffectTag; +import com.google.devtools.common.options.OptionMetadataTag; import com.google.devtools.common.options.OptionsBase; import java.util.EnumSet; import java.util.List; @@ -153,6 +154,26 @@ public Set toSettings() { return settings; } + /////////////////////////////////////////////////////////// + // LOCATION OUTPUT FORMATTER OPTIONS // + /////////////////////////////////////////////////////////// + + // TODO(tanzhengwei): Clean up in next major release + @Option( + name = "incompatible_display_source_file_location", + defaultValue = "false", + documentationCategory = OptionDocumentationCategory.QUERY, + effectTags = {OptionEffectTag.TERMINAL_OUTPUT}, + metadataTags = { + OptionMetadataTag.INCOMPATIBLE_CHANGE, + OptionMetadataTag.TRIGGERED_BY_ALL_INCOMPATIBLE_CHANGES + }, + help = + "False by default, displays the target of the source file. " + + "If true, displays the location of line 1 of source files in location outputs. " + + "This flag only exists for migration purposes.") + public boolean displaySourceFileLocation; + /////////////////////////////////////////////////////////// // PROTO OUTPUT FORMATTER OPTIONS // /////////////////////////////////////////////////////////// diff --git a/src/main/java/com/google/devtools/build/lib/query2/query/output/FormatUtils.java b/src/main/java/com/google/devtools/build/lib/query2/query/output/FormatUtils.java index 7a12a7f4e8fa68..cfdc613d81fad9 100644 --- a/src/main/java/com/google/devtools/build/lib/query2/query/output/FormatUtils.java +++ b/src/main/java/com/google/devtools/build/lib/query2/query/output/FormatUtils.java @@ -15,6 +15,7 @@ package com.google.devtools.build.lib.query2.query.output; import com.google.devtools.build.lib.packages.DependencyFilter; +import com.google.devtools.build.lib.packages.InputFile; import com.google.devtools.build.lib.packages.Package; import com.google.devtools.build.lib.packages.Rule; import com.google.devtools.build.lib.packages.Target; @@ -80,4 +81,30 @@ static Location getRootRelativeLocation(Location location, @Nullable Package bas } return location; } + + /** + * Returns the target label string. For {@link InputFile} targets, displays the location of line 1 + * of the actual source file by default. + * + * @param target the target to get the label from + * @param displaySourceFileLocation displays the location of line 1 of the actual source file + * instead of its target if true. + * @param relativeLocations displays the location of the source file relative to its package's + * source root directory + */ + static String getLabel( + Target target, boolean displaySourceFileLocation, boolean relativeLocations) { + if (!(target instanceof InputFile) || !displaySourceFileLocation) { + return target.getLabel().getDefaultCanonicalForm(); + } + + // Default behaviour for source files without the incompatible_display_source_file_location flag + PathFragment packageDir = target.getPackage().getPackageDirectory().asFragment(); + Location sourceFileLoc = + Location.fromFileLineColumn(packageDir.getRelative(target.getName()).toString(), 1, 1); + if (relativeLocations) { + sourceFileLoc = getRootRelativeLocation(sourceFileLoc, target.getPackage()); + } + return sourceFileLoc.toString(); + } } diff --git a/src/main/java/com/google/devtools/build/lib/query2/query/output/LocationOutputFormatter.java b/src/main/java/com/google/devtools/build/lib/query2/query/output/LocationOutputFormatter.java index 754bacd37a3729..e1bd0718b63807 100644 --- a/src/main/java/com/google/devtools/build/lib/query2/query/output/LocationOutputFormatter.java +++ b/src/main/java/com/google/devtools/build/lib/query2/query/output/LocationOutputFormatter.java @@ -40,6 +40,7 @@ class LocationOutputFormatter extends AbstractUnorderedFormatter { private boolean relativeLocations; + private boolean displaySourceFileLocation; @Override public String getName() { @@ -51,6 +52,7 @@ public void setOptions( CommonQueryOptions options, AspectResolver aspectResolver, HashFunction hashFunction) { super.setOptions(options, aspectResolver, hashFunction); this.relativeLocations = options.relativeLocations; + this.displaySourceFileLocation = options.displaySourceFileLocation; } @Override @@ -85,7 +87,7 @@ public void processOutput(Iterable partialResult) throws IOException { .append(": ") .append(target.getTargetKind()) .append(" ") - .append(target.getLabel().getDefaultCanonicalForm()) + .append(FormatUtils.getLabel(target, displaySourceFileLocation, relativeLocations)) .append(lineTerm); } } diff --git a/src/test/shell/integration/bazel_query_test.sh b/src/test/shell/integration/bazel_query_test.sh index 7f50faa42867e6..9cf0a58928c7fd 100755 --- a/src/test/shell/integration/bazel_query_test.sh +++ b/src/test/shell/integration/bazel_query_test.sh @@ -497,6 +497,57 @@ EOF expect_log "//foo:foo" } +test_location_output_source_files() { + rm -rf foo + mkdir -p foo + cat > foo/BUILD <& $TEST_log || fail "Expected success" + expect_log "source file ${TEST_TMPDIR}/.*/foo/main.py:1:1" + expect_not_log "source file //foo:main.py" + + # Default behavior overridden by noincompatible_display_source_file_location + # flag to display the source file target instead + bazel query \ + --output=location \ + --noincompatible_display_source_file_location \ + '//foo:main.py' >& $TEST_log || fail "Expected success" + expect_log "source file //foo:main.py" + expect_not_log "source file ${TEST_TMPDIR}/.*/foo/main.py:1:1" + + # Adding relative_locations flag should modify default behavior and + # make location of source file be relative + bazel query \ + --output=location \ + --relative_locations \ + --incompatible_display_source_file_location \ + '//foo:main.py' >& $TEST_log || fail "Expected success" + expect_log "source file foo/main.py:1:1" + expect_not_log "source file ${TEST_TMPDIR}/.*/foo/main.py:1:1" + + # Adding noincompatible_display_source_file_location flag should still + # override default behaviour regardless of relative_locations to + # display the source file target + bazel query --output=location \ + --relative_locations \ + --noincompatible_display_source_file_location \ + '//foo:main.py' >& $TEST_log || fail "Expected success" + expect_log "source file //foo:main.py" + expect_not_log "source file foo/main.py:1:1" + expect_not_log "source file ${TEST_TMPDIR}/.*/foo/main.py:1:1" +} + function test_subdirectory_named_external() { mkdir -p foo/external foo/bar cat > foo/external/BUILD <