Multiple fixes for ACTIVITY mode.android-s-beta-4android-s-beta-3android-s-beta-4

Improved searching for classpath jars which might be in an
Apex intermediates directory.  Fail fast with a build suggestion
if not all are found.

Handle d8 changes (single dex file no longer supported as an
output option).

Replace hyphens in package names in manifest as these are not legal.

Add min and target sdkVersion to manifest.xml - prevents warning
popups on recent Androids.

Test: Manual tests
Change-Id: Iaf72d6213f9e3cce17190cc007fe2c2224533f79

4 files changed, 73 insertions, 20 deletions

diff --git a/src/vogar/Run.java b/src/vogar/Run.java
index 4a61a76..ba36594 100644
--- a/src/vogar/Run.java
+++ b/src/vogar/Run.java
@@ -105,6 +105,7 @@ public final class Run {
     public final Toolchain toolchain;
     public final boolean checkJni;
     public final boolean debugging;
+    public final Integer sdkVersion;
 
     public Run(Vogar vogar, Toolchain toolchain, Console console, Mkdir mkdir,
             AndroidSdk androidSdk, Rm rm, Target target, File runnerDir)
@@ -184,6 +185,7 @@ public final class Run {
         this.taskQueue = new TaskQueue(console, maxConcurrentActions);
         this.checkJni = vogar.checkJni;
         this.debugging = (vogar.debugPort != null) || vogar.debugApp;
+        this.sdkVersion = vogar.sdkVersion;
     }
 
     private Mode createMode(ModeId modeId, Variant variant) {
diff --git a/src/vogar/Vogar.java b/src/vogar/Vogar.java
index 1bf8846..42736f4 100644
--- a/src/vogar/Vogar.java
+++ b/src/vogar/Vogar.java
@@ -218,6 +218,9 @@ public final class Vogar {
     @Option(names = {"--runner-type"})
     RunnerType runnerType;
 
+    @Option(names = {"--sdk-version"})
+    Integer sdkVersion = 28;
+
     @VisibleForTesting public Vogar() {}
 
     private void printUsage() {
@@ -403,6 +406,11 @@ public final class Vogar {
         System.out.println("      Disable with --no-multidex.");
         System.out.println("      Default is: " + multidex);
         System.out.println();
+        System.out.println("  --sdk-version <argument>: min and target sdk version.");
+        System.out.println("      Used in the app manifest for ACTIVITY mode");
+        System.out.println("      to prevent warning popups about old applications");
+        System.out.println("      Default is: " + sdkVersion);
+        System.out.println();
         System.out.println("  --dalvik-cache <argument>: override default dalvik-cache location.");
         System.out.println("      Default is: " + dalvikCache);
         System.out.println();
diff --git a/src/vogar/android/AndroidSdk.java b/src/vogar/android/AndroidSdk.java
index 45f11bb..81906db 100644
--- a/src/vogar/android/AndroidSdk.java
+++ b/src/vogar/android/AndroidSdk.java
@@ -184,6 +184,7 @@ public class AndroidSdk {
 
             String[] jarNames = modeId.getJarNames();
             compilationClasspath = new File[jarNames.length];
+            List<String> missingJars = new ArrayList<>();
             for (int i = 0; i < jarNames.length; i++) {
                 String jar = jarNames[i];
                 File file;
@@ -195,28 +196,19 @@ public class AndroidSdk {
                     }
                     file = new File(String.format(pattern, jar));
                 } else {
-                    final String apexPackage;
-                    // With unbundled ART, the intermediate directory storing the jar file
-                    // outside ART APEX doesn't contain the apex package name.
-                    final boolean tryNonApexIntermediate;
-                    if ("conscrypt".equals(jar)) {
-                        apexPackage = "com.android.conscrypt";
-                        tryNonApexIntermediate = true;
-                    } else if ("core-icu4j".equals(jar)) {
-                        apexPackage = "com.android.i18n";
-                        tryNonApexIntermediate = true;
+                    file = findApexJar(jar, pattern);
+                    if (file.exists()) {
+                        log.verbose("Using jar " + jar + " from " + file);
                     } else {
-                        apexPackage = "com.android.art.testing";
-                        tryNonApexIntermediate = false;
-                    }
-
-                    file = new File(String.format(pattern, jar + "." + apexPackage));
-                    if (tryNonApexIntermediate && !file.exists()) {
-                        file = new File(String.format(pattern, jar));
+                        missingJars.add(jar);
                     }
                 }
                 compilationClasspath[i] = file;
             }
+            if (!missingJars.isEmpty()) {
+                logMissingJars(log, missingJars);
+                throw new RuntimeException("Unable to locate all jars needed for compilation");
+            }
         } else {
             throw new RuntimeException("Couldn't derive Android home from "
                     + ARBITRARY_BUILD_TOOL_NAME);
@@ -226,6 +218,38 @@ public class AndroidSdk {
                 new HostFileCache(log, mkdir), language);
     }
 
+    /** Logs jars that couldn't be found ands suggests a command for building them */
+    private static void logMissingJars(Log log, List<String> missingJars) {
+        StringBuilder makeCommand = new StringBuilder().append("m ");
+        for (String jarName : missingJars) {
+            log.warn("Missing compilation jar " + jarName + " from APEX " + apexForJar(jarName));
+            makeCommand.append(jarName).append(" ");
+        }
+        log.info("Suggested make command: " + makeCommand);
+    }
+
+    /** Returns the name of the APEX a particular jar might be located in */
+    private static String apexForJar(String jar) {
+        if ("conscrypt".equals(jar)) {
+            return "com.android.conscrypt";
+        } else if ("core-icu4j".equals(jar)) {
+            return "com.android.i18n";
+        }
+        return "com.android.art.testing";
+    }
+
+    /**
+     * Depending on the build setup, jars might be located in the intermediates directory
+     * for their APEX or not, so look in both places.  Return null if not found.
+     */
+    private static File findApexJar(String jar, String filePattern) {
+        File file = new File(String.format(filePattern, jar + "." + apexForJar(jar)));
+        if (file.exists()) {
+            return file;
+        }
+        return new File(String.format(filePattern, jar));
+    }
+
     @VisibleForTesting
     AndroidSdk(Log log, Mkdir mkdir, File[] compilationClasspath, String androidJarPath,
                String desugarJarPath, HostFileCache hostFileCache, Language language) {
@@ -371,11 +395,27 @@ public class AndroidSdk {
                 }
                 builder.args(D8_COMMAND_NAME);
                 builder.args("-JXms16M").args("-JXmx1536M");
+
+                // d8 will not allow compiling with a single dex file as the target, but if given
+                // a directory name will start its output in classes.dex but may overflow into
+                // multiple dex files. See b/189327238
+                String outputPath = output.toString();
+                String dexOverflowPath = null;
+                if (outputPath.endsWith("/classes.dex")) {
+                    dexOverflowPath = outputPath.replace("classes.dex", "classes2.dex");
+                    outputPath = output.getParentFile().toString();
+                }
                 builder
                     .args("--min-api").args(language.getMinApiLevel())
-                    .args("--output").args(output)
+                    .args("--output").args(outputPath)
                     .args(sanitizedDesugarOutputFilePaths);
                 builder.execute();
+                if (dexOverflowPath != null && new File(dexOverflowPath).exists()) {
+                    // If we were expecting a single dex file and d8 overflows into two
+                    // or more files than fail.
+                    throw new RuntimeException("Dex file overflow " + dexOverflowPath
+                        + ", try --multidex");
+                }
                 if (output.toString().endsWith(".jar")) {
                     try {
                         fixD8JarOutput(output, desugarOutputFilePaths);
diff --git a/src/vogar/android/InstallApkTask.java b/src/vogar/android/InstallApkTask.java
index 8e2479d..a62e85a 100644
--- a/src/vogar/android/InstallApkTask.java
+++ b/src/vogar/android/InstallApkTask.java
@@ -83,6 +83,8 @@ public final class InstallApkTask extends Task {
             "<manifest xmlns:android=\"http://schemas.android.com/apk/res/android\"\n" +
             "      package=\"" + packageName(action) + "\">\n" +
             "    <uses-permission android:name=\"android.permission.INTERNET\" />\n" +
+            "    <uses-sdk android:minSdkVersion=\"" + run.sdkVersion + "\"\n" +
+            "          android:targetSdkVersion=\"" + run.sdkVersion + "\" />\n" +
             "    <application" +
                     ((run.debugging) ? " android:debuggable=\"true\"" : "") + ">\n" +
             "        <activity android:name=\"" + ACTIVITY_CLASS + "\">\n" +
@@ -114,10 +116,11 @@ public final class InstallApkTask extends Task {
      * According to android.content.pm.PackageParser, package name
      * "must have at least one '.' separator" Since the qualified name
      * may not contain a dot, we prefix containing one to ensure we
-     * are compliant.
+     * are compliant.  Also transform any hyphens to underscores as they
+     * are illegal in package names.
      */
     public static String packageName(Action action) {
-        return "vogar.test." + action.getName();
+        return "vogar.test." + action.getName().replaceAll("-", "_");
     }
 
     private void signApk(File apkUnsigned) {