From c018f607c9b52d301841c4d0fd2d8771c711d299 Mon Sep 17 00:00:00 2001 From: dchai Date: Mon, 12 Aug 2019 14:44:34 -0700 Subject: [PATCH] Remove obsolete code for config splits Bazel support was never launched, and aapt1 is already obsolete. Most of the deleted logic (SplitConfigurationFilter) deals with inferring a split's configuration from its filename, but the right way is to read this directly from its manifest: * https://android.googlesource.com/platform/frameworks/base/+/refs/tags/platform-tools-29.0.2/tools/aapt2/cmd/Util.cpp#219 PiperOrigin-RevId: 263009083 --- .../com/google/devtools/build/android/BUILD | 14 - .../android/SplitConfigurationFilterTest.java | 327 ---------------- .../AndroidResourceProcessingAction.java | 7 +- .../android/AndroidResourceProcessor.java | 71 +--- .../AndroidResourceValidatorAction.java | 2 - .../build/android/ResourceShrinkerAction.java | 1 - .../android/SplitConfigurationFilter.java | 366 ------------------ 7 files changed, 3 insertions(+), 785 deletions(-) delete mode 100644 src/test/java/com/google/devtools/build/android/SplitConfigurationFilterTest.java delete mode 100644 src/tools/android/java/com/google/devtools/build/android/SplitConfigurationFilter.java diff --git a/src/test/java/com/google/devtools/build/android/BUILD b/src/test/java/com/google/devtools/build/android/BUILD index a96be108a7fda4..a73dec0ba8011c 100644 --- a/src/test/java/com/google/devtools/build/android/BUILD +++ b/src/test/java/com/google/devtools/build/android/BUILD @@ -257,20 +257,6 @@ java_test( ], ) -java_test( - name = "SplitConfigurationFilterTest", - size = "small", - srcs = ["SplitConfigurationFilterTest.java"], - deps = [ - "//src/tools/android/java/com/google/devtools/build/android:android_builder_lib", - "//third_party:guava", - "//third_party:guava-testlib", - "//third_party:jsr305", - "//third_party:junit4", - "//third_party:truth", - ], -) - java_test( name = "UnvalidatedAndroidDataTest", size = "small", diff --git a/src/test/java/com/google/devtools/build/android/SplitConfigurationFilterTest.java b/src/test/java/com/google/devtools/build/android/SplitConfigurationFilterTest.java deleted file mode 100644 index 9e58672c35cd60..00000000000000 --- a/src/test/java/com/google/devtools/build/android/SplitConfigurationFilterTest.java +++ /dev/null @@ -1,327 +0,0 @@ -// Copyright 2017 The Bazel Authors. All rights reserved. -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. -package com.google.devtools.build.android; - -import static com.google.common.truth.Truth.assertThat; -import static com.google.common.truth.Truth.assertWithMessage; -import static com.google.devtools.build.android.SplitConfigurationFilter.mapFilenamesToSplitFlags; - -import com.google.common.collect.ImmutableList; -import com.google.common.testing.EqualsTester; -import com.google.common.truth.BooleanSubject; -import com.google.devtools.build.android.SplitConfigurationFilter.ResourceConfiguration; -import com.google.devtools.build.android.SplitConfigurationFilter.UnrecognizedSplitsException; -import javax.annotation.CheckReturnValue; -import org.junit.Test; -import org.junit.runner.RunWith; -import org.junit.runners.JUnit4; - -/** Tests for {@link SplitConfigurationFilter}. */ -@RunWith(JUnit4.class) -public final class SplitConfigurationFilterTest { - - @Test - public void mapFilenamesToSplitFlagsShouldReturnEmptyMapForEmptyInput() - throws UnrecognizedSplitsException { - assertThat(mapFilenamesToSplitFlags(ImmutableList.of(), ImmutableList.of())) - .isEmpty(); - } - - @Test - public void mapFilenamesToSplitFlagsShouldMatchIdenticalFilenames() - throws UnrecognizedSplitsException { - assertThat(mapFilenamesToSplitFlags(ImmutableList.of("en"), ImmutableList.of("en"))) - .containsExactly("en", "en"); - assertThat(mapFilenamesToSplitFlags(ImmutableList.of("fr-v7"), ImmutableList.of("fr-v7"))) - .containsExactly("fr-v7", "fr-v7"); - assertThat(mapFilenamesToSplitFlags(ImmutableList.of("en_fr-v7"), ImmutableList.of("en,fr-v7"))) - .containsExactly("en_fr-v7", "en_fr-v7"); - assertThat( - mapFilenamesToSplitFlags( - ImmutableList.of("en", "fr-v7"), ImmutableList.of("en", "fr-v7"))) - .containsExactly("en", "en", "fr-v7", "fr-v7"); - assertThat( - mapFilenamesToSplitFlags( - ImmutableList.of("fr-v4", "fr-v7"), ImmutableList.of("fr-v4", "fr-v7"))) - .containsExactly("fr-v4", "fr-v4", "fr-v7", "fr-v7"); - } - - @Test - public void mapFilenamesToSplitFlagsShouldMatchReorderedFilenames() - throws UnrecognizedSplitsException { - assertThat(mapFilenamesToSplitFlags(ImmutableList.of("en_fr_ja"), ImmutableList.of("ja,fr,en"))) - .containsExactly("en_fr_ja", "ja_fr_en"); - assertThat( - mapFilenamesToSplitFlags( - ImmutableList.of("zu_12key", "zu"), ImmutableList.of("12key,zu", "zu"))) - .containsExactly("zu_12key", "12key_zu", "zu", "zu"); - assertThat( - mapFilenamesToSplitFlags( - ImmutableList.of("land_car", "round_port"), - ImmutableList.of("car,land", "port,round"))) - .containsExactly("land_car", "car_land", "round_port", "port_round"); - } - - @Test - public void mapFilenamesToSplitFlagsShouldMatchVersionUpgradedFilenames() - throws UnrecognizedSplitsException { - assertThat(mapFilenamesToSplitFlags(ImmutableList.of("round-v23"), ImmutableList.of("round"))) - .containsExactly("round-v23", "round"); - assertThat( - mapFilenamesToSplitFlags( - ImmutableList.of("watch-v20", "watch-v23"), ImmutableList.of("watch", "watch-v23"))) - .containsExactly("watch-v20", "watch", "watch-v23", "watch-v23"); - } - - @Test - public void equivalentFiltersShouldMatchFilterFromFilename() { - // identical inputs (barring commas -> underscores) should naturally match - assertMatchesFilterFromFilename("abc-def", "abc-def").isTrue(); - assertMatchesFilterFromFilename("abc-def-v5,ghi,jkl-lmno", "abc-def-v5_ghi_jkl-lmno").isTrue(); - // anything that results in matching sets should work, so case should be ignored - assertMatchesFilterFromFilename("abc-dEF", "aBC-def").isTrue(); - assertMatchesFilterFromFilename("aBC-def", "abc-dEF").isTrue(); - // additionally, order of elements in the set should be ignored - assertMatchesFilterFromFilename("abc,def", "def_abc").isTrue(); - assertMatchesFilterFromFilename("def,abc", "abc_def").isTrue(); - } - - @Test - public void matchingSpecifiersShouldMatchFilterFromFilenameWhenVersionsAreHigherNotWhenLower() { - // different (higher) versions, but same specifiers and same order - assertMatchesFilterFromFilename("x-v5,y-v6", "x-v7_y-v9").isTrue(); - assertMatchesFilterFromFilename("x-v7,y-v9", "x-v5_y-v6").isFalse(); - // order of sorted set changes but matching specifiers still match and filename version > flag - assertMatchesFilterFromFilename("x-v3,y-v6", "x-v23_y-v9").isTrue(); - assertMatchesFilterFromFilename("x-v23,y-v9", "x-v3_y-v6").isFalse(); - // specifier sets of multiple configs are the same - assertMatchesFilterFromFilename("x-v3,x-v17", "x-v3_x-v17").isTrue(); - assertMatchesFilterFromFilename("x-v3,x-v17", "x-v17_x-v3").isTrue(); - assertMatchesFilterFromFilename("x-v17,x-v3", "x-v17_x-v3").isTrue(); - assertMatchesFilterFromFilename("x-v17,x-v3", "x-v3_x-v17").isTrue(); - // specifier sets are the same, but one of them got a version bump - assertMatchesFilterFromFilename("x-v3,x-v17", "x-v14_x-v17").isTrue(); - assertMatchesFilterFromFilename("x-v14,x-v17", "x-v3_x-v17").isFalse(); - } - - @Test - public void nonMatchingSpecifiersShouldNotMatchFilterFromFilename() { - // completely disjoint specifier sets - assertMatchesFilterFromFilename("x,y,z", "a_b_c").isFalse(); - assertMatchesFilterFromFilename("a,b,c", "x_y_z").isFalse(); - // different number of specifier sets, which otherwise match - assertMatchesFilterFromFilename("x,y,z", "x_y").isFalse(); - assertMatchesFilterFromFilename("x,y", "x_y_z").isFalse(); - // same number of specifiers with one non-match - assertMatchesFilterFromFilename("x,y,z", "a_y_z").isFalse(); - assertMatchesFilterFromFilename("a,b,c", "a_b_z").isFalse(); - } - - @CheckReturnValue - private BooleanSubject assertMatchesFilterFromFilename(String flagFilter, String filenameFilter) { - return assertWithMessage( - "The split flag '%s' would be a match for a filename containing '%s'", - flagFilter, filenameFilter) - .that( - SplitConfigurationFilter.fromSplitFlag(flagFilter) - .matchesFilterFromFilename( - SplitConfigurationFilter.fromFilenameSuffix(filenameFilter))); - } - - @Test - public void splitConfigurationFilterShouldBeOrderedByConfigsThenFilename() { - assertThat( - ImmutableList.of( - // If all else is equal, break ties via filename (case does matter here) - SplitConfigurationFilter.fromFilenameSuffix("A"), - SplitConfigurationFilter.fromFilenameSuffix("a"), - // In filters with the same number of configs, the highest version wins - SplitConfigurationFilter.fromFilenameSuffix("d-v5_e-v5_f-v5"), - SplitConfigurationFilter.fromFilenameSuffix("a_b_c-v6"), - // It doesn't matter where in the input order that version is - SplitConfigurationFilter.fromFilenameSuffix("a-v7_b_c"), - // Specifiers break ties on number of configs + highest version - SplitConfigurationFilter.fromFilenameSuffix("d-v7_b_c"), - // Second highest version breaks ties (etc.) - SplitConfigurationFilter.fromFilenameSuffix("b-v2_d-v7_c"), - // Order doesn't matter but it does change the filename, hence sort order - SplitConfigurationFilter.fromFilenameSuffix("d-v7_b-v2_c"), - // Number of configs is the main criterion, so more sets of configs means later sort - SplitConfigurationFilter.fromFilenameSuffix("a_b_c_d_e_f_g_h_i_j_k_l_m"))) - .isInStrictOrder(); - - // if they are actually equal, they will compare equal - assertThat(SplitConfigurationFilter.fromFilenameSuffix("c-v13")) - .isEquivalentAccordingToCompareTo(SplitConfigurationFilter.fromFilenameSuffix("c-v13")); - - // if the only difference is split flag vs. filename suffix they will compare equal - assertThat(SplitConfigurationFilter.fromSplitFlag("split,split")) - .isEquivalentAccordingToCompareTo( - SplitConfigurationFilter.fromFilenameSuffix("split_split")); - } - - @Test - public void splitConfigurationFilterEqualsShouldPassEqualsTester() { - new EqualsTester() - .addEqualityGroup( - // base example - SplitConfigurationFilter.fromFilenameSuffix("abc-def_ghi_jkl"), - // identical clone - SplitConfigurationFilter.fromFilenameSuffix("abc-def_ghi_jkl"), - // commas are converted for split flags, the result is equal - SplitConfigurationFilter.fromSplitFlag("abc-def,ghi,jkl")) - .addEqualityGroup( - // case matters for filenames - SplitConfigurationFilter.fromFilenameSuffix("aBC-dEF_ghi_jkl")) - .addEqualityGroup( - // different filename producing equal set (elements parse the same) is still different - SplitConfigurationFilter.fromFilenameSuffix("abc-def-v0_ghi_jkl")) - .addEqualityGroup( - // different filename producing equal set (input order is different) is still different - SplitConfigurationFilter.fromFilenameSuffix("ghi_abc-def_jkl")) - .addEqualityGroup( - // totally different set is also very clearly different - SplitConfigurationFilter.fromFilenameSuffix("some-other_specifiers")) - .testEquals(); - } - - @Test - public void equalInputsShouldMatchConfigurationFromFilename() { - // identical inputs should naturally match - assertMatchesConfigurationFromFilename("abc-def", "abc-def").isTrue(); - assertMatchesConfigurationFromFilename("abc-def-v5", "abc-def-v5").isTrue(); - // case should be ignored - assertMatchesConfigurationFromFilename("abc-dEF", "aBC-def").isTrue(); - assertMatchesConfigurationFromFilename("aBC-def", "abc-dEF").isTrue(); - // wildcards should be ignored - assertMatchesConfigurationFromFilename("any-abc-def", "abc-any-def").isTrue(); - assertMatchesConfigurationFromFilename("abc-any-def", "any-abc-def").isTrue(); - // v0 should be ignored - assertMatchesConfigurationFromFilename("abc-def", "abc-def-v0").isTrue(); - assertMatchesConfigurationFromFilename("abc-def-v0", "abc-def").isTrue(); - } - - @Test - public void higherVersionSameSpecifiersShouldMatchConfigurationFromFilename() { - // any version is higher than the default v0 - assertMatchesConfigurationFromFilename("abc-def", "abc-def-v1").isTrue(); - assertMatchesConfigurationFromFilename("abc-def-v1", "abc-def").isFalse(); - // same deal for explicit v0 - assertMatchesConfigurationFromFilename("abc-def-v0", "abc-def-v1").isTrue(); - assertMatchesConfigurationFromFilename("abc-def-v1", "abc-def-v0").isFalse(); - // higher versions are better of course - assertMatchesConfigurationFromFilename("abc-def-v1", "abc-def-v999").isTrue(); - assertMatchesConfigurationFromFilename("abc-def-v999", "abc-def-v1").isFalse(); - } - - @Test - public void nonMatchingSpecifiersShouldNotMatchConfigurationFromFilename() { - // same version - assertMatchesConfigurationFromFilename("abc", "ghi").isFalse(); - assertMatchesConfigurationFromFilename("abc-v0", "ghi-v0").isFalse(); - assertMatchesConfigurationFromFilename("abc-v15", "ghi-v15").isFalse(); - // different version - assertMatchesConfigurationFromFilename("abc", "ghi-v15").isFalse(); - assertMatchesConfigurationFromFilename("abc-v0", "ghi-v15").isFalse(); - assertMatchesConfigurationFromFilename("abc-v15", "ghi-v19").isFalse(); - } - - @CheckReturnValue - private BooleanSubject assertMatchesConfigurationFromFilename( - String flagConfiguration, String fileConfiguration) { - return assertWithMessage( - "The split flag '%s' would be a match for a filename containing '%s'", - flagConfiguration, fileConfiguration) - .that( - ResourceConfiguration.fromString(flagConfiguration) - .matchesConfigurationFromFilename( - ResourceConfiguration.fromString(fileConfiguration))); - } - - @Test - public void resourceConfigurationShouldBeOrderedByApiVersionThenSpecifiers() { - assertThat( - ImmutableList.of( - // -v0 and no -v0 sort equal given the same specifiers, so it's a tie - ResourceConfiguration.fromString("a"), - // version ties are broken based on lexicographic string ordering - ResourceConfiguration.fromString("b-v0"), - // string ordering ignores case of the specifiers - ResourceConfiguration.fromString("Z"), - // higher API versions sort later regardless of the specifiers - ResourceConfiguration.fromString("z-v6"), - ResourceConfiguration.fromString("b-v13"), - // wildcards ("any") are ignored when considering specifier order - ResourceConfiguration.fromString("any-c-v13"))) - .isInStrictOrder(); - - // if they are actually equal, they will compare equal - assertThat(ResourceConfiguration.fromString("c-v13")) - .isEquivalentAccordingToCompareTo(ResourceConfiguration.fromString("c-v13")); - - // if the only difference is wildcards they will compare equal - assertThat(ResourceConfiguration.fromString("any-c-v13")) - .isEquivalentAccordingToCompareTo(ResourceConfiguration.fromString("c-v13")); - - // if the only difference is specifying (or not specifying) -v0 they will compare equal - assertThat(ResourceConfiguration.fromString("a-v0")) - .isEquivalentAccordingToCompareTo(ResourceConfiguration.fromString("a")); - - // if the only difference is case they will compare equal - assertThat(ResourceConfiguration.fromString("z")) - .isEquivalentAccordingToCompareTo(ResourceConfiguration.fromString("Z")); - } - - @Test - public void resourceConfigurationShouldPassEqualsTester() { - new EqualsTester() - .addEqualityGroup( - // base example - ResourceConfiguration.fromString("abc-def"), - // identical clone - ResourceConfiguration.fromString("abc-def"), - // case doesn't matter - ResourceConfiguration.fromString("aBC-dEF"), - // absent version code is equivalent to -v0 - ResourceConfiguration.fromString("abc-def-v0"), - // skip "any" - ResourceConfiguration.fromString("any-abc-any-def-any")) - .addEqualityGroup( - // order matters - ResourceConfiguration.fromString("def-abc")) - .addEqualityGroup( - // empty segments are not collapsed - ResourceConfiguration.fromString("abc---def")) - .addEqualityGroup( - // different version codes are parsed but result in a non-equal instance - ResourceConfiguration.fromString("abc-def-v15")) - .addEqualityGroup( - // two configurations with version codes must also have EQUAL version codes - ResourceConfiguration.fromString("abc-def-v12")) - .addEqualityGroup( - // empty strings are equal - ResourceConfiguration.fromString(""), - // yes, even if they have a v0 - ResourceConfiguration.fromString("v0"), - // or if there are some wildcards which will collapse to nothing - ResourceConfiguration.fromString("any"), - ResourceConfiguration.fromString("any-v0"), - ResourceConfiguration.fromString("any-any-any"), - ResourceConfiguration.fromString("any-any-any-v0")) - .addEqualityGroup( - // not if the version is nonzero though - ResourceConfiguration.fromString("v15"), ResourceConfiguration.fromString("any-v15")) - .testEquals(); - } -} diff --git a/src/tools/android/java/com/google/devtools/build/android/AndroidResourceProcessingAction.java b/src/tools/android/java/com/google/devtools/build/android/AndroidResourceProcessingAction.java index 11131acc9fd2c4..d94db60968d82f 100644 --- a/src/tools/android/java/com/google/devtools/build/android/AndroidResourceProcessingAction.java +++ b/src/tools/android/java/com/google/devtools/build/android/AndroidResourceProcessingAction.java @@ -38,7 +38,6 @@ import com.google.devtools.build.android.Converters.SerializedAndroidDataListConverter; import com.google.devtools.build.android.Converters.UnvalidatedAndroidDataConverter; import com.google.devtools.build.android.Converters.VariantTypeConverter; -import com.google.devtools.build.android.SplitConfigurationFilter.UnrecognizedSplitsException; import com.google.devtools.common.options.Converters; import com.google.devtools.common.options.Converters.CommaSeparatedOptionListConverter; import com.google.devtools.common.options.Option; @@ -476,7 +475,6 @@ public static void main(String[] args) throws Exception { options.packageForR, new FlagAaptOptions(aaptConfigOptions), aaptConfigOptions.resourceConfigs, - aaptConfigOptions.splits, processedData, resourceData, generatedSources, @@ -524,10 +522,7 @@ public static void main(String[] args) throws Exception { } catch (MergingException e) { logger.log(java.util.logging.Level.SEVERE, "Error during merging resources", e); throw e; - } catch (IOException - | InterruptedException - | LoggedErrorException - | UnrecognizedSplitsException e) { + } catch (IOException | InterruptedException | LoggedErrorException e) { logger.log(java.util.logging.Level.SEVERE, "Error during processing resources", e); throw e; } catch (AndroidManifestProcessor.ManifestProcessingException e) { diff --git a/src/tools/android/java/com/google/devtools/build/android/AndroidResourceProcessor.java b/src/tools/android/java/com/google/devtools/build/android/AndroidResourceProcessor.java index 86616a9e903511..102d2888ec0cee 100644 --- a/src/tools/android/java/com/google/devtools/build/android/AndroidResourceProcessor.java +++ b/src/tools/android/java/com/google/devtools/build/android/AndroidResourceProcessor.java @@ -36,7 +36,6 @@ import com.google.common.util.concurrent.MoreExecutors; import com.google.devtools.build.android.Converters.ExistingPathConverter; import com.google.devtools.build.android.Converters.RevisionConverter; -import com.google.devtools.build.android.SplitConfigurationFilter.UnrecognizedSplitsException; import com.google.devtools.build.android.junctions.JunctionCreator; import com.google.devtools.build.android.junctions.NoopJunctionCreator; import com.google.devtools.build.android.junctions.WindowsJunctionCreator; @@ -51,7 +50,6 @@ import java.io.IOException; import java.io.PrintStream; import java.nio.charset.StandardCharsets; -import java.nio.file.DirectoryStream; import java.nio.file.Files; import java.nio.file.Path; import java.util.ArrayList; @@ -180,32 +178,6 @@ public static final class AaptConfigOptions extends OptionsBase { help = "A list of resource config filters to pass to aapt." ) public List resourceConfigs; - - private static final String ANDROID_SPLIT_DOCUMENTATION_URL = - "https://developer.android.com/guide/topics/resources/providing-resources.html" - + "#QualifierRules"; - - @Option( - name = "split", - defaultValue = "required but ignored due to allowMultiple", - category = "config", - documentationCategory = OptionDocumentationCategory.UNCATEGORIZED, - effectTags = {OptionEffectTag.UNKNOWN}, - allowMultiple = true, - help = - "An individual split configuration to pass to aapt." - + " Each split is a list of configuration filters separated by commas." - + " Configuration filters are lists of configuration qualifiers separated by dashes," - + " as used in resource directory names and described on the Android developer site: " - + ANDROID_SPLIT_DOCUMENTATION_URL - + " For example, a split might be 'en-television,en-xxhdpi', containing English" - + " assets which either are for TV screens or are extra extra high resolution." - + " Multiple splits can be specified by passing this flag multiple times." - + " Each split flag will produce an additional output file, named by replacing the" - + " commas in the split specification with underscores, and appending the result to" - + " the output package name following an underscore." - ) - public List splits; } /** {@link AaptOptions} backed by an {@link AaptConfigOptions}. */ @@ -275,7 +247,6 @@ public MergedAndroidData processResources( String customPackageForR, AaptOptions aaptOptions, Collection resourceConfigs, - Collection splits, MergedAndroidData primaryData, List dependencyData, @Nullable Path sourceOut, @@ -284,7 +255,7 @@ public MergedAndroidData processResources( @Nullable Path mainDexProguardOut, @Nullable Path publicResourcesOut, @Nullable Path dataBindingInfoOut) - throws IOException, InterruptedException, LoggedErrorException, UnrecognizedSplitsException { + throws IOException, InterruptedException, LoggedErrorException { Path androidManifest = primaryData.getManifest(); final Path resourceDir = processDataBindings( @@ -308,7 +279,6 @@ public MergedAndroidData processResources( customPackageForR, aaptOptions, resourceConfigs, - splits, androidManifest, resourceDir, assetsDir, @@ -323,12 +293,6 @@ public MergedAndroidData processResources( writeDependencyPackageRJavaFiles( dependencyData, customPackageForR, androidManifest, sourceOut); } - // Reset the output date stamps. - if (packageOut != null) { - if (!splits.isEmpty()) { - renameSplitPackages(packageOut, splits); - } - } return new MergedAndroidData(resourceDir, assetsDir, androidManifest); } @@ -342,7 +306,6 @@ public void runAapt( String customPackageForR, AaptOptions aaptOptions, Collection resourceConfigs, - Collection splits, Path androidManifest, Path resourceDir, Path assetsDir, @@ -407,10 +370,7 @@ public void runAapt( // Add custom no-compress extensions. .addRepeated("-0", aaptOptions.getNoCompress()) // Filter by resource configuration type. - .add("-c", Joiner.on(',').join(resourceConfigs)) - // Split APKs if any splits were specified. - .whenVersionIsAtLeast(new Revision(23)) - .thenAddRepeated("--split", splits); + .add("-c", Joiner.on(',').join(resourceConfigs)); for (String additional : aaptOptions.getAdditionalParameters()) { commandBuilder.add(additional); } @@ -575,33 +535,6 @@ void writeDependencyPackageRJavaFiles( } } - /** Renames aapt's split outputs according to the input flags. */ - private void renameSplitPackages(Path packageOut, Iterable splits) - throws UnrecognizedSplitsException, IOException { - String prefix = packageOut.getFileName().toString() + "_"; - // The regex java string literal below is received as [\\{}\[\]*?] by the regex engine, - // which produces a character class containing \{}[]*? - // The replacement string literal is received as \\$0 by the regex engine, which places - // a backslash before the match. - String prefixGlob = prefix.replaceAll("[\\\\{}\\[\\]*?]", "\\\\$0") + "*"; - Path outputDirectory = packageOut.getParent(); - ImmutableList.Builder filenameSuffixes = new ImmutableList.Builder<>(); - try (DirectoryStream glob = Files.newDirectoryStream(outputDirectory, prefixGlob)) { - for (Path file : glob) { - filenameSuffixes.add(file.getFileName().toString().substring(prefix.length())); - } - } - Map outputs = - SplitConfigurationFilter.mapFilenamesToSplitFlags(filenameSuffixes.build(), splits); - for (Map.Entry splitMapping : outputs.entrySet()) { - Path resultPath = packageOut.resolveSibling(prefix + splitMapping.getValue()); - if (!splitMapping.getKey().equals(splitMapping.getValue())) { - Path sourcePath = packageOut.resolveSibling(prefix + splitMapping.getKey()); - Files.move(sourcePath, resultPath); - } - } - } - /** A logger that will print messages to a target OutputStream. */ static final class PrintStreamLogger implements ILogger { private final PrintStream out; diff --git a/src/tools/android/java/com/google/devtools/build/android/AndroidResourceValidatorAction.java b/src/tools/android/java/com/google/devtools/build/android/AndroidResourceValidatorAction.java index 61278655d45463..65d3c729225077 100644 --- a/src/tools/android/java/com/google/devtools/build/android/AndroidResourceValidatorAction.java +++ b/src/tools/android/java/com/google/devtools/build/android/AndroidResourceValidatorAction.java @@ -17,7 +17,6 @@ import com.android.utils.StdLogger; import com.google.common.base.Preconditions; import com.google.common.base.Stopwatch; -import com.google.common.collect.ImmutableList; import com.google.devtools.build.android.AndroidResourceProcessor.AaptConfigOptions; import com.google.devtools.build.android.AndroidResourceProcessor.FlagAaptOptions; import com.google.devtools.build.android.Converters.ExistingPathConverter; @@ -156,7 +155,6 @@ public static void main(String[] args) throws Exception { options.packageForR, new FlagAaptOptions(aaptConfigOptions), aaptConfigOptions.resourceConfigs, - ImmutableList.of(), AndroidManifest.parseFrom(options.manifest) .writeDummyManifestForAapt(dummyManifestDirectory, options.packageForR), resources, diff --git a/src/tools/android/java/com/google/devtools/build/android/ResourceShrinkerAction.java b/src/tools/android/java/com/google/devtools/build/android/ResourceShrinkerAction.java index 6b5794b5b4e915..b2189d77d8bcba 100644 --- a/src/tools/android/java/com/google/devtools/build/android/ResourceShrinkerAction.java +++ b/src/tools/android/java/com/google/devtools/build/android/ResourceShrinkerAction.java @@ -301,7 +301,6 @@ public static void main(String[] args) throws Exception { null /* packageForR */, new FlagAaptOptions(aaptConfigOptions), aaptConfigOptions.resourceConfigs, - aaptConfigOptions.splits, new MergedAndroidData( shrunkResources, resourceFiles.resolve("assets"), options.primaryManifest), ImmutableList.of() /* libraries */, diff --git a/src/tools/android/java/com/google/devtools/build/android/SplitConfigurationFilter.java b/src/tools/android/java/com/google/devtools/build/android/SplitConfigurationFilter.java deleted file mode 100644 index 4103b800a280cd..00000000000000 --- a/src/tools/android/java/com/google/devtools/build/android/SplitConfigurationFilter.java +++ /dev/null @@ -1,366 +0,0 @@ -// Copyright 2016 The Bazel Authors. All rights reserved. -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -package com.google.devtools.build.android; - -import com.google.common.base.Joiner; -import com.google.common.base.Optional; -import com.google.common.base.Predicate; -import com.google.common.base.Splitter; -import com.google.common.collect.ComparisonChain; -import com.google.common.collect.ImmutableList; -import com.google.common.collect.ImmutableMap; -import com.google.common.collect.ImmutableSortedSet; -import com.google.common.collect.Iterables; -import com.google.common.collect.Ordering; -import java.util.ArrayList; -import java.util.List; -import java.util.Locale; -import java.util.Map; -import java.util.Objects; -import java.util.TreeSet; -import java.util.regex.Matcher; -import java.util.regex.Pattern; - -/** - * A parsed set of configuration filters for a split flag or an output filename. - * - *

The natural ordering of this class sorts by number of configurations, then by highest required - * API version, if any, then by other specifiers (case-insensitive), with ties broken by the - * filename or split flag originally used to create the instance (case-sensitive). - * - *

This has the following useful property:
- * Given two sets of {@link SplitConfigurationFilter}s, one from the input split flags, and one from - * aapt's outputs... Each member of the output set can be matched to the greatest member of the - * input set for which {@code input.matchesFilterFromFilename(output)} is true. - */ -final class SplitConfigurationFilter implements Comparable { - - /** - * Finds a mapping from filename suffixes to the split flags which could have spawned them. - * - * @param filenames The suffixes of the original apk filenames output by aapt, not including the - * underscore used to set it off from the base filename or the base filename itself. - * @param splitFlags The split flags originally passed to aapt. - * @return A map whose keys are the filenames from {@code filenames} and whose values are - * predictable filenames based on the split flags - that is, the commas present in the input - * have been replaced with underscores. - * @throws UnrecognizedSplitException if any of the inputs are unused or could not be matched - */ - static Map mapFilenamesToSplitFlags( - Iterable filenames, Iterable splitFlags) throws UnrecognizedSplitsException { - TreeSet filenameFilters = new TreeSet<>(); - for (String filename : filenames) { - filenameFilters.add(SplitConfigurationFilter.fromFilenameSuffix(filename)); - } - TreeSet flagFilters = new TreeSet<>(); - for (String splitFlag : splitFlags) { - flagFilters.add(SplitConfigurationFilter.fromSplitFlag(splitFlag)); - } - ImmutableMap.Builder result = ImmutableMap.builder(); - List unidentifiedFilenames = new ArrayList<>(); - for (SplitConfigurationFilter filenameFilter : filenameFilters) { - Optional matched = - Iterables.tryFind(flagFilters, new MatchesFilterFromFilename(filenameFilter)); - if (matched.isPresent()) { - result.put(filenameFilter.filename, matched.get().filename); - flagFilters.remove(matched.get()); - } else { - unidentifiedFilenames.add(filenameFilter.filename); - } - } - if (!(unidentifiedFilenames.isEmpty() && flagFilters.isEmpty())) { - ImmutableList.Builder unidentifiedFlags = ImmutableList.builder(); - for (SplitConfigurationFilter flagFilter : flagFilters) { - unidentifiedFlags.add(flagFilter.filename); - } - throw new UnrecognizedSplitsException( - unidentifiedFlags.build(), unidentifiedFilenames, result.build()); - } - return result.build(); - } - - /** - * Exception thrown when mapFilenamesToSplitFlags fails to find matches for all elements of both - * input sets. - */ - static final class UnrecognizedSplitsException extends Exception { - private final ImmutableList unidentifiedSplits; - private final ImmutableList unidentifiedFilenames; - private final ImmutableMap identifiedSplits; - - UnrecognizedSplitsException( - Iterable unidentifiedSplits, - Iterable unidentifiedFilenames, - Map identifiedSplits) { - super( - "Could not find matching filenames for these split flags:\n" - + Joiner.on("\n").join(unidentifiedSplits) - + "\nnor matching split flags for these filenames:\n" - + Joiner.on(", ").join(unidentifiedFilenames) - + "\nFound these (filename => split flag) matches though:\n" - + Joiner.on("\n").withKeyValueSeparator(" => ").join(identifiedSplits)); - this.unidentifiedSplits = ImmutableList.copyOf(unidentifiedSplits); - this.unidentifiedFilenames = ImmutableList.copyOf(unidentifiedFilenames); - this.identifiedSplits = ImmutableMap.copyOf(identifiedSplits); - } - - /** Returns the list of split flags which did not find a match. */ - ImmutableList getUnidentifiedSplits() { - return unidentifiedSplits; - } - - /** Returns the list of filename suffixes which did not find a match. */ - ImmutableList getUnidentifiedFilenames() { - return unidentifiedFilenames; - } - - /** Returns the mapping from filename suffix to split flag for splits that did match. */ - ImmutableMap getIdentifiedSplits() { - return identifiedSplits; - } - } - - /** Generates a SplitConfigurationFilter from a split flag. */ - static SplitConfigurationFilter fromSplitFlag(String flag) { - return SplitConfigurationFilter.fromFilenameSuffix(flag.replace(',', '_')); - } - - /** Generates a SplitConfigurationFilter from the suffix of a split generated by aapt. */ - static SplitConfigurationFilter fromFilenameSuffix(String suffix) { - ImmutableSortedSet.Builder configs = ImmutableSortedSet.reverseOrder(); - for (String configuration : Splitter.on('_').split(suffix)) { - configs.add(ResourceConfiguration.fromString(configuration)); - } - return new SplitConfigurationFilter(suffix, configs.build()); - } - - /** - * The suffix to be appended to the output package for this split configuration. - * - *

When created with {@link fromFilenameSuffix}, this will be the original filename from aapt; - * when created with {@link fromSplitFlag}, this will be the filename to rename to. - */ - private final String filename; - - /** - * A set of resource configurations which will be included in this split, sorted so that the - * configs with the highest API versions come first. - * - *

It's okay for this to collapse duplicates, because aapt forbids duplicate resource - * configurations across all splits in the same invocation anyway. - */ - private final ImmutableSortedSet configs; - - private SplitConfigurationFilter( - String filename, ImmutableSortedSet configs) { - this.filename = filename; - this.configs = configs; - } - - /** - * Checks if the {@code other} split configuration filter could have been produced as a filename - * by aapt based on this configuration filter being passed as a split flag. - * - *

This means that there must be a one-to-one mapping from each configuration in this filter to - * a configuration in the {@code other} filter such that the non-API-version specifiers of the two - * configurations match and the API version specifier of the {@code other} filter's configuration - * is greater than or equal to the API version specifier of this filter's configuration. - * - *

Order of whole configurations doesn't matter, as aapt will reorder the configurations - * according to complicated internal logic (yes, logic even more complicated than this!). - * - *

Care is needed with API version specifiers because aapt may add or change minimum API - * version specifiers to configurations according to whether they had specifiers which are only - * supported in certain versions of Android. It will only ever increase the minimum version or - * leave it the same. - * - *

The other (non-wildcard) specifiers should be case-insensitive identical, including order; - * aapt will not allow parts of a single configuration to be parsed out of order. - * - * @see ResourceConfiguration#matchesConfigurationFromFilename(ResourceConfiguration) - */ - boolean matchesFilterFromFilename(SplitConfigurationFilter filenameFilter) { - if (filenameFilter.configs.size() != this.configs.size()) { - return false; - } - - List unmatchedConfigs = new ArrayList<>(this.configs); - for (ResourceConfiguration filenameConfig : filenameFilter.configs) { - Optional matched = - Iterables.tryFind( - unmatchedConfigs, - new ResourceConfiguration.MatchesConfigurationFromFilename(filenameConfig)); - if (!matched.isPresent()) { - return false; - } - unmatchedConfigs.remove(matched.get()); - } - return true; - } - - static final class MatchesFilterFromFilename implements Predicate { - private final SplitConfigurationFilter filenameFilter; - - MatchesFilterFromFilename(SplitConfigurationFilter filenameFilter) { - this.filenameFilter = filenameFilter; - } - - @Override - public boolean apply(SplitConfigurationFilter flagFilter) { - return flagFilter.matchesFilterFromFilename(filenameFilter); - } - } - - private static final Ordering> CONFIG_LEXICOGRAPHICAL = - Ordering.natural().lexicographical(); - - @Override - public int compareTo(SplitConfigurationFilter other) { - return ComparisonChain.start() - .compare(this.configs.size(), other.configs.size()) - .compare(this.configs, other.configs, CONFIG_LEXICOGRAPHICAL) - .compare(this.filename, other.filename) - .result(); - } - - @Override - public int hashCode() { - return Objects.hash(configs, filename); - } - - @Override - public boolean equals(Object object) { - if (object instanceof SplitConfigurationFilter) { - SplitConfigurationFilter other = (SplitConfigurationFilter) object; - // the configs are derived from the filename, so we can be assured they are equal if the - // filenames are. - return Objects.equals(this.filename, other.filename); - } - return false; - } - - @Override - public String toString() { - return "SplitConfigurationFilter{" + filename + "}"; - } - - /** - * An individual set of configuration specifiers, for the purposes of split name parsing. - * - *

The natural ordering of this class sorts by required API version, if any, then by other - * specifiers. - * - *

This has the following useful property:
- * Given two sets of {@link ResourceConfiguration}s, one from an input split flag, and one from - * aapt's output... Each member of the output set can be matched to the greatest member of the - * input set for which {@code input.matchesConfigurationFromFilename(output)} is true. - */ - static final class ResourceConfiguration implements Comparable { - /** - * Pattern to match wildcard parts ("any"), which can be safely ignored - aapt drops them. - * - *

Matches an 'any' part and the dash following it, or for an 'any' part which is the last - * specifier, the dash preceding it. In the former case, it must be a full part - that is, - * preceded by the beginning of the string or a dash, which will not be consumed. - */ - private static final Pattern WILDCARD_SPECIFIER = Pattern.compile("(?<=^|-)any(?:-|$)|-any$"); - /** - * Pattern to match the API version and capture the version number. - * - *

It must always be the last specifier in a config, although it may also be the first if - * there are no other specifiers. - */ - private static final Pattern API_VERSION = Pattern.compile("(?:-|^)v(\\d+)$"); - - /** Parses a resource configuration into a form that can be compared to other configurations. */ - static ResourceConfiguration fromString(String text) { - // Case is ignored for resource configurations (aapt lowercases internally), - // and wildcards can be dropped. - String cleanSpecifiers = - WILDCARD_SPECIFIER.matcher(text.toLowerCase(Locale.ENGLISH)).replaceAll(""); - Matcher apiVersionMatcher = API_VERSION.matcher(cleanSpecifiers); - if (apiVersionMatcher.find()) { - return new ResourceConfiguration( - cleanSpecifiers.substring(0, apiVersionMatcher.start()), - Integer.parseInt(apiVersionMatcher.group(1))); - } else { - return new ResourceConfiguration(cleanSpecifiers, 0); - } - } - - /** The specifiers for this resource configuration, besides API version, in lowercase. */ - private final String specifiers; - - /** The API version, or 0 to indicate that no API version was present in the original config. */ - private final int apiVersion; - - private ResourceConfiguration(String specifiers, int apiVersion) { - this.specifiers = specifiers; - this.apiVersion = apiVersion; - } - - /** - * Checks that the {@code other} configuration could be a filename generated from this one. - * - * @see SplitConfigurationFilter#matchesFilterFromFilename(SplitConfigurationFilter) - */ - boolean matchesConfigurationFromFilename(ResourceConfiguration other) { - return Objects.equals(other.specifiers, this.specifiers) - && other.apiVersion >= this.apiVersion; - } - - static final class MatchesConfigurationFromFilename - implements Predicate { - private final ResourceConfiguration filenameConfig; - - MatchesConfigurationFromFilename(ResourceConfiguration filenameConfig) { - this.filenameConfig = filenameConfig; - } - - @Override - public boolean apply(ResourceConfiguration flagConfig) { - return flagConfig.matchesConfigurationFromFilename(filenameConfig); - } - } - - @Override - public int compareTo(ResourceConfiguration other) { - return ComparisonChain.start() - .compare(this.apiVersion, other.apiVersion) - .compare(this.specifiers, other.specifiers) - .result(); - } - - @Override - public int hashCode() { - return Objects.hash(specifiers, apiVersion); - } - - @Override - public boolean equals(Object object) { - if (object instanceof ResourceConfiguration) { - ResourceConfiguration other = (ResourceConfiguration) object; - return Objects.equals(this.specifiers, other.specifiers) - && this.apiVersion == other.apiVersion; - } - return false; - } - - @Override - public String toString() { - return "ResourceConfiguration{" + specifiers + "-v" + Integer.toString(apiVersion) + "}"; - } - } -}