Add support for --force-jumbo in DxStep. Summary: We need this for the upcoming predex diff. Test Plan: Comprehensive unit test for DxStep: DxStepTest.
diff --git a/src/com/facebook/buck/android/DxStep.java b/src/com/facebook/buck/android/DxStep.java index de51a3d..cde9cc9 100644 --- a/src/com/facebook/buck/android/DxStep.java +++ b/src/com/facebook/buck/android/DxStep.java
@@ -24,26 +24,47 @@ import com.google.common.base.Preconditions; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableSet; +import com.google.common.collect.Sets; import java.nio.file.Path; +import java.util.EnumSet; import java.util.Set; public class DxStep extends ShellStep { + /** Options to pass to {@code dx}. */ + public static enum Option { + /** Specify the {@code --no-optimize} flag when running {@code dx}. */ + NO_OPTIMIZE, + + /** Specify the {@code --no-optimize} flag when running {@code dx}. */ + FORCE_JUMBO, + ; + } + private final String outputDexFile; private final Set<Path> filesToDex; - private final boolean optimize; + private final Set<Option> options; /** - * @param outputDexFile path to the file where the generated classes.dex should go + * @param outputDexFile path to the file where the generated classes.dex should go. * @param filesToDex each element in this set is a path to a .class file, a zip file of .class - * files, or a directory of .class files - * @param optimize If false, specify {@code --no-optimize}. + * files, or a directory of .class files. */ - public DxStep(String outputDexFile, Iterable<Path> filesToDex, boolean optimize) { + public DxStep(String outputDexFile, Iterable<Path> filesToDex) { + this(outputDexFile, filesToDex, EnumSet.noneOf(DxStep.Option.class)); + } + + /** + * @param outputDexFile path to the file where the generated classes.dex should go. + * @param filesToDex each element in this set is a path to a .class file, a zip file of .class + * files, or a directory of .class files. + * @param options to pass to {@code dx}. + */ + public DxStep(String outputDexFile, Iterable<Path> filesToDex, EnumSet<Option> options) { this.outputDexFile = Preconditions.checkNotNull(outputDexFile); this.filesToDex = ImmutableSet.copyOf(filesToDex); - this.optimize = optimize; + this.options = Sets.immutableEnumSet(options); } @Override @@ -59,10 +80,14 @@ builder.add("--statistics"); } - if (!optimize) { + if (options.contains(Option.NO_OPTIMIZE)) { builder.add("--no-optimize"); } + if (options.contains(Option.FORCE_JUMBO)) { + builder.add("--force-jumbo"); + } + // verbose flag, if appropriate. if (context.getVerbosity().shouldUseVerbosityFlagIfAvailable()) { builder.add("--verbose");
diff --git a/src/com/facebook/buck/android/SmartDexingStep.java b/src/com/facebook/buck/android/SmartDexingStep.java index 7f389d5..f329a2f 100644 --- a/src/com/facebook/buck/android/SmartDexingStep.java +++ b/src/com/facebook/buck/android/SmartDexingStep.java
@@ -15,6 +15,7 @@ */ package com.facebook.buck.android; +import com.facebook.buck.android.DxStep.Option; import com.facebook.buck.java.classes.ClasspathTraversal; import com.facebook.buck.java.classes.ClasspathTraverser; import com.facebook.buck.java.classes.DefaultClasspathTraverser; @@ -52,6 +53,7 @@ import java.io.File; import java.io.IOException; import java.nio.file.Path; +import java.util.EnumSet; import java.util.List; import java.util.Set; import java.util.concurrent.Executors; @@ -440,9 +442,13 @@ Preconditions.checkState(newInputsHash != null, "Must call checkIsCached first!"); List<Step> steps = Lists.newArrayList(); + + EnumSet<Option> dxOptions = optimizeDex + ? EnumSet.noneOf(DxStep.Option.class) + : EnumSet.of(DxStep.Option.NO_OPTIMIZE); if (useXzCompression()) { String tempDexJarOutput = outputPath.replaceAll("\\.jar\\.xz$", ".tmp.jar"); - steps.add(new DxStep(tempDexJarOutput, srcs, optimizeDex)); + steps.add(new DxStep(tempDexJarOutput, srcs, dxOptions)); // We need to make sure classes.dex is STOREd in the .dex.jar file, otherwise .XZ // compression won't be effective. String repackedJar = outputPath.replaceAll("\\.xz$", ""); @@ -455,7 +461,7 @@ steps.add(new RmStep(tempDexJarOutput, true)); steps.add(new XzStep(repackedJar)); } else { - steps.add(new DxStep(outputPath, srcs, optimizeDex)); + steps.add(new DxStep(outputPath, srcs, dxOptions)); } steps.add(new WriteFileStep(newInputsHash, outputHashPath));
diff --git a/src/com/facebook/buck/cli/BUCK b/src/com/facebook/buck/cli/BUCK index acbbd5b..74081e7 100644 --- a/src/com/facebook/buck/cli/BUCK +++ b/src/com/facebook/buck/cli/BUCK
@@ -78,6 +78,7 @@ ], visibility = [ '//test/com/facebook/buck/cli:', + '//test/com/facebook/buck/android:android', '//test/com/facebook/buck/testutil/integration:integration', '//src/com/facebook/buck/event:dependencies-for-external-projects-inner', ],
diff --git a/src/com/facebook/buck/cli/VerbosityParser.java b/src/com/facebook/buck/cli/VerbosityParser.java index ddafd3f..442ca80 100644 --- a/src/com/facebook/buck/cli/VerbosityParser.java +++ b/src/com/facebook/buck/cli/VerbosityParser.java
@@ -19,7 +19,7 @@ import com.facebook.buck.util.Verbosity; import com.google.common.annotations.VisibleForTesting; -class VerbosityParser { +public class VerbosityParser { @VisibleForTesting static final String VERBOSE_LONG_ARG = "--verbose"; @@ -42,7 +42,7 @@ return DEFAULT_VERBOSITY; } - private static Verbosity getVerbosityForLevel(int verbosityLevel) { + public static Verbosity getVerbosityForLevel(int verbosityLevel) { if (verbosityLevel >= 8) { return Verbosity.ALL; } else if (verbosityLevel >= 5) {
diff --git a/test/com/facebook/buck/android/BUCK b/test/com/facebook/buck/android/BUCK index 83bdcd4..4d5fd00 100644 --- a/test/com/facebook/buck/android/BUCK +++ b/test/com/facebook/buck/android/BUCK
@@ -12,6 +12,7 @@ '//src/com/facebook/buck/android:rules', '//src/com/facebook/buck/android:split_dex', '//src/com/facebook/buck/android:steps', + '//src/com/facebook/buck/cli:cli', '//src/com/facebook/buck/dalvik:dalvik', '//src/com/facebook/buck/event:event', '//src/com/facebook/buck/graph:graph',
diff --git a/test/com/facebook/buck/android/DxStepTest.java b/test/com/facebook/buck/android/DxStepTest.java new file mode 100644 index 0000000..939a04c --- /dev/null +++ b/test/com/facebook/buck/android/DxStepTest.java
@@ -0,0 +1,183 @@ +/* + * Copyright 2013-present Facebook, Inc. + * + * 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.facebook.buck.android; + +import static org.junit.Assert.assertTrue; + +import com.facebook.buck.android.DxStep.Option; +import com.facebook.buck.cli.VerbosityParser; +import com.facebook.buck.step.ExecutionContext; +import com.facebook.buck.step.Step; +import com.facebook.buck.step.TestExecutionContext; +import com.facebook.buck.testutil.MoreAsserts; +import com.facebook.buck.testutil.TestConsole; +import com.facebook.buck.util.AndroidPlatformTarget; +import com.facebook.buck.util.Verbosity; +import com.google.common.base.Function; +import com.google.common.base.Joiner; +import com.google.common.base.Optional; +import com.google.common.collect.ImmutableList; +import com.google.common.collect.ImmutableSet; +import com.google.common.collect.Iterables; + +import org.easymock.EasyMock; +import org.easymock.EasyMockSupport; +import org.junit.Before; +import org.junit.Test; + +import java.io.File; +import java.nio.file.Path; +import java.nio.file.Paths; +import java.util.EnumSet; + +public class DxStepTest extends EasyMockSupport { + + private static Path SAMPLE_OUTPUT_PATH = Paths.get("buck-out/gen/classes.dex"); + + private static ImmutableSet<Path> SAMPLE_FILES_TO_DEX = ImmutableSet.of( + Paths.get("buck-out/gen/foo.dex.jar"), + Paths.get("buck-out/gen/bar.dex.jar")); + + private Optional<AndroidPlatformTarget> androidPlatformTargetOptional; + + @Before + public void setUp() { + AndroidPlatformTarget androidPlatformTarget = createMock(AndroidPlatformTarget.class); + EasyMock.expect(androidPlatformTarget.getDxExecutable()).andReturn(new File("/usr/bin/dx")); + androidPlatformTargetOptional = Optional.of(androidPlatformTarget); + replayAll(); + } + + @Test + public void testDxCommandNoOptimizeNoJumbo() { + // Context with --verbose 2. + ExecutionContext context = createExecutionContext(2); + Function<Path, Path> pathAbsolutifier = context.getProjectFilesystem().getAbsolutifier(); + + DxStep dx = new DxStep(SAMPLE_OUTPUT_PATH.toString(), + SAMPLE_FILES_TO_DEX, + EnumSet.of(Option.NO_OPTIMIZE)); + + String expected = String.format("/usr/bin/dx --dex --no-optimize --output %s %s", + SAMPLE_OUTPUT_PATH, + Joiner.on(' ').join(Iterables.transform(SAMPLE_FILES_TO_DEX, pathAbsolutifier))); + MoreAsserts.assertShellCommands( + "--no-optimize should be present, but --force-jumbo should not.", + ImmutableList.of(expected), + ImmutableList.<Step>of(dx), + context); + verifyAll(); + } + + @Test + public void testDxCommandOptimizeNoJumbo() { + // Context with --verbose 2. + ExecutionContext context = createExecutionContext(2); + Function<Path, Path> pathAbsolutifier = context.getProjectFilesystem().getAbsolutifier(); + + DxStep dx = new DxStep(SAMPLE_OUTPUT_PATH.toString(), SAMPLE_FILES_TO_DEX); + + String expected = String.format("/usr/bin/dx --dex --output %s %s", + SAMPLE_OUTPUT_PATH, + Joiner.on(' ').join(Iterables.transform(SAMPLE_FILES_TO_DEX, pathAbsolutifier))); + MoreAsserts.assertShellCommands( + "Neither --no-optimize nor --force-jumbo should be present.", + ImmutableList.of(expected), + ImmutableList.<Step>of(dx), + context); + verifyAll(); + } + + @Test + public void testDxCommandNoOptimizeForceJumbo() { + // Context with --verbose 2. + ExecutionContext context = createExecutionContext(2); + Function<Path, Path> pathAbsolutifier = context.getProjectFilesystem().getAbsolutifier(); + + DxStep dx = new DxStep(SAMPLE_OUTPUT_PATH.toString(), + SAMPLE_FILES_TO_DEX, + EnumSet.of(DxStep.Option.NO_OPTIMIZE, DxStep.Option.FORCE_JUMBO)); + + String expected = String.format("/usr/bin/dx --dex --no-optimize --force-jumbo --output %s %s", + SAMPLE_OUTPUT_PATH, + Joiner.on(' ').join(Iterables.transform(SAMPLE_FILES_TO_DEX, pathAbsolutifier))); + MoreAsserts.assertShellCommands( + "Both --no-optimize and --force-jumbo should be present.", + ImmutableList.of(expected), + ImmutableList.<Step>of(dx), + context); + verifyAll(); + } + + @Test + public void testVerbose3AddsStatisticsFlag() { + // Context with --verbose 3. + ExecutionContext context = createExecutionContext(3); + Function<Path, Path> pathAbsolutifier = context.getProjectFilesystem().getAbsolutifier(); + + DxStep dx = new DxStep(SAMPLE_OUTPUT_PATH.toString(), SAMPLE_FILES_TO_DEX); + + String expected = String.format("/usr/bin/dx --dex --statistics --output %s %s", + SAMPLE_OUTPUT_PATH, + Joiner.on(' ').join(Iterables.transform(SAMPLE_FILES_TO_DEX, pathAbsolutifier))); + MoreAsserts.assertShellCommands( + "Ensure that the --statistics flag is present.", + ImmutableList.of(expected), + ImmutableList.<Step>of(dx), + context); + + assertTrue("Should print stdout to show statistics.", + dx.shouldPrintStdout(context.getVerbosity())); + assertTrue("Should print stderr to show statistics.", + dx.shouldPrintStderr(context.getVerbosity())); + verifyAll(); + } + + @Test + public void testVerbose10AddsVerboseFlagToDx() { + // Context with --verbose 10. + ExecutionContext context = createExecutionContext(10); + Function<Path, Path> pathAbsolutifier = context.getProjectFilesystem().getAbsolutifier(); + + DxStep dx = new DxStep(SAMPLE_OUTPUT_PATH.toString(), SAMPLE_FILES_TO_DEX); + + String expected = String.format("/usr/bin/dx --dex --statistics --verbose --output %s %s", + SAMPLE_OUTPUT_PATH, + Joiner.on(' ').join(Iterables.transform(SAMPLE_FILES_TO_DEX, pathAbsolutifier))); + MoreAsserts.assertShellCommands( + "Ensure that the --statistics flag is present.", + ImmutableList.of(expected), + ImmutableList.<Step>of(dx), + context); + + assertTrue("Should print stdout since `dx --verbose` is enabled.", + dx.shouldPrintStdout(context.getVerbosity())); + assertTrue("Should print stdout since `dx --verbose` is enabled.", + dx.shouldPrintStderr(context.getVerbosity())); + verifyAll(); +} + + private ExecutionContext createExecutionContext(int verbosityLevel) { + TestConsole console = new TestConsole(); + Verbosity verbosity = VerbosityParser.getVerbosityForLevel(verbosityLevel); + console.setVerbosity(verbosity); + return TestExecutionContext.newBuilder() + .setConsole(console) + .setAndroidPlatformTarget(androidPlatformTargetOptional) + .build(); + } +}