Skip to content

Commit

Permalink
Remove the --incompatible_display_source_file_location command line o…
Browse files Browse the repository at this point in the history
…ption.

RELNOTES[INC]: The --incompatible_display_source_file_location command line option is not available anymore.

PiperOrigin-RevId: 569117511
Change-Id: Idc4748ba6bf11e42063acdd5c459df83e175ccd4
  • Loading branch information
lberki authored and Copybara-Service committed Sep 28, 2023
1 parent 65b5640 commit 8c24877
Show file tree
Hide file tree
Showing 6 changed files with 8 additions and 84 deletions.
Expand Up @@ -190,23 +190,6 @@ public LabelPrinter getLabelPrinter(
: LabelPrinter.displayForm(mainRepoMapping);
}

///////////////////////////////////////////////////////////
// LOCATION OUTPUT FORMATTER OPTIONS //
///////////////////////////////////////////////////////////

// TODO(tanzhengwei): Clean up in next major release
@Option(
name = "incompatible_display_source_file_location",
defaultValue = "true",
documentationCategory = OptionDocumentationCategory.QUERY,
effectTags = {OptionEffectTag.TERMINAL_OUTPUT},
metadataTags = {OptionMetadataTag.INCOMPATIBLE_CHANGE},
help =
"True 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
Expand Up @@ -53,29 +53,15 @@ public int compare(Target o1, Target o2) {
}
}

/**
* Returns the target location string, optionally relative to its package's source root directory.
*/
static String getLocation(Target target, boolean relative) {
Location loc = target.getLocation();

if (relative) {
loc = getRootRelativeLocation(loc, target.getPackage());
}
return loc.toString();
}

/**
* Returns the target location string, optionally relative to its package's source root directory
* and optionally to display the location of source files.
*
* @param relative flag to display the location relative to its package's source root directory.
* @param displaySourceFileLocation flag to display the location of line 1 of the actual source
* file instead of its location in the BUILD file.
*/
static String getLocation(Target target, boolean relative, boolean displaySourceFileLocation) {
static String getLocation(Target target, boolean relative) {
Location loc = target.getLocation();
if (target instanceof InputFile && displaySourceFileLocation) {
if (target instanceof InputFile) {
PathFragment packageDir = target.getPackage().getPackageDirectory().asFragment();
loc = Location.fromFileLineColumn(packageDir.getRelative(target.getName()).toString(), 1, 1);
}
Expand Down
Expand Up @@ -41,7 +41,6 @@
class LocationOutputFormatter extends AbstractUnorderedFormatter {

private boolean relativeLocations;
private boolean displaySourceFileLocation;

@Override
public String getName() {
Expand All @@ -53,7 +52,6 @@ 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 @@ -84,7 +82,7 @@ public void processOutput(Iterable<Target> partialResult) throws IOException {
final String lineTerm = options.getLineTerminator();
for (Target target : partialResult) {
writer
.append(FormatUtils.getLocation(target, relativeLocations, displaySourceFileLocation))
.append(FormatUtils.getLocation(target, relativeLocations))
.append(": ")
.append(target.getTargetKind())
.append(" ")
Expand Down
Expand Up @@ -101,7 +101,6 @@ public class ProtoOutputFormatter extends AbstractUnorderedFormatter {
private DependencyFilter dependencyFilter;
private boolean packageGroupIncludesDoubleSlash;
private boolean relativeLocations;
private boolean displaySourceFileLocation;
private boolean includeDefaultValues = true;
private Predicate<String> ruleAttributePredicate = Predicates.alwaysTrue();
private boolean flattenSelects = true;
Expand All @@ -128,7 +127,6 @@ public void setOptions(
this.dependencyFilter = FormatUtils.getDependencyFilter(options);
this.packageGroupIncludesDoubleSlash = options.incompatiblePackageGroupIncludesDoubleSlash;
this.relativeLocations = options.relativeLocations;
this.displaySourceFileLocation = options.displaySourceFileLocation;
this.includeDefaultValues = options.protoIncludeDefaultValues;
this.ruleAttributePredicate = newAttributePredicate(options.protoOutputRuleAttributes);
this.flattenSelects = options.protoFlattenSelects;
Expand Down Expand Up @@ -298,8 +296,7 @@ public Build.Target toTargetProtoBuffer(
Build.SourceFile.newBuilder().setName(labelPrinter.toString(label));

if (includeLocations) {
input.setLocation(
FormatUtils.getLocation(target, relativeLocations, displaySourceFileLocation));
input.setLocation(FormatUtils.getLocation(target, relativeLocations));
}

if (inputFile.getName().equals("BUILD")) {
Expand Down
Expand Up @@ -64,7 +64,6 @@ class XmlOutputFormatter extends AbstractUnorderedFormatter {
private DependencyFilter dependencyFilter;
private boolean packageGroupIncludesDoubleSlash;
private boolean relativeLocations;
private boolean displaySourceFileLocation;
private QueryOptions queryOptions;

@Override
Expand All @@ -87,7 +86,6 @@ public void setOptions(
this.dependencyFilter = FormatUtils.getDependencyFilter(options);
this.packageGroupIncludesDoubleSlash = options.incompatiblePackageGroupIncludesDoubleSlash;
this.relativeLocations = options.relativeLocations;
this.displaySourceFileLocation = options.displaySourceFileLocation;

Preconditions.checkArgument(options instanceof QueryOptions);
this.queryOptions = (QueryOptions) options;
Expand Down Expand Up @@ -247,7 +245,7 @@ private Element createTargetElement(Document doc, Target target, LabelPrinter la
}

elem.setAttribute("name", labelPrinter.toString(target.getLabel()));
String location = FormatUtils.getLocation(target, relativeLocations, displaySourceFileLocation);
String location = FormatUtils.getLocation(target, relativeLocations);
if (!queryOptions.xmlLineNumbers) {
int firstColon = location.indexOf(':');
if (firstColon != -1) {
Expand Down
44 changes: 3 additions & 41 deletions src/test/shell/integration/bazel_query_test.sh
Expand Up @@ -508,47 +508,23 @@ py_binary(
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
# Check that Bazel 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 //foo:main.py"
expect_log "^${TEST_TMPDIR}/.*/foo/main.py:1:1"
expect_not_log "^${TEST_TMPDIR}/.*/foo/BUILD:[0-9]*:[0-9]*"

# The noincompatible_display_source_file_location flag displays its location
# in the BUILD file
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_log "^${TEST_TMPDIR}/.*/foo/BUILD:[0-9]*:[0-9]*"
expect_not_log "^${TEST_TMPDIR}/.*/foo/main.py:1:1"

# The incompatible_display_source_file_location should still be affected by
# relative_locations flag to display the relative location of the source file
# Location should still be affected by relative_locations flag to display the
# relative location of the source file
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"
expect_log "^foo/main.py:1:1"
expect_not_log "^${TEST_TMPDIR}/.*/foo/main.py:1:1"

# The noincompatible_display_source_file_location flag should still be
# affected by relative_locations flag to display the relative location of
# the BUILD file.
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_log "^foo/BUILD:[0-9]*:[0-9]*"
expect_not_log "^${TEST_TMPDIR}/.*/foo/BUILD:[0-9]*:[0-9]*"
}

function test_proto_output_source_files() {
Expand All @@ -563,17 +539,10 @@ EOF
touch foo/main.py || fail "Could not touch foo/main.py"

bazel query --output=proto \
--incompatible_display_source_file_location \
'//foo:main.py' >& $TEST_log || fail "Expected success"

expect_log "${TEST_TMPDIR}/.*/foo/main.py:1:1" $TEST_log
expect_not_log "${TEST_TMPDIR}/.*/foo/BUILD:[0-9]*:[0-9]*" $TEST_log

bazel query --output=proto \
--noincompatible_display_source_file_location \
'//foo:main.py' >& $TEST_log || fail "Expected success"
expect_log "${TEST_TMPDIR}/.*/foo/BUILD:[0-9]*:[0-9]*" $TEST_log
expect_not_log "${TEST_TMPDIR}/.*/foo/main.py:1:1" $TEST_log
}

function test_xml_output_source_files() {
Expand All @@ -588,16 +557,9 @@ EOF
touch foo/main.py || fail "Could not touch foo/main.py"

bazel query --output=xml \
--incompatible_display_source_file_location \
'//foo:main.py' >& $TEST_log || fail "Expected success"
expect_log "location=\"${TEST_TMPDIR}/.*/foo/main.py:1:1"
expect_not_log "location=\"${TEST_TMPDIR}/.*/foo/BUILD:[0-9]*:[0-9]*"

bazel query --output=xml \
--noincompatible_display_source_file_location \
'//foo:main.py' >& $TEST_log || fail "Expected success"
expect_log "location=\"${TEST_TMPDIR}/.*/foo/BUILD:[0-9]*:[0-9]*"
expect_not_log "location=\"${TEST_TMPDIR}/.*/foo/main.py:1:1"
}

function test_subdirectory_named_external() {
Expand Down

0 comments on commit 8c24877

Please sign in to comment.