bazelbuild / bazel

a fast, scalable, multi-language and extensible build system
https://bazel.build
Apache License 2.0
23.13k stars 4.05k forks source link

Bazel cannot query //tools #3122

Closed abergmeier closed 7 years ago

abergmeier commented 7 years ago

Description of the problem / feature request / question:

There are a few "virtual" (don't know a better word) packages like //tools/cpp, which one can can retrieve with buildfiles(deps(...)) --output package. Trying to then query these with kind("source file", deps(//tools/cpp, 1)) does not work however. IMO either they should be totally ignored or work like any other package everywhere.

If possible, provide a minimal example to reproduce the problem:

bazel query 'kind("source file", deps(//tools/cpp, 1))'

Environment info

aj-michael commented 7 years ago

I think that --output package is just missing the workspace name. //tools/cpp should be @bazel_tools//tools/cpp.

$ bazel query 'kind("source file", deps(@bazel_tools//tools/cpp:all, 1))'
@bazel_tools//tools/cpp:wrapper/bin/pydir/msvc_tools.py.tpl
@bazel_tools//tools/cpp:wrapper/bin/pydir/msvc_link.py
@bazel_tools//tools/cpp:wrapper/bin/pydir/msvc_cl.py
@bazel_tools//tools/cpp:wrapper/bin/msvc_nop.bat
@bazel_tools//tools/cpp:wrapper/bin/msvc_link.bat
@bazel_tools//tools/cpp:wrapper/bin/msvc_cl.bat
@bazel_tools//tools/cpp:wrapper/bin/call_python.bat.tpl
@bazel_tools//tools/cpp:osx_cc_wrapper.sh.tpl
@bazel_tools//tools/cpp:osx_cc_wrapper.sh
@bazel_tools//tools/cpp:linux_cc_wrapper.sh.tpl
@bazel_tools//tools/cpp:link_dynamic_library.sh
@bazel_tools//tools/cpp:cc_configure.bzl
@bazel_tools//tools/cpp:build_interface_so
@bazel_tools//tools/cpp:CROSSTOOL.tpl
@bazel_tools//tools/cpp:CROSSTOOL
@bazel_tools//tools/cpp:BUILD.tpl
@bazel_tools//tools/cpp:BUILD.static
@bazel_tools//tools/cpp:BUILD
abergmeier commented 7 years ago

I think that --output package is just missing the workspace name

Is that a bug?

aj-michael commented 7 years ago

I think so yea, the docs don't mention anything about stripping workspace names.

https://bazel.build/versions/master/docs/query.html#output-package

aj-michael commented 7 years ago

FWIW, this is not specific to the @bazel_tools repository. --output package gives just the "package", and I guess the "package" is a separate part of the label syntax from the "repository". So IDK if this is necessarily a bug, but I think it was implemented before the concept of "repository" existed.

The relevant code is https://github.com/bazelbuild/bazel/blob/8e042300d8e1eb6b9eda8a59bdb809aadb91d23b/src/main/java/com/google/devtools/build/lib/query2/output/OutputFormatter.java#L334

I think a diff like this would do what you would expect

diff --git a/src/main/java/com/google/devtools/build/lib/query2/output/OutputFormatter.java b/src/main/java/com/google/devtools/build/lib/query2/output/OutputFormatter.java
index 2fa79b68a..78231f9d8 100644
--- a/src/main/java/com/google/devtools/build/lib/query2/output/OutputFormatter.java
+++ b/src/main/java/com/google/devtools/build/lib/query2/output/OutputFormatter.java
@@ -331,7 +331,7 @@ public abstract class OutputFormatter implements Serializable {
         public void processOutput(Iterable<Target> partialResult) {

           for (Target target : partialResult) {
-            packageNames.add(target.getLabel().getPackageName());
+            packageNames.add(target.getPackage().getPackageIdentifier().toString());
           }
         }

@kchodorow or @haxorz , do you think this is reasonable? It would break backwards compatibility if someone was depending on this.

kchodorow commented 7 years ago

I think you're right and that patch looks correct.