Skip to content

Commit

Permalink
Add optional functionality to display source file locations in `outpu…
Browse files Browse the repository at this point in the history
…t=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 #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 #8900.
PiperOrigin-RevId: 338240601
  • Loading branch information
zhengwei143 authored and copybara-github committed Oct 21, 2020
1 parent 7b48943 commit f00f911
Show file tree
Hide file tree
Showing 4 changed files with 102 additions and 1 deletion.
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -153,6 +154,26 @@ public Set<Setting> 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 //
///////////////////////////////////////////////////////////
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@
class LocationOutputFormatter extends AbstractUnorderedFormatter {

private boolean relativeLocations;
private boolean displaySourceFileLocation;

@Override
public String getName() {
Expand All @@ -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
Expand Down Expand Up @@ -85,7 +87,7 @@ public void processOutput(Iterable<Target> partialResult) throws IOException {
.append(": ")
.append(target.getTargetKind())
.append(" ")
.append(target.getLabel().getDefaultCanonicalForm())
.append(FormatUtils.getLabel(target, displaySourceFileLocation, relativeLocations))
.append(lineTerm);
}
}
Expand Down
51 changes: 51 additions & 0 deletions src/test/shell/integration/bazel_query_test.sh
Original file line number Diff line number Diff line change
Expand Up @@ -497,6 +497,57 @@ EOF
expect_log "//foo:foo"
}

test_location_output_source_files() {
rm -rf foo
mkdir -p foo
cat > foo/BUILD <<EOF
py_binary(
name = "main",
srcs = ["main.py"],
)
EOF
touch foo/main.py || fail "Could not touch foo/main.py"

# The incompatible_display_source_file_location flag displays the location of
# line 1 of the actual source file
bazel query \
--output=location \
--incompatible_display_source_file_location \
'//foo:main.py' >& $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 <<EOF
Expand Down

0 comments on commit f00f911

Please sign in to comment.