unknown commit of 5: Fixing the merge warning messages in transitive library overwrites.

Record the sources are overwritten during merging.

--
MOS_MIGRATED_REVID=140654137

11 files changed, 175 insertions, 57 deletions

diff --git a/src/tools/android/java/com/google/devtools/build/android/AndroidDataMerger.java b/src/tools/android/java/com/google/devtools/build/android/AndroidDataMerger.java
index 3ee002a98d..34b424b521 100644
--- a/src/tools/android/java/com/google/devtools/build/android/AndroidDataMerger.java
+++ b/src/tools/android/java/com/google/devtools/build/android/AndroidDataMerger.java
@@ -36,9 +36,7 @@ import java.util.concurrent.ExecutorService;
 import java.util.concurrent.TimeUnit;
 import java.util.logging.Logger;
 
-/**
- * Handles the Merging of ParsedAndroidData.
- */
+/** Handles the Merging of ParsedAndroidData. */
 public class AndroidDataMerger {
 
   private static final Logger logger = Logger.getLogger(AndroidDataMerger.class.getCanonicalName());
@@ -128,7 +126,6 @@ public class AndroidDataMerger {
         }
       }
     }
-
   }
 
   static class NoopSourceChecker implements SourceChecker {
@@ -152,8 +149,7 @@ public class AndroidDataMerger {
 
   /** Creates a merger with a custom deduplicator and a default {@link ExecutorService}. */
   public static AndroidDataMerger createWithDefaultThreadPool(SourceChecker deDuplicator) {
-    return new AndroidDataMerger(deDuplicator,
-        MoreExecutors.newDirectExecutorService());
+    return new AndroidDataMerger(deDuplicator, MoreExecutors.newDirectExecutorService());
   }
 
   /** Creates a merger with a custom deduplicator and an {@link ExecutorService}. */
@@ -224,18 +220,20 @@ public class AndroidDataMerger {
 
   /**
    * Merges DataResources into an UnwrittenMergedAndroidData.
-   * <p>
-   * This method has two basic states, library and binary. These are distinguished by
+   *
+   * <p>This method has two basic states, library and binary. These are distinguished by
    * allowPrimaryOverrideAll, which allows the primary data to overwrite any value in the closure, a
    * trait associated with binaries, as a binary is a leaf node. The other semantics are slightly
    * more complicated: a given resource can be overwritten only if it resides in the direct
    * dependencies of primary data. This forces an explicit simple priority for each resource,
    * instead of the more subtle semantics of multiple layers of libraries with potential overwrites.
-   * <p>
-   * The UnwrittenMergedAndroidData contains only one of each DataKey in both the direct and
+   *
+   * <p>The UnwrittenMergedAndroidData contains only one of each DataKey in both the direct and
    * transitive closure.
    *
-   * The merge semantics for overwriting resources (non id and styleable) are as follows: <pre>
+   * <p>The merge semantics for overwriting resources (non id and styleable) are as follows:
+   *
+   * <pre>
    *   Key:
    *     A(): package A
    *     A(foo): package A with resource symbol foo
@@ -266,22 +264,24 @@ public class AndroidDataMerger {
    *     A(foo),B(foo) -> C() -> D(foo) == Valid
    *     A() -> B(foo),C(foo) -> D(foo) == Valid
    * </pre>
-   * <p>
-   * Combining resources are much simpler -- since a combining (id and styleable) resource does not
-   * get replaced when redefined, they are simply combined: <pre>
+   *
+   * <p>Combining resources are much simpler -- since a combining (id and styleable) resource does
+   * not get replaced when redefined, they are simply combined:
+   *
+   * <pre>
    *     A(foo) -> B(foo) -> C(foo) == Valid
-   *     
+   *
    * </pre>
-   * 
+   *
    * @param transitive The transitive dependencies to merge.
    * @param direct The direct dependencies to merge.
    * @param primaryData The primary data to merge against.
    * @param allowPrimaryOverrideAll Boolean that indicates if the primary data will be considered
-   *    the ultimate source of truth, provided it doesn't conflict with itself.
+   *     the ultimate source of truth, provided it doesn't conflict with itself.
    * @return An UnwrittenMergedAndroidData, containing DataResource objects that can be written to
-   *    disk for aapt processing or serialized for future merge passes.
+   *     disk for aapt processing or serialized for future merge passes.
    * @throws MergingException if there are merge conflicts or issues with parsing resources from
-   *    primaryData.
+   *     primaryData.
    */
   UnwrittenMergedAndroidData merge(
       ParsedAndroidData transitive,
@@ -314,6 +314,8 @@ public class AndroidDataMerger {
       final KeyValueConsumers primaryConsumers = primaryBuilder.consumers();
 
       final Set<MergeConflict> conflicts = new HashSet<>();
+      
+      // Find all internal conflicts.
       conflicts.addAll(parsedPrimary.conflicts());
       for (MergeConflict conflict : Iterables.concat(direct.conflicts(), transitive.conflicts())) {
         if (allowPrimaryOverrideAll
@@ -326,7 +328,12 @@ public class AndroidDataMerger {
 
       // overwriting resources
       for (Entry<DataKey, DataResource> entry : parsedPrimary.iterateOverwritableEntries()) {
-        primaryConsumers.overwritingConsumer.consume(entry.getKey(), entry.getValue());
+        if (direct.containsOverwritable(entry.getKey())) {
+          primaryConsumers.overwritingConsumer.consume(
+              entry.getKey(), entry.getValue().overwrite(direct.getOverwritable(entry.getKey())));
+        } else {
+          primaryConsumers.overwritingConsumer.consume(entry.getKey(), entry.getValue());
+        }
       }
 
       for (Map.Entry<DataKey, DataResource> entry : direct.iterateOverwritableEntries()) {
@@ -379,7 +386,12 @@ public class AndroidDataMerger {
 
       // assets
       for (Entry<DataKey, DataAsset> entry : parsedPrimary.iterateAssetEntries()) {
-        primaryConsumers.assetConsumer.consume(entry.getKey(), entry.getValue());
+        if (direct.containsAsset(entry.getKey())) {
+          primaryConsumers.assetConsumer.consume(
+              entry.getKey(), entry.getValue().overwrite(direct.getAsset(entry.getKey())));
+        } else {
+          primaryConsumers.assetConsumer.consume(entry.getKey(), entry.getValue());
+        }
       }
 
       for (Map.Entry<DataKey, DataAsset> entry : direct.iterateAssetEntries()) {
@@ -412,9 +424,7 @@ public class AndroidDataMerger {
       if (!conflicts.isEmpty()) {
         List<String> messages = new ArrayList<>();
         for (MergeConflict conflict : conflicts) {
-          if (!conflict.first().equals(conflict.second())
-              && !deDuplicator.checkEquality(
-                  conflict.first().source(), conflict.second().source())) {
+          if (conflict.isValidWith(deDuplicator)) {
             messages.add(conflict.toConflictMessage());
           }
         }
@@ -423,11 +433,8 @@ public class AndroidDataMerger {
           logger.warning(Joiner.on("").join(messages));
         }
       }
-
       return UnwrittenMergedAndroidData.of(
-          primaryManifest,
-          primaryBuilder.build(),
-          transitiveBuilder.build());
+          primaryManifest, primaryBuilder.build(), transitiveBuilder.build());
     } catch (IOException e) {
       throw MergingException.wrapException(e).build();
     }
diff --git a/src/tools/android/java/com/google/devtools/build/android/DataAsset.java b/src/tools/android/java/com/google/devtools/build/android/DataAsset.java
index cbb2f77101..b78b07fd30 100644
--- a/src/tools/android/java/com/google/devtools/build/android/DataAsset.java
+++ b/src/tools/android/java/com/google/devtools/build/android/DataAsset.java
@@ -24,4 +24,7 @@ public interface DataAsset extends DataValue {
    */
   void writeAsset(RelativeAssetPath key, AndroidDataWritingVisitor mergedDataWriter)
       throws IOException;
+  
+  /** Overwrite another {@link DataAsset}. */
+  DataAsset overwrite(DataAsset other);
 }
diff --git a/src/tools/android/java/com/google/devtools/build/android/DataResource.java b/src/tools/android/java/com/google/devtools/build/android/DataResource.java
index d1836b8a13..b177b9e49b 100644
--- a/src/tools/android/java/com/google/devtools/build/android/DataResource.java
+++ b/src/tools/android/java/com/google/devtools/build/android/DataResource.java
@@ -14,7 +14,6 @@
 package com.google.devtools.build.android;
 
 import com.android.ide.common.res2.MergingException;
-
 import java.io.IOException;
 
 /**
@@ -42,4 +41,7 @@ public interface DataResource extends DataValue {
   void writeResourceToClass(
       FullyQualifiedName key,
       AndroidResourceClassWriter resourceClassWriter);
+  
+  /** Overwrite another {@link DataResource}. */
+  DataResource overwrite(DataResource other);
 }
diff --git a/src/tools/android/java/com/google/devtools/build/android/DataResourceXml.java b/src/tools/android/java/com/google/devtools/build/android/DataResourceXml.java
index 1fbbd8c81a..39f230dbec 100644
--- a/src/tools/android/java/com/google/devtools/build/android/DataResourceXml.java
+++ b/src/tools/android/java/com/google/devtools/build/android/DataResourceXml.java
@@ -311,12 +311,16 @@ public class DataResourceXml implements DataResource {
     }
     DataResourceXml xmlResource = (DataResourceXml) resource;
     return createWithNamespaces(
-        combineSources(xmlResource.source),
+        source.combine(xmlResource.source),
         xml.combineWith(xmlResource.xml),
         namespaces.union(xmlResource.namespaces));
   }
 
-  private DataSource combineSources(DataSource otherSource) {
-    return source.combine(otherSource);
+  @Override
+  public DataResource overwrite(DataResource resource) {
+    if (equals(resource)) {
+      return this;
+    }
+    return createWithNamespaces(source.overwrite(resource.source()), xml, namespaces);
   }
 }
diff --git a/src/tools/android/java/com/google/devtools/build/android/DataSource.java b/src/tools/android/java/com/google/devtools/build/android/DataSource.java
index 5c51ea729a..1e4580eec9 100644
--- a/src/tools/android/java/com/google/devtools/build/android/DataSource.java
+++ b/src/tools/android/java/com/google/devtools/build/android/DataSource.java
@@ -14,7 +14,10 @@
 package com.google.devtools.build.android;
 
 import com.android.SdkConstants;
+import com.google.common.base.MoreObjects;
 import com.google.common.base.Objects;
+import com.google.common.base.Preconditions;
+import com.google.common.collect.ImmutableSet;
 import com.google.devtools.build.android.proto.SerializeFormat.ProtoSource;
 import java.io.BufferedInputStream;
 import java.io.IOException;
@@ -32,14 +35,16 @@ public class DataSource implements Comparable<DataSource> {
     return of(path);
   }
 
-  public static DataSource of(Path source) {
-    return new DataSource(source);
+  public static DataSource of(Path sourcePath) {
+    return new DataSource(sourcePath, ImmutableSet.<DataSource>of());
   }
 
   private final Path path;
+  private final ImmutableSet<DataSource> overrides;
 
-  private DataSource(Path path) {
+  private DataSource(Path path, ImmutableSet<DataSource> overrides) {
     this.path = path;
+    this.overrides = overrides;
   }
 
   public Path getPath() {
@@ -58,12 +63,13 @@ public class DataSource implements Comparable<DataSource> {
     if (o == null || getClass() != o.getClass()) {
       return false;
     }
-    return Objects.equal(path, ((DataSource) o).path);
+    return Objects.equal(path, ((DataSource) o).path)
+        && Objects.equal(overrides, ((DataSource) o).overrides);
   }
 
   @Override
   public int hashCode() {
-    return Objects.hashCode(path);
+    return Objects.hashCode(path, overrides);
   }
 
   @Override
@@ -92,7 +98,30 @@ public class DataSource implements Comparable<DataSource> {
     return this;
   }
 
+  public DataSource overwrite(DataSource... sources) {
+    ImmutableSet<DataSource> overrides =
+        ImmutableSet.<DataSource>builder().addAll(this.overrides).add(sources).build();
+    Preconditions.checkArgument(!overrides.contains(this));
+    return new DataSource(path, overrides);
+  }
+
+  public ImmutableSet<DataSource> overrides() {
+    return overrides;
+  }
+
   public boolean isInValuesFolder() {
     return path.getParent().getFileName().toString().startsWith(SdkConstants.FD_RES_VALUES);
   }
+
+  public boolean hasOveridden(DataSource source) {
+    return overrides.contains(source);
+  }
+
+  @Override
+  public String toString() {
+    return MoreObjects.toStringHelper(getClass())
+        .add("path", path)
+        .add("overrides", overrides)
+        .toString();
+  }
 }
diff --git a/src/tools/android/java/com/google/devtools/build/android/DataSourceTable.java b/src/tools/android/java/com/google/devtools/build/android/DataSourceTable.java
index 0a26fc092a..53be389d9f 100644
--- a/src/tools/android/java/com/google/devtools/build/android/DataSourceTable.java
+++ b/src/tools/android/java/com/google/devtools/build/android/DataSourceTable.java
@@ -13,6 +13,10 @@
 // limitations under the License.
 package com.google.devtools.build.android;
 
+import com.google.common.base.Function;
+import com.google.common.collect.Collections2;
+import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableSet;
 import com.google.devtools.build.android.proto.SerializeFormat;
 import com.google.devtools.build.android.proto.SerializeFormat.Header;
 import com.google.devtools.build.android.proto.SerializeFormat.ProtoSource;
@@ -20,18 +24,26 @@ import java.io.IOException;
 import java.io.InputStream;
 import java.io.OutputStream;
 import java.nio.file.FileSystem;
-import java.util.HashMap;
+import java.util.LinkedHashMap;
+import java.util.LinkedList;
+import java.util.List;
 import java.util.Map;
 import java.util.NavigableMap;
 
-
 /**
  * Tracks mappings from resource source paths (/foo/bar/res/values/colors.xml) to an ID for a more
  * compact serialization format.
  */
 class DataSourceTable {
 
-  private final Map<DataSource, Integer> sourceTable = new HashMap<>();
+  private static final Function<DataValue, DataSource> VALUE_TO_SOURCE =
+      new Function<DataValue, DataSource>() {
+        @Override
+        public DataSource apply(DataValue input) {
+          return input.source();
+        }
+      };
+  private final Map<DataSource, Integer> sourceTable = new LinkedHashMap<>();
   private DataSource[] idToSource;
 
   /**
@@ -60,17 +72,35 @@ class DataSourceTable {
   private void writeSourceInfo(NavigableMap<DataKey, DataValue> map, OutputStream outStream)
       throws IOException {
     int sourceNumber = 0;
-    for (Map.Entry<DataKey, DataValue> entry : map.entrySet()) {
-      DataSource source = entry.getValue().source();
+    LinkedList<DataSource> sourceQueue =
+        new LinkedList<>(Collections2.transform(map.values(), VALUE_TO_SOURCE));
+    while (!sourceQueue.isEmpty()) {
+      DataSource source = sourceQueue.pop();
       if (!sourceTable.containsKey(source)) {
         sourceTable.put(source, sourceNumber);
         ++sourceNumber;
-        ProtoSource.newBuilder()
-            .setFilename(source.getPath().toString())
-            .build()
-            .writeDelimitedTo(outStream);
+        sourceQueue.addAll(source.overrides());
+      }
+    }
+    for (DataSource dataSource : sourceTable.keySet()) {
+      ProtoSource.newBuilder()
+          .setFilename(dataSource.getPath().toString())
+          .addAllOverwritten(sourcesToIds(dataSource.overrides()))
+          .build()
+          .writeDelimitedTo(outStream);
+    }
+  }
+
+  private List<Integer> sourcesToIds(ImmutableSet<DataSource> overrides) {
+    ImmutableList.Builder<Integer> idsBuilder = ImmutableList.builder();
+    for (DataSource dataSource : overrides) {
+      if (!sourceTable.containsKey(dataSource)) {
+        throw new IllegalArgumentException(
+            "Cannot find data source: " + dataSource.toString() + " in " + sourceTable.keySet());
       }
+      idsBuilder.add(sourceTable.get(dataSource));
     }
+    return idsBuilder.build();
   }
 
   /** Fill in the serialize format header information required to deserialize */
diff --git a/src/tools/android/java/com/google/devtools/build/android/DataValueFile.java b/src/tools/android/java/com/google/devtools/build/android/DataValueFile.java
index a54c8ef6da..b24d1278af 100644
--- a/src/tools/android/java/com/google/devtools/build/android/DataValueFile.java
+++ b/src/tools/android/java/com/google/devtools/build/android/DataValueFile.java
@@ -42,6 +42,7 @@ public class DataValueFile implements DataResource, DataAsset {
   public static DataValueFile of(DataSource source) {
     return new DataValueFile(source);
   }
+
   /**
    * Creates a {@link DataValueFile} from a {@link SerializeFormat.DataValue}.
    */
@@ -101,6 +102,22 @@ public class DataValueFile implements DataResource, DataAsset {
   }
 
   @Override
+  public DataResource overwrite(DataResource resource) {
+    if (equals(resource)) {
+      return this;
+    }
+    return of(source.overwrite(resource.source()));
+  }
+
+  @Override
+  public DataAsset overwrite(DataAsset asset) {
+    if (equals(asset)) {
+      return this;
+    }
+    return of(source.overwrite(asset.source()));
+  }
+
+  @Override
   public void writeResourceToClass(FullyQualifiedName key,
       AndroidResourceClassWriter resourceClassWriter) {
     resourceClassWriter.writeSimpleResource(key.type(), key.name());
diff --git a/src/tools/android/java/com/google/devtools/build/android/MergeConflict.java b/src/tools/android/java/com/google/devtools/build/android/MergeConflict.java
index cbaa642fab..215c895550 100644
--- a/src/tools/android/java/com/google/devtools/build/android/MergeConflict.java
+++ b/src/tools/android/java/com/google/devtools/build/android/MergeConflict.java
@@ -13,12 +13,12 @@
 // limitations under the License.
 package com.google.devtools.build.android;
 
-import com.google.common.base.MoreObjects;
-import com.google.common.base.Preconditions;
-
 import com.android.annotations.VisibleForTesting;
 import com.android.annotations.concurrency.Immutable;
-
+import com.google.common.base.MoreObjects;
+import com.google.common.base.Preconditions;
+import com.google.devtools.build.android.AndroidDataMerger.SourceChecker;
+import java.io.IOException;
 import java.util.Objects;
 
 /**
@@ -82,6 +82,13 @@ public class MergeConflict {
     return second;
   }
 
+  boolean isValidWith(SourceChecker checker) throws IOException {
+    return !first.equals(second)
+        && !first.source().hasOveridden(second.source())
+        && !second.source().hasOveridden(first.source())
+        && !checker.checkEquality(first.source(), second.source());
+  }
+
   @Override
   public String toString() {
     return MoreObjects.toStringHelper(this)
diff --git a/src/tools/android/java/com/google/devtools/build/android/ParsedAndroidData.java b/src/tools/android/java/com/google/devtools/build/android/ParsedAndroidData.java
index 2470a3f87b..e1d29d0515 100644
--- a/src/tools/android/java/com/google/devtools/build/android/ParsedAndroidData.java
+++ b/src/tools/android/java/com/google/devtools/build/android/ParsedAndroidData.java
@@ -176,10 +176,15 @@ public class ParsedAndroidData {
     @Override
     public void consume(K key, V value) {
       if (target.containsKey(key)) {
-        conflicts.add(MergeConflict.between(key, value, target.get(key)));
+        V other = target.get(key);
+        conflicts.add(MergeConflict.between(key, value, other));
+        if (!other.source().hasOveridden(value.source())) {
+          // Only replace it if the previous value has explicitly replaced the current.
+          target.put(key, value);
+        }
+      } else {
+        target.put(key, value);
       }
-      // Always record the value, conflict or not, to maintain backwards compatibility.
-      target.put(key, value);
     }
   }
 
@@ -463,13 +468,17 @@ public class ParsedAndroidData {
     return assets;
   }
 
-  boolean containsOverwritable(DataKey name) {
+  public boolean containsOverwritable(DataKey name) {
     return overwritingResources.containsKey(name);
   }
 
   public boolean containsCombineable(DataKey key) {
     return combiningResources.containsKey(key);
   }
+  
+  public DataResource getOverwritable(DataKey name) {
+    return overwritingResources.get(name);
+  }
 
   Iterable<Entry<DataKey, DataResource>> iterateOverwritableEntries() {
     return overwritingResources.entrySet();
@@ -503,4 +512,8 @@ public class ParsedAndroidData {
   ImmutableSet<MergeConflict> conflicts() {
     return conflicts;
   }
+
+  public DataAsset getAsset(DataKey key) {
+    return assets.get(key);
+  }
 }
diff --git a/src/tools/android/java/com/google/devtools/build/android/proto/serialize_format.proto b/src/tools/android/java/com/google/devtools/build/android/proto/serialize_format.proto
index 3c24f5f54d..122d3569da 100644
--- a/src/tools/android/java/com/google/devtools/build/android/proto/serialize_format.proto
+++ b/src/tools/android/java/com/google/devtools/build/android/proto/serialize_format.proto
@@ -58,6 +58,8 @@ message DataValue {
 message ProtoSource {
   // Required
   optional string filename = 1;
+  // The indexes of sources this source replaces.
+  repeated uint32 overwritten = 2;
 }
 
 // The container for a serialized xml value.
diff --git a/src/tools/android/java/com/google/devtools/build/android/xml/Namespaces.java b/src/tools/android/java/com/google/devtools/build/android/xml/Namespaces.java
index cbaabea740..27947d6e01 100644
--- a/src/tools/android/java/com/google/devtools/build/android/xml/Namespaces.java
+++ b/src/tools/android/java/com/google/devtools/build/android/xml/Namespaces.java
@@ -13,6 +13,7 @@
 // limitations under the License.
 package com.google.devtools.build.android.xml;
 
+import com.google.common.base.MoreObjects;
 import com.google.common.collect.ImmutableMap;
 import com.google.devtools.build.android.DataResourceXml;
 import com.google.devtools.build.android.XmlResourceValue;
@@ -43,9 +44,7 @@ public class Namespaces implements Iterable<Entry<String, String>> {
   private static final Namespaces EMPTY_INSTANCE =
       new Namespaces(ImmutableMap.<String, String>of());
 
-  /**
-   * Collects prefix and uri pairs from elements.
-   */
+  /** Collects prefix and uri pairs from elements. */
   public static class Collector {
     private Map<String, String> prefixToUri = new HashMap<>();
 
@@ -149,6 +148,11 @@ public class Namespaces implements Iterable<Entry<String, String>> {
   }
 
   @Override
+  public String toString() {
+    return MoreObjects.toStringHelper(getClass()).add("prefixToUri", prefixToUri).toString();
+  }
+
+  @Override
   public boolean equals(Object obj) {
     if (obj instanceof Namespaces) {
       Namespaces other = (Namespaces) obj;