Merge PackageSerializer's and ProtoOutputFormatter's duplicated

serialization logic.

Among other things, this fixes an out-of-sync bug where
ProtoOutputFormatter's version knew how to handle configurable
attributes while PackageSerializer's version crashed.

The merged logic preserves ProtoOutputFormatter's semantics:
configurable attributes work, but their values are merged together
into a flattened list, so the original select structure can't be
reproduced later.

--
MOS_MIGRATED_REVID=89435116

1 files changed, 2 insertions, 13 deletions

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 9272e1d285..e8247c540a 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
@@ -21,8 +21,8 @@ import com.google.common.collect.Sets;
 import com.google.devtools.build.lib.events.Location;
 import com.google.devtools.build.lib.graph.Digraph;
 import com.google.devtools.build.lib.graph.Node;
-import com.google.devtools.build.lib.packages.AggregatingAttributeMapper;
 import com.google.devtools.build.lib.packages.Attribute;
+import com.google.devtools.build.lib.packages.PackageSerializer;
 import com.google.devtools.build.lib.packages.Rule;
 import com.google.devtools.build.lib.packages.Target;
 import com.google.devtools.build.lib.syntax.EvalUtils;
@@ -40,7 +40,6 @@ import java.util.Comparator;
 import java.util.HashMap;
 import java.util.HashSet;
 import java.util.LinkedHashSet;
-import java.util.LinkedList;
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
@@ -450,10 +449,6 @@ public abstract class OutputFormatter implements Serializable {
    * non-configured attributes, this is a single value. For configurable attributes, this
    * may be multiple values.
    *
-   * <p>This is needed because the visibility attribute is replaced with an empty list
-   * during package loading if it is public or private in order not to visit
-   * the package called 'visibility'.
-   *
    * @return a pair, where the first value is the set of possible values and the
    *     second is an enum that tells where the values come from (declared on the
    *     rule, declared as a package level default or a
@@ -461,11 +456,9 @@ public abstract class OutputFormatter implements Serializable {
    */
   protected static Pair<Iterable<Object>, AttributeValueSource> getAttributeValues(
       Rule rule, Attribute attr) {
-    List<Object> values = new LinkedList<>(); // Not an ImmutableList: may host null values.
     AttributeValueSource source;
 
     if (attr.getName().equals("visibility")) {
-      values.add(rule.getVisibility().getDeclaredLabels());
       if (rule.isVisibilitySpecified()) {
         source = AttributeValueSource.RULE;
       } else if (rule.getPackage().isDefaultVisibilitySet()) {
@@ -474,15 +467,11 @@ public abstract class OutputFormatter implements Serializable {
         source = AttributeValueSource.DEFAULT;
       }
     } else {
-      for (Object o :
-          AggregatingAttributeMapper.of(rule).visitAttribute(attr.getName(), attr.getType())) {
-        values.add(o);
-      }
       source = rule.isAttributeValueExplicitlySpecified(attr)
           ? AttributeValueSource.RULE : AttributeValueSource.DEFAULT;
     }
 
-    return Pair.of((Iterable<Object>) values, source);
+    return Pair.of(PackageSerializer.getAttributeValues(rule, attr), source);
   }
 
   /**