Support --positions flag in DexBuilder, for consistency with dx

RELNOTES: none

PiperOrigin-RevId: 170379445

5 files changed, 77 insertions, 15 deletions

diff --git a/src/test/java/com/google/devtools/build/android/dexer/DexBuilderTest.java b/src/test/java/com/google/devtools/build/android/dexer/DexBuilderTest.java
index 01236b3e4c..f5e01e46b7 100644
--- a/src/test/java/com/google/devtools/build/android/dexer/DexBuilderTest.java
+++ b/src/test/java/com/google/devtools/build/android/dexer/DexBuilderTest.java
@@ -17,6 +17,7 @@ import static com.google.common.truth.Truth.assertThat;
 import static java.nio.charset.StandardCharsets.UTF_8;
 
 import com.android.dex.Dex;
+import com.android.dx.dex.code.PositionList;
 import com.google.common.io.ByteStreams;
 import java.nio.file.FileSystems;
 import java.nio.file.Path;
@@ -43,7 +44,10 @@ public class DexBuilderTest {
     options.outputZip =
         FileSystems.getDefault().getPath(System.getenv("TEST_TMPDIR"), "dex_builder_test.zip");
     options.maxThreads = 1;
-    DexBuilder.buildDexArchive(options, new Dexing(new Dexing.DexingOptions()));
+    Dexing.DexingOptions dexingOptions = new Dexing.DexingOptions();
+    dexingOptions.optimize = true;
+    dexingOptions.positionInfo = PositionList.LINES;
+    DexBuilder.buildDexArchive(options, new Dexing(dexingOptions));
     assertThat(options.outputZip.toFile().exists()).isTrue();
 
     HashSet<String> files = new HashSet<>();
diff --git a/src/test/java/com/google/devtools/build/android/dexer/DexConversionEnqueuerTest.java b/src/test/java/com/google/devtools/build/android/dexer/DexConversionEnqueuerTest.java
index 5f1bb828dc..b032f964ee 100644
--- a/src/test/java/com/google/devtools/build/android/dexer/DexConversionEnqueuerTest.java
+++ b/src/test/java/com/google/devtools/build/android/dexer/DexConversionEnqueuerTest.java
@@ -23,6 +23,7 @@ import com.android.dex.Dex;
 import com.android.dx.command.dexer.DxContext;
 import com.android.dx.dex.DexOptions;
 import com.android.dx.dex.cf.CfOptions;
+import com.android.dx.dex.code.PositionList;
 import com.google.common.cache.Cache;
 import com.google.common.cache.CacheBuilder;
 import com.google.common.collect.ImmutableList;
@@ -149,10 +150,16 @@ public class DexConversionEnqueuerTest {
 
     Dex dex = new Dex(dexcode);
     assertThat(dex.classDefs()).hasSize(1);
-    assertThat(cache.getIfPresent(DexingKey.create(false, false, bytecode))).isSameAs(dexcode);
-    assertThat(cache.getIfPresent(DexingKey.create(true, false, bytecode))).isNull();
-    assertThat(cache.getIfPresent(DexingKey.create(false, true, bytecode))).isNull();
-    assertThat(cache.getIfPresent(DexingKey.create(true, true, bytecode))).isNull();
+    assertThat(cache.getIfPresent(DexingKey.create(false, false, PositionList.LINES, bytecode)))
+        .isSameAs(dexcode);
+    assertThat(cache.getIfPresent(DexingKey.create(false, false, PositionList.NONE, bytecode)))
+        .isNull();
+    assertThat(cache.getIfPresent(DexingKey.create(true, false, PositionList.LINES, bytecode)))
+        .isNull();
+    assertThat(cache.getIfPresent(DexingKey.create(false, true, PositionList.LINES, bytecode)))
+        .isNull();
+    assertThat(cache.getIfPresent(DexingKey.create(true, true, PositionList.LINES, bytecode)))
+        .isNull();
     return dexcode;
   }
 
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 38c7f4f5a9..7ea6231d29 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
@@ -20,6 +20,7 @@ import static org.junit.Assert.fail;
 import com.android.dex.ClassDef;
 import com.android.dex.Dex;
 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;
@@ -292,7 +293,10 @@ public class DexFileMergerTest {
     options.outputZip =
         FileSystems.getDefault().getPath(System.getenv("TEST_TMPDIR"), "libtests.dex.zip");
     options.maxThreads = 1;
-    DexBuilder.buildDexArchive(options, new Dexing(new DxContext(), new Dexing.DexingOptions()));
+    Dexing.DexingOptions dexingOptions = new Dexing.DexingOptions();
+    dexingOptions.optimize = true;
+    dexingOptions.positionInfo = PositionList.LINES;
+    DexBuilder.buildDexArchive(options, new Dexing(new DxContext(), dexingOptions));
     return options.outputZip;
   }
 
diff --git a/src/test/java/com/google/devtools/build/android/dexer/DexingKeyTest.java b/src/test/java/com/google/devtools/build/android/dexer/DexingKeyTest.java
index f7250c9bbb..5cc1d9afae 100644
--- a/src/test/java/com/google/devtools/build/android/dexer/DexingKeyTest.java
+++ b/src/test/java/com/google/devtools/build/android/dexer/DexingKeyTest.java
@@ -30,9 +30,10 @@ public class DexingKeyTest {
 
   @Test
   public void testOrderMatters() {
-    DexingKey key = DexingKey.create(false, true, new byte[0]);
+    DexingKey key = DexingKey.create(false, true, 2, new byte[0]);
     assertThat(key.localInfo()).isFalse();
     assertThat(key.optimize()).isTrue();
+    assertThat(key.positionInfo()).isEqualTo(2);
   }
 
   /**
@@ -41,10 +42,10 @@ public class DexingKeyTest {
    */
   @Test
   public void testContentMatters() {
-    assertThat(DexingKey.create(false, false, new byte[] { 1, 2, 3 }))
-        .isEqualTo(DexingKey.create(false, false, new byte[] { 1, 2, 3 }));
-    assertThat(DexingKey.create(false, false, new byte[] { 1, 2, 3 }))
-        .isNotEqualTo(DexingKey.create(false, false, new byte[] { 1, 3, 3 }));
+    assertThat(DexingKey.create(false, false, 1, new byte[] { 1, 2, 3 }))
+        .isEqualTo(DexingKey.create(false, false, 1, new byte[] { 1, 2, 3 }));
+    assertThat(DexingKey.create(false, false, 1, new byte[] { 1, 2, 3 }))
+        .isNotEqualTo(DexingKey.create(false, false, 1, new byte[] { 1, 3, 3 }));
   }
 
   /**
diff --git a/src/tools/android/java/com/google/devtools/build/android/dexer/Dexing.java b/src/tools/android/java/com/google/devtools/build/android/dexer/Dexing.java
index ca5dba24ae..d5583e79b1 100644
--- a/src/tools/android/java/com/google/devtools/build/android/dexer/Dexing.java
+++ b/src/tools/android/java/com/google/devtools/build/android/dexer/Dexing.java
@@ -26,11 +26,14 @@ import com.android.dx.util.ByteArray;
 import com.google.auto.value.AutoValue;
 import com.google.common.annotations.VisibleForTesting;
 import com.google.common.io.ByteStreams;
+import com.google.devtools.common.options.Converter;
 import com.google.devtools.common.options.Option;
 import com.google.devtools.common.options.OptionDocumentationCategory;
 import com.google.devtools.common.options.OptionEffectTag;
 import com.google.devtools.common.options.OptionsBase;
+import com.google.devtools.common.options.OptionsParsingException;
 import java.io.PrintStream;
+import java.lang.reflect.Field;
 
 /**
  * Common helper class that encodes Java classes into {@link DexFile}s.
@@ -40,6 +43,30 @@ class Dexing {
   static final PrintStream nullout = new PrintStream(ByteStreams.nullOutputStream());
 
   /**
+   * Parser for positions options based on the integer field names in {@link PositionList}.
+   */
+  public static class PositionGranularityConverter implements Converter<Integer> {
+    @Override
+    public Integer convert(String input) throws OptionsParsingException {
+      for (Field field : PositionList.class.getFields()) {
+        if (field.getName().equalsIgnoreCase(input)) {
+          try {
+            return field.getInt(null);
+          } catch (RuntimeException | IllegalAccessException e) {
+            throw new OptionsParsingException("Can't parse positions option", input, e);
+          }
+        }
+      }
+      throw new OptionsParsingException("Unknown positions option", input);
+    }
+
+    @Override
+    public String getTypeDescription() {
+      return "One of the options from dx's --positions flag";
+    }
+  }
+
+  /**
    * Common command line options for use with {@link Dexing}.
    */
   public static class DexingOptions extends OptionsBase {
@@ -67,6 +94,20 @@ class Dexing {
     public boolean optimize;
 
     @Option(
+      name = "positions",
+      defaultValue = "lines", // dx's default
+      category = "semantics",
+      documentationCategory = OptionDocumentationCategory.UNCATEGORIZED,
+      effectTags = {OptionEffectTag.UNKNOWN},
+      allowMultiple = false,
+      converter = PositionGranularityConverter.class,
+      help = "How densely to emit line number information."
+    )
+    // Note this field must be initialized (usually done by an options parser) since the implicit
+    // initial value 0 is not valid.
+    public int positionInfo;
+
+    @Option(
       name = "warning",
       defaultValue = "true", // dx's default
       category = "misc",
@@ -85,7 +126,7 @@ class Dexing {
       // Use dx's defaults
       result.optimizeListFile = null;
       result.dontOptimizeListFile = null;
-      result.positionInfo = PositionList.LINES;
+      result.positionInfo = positionInfo;
       result.strictNameCheck = true;
       result.statistics = false; // we're not supporting statistics anyways
       return result;
@@ -103,9 +144,10 @@ class Dexing {
    */
   @AutoValue
   abstract static class DexingKey {
-    static DexingKey create(boolean localInfo, boolean optimize, byte[] classfileContent) {
+    static DexingKey create(
+        boolean localInfo, boolean optimize, int positionInfo, byte[] classfileContent) {
       // TODO(bazel-team): Maybe we can use a minimal collision hash instead of full content
-      return new AutoValue_Dexing_DexingKey(localInfo, optimize, classfileContent);
+      return new AutoValue_Dexing_DexingKey(localInfo, optimize, positionInfo, classfileContent);
     }
 
     /** Returns whether {@link CfOptions#localInfo local variable information} is included. */
@@ -114,6 +156,9 @@ class Dexing {
     /** Returns whether {@link CfOptions#optimize SSA/register optimization} is performed. */
     abstract boolean optimize();
 
+    /** Returns how much line number information is emitted as a {@link PositionList} constant. */
+    abstract int positionInfo();
+
     /** Returns the class file to dex, <b>not</b> the dexed class. Don't modify the return value! */
     @SuppressWarnings("mutable") abstract byte[] classfileContent();
   }
@@ -162,6 +207,7 @@ class Dexing {
   }
 
   public DexingKey getDexingKey(byte[] classfile) {
-    return DexingKey.create(cfOptions.localInfo, cfOptions.optimize, classfile);
+    return DexingKey.create(
+        cfOptions.localInfo, cfOptions.optimize, cfOptions.positionInfo, classfile);
   }
 }