Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

incompatible_display_source_file_location: incompatible flag for query output location #12322

Closed
zhengwei143 opened this issue Oct 21, 2020 · 6 comments
Assignees
Labels
incompatible-change Incompatible/breaking change P2 We'll consider working on this in future. (Assignee optional) team-Performance Issues for Performance teams type: bug

Comments

@zhengwei143
Copy link
Contributor

zhengwei143 commented Oct 21, 2020

Flag: --incompatible_display_source_file_location
Available since: Yet to be released
Will be flipped in: To be confirmed

Motivation

Brought to light by github issue.

Current documentation defines the --output=location flag to print the location of line 1 of source files. However, current implementations only print the target of the source file. For example:

[Expected] Current definition:

$ bazel query bazeltest:main.py --output=location
/home/tanzhengwei/bazeltest/main.py:1:1: source file //bazeltest:main.py

[Actual] Current implementation:

$ bazel query bazeltest:main.py --output=location
/home/tanzhengwei/bazeltest/BUILD:1:10: source file //bazeltest:main.py

Changes

Appending the flag --incompatible_display_source_file_location will change the current implementation to fit the expected definition.

After the flip, modifies the current implementation to match the current definition. The default will use the --incompatible_display_source_file_location flag and print out the location of line 1 of source files instead of their targets.

EDIT: Similarly, when using --output=proto and --output=xml together with --incompatible_display_source_file_location, the location of source files will printed similar to --output=location.

Migration

After the flip, scripts that use the output of bazel query --output=location and more specifically the source file location /home/tanzhengwei/bazeltest/BUILD:1:10 will be affected by this change. To fix your scripts, you would likely need to use some sed transformation to modify the output accordingly.

However, it might be a problem if your script requires the use of the location (specifically the row and column) of the source file in the BUILD file, since this is being phased out and will not contain any information about it.

@zhengwei143 zhengwei143 self-assigned this Oct 21, 2020
@zhengwei143 zhengwei143 added incompatible-change Incompatible/breaking change team-Performance Issues for Performance teams labels Oct 21, 2020
bazel-io pushed a commit that referenced this issue Oct 21, 2020
…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
@benjaminp
Copy link
Collaborator

Is it intentional that the source files names are all suffixed with :1:1?

@zhengwei143
Copy link
Contributor Author

Yes, by definition, other than the path, the location also includes the row and column in the file. This is aligned with the definition as currently stated in the docs "For source files, the location of line 1 of the actual file is printed".

@benjaminp
Copy link
Collaborator

That documentation doesn't justify the new behavior. The first paragraph of the --output location documentation states that the first field of each line is a location. The second paragraph describes what that location is for each kind of target. In fact, source files are not prefixed by the "the location of line 1 of the actual file" in the current implementation; the prefix is a location in the containing package's BUILD file.

It would make sense to keep the :1:1 location if the prefix was actually the path to the source file to preserve the consistency of the "grep format".

If changing the location prefix to cohere with the documentation isn't in the works, the new output should be evaluated for its utility. Semantically, a source file target refers to the whole file not in particular its first line and column. On a practical level, the :1:1 must be stripped by any tool making use of --output location specifically for source files because it is meaningless.

@zhengwei143
Copy link
Contributor Author

You're right, I've misunderstood the documentation previously. Thanks for pointing it out.

I've added you as a reviewer to the PR that fixes this and modified the description above.

bazel-io pushed a commit that referenced this issue Oct 23, 2020
This PR is a patch to fix this [commit](f00f911) after being brought up [here](#12322 (comment)). This aims to provide a fix for #8900.

The goal of this PR is to align the output of `--output=location` with its current definition in the [documentation](https://docs.bazel.build/versions/master/query.html#output-location). With the flag `--output=location`, source files are supposed to output "the location of line 1 of the actual file" but only displayed its location in the BUILD file prior to the commit.

**Expected behavior by definition:**
```
bazel query bazeltest:main.py --output=location
/home/tanzhengwei/bazeltest/main.py:1:1: source file //bazeltest:main.py
```

The previous [commit](f00f911) wrongly modified the output to the following (with the `--incompatible_display_source_file_location` flag):
```
bazel query bazeltest:main.py --output=location --incompatible_display_source_file_location
/home/tanzhengwei/bazeltest/BUILD:1:10: source file /home/tanzhengwei/bazeltest/main.py:1:1
```

This PR fixes it to output to the correct implementation (with the `--incompatible_display_source_file_location` flag):
```
bazel query bazeltest:main.py --output=location --incompatible_display_source_file_location
/home/tanzhengwei/bazeltest/main.py:1:1: source file //bazeltest:main.py
```

Closes #12329.

PiperOrigin-RevId: 338620611
@meisterT meisterT added type: bug P1 I'll work on this now. (Assignee required) labels Nov 9, 2020
@alandonovan
Copy link
Contributor

alandonovan commented Dec 7, 2020

This feature cannot be implemented as described, because the actual location of a generated file target is not known during package loading; it is a function of the configuration, computed during analysis. In other words, the best blaze query can do is a white lie:

$ cat p/BUILD
genrule(name='g', outs=['o'], cmd=':')

$ blaze query --output=location //p:o | sed -e s?$(pwd)/??
p/o:1:8: generated file //p:o  # actual result today is p/BUILD. Correct result is blaze-bin/.../p/o

EDIT: Never mind, this feature is restricted to source files. Carry on.

@zhengwei143 zhengwei143 added P2 We'll consider working on this in future. (Assignee optional) and removed P1 I'll work on this now. (Assignee required) labels Dec 8, 2020
@noscript
Copy link

noscript commented Jun 3, 2022

It seems the new behavior is already present in Bazel 5.1.1 but that change broke my tools. Is there a way to get the old behavior back? I can no longer query the build file, along with the line and column number.

UPDATE: --noincompatible_display_source_file_location reverts the old behavior.

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
incompatible-change Incompatible/breaking change P2 We'll consider working on this in future. (Assignee optional) team-Performance Issues for Performance teams type: bug
Projects
None yet
Development

No branches or pull requests

5 participants