Repository: bazelbuild/bazel
Issue: add `label_kind` to `--output` for cquery
Labels: P3
good first issue
team-Configurability

`bazel cquery` with `--output label_kind` gives

    ERROR: Invalid output format 'label_kind'. Valid values are: label, transitions, proto, textproto, build

Issue comments:
Contributions welcome. I expect this'd be easy by simply adding a new entry here:

https://github.com/bazelbuild/bazel/blob/605eaccabe2fed0a1d188d77baa2206f6346becd/src/main/java/com/google/devtools/build/lib/query2/cquery/ConfiguredTargetQueryEnvironment.java#L207-L209

i.e. clone and modify:

https://github.com/bazelbuild/bazel/blob/966dc2328a8565103e514f898673fd8eb037b9c0/src/main/java/com/google/devtools/build/lib/query2/cquery/LabelAndConfigurationOutputFormatterCallback.java

Hi, I would like to work on this issue, since I'm new to bazel i might need some guidance in the future. 
Hi Aman,

Happy to hear!. Feel free to reach out to me at any point and I'll offer help as I can (here or gregce@bazel.build). 
I have updated 'label' to 'label_kind' in https://github.com/bazelbuild/bazel/blob/966dc2328a8565103e514f898673fd8eb037b9c0/src/main/java/com/google/devtools/build/lib/query2/cquery/LabelAndConfigurationOutputFormatterCallback.java,

and added a new entry in https://github.com/bazelbuild/bazel/src/main/java/com/google/devtools/build/lib/query2/cquery/ConfiguredTargetQueryEnvironment.java
Great! I'll respond more on https://github.com/bazelbuild/bazel/pull/10632.

Pull request review comments:
Does your PR included a file that defines this class?

Ok, so what I'm reading from your last error report is compilation fails because the parameters you're passing to CqueryThreadsafeCallback's constructor are different than what the constructor expects:  https://github.com/bazelbuild/bazel/blob/ebca24c525ec93f34bb65053370376082765d800/src/main/java/com/google/devtools/build/lib/query2/cquery/CqueryThreadsafeCallback.java#L48-L53

The link above doesn't know what to do with an extra `ConfiguredTargetAccessor` argument.

But I just realized something that can make this all simpler! You're already accepting a `TargetAccessor<ConfiguredTarget> accessor` above, and `ConfiguredTargetAccessor` is already an instance of `TargetAccessor`!

https://github.com/bazelbuild/bazel/blob/f2cb3c69413c74569f19232f89a5f0e86d4f15ac/src/main/java/com/google/devtools/build/lib/query2/cquery/ConfiguredTargetAccessor.java#L56

So you don't have to pass the new `ConfiguredTargetAccessor configuredTargetAccessor` parameter at all (my bad). Instead, simply call `accessor.getTargetKind()` in `processOutput` below. And there's no need to change anything  here in the constructor.


Following my longer comment: you don't need `configuredTargetAccessor` here or in the above line.
Following my longer comment: just change this to `accessor.getTargetKind(configuredTarget)` and I think it'll just work.
I don't think you need `showKind` on this line: since it's private to  `LabelAndConfigurationOutputFormatterCallback`, `CqueryThreadsafeCallback` doesn't know (or need to know) it exists.

But you do want to set the private final field again, i.e. `this.showKind = showKind`.
Since most of the lines here and in the other branch are repeated, what if `if (showKind)` just sets some string `result` (or some similar name)? Then you only have to write:

    ...
    append(result)

and all those other lines once.
99% of the way there. But this and the line below are going to fail because it's not actually outputting `//$pkg:maple` now, which is what the `label_kind` option does, right?

From the red test results attached to this PR:

https://storage.googleapis.com/bazel-untrusted-buildkite-artifacts/270d91f7-5738-4269-a656-c3c98f7c3550/src/test/shell/integration/configured_query_test/attempt_1.log

You can see the actual output looks like:

    sh_library rule 
    platform rule
    constraint_value rule
    ...
Yea the task what we wanted is being achieved before only. We dont require these two lines, right?

Patch that resolved the issue:
diff --git a/src/main/java/com/google/devtools/build/lib/query2/cquery/ConfiguredTargetQueryEnvironment.java b/src/main/java/com/google/devtools/build/lib/query2/cquery/ConfiguredTargetQueryEnvironment.java
index 61ce4d8e8e9f09..ae05a7050eb013 100644
--- a/src/main/java/com/google/devtools/build/lib/query2/cquery/ConfiguredTargetQueryEnvironment.java
+++ b/src/main/java/com/google/devtools/build/lib/query2/cquery/ConfiguredTargetQueryEnvironment.java
@@ -219,7 +219,9 @@ private static ImmutableMap<String, BuildConfiguration> getTransitiveConfigurati
         cqueryOptions.aspectDeps.createResolver(packageManager, eventHandler);
     return ImmutableList.of(
         new LabelAndConfigurationOutputFormatterCallback(
-            eventHandler, cqueryOptions, out, skyframeExecutor, accessor),
+            eventHandler, cqueryOptions, out, skyframeExecutor, accessor, true),
+        new LabelAndConfigurationOutputFormatterCallback(
+            eventHandler, cqueryOptions, out, skyframeExecutor, accessor, false),
         new TransitionsOutputFormatterCallback(
             eventHandler,
             cqueryOptions,
@@ -486,4 +488,3 @@ public ThreadSafeMutableSet<ConfiguredTarget> createThreadSafeMutableSet() {
         SkyQueryEnvironment.DEFAULT_THREAD_COUNT);
   }
 }
-
diff --git a/src/main/java/com/google/devtools/build/lib/query2/cquery/CqueryOptions.java b/src/main/java/com/google/devtools/build/lib/query2/cquery/CqueryOptions.java
index 2de9e3ab8ab48e..14aea6cb2d2b01 100644
--- a/src/main/java/com/google/devtools/build/lib/query2/cquery/CqueryOptions.java
+++ b/src/main/java/com/google/devtools/build/lib/query2/cquery/CqueryOptions.java
@@ -45,7 +45,7 @@ public enum Transitions {
       effectTags = {OptionEffectTag.TERMINAL_OUTPUT},
       help =
           "The format in which the cquery results should be printed. Allowed values for cquery "
-              + "are: label, textproto, transitions, proto, jsonproto. If you select "
+              + "are: label, label_kind, textproto, transitions, proto, jsonproto. If you select "
               + "'transitions', you also have to specify the --transitions=(lite|full) option.")
   public String outputFormat;
 
diff --git a/src/main/java/com/google/devtools/build/lib/query2/cquery/LabelAndConfigurationOutputFormatterCallback.java b/src/main/java/com/google/devtools/build/lib/query2/cquery/LabelAndConfigurationOutputFormatterCallback.java
index 0f6bc91bcd11a0..d0279f1ab78bc5 100644
--- a/src/main/java/com/google/devtools/build/lib/query2/cquery/LabelAndConfigurationOutputFormatterCallback.java
+++ b/src/main/java/com/google/devtools/build/lib/query2/cquery/LabelAndConfigurationOutputFormatterCallback.java
@@ -18,25 +18,30 @@
 import com.google.devtools.build.lib.analysis.config.BuildConfiguration;
 import com.google.devtools.build.lib.analysis.config.CoreOptions.IncludeConfigFragmentsEnum;
 import com.google.devtools.build.lib.events.ExtendedEventHandler;
+import com.google.devtools.build.lib.packages.Target;
 import com.google.devtools.build.lib.query2.engine.QueryEnvironment.TargetAccessor;
 import com.google.devtools.build.lib.skyframe.SkyframeExecutor;
 import java.io.OutputStream;
 
 /** Default Output callback for cquery. Prints a label and configuration pair per result. */
 public class LabelAndConfigurationOutputFormatterCallback extends CqueryThreadsafeCallback {
-
+  private final boolean showKind;
   public LabelAndConfigurationOutputFormatterCallback(
       ExtendedEventHandler eventHandler,
       CqueryOptions options,
       OutputStream out,
       SkyframeExecutor skyframeExecutor,
-      TargetAccessor<ConfiguredTarget> accessor) {
+      TargetAccessor<ConfiguredTarget> accessor,
+      boolean showKind
+      ) {
     super(eventHandler, options, out, skyframeExecutor, accessor);
+    this.showKind = showKind;
   }
 
+
   @Override
   public String getName() {
-    return "label";
+    return this.showKind ? "label_kind" : "label";
   }
 
   @Override
@@ -44,10 +49,19 @@ public void processOutput(Iterable<ConfiguredTarget> partialResult) {
     for (ConfiguredTarget configuredTarget : partialResult) {
       BuildConfiguration config =
           skyframeExecutor.getConfiguration(eventHandler, configuredTarget.getConfigurationKey());
-
-      StringBuilder output =
-          new StringBuilder()
-              .append(configuredTarget.getLabel())
+      StringBuilder output = new StringBuilder();
+      if(showKind){
+            Target actualTarget = accessor.getTargetFromConfiguredTarget(configuredTarget);
+            output =
+                output
+                .append(actualTarget.getTargetKind());
+      }
+      else if(!showKind){
+            output =
+                output
+                .append(configuredTarget.getLabel());
+      }
+      output = output
               .append(" (")
               .append(config != null && config.isHostConfiguration() ? "HOST" : config)
               .append(")");
@@ -66,4 +80,4 @@ public void processOutput(Iterable<ConfiguredTarget> partialResult) {
       addResult(output.toString());
     }
   }
-}
\ No newline at end of file
+}


Test patch:
diff --git a/src/test/shell/integration/configured_query_test.sh b/src/test/shell/integration/configured_query_test.sh
index e3019fdc333358..6f73a165636bc5 100755
--- a/src/test/shell/integration/configured_query_test.sh
+++ b/src/test/shell/integration/configured_query_test.sh
@@ -90,6 +90,17 @@ EOF
  assert_contains "name: \"//$pkg:japanese\"" output
 }
 
+function test_basic_query_output_labelkind() {
+  local -r pkg=$FUNCNAME
+  mkdir -p $pkg
+  cat > $pkg/BUILD <<EOF
+sh_library(name='maple', deps=[':japanese'])
+sh_library(name='japanese')
+EOF
+
+ bazel cquery --output=label_kind "deps(//$pkg:maple)" > output 2>"$TEST_log" || fail "Expected success"
+}
+
 function test_respects_selects() {
   local -r pkg=$FUNCNAME
   mkdir -p $pkg