unknown commit of 5: Fixing the merge warning messages in transitive library overwrites. Record the sources are overwritten during merging. -- MOS_MIGRATED_REVID=140654137
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 3ee002a..34b424b 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.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 @@ } } } - } static class NoopSourceChecker implements SourceChecker { @@ -152,8 +149,7 @@ /** 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 @@ /** * 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 @@ * 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 @@ 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 @@ // 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 @@ // 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 @@ 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 @@ 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 cbb2f77..b78b07f 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 @@ */ 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 d1836b8..b177b9e 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 @@ 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 1fbbd8c..39f230d 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 @@ } 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 5c51ea7..1e4580e 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 @@ 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 @@ 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 @@ 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 0a26fc0..53be389 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.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 @@ 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 a54c8ef..b24d127 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 static DataValueFile of(DataSource source) { return new DataValueFile(source); } + /** * Creates a {@link DataValueFile} from a {@link SerializeFormat.DataValue}. */ @@ -101,6 +102,22 @@ } @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 cbaa642..215c895 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 @@ 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 2470a3f..e1d29d0 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 @@ @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 @@ 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 @@ 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 3c24f5f..122d356 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 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 cbaabea..27947d6 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 @@ 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 @@ } @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;