diff options
6 files changed, 199 insertions, 51 deletions
diff --git a/src/test/java/com/google/devtools/build/android/dexer/BUILD b/src/test/java/com/google/devtools/build/android/dexer/BUILD index c60e7474d7..08941fedb3 100644 --- a/src/test/java/com/google/devtools/build/android/dexer/BUILD +++ b/src/test/java/com/google/devtools/build/android/dexer/BUILD @@ -22,8 +22,6 @@ java_library( exclude = [ "NoAndroidSdkStubTest.java", "AllTests.java", - # TODO(kmb,ajmichael): need 2nd Java 7 Jar for this test - "DexFileSplitterTest.java", ], ), "//conditions:default": ["NoAndroidSdkStubTest.java"], @@ -42,16 +40,23 @@ java_library( }), ) +java_library( + name = "testdata", + srcs = glob(["testdata/**/*.java"]), +) + java_test( name = "AllTests", size = "small", data = [ "test_main_dex_list.txt", + ":testdata", ":tests", ], jvm_flags = [ "-Dtestmaindexlist=$(location :test_main_dex_list.txt)", "-Dtestinputjar=$(location :tests)", + "-Dtestinputjar2=$(location :testdata)", ], runtime_deps = [ ":tests", diff --git a/src/test/java/com/google/devtools/build/android/dexer/DexFileMergerTest.java b/src/test/java/com/google/devtools/build/android/dexer/DexFileMergerTest.java index c772e427e8..b459b70a36 100644 --- a/src/test/java/com/google/devtools/build/android/dexer/DexFileMergerTest.java +++ b/src/test/java/com/google/devtools/build/android/dexer/DexFileMergerTest.java @@ -19,11 +19,13 @@ import static org.junit.Assert.fail; import com.android.dex.ClassDef; import com.android.dex.Dex; +import com.android.dex.DexException; import com.android.dx.command.dexer.DxContext; import com.android.dx.dex.code.PositionList; import com.google.common.base.Function; import com.google.common.base.Predicates; import com.google.common.collect.HashMultimap; +import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableSet; import com.google.common.collect.Iterators; import com.google.common.collect.Multimap; @@ -50,11 +52,12 @@ public class DexFileMergerTest { private static final Path WORKING_DIR = Paths.get(System.getProperty("user.dir")); private static final Path INPUT_JAR = WORKING_DIR.resolve(System.getProperty("testinputjar")); + private static final Path INPUT_JAR2 = WORKING_DIR.resolve(System.getProperty("testinputjar2")); private static final Path MAIN_DEX_LIST_FILE = WORKING_DIR.resolve(System.getProperty("testmaindexlist")); static final String DEX_PREFIX = "classes"; - /** Exercises DexFileMerger in monodex mode. */ + /** Exercises DexFileMerger to write a single .dex file. */ @Test public void testMergeDexArchive_singleOutputDex() throws Exception { Path dexArchive = buildDexArchive(); @@ -64,6 +67,43 @@ public class DexFileMergerTest { assertSingleDexOutput(expectedClassCount, outputArchive, "classes.dex"); } + @Test + public void testMergeDexArchive_duplicateInputFails() throws Exception { + Path dexArchive = buildDexArchive(); + try { + runDexFileMerger( + 256 * 256, + /*forceJumbo=*/ false, + "duplicate.dex.zip", + MultidexStrategy.MINIMAL, + /*mainDexList=*/ null, + /*minimalMainDex=*/ false, + DEX_PREFIX, + dexArchive, + dexArchive); // input Jar twice to induce failure + fail("DexException expected"); + } catch (DexException expected) {} + } + + /** Similar to {@link #testMergeDexArchive_singleOutputDex} but uses --multidex=given_shard. */ + @Test + public void testMergeDexArchive_givenShard() throws Exception { + Path dexArchive = buildDexArchive(INPUT_JAR, "3.classes.jar"); + Path outputArchive = + runDexFileMerger( + 256 * 256, + /*forceJumbo=*/ false, + "given_shard.dex.zip", + MultidexStrategy.GIVEN_SHARD, + /*mainDexList=*/ null, + /*minimalMainDex=*/ false, + DEX_PREFIX, + dexArchive); + + int expectedClassCount = matchingFileCount(dexArchive, ".*\\.class.dex$"); + assertSingleDexOutput(expectedClassCount, outputArchive, "classes3.dex"); + } + /** * Similar to {@link #testMergeDexArchive_singleOutputDex} but different name for output dex file. */ @@ -72,19 +112,41 @@ public class DexFileMergerTest { Path dexArchive = buildDexArchive(); Path outputArchive = runDexFileMerger( - dexArchive, 256 * 256, /*forceJumbo=*/ false, - "from_dex_archive.dex.zip", + "prefix.dex.zip", MultidexStrategy.MINIMAL, /*mainDexList=*/ null, /*minimalMainDex=*/ false, - "noname"); + "noname", + dexArchive); int expectedClassCount = matchingFileCount(dexArchive, ".*\\.class.dex$"); assertSingleDexOutput(expectedClassCount, outputArchive, "noname.dex"); } + /** Exercises DexFileMerger with two input archives. */ + @Test + public void testMergeDexArchive_multipleInputs() throws Exception { + Path dexArchive = buildDexArchive(); + Path dexArchive2 = buildDexArchive(INPUT_JAR2, "libtestdata.jar.dex.zip"); + Path outputArchive = + runDexFileMerger( + 256 * 256, + /*forceJumbo=*/ false, + "multiple_inputs.dex.zip", + MultidexStrategy.MINIMAL, + /*mainDexList=*/ null, + /*minimalMainDex=*/ false, + DEX_PREFIX, + dexArchive, + dexArchive2); + + int expectedClassCount = matchingFileCount(dexArchive, ".*\\.class.dex$"); + expectedClassCount += matchingFileCount(dexArchive2, ".*\\.class.dex$"); + assertSingleDexOutput(expectedClassCount, outputArchive, "classes.dex"); + } + /** * Similar to {@link #testMergeDexArchive_singleOutputDex} but forces multiple output dex files. */ @@ -102,14 +164,14 @@ public class DexFileMergerTest { Path dexArchive = buildDexArchive(); Path outputArchive = runDexFileMerger( - dexArchive, 200, /*forceJumbo=*/ false, "main_dex_list.dex.zip", MultidexStrategy.MINIMAL, MAIN_DEX_LIST_FILE, /*minimalMainDex=*/ false, - DEX_PREFIX); + DEX_PREFIX, + dexArchive); int expectedClassCount = matchingFileCount(dexArchive, ".*\\.class.dex$"); assertMainDexOutput(expectedClassCount, outputArchive, false); @@ -120,14 +182,14 @@ public class DexFileMergerTest { Path dexArchive = buildDexArchive(); Path outputArchive = runDexFileMerger( - dexArchive, 256 * 256, /*forceJumbo=*/ false, "minimal_main_dex.dex.zip", MultidexStrategy.MINIMAL, MAIN_DEX_LIST_FILE, /*minimalMainDex=*/ true, - DEX_PREFIX); + DEX_PREFIX, + dexArchive); int expectedClassCount = matchingFileCount(dexArchive, ".*\\.class.dex$"); assertMainDexOutput(expectedClassCount, outputArchive, true); @@ -138,14 +200,14 @@ public class DexFileMergerTest { Path dexArchive = buildDexArchive(); try { runDexFileMerger( - dexArchive, 200, /*forceJumbo=*/ false, "classes.dex.zip", MultidexStrategy.OFF, /*mainDexList=*/ null, /*minimalMainDex=*/ true, - DEX_PREFIX); + DEX_PREFIX, + dexArchive); fail("Expected DexFileMerger to fail"); } catch (IllegalArgumentException e) { assertThat(e) @@ -154,14 +216,14 @@ public class DexFileMergerTest { } try { runDexFileMerger( - dexArchive, 200, /*forceJumbo=*/ false, "classes.dex.zip", MultidexStrategy.OFF, MAIN_DEX_LIST_FILE, /*minimalMainDex=*/ false, - DEX_PREFIX); + DEX_PREFIX, + dexArchive); fail("Expected DexFileMerger to fail"); } catch (IllegalArgumentException e) { assertThat(e) @@ -175,9 +237,9 @@ public class DexFileMergerTest { Path dexArchive = buildDexArchive(); Path outputArchive; try { - outputArchive = runDexFileMerger(dexArchive, 256 * 256, /*forceJumbo=*/ true, - "from_dex_archive.dex.zip", MultidexStrategy.OFF, /*mainDexList=*/ null, - /*minimalMainDex=*/ false, DEX_PREFIX); + outputArchive = runDexFileMerger(256 * 256, /*forceJumbo=*/ true, "from_dex_archive.dex.zip", + MultidexStrategy.OFF, /*mainDexList=*/ null, /*minimalMainDex=*/ false, DEX_PREFIX, + dexArchive); } catch (IllegalStateException e) { assertThat(e).hasMessage("--forceJumbo flag not supported"); System.err.println("Skipping this test due to missing --forceJumbo support in Android SDK."); @@ -275,28 +337,28 @@ public class DexFileMergerTest { private Path runDexFileMerger(Path dexArchive, int maxNumberOfIdxPerDex, String outputBasename) throws IOException { return runDexFileMerger( - dexArchive, maxNumberOfIdxPerDex, /*forceJumbo=*/ false, outputBasename, MultidexStrategy.MINIMAL, /*mainDexList=*/ null, /*minimalMainDex=*/ false, - DEX_PREFIX); + DEX_PREFIX, + dexArchive); } private Path runDexFileMerger( - Path dexArchive, int maxNumberOfIdxPerDex, boolean forceJumbo, String outputBasename, MultidexStrategy multidexMode, @Nullable Path mainDexList, boolean minimalMainDex, - String dexPrefix) + String dexPrefix, + Path... dexArchives) throws IOException { DexFileMerger.Options options = new DexFileMerger.Options(); - options.inputArchive = dexArchive; + options.inputArchives = ImmutableList.copyOf(dexArchives); options.outputArchive = FileSystems.getDefault().getPath(System.getenv("TEST_TMPDIR"), outputBasename); options.multidexMode = multidexMode; @@ -311,11 +373,13 @@ public class DexFileMergerTest { } private Path buildDexArchive() throws Exception { + return buildDexArchive(INPUT_JAR, "libtests.dex.zip"); + } + + private Path buildDexArchive(Path inputJar, String outputZip) throws Exception { DexBuilder.Options options = new DexBuilder.Options(); - // Use Jar file that has this test in it as the input Jar - options.inputJar = INPUT_JAR; - options.outputZip = - FileSystems.getDefault().getPath(System.getenv("TEST_TMPDIR"), "libtests.dex.zip"); + options.inputJar = inputJar; + options.outputZip = FileSystems.getDefault().getPath(System.getenv("TEST_TMPDIR"), outputZip); options.maxThreads = 1; Dexing.DexingOptions dexingOptions = new Dexing.DexingOptions(); dexingOptions.optimize = true; diff --git a/src/test/java/com/google/devtools/build/android/dexer/testdata/AnotherClass.java b/src/test/java/com/google/devtools/build/android/dexer/testdata/AnotherClass.java new file mode 100644 index 0000000000..5aba303964 --- /dev/null +++ b/src/test/java/com/google/devtools/build/android/dexer/testdata/AnotherClass.java @@ -0,0 +1,20 @@ +// 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.dexer.testdata; + +public class AnotherClass { + public String bar() { + return ""; + } +} diff --git a/src/test/java/com/google/devtools/build/android/dexer/testdata/SomeClass.java b/src/test/java/com/google/devtools/build/android/dexer/testdata/SomeClass.java new file mode 100644 index 0000000000..f298ea98a6 --- /dev/null +++ b/src/test/java/com/google/devtools/build/android/dexer/testdata/SomeClass.java @@ -0,0 +1,20 @@ +// 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.dexer.testdata; + +public class SomeClass { + public int foo() { + return 0; + } +} diff --git a/src/tools/android/java/com/google/devtools/build/android/dexer/DexFileMerger.java b/src/tools/android/java/com/google/devtools/build/android/dexer/DexFileMerger.java index e892f7a94a..fded7b9562 100644 --- a/src/tools/android/java/com/google/devtools/build/android/dexer/DexFileMerger.java +++ b/src/tools/android/java/com/google/devtools/build/android/dexer/DexFileMerger.java @@ -47,7 +47,10 @@ import java.nio.file.Files; import java.nio.file.Path; import java.util.ArrayList; import java.util.Collections; +import java.util.List; import java.util.concurrent.Executors; +import java.util.regex.Matcher; +import java.util.regex.Pattern; import java.util.zip.ZipEntry; import java.util.zip.ZipFile; import java.util.zip.ZipOutputStream; @@ -71,15 +74,18 @@ class DexFileMerger { public static class Options extends OptionsBase { @Option( name = "input", - defaultValue = "null", + allowMultiple = true, + defaultValue = "", category = "input", documentationCategory = OptionDocumentationCategory.UNCATEGORIZED, effectTags = {OptionEffectTag.UNKNOWN}, converter = ExistingPathConverter.class, abbrev = 'i', - help = "Input file to read to aggregate." + help = "Input archives with .dex files to merge. Inputs are processed in given order, so " + + "classes from later inputs will be added after earlier inputs. Classes mustn't appear " + + "more than once." ) - public Path inputArchive; + public List<Path> inputArchives; @Option( name = "output", @@ -164,7 +170,6 @@ class DexFileMerger { defaultValue = "false", // dx's default documentationCategory = OptionDocumentationCategory.UNDOCUMENTED, effectTags = {OptionEffectTag.UNKNOWN}, - allowMultiple = false, help = "Typically not needed flag intended to imitate dx's --forceJumbo." ) public boolean forceJumbo; @@ -175,7 +180,6 @@ class DexFileMerger { category = "misc", documentationCategory = OptionDocumentationCategory.UNCATEGORIZED, effectTags = {OptionEffectTag.UNKNOWN}, - allowMultiple = false, help = "Dex file output prefix." ) public String dexPrefix; @@ -197,6 +201,10 @@ class DexFileMerger { @VisibleForTesting static void buildMergedDexFiles(Options options) throws IOException { ListeningExecutorService executor; + checkArgument(!options.inputArchives.isEmpty(), "Need at least one --input"); + checkArgument( + options.mainDexListFile == null || options.inputArchives.size() == 1, + "--main-dex-list only supported with exactly one --input, use DexFileSplitter for more"); if (options.multidexMode.isMultidexAllowed()) { executor = createThreadPool(); } else { @@ -216,31 +224,37 @@ class DexFileMerger { ? ImmutableSet.copyOf(Files.readAllLines(options.mainDexListFile, UTF_8)) : null; PrintStream originalStdOut = System.out; - try (ZipFile zip = new ZipFile(options.inputArchive.toFile()); - DexFileAggregator out = createDexFileAggregator(options, executor)) { - ArrayList<ZipEntry> dexFiles = filesToProcess(zip); - + try (DexFileAggregator out = createDexFileAggregator(options, executor)) { if (!options.verbose) { // com.android.dx.merge.DexMerger prints status information to System.out that we silence // here unless it was explicitly requested. (It also prints debug info to DxContext.out, // which we populate accordingly below.) System.setOut(Dexing.nullout); } - if (classesInMainDex == null) { - processDexFiles(zip, dexFiles, out); - } else { - // To honor --main_dex_list make two passes: - // 1. process only the classes listed in the given file - // 2. process the remaining files - Predicate<ZipEntry> mainDexFilter = ZipEntryPredicates.classFileFilter(classesInMainDex); - processDexFiles(zip, Iterables.filter(dexFiles, mainDexFilter), out); - // Fail if main_dex_list is too big, following dx's example - checkState(out.getDexFilesWritten() == 0, "Too many classes listed in main dex list file " - + "%s, main dex capacity exceeded", options.mainDexListFile); - if (options.minimalMainDex) { - out.flush(); // Start new .dex file if requested + + for (Path inputArchive : options.inputArchives) { + // Simply merge files from inputs in order. Doing that with a main dex list doesn't work, + // but we rule out more than one input with a main dex list above. + try (ZipFile zip = new ZipFile(inputArchive.toFile())) { + ArrayList<ZipEntry> dexFiles = filesToProcess(zip); + if (classesInMainDex == null) { + processDexFiles(zip, dexFiles, out); + } else { + // To honor --main_dex_list make two passes: + // 1. process only the classes listed in the given file + // 2. process the remaining files + Predicate<ZipEntry> mainDexFilter = + ZipEntryPredicates.classFileFilter(classesInMainDex); + processDexFiles(zip, Iterables.filter(dexFiles, mainDexFilter), out); + // Fail if main_dex_list is too big, following dx's example + checkState(out.getDexFilesWritten() == 0, "Too many classes listed in main dex list " + + "file %s, main dex capacity exceeded", options.mainDexListFile); + if (options.minimalMainDex) { + out.flush(); // Start new .dex file if requested + } + processDexFiles(zip, Iterables.filter(dexFiles, Predicates.not(mainDexFilter)), out); + } } - processDexFiles(zip, Iterables.filter(dexFiles, Predicates.not(mainDexFilter)), out); } } finally { // Kill threads in the pool so we don't hang @@ -280,6 +294,21 @@ class DexFileMerger { private static DexFileAggregator createDexFileAggregator( Options options, ListeningExecutorService executor) throws IOException { + String filePrefix = options.dexPrefix; + if (options.multidexMode == MultidexStrategy.GIVEN_SHARD) { + checkArgument(options.inputArchives.size() == 1, + "--multidex=given_shard requires exactly one --input"); + Pattern namingPattern = Pattern.compile("([0-9]+)\\..*"); + Matcher matcher = namingPattern.matcher(options.inputArchives.get(0).toFile().getName()); + checkArgument(matcher.matches(), + "expect input named <N>.xxx.zip for --multidex=given_shard but got %s", + options.inputArchives.get(0).toFile().getName()); + int shard = Integer.parseInt(matcher.group(1)); + checkArgument(shard > 0, "expect positive N in input named <N>.xxx.zip but got %s", shard); + if (shard > 1) { // first shard conventionally isn't numbered + filePrefix += shard; + } + } return new DexFileAggregator( new DxContext(options.verbose ? System.out : ByteStreams.nullOutputStream(), System.err), new DexFileArchive( @@ -290,7 +319,7 @@ class DexFileMerger { options.forceJumbo, options.maxNumberOfIdxPerDex, options.wasteThresholdPerDex, - options.dexPrefix); + filePrefix); } /** diff --git a/src/tools/android/java/com/google/devtools/build/android/dexer/MultidexStrategy.java b/src/tools/android/java/com/google/devtools/build/android/dexer/MultidexStrategy.java index f1903c7e3b..70a49e4dcd 100644 --- a/src/tools/android/java/com/google/devtools/build/android/dexer/MultidexStrategy.java +++ b/src/tools/android/java/com/google/devtools/build/android/dexer/MultidexStrategy.java @@ -19,6 +19,8 @@ package com.google.devtools.build.android.dexer; public enum MultidexStrategy { /** Create exactly one .dex file. The operation will fail if .dex limits are exceeded. */ OFF, + /** Create exactly one <prefixN>.dex file with N taken from the (single) input archive. */ + GIVEN_SHARD, /** * Assemble .dex files similar to {@link com.android.dx.command.dexer.Main dx}, with all but one * file as large as possible. @@ -31,6 +33,14 @@ public enum MultidexStrategy { BEST_EFFORT; public boolean isMultidexAllowed() { - return this != OFF; + switch (this) { + case OFF: + case GIVEN_SHARD: + return false; + case MINIMAL: + case BEST_EFFORT: + return true; + } + throw new AssertionError("Unknown: " + this); } } |