Add support of '--min-api' d8 argument (#1000)

Author: ekharkunov

server/src/main/java/com/defold/extender/Extender.java

@@ -2199,6 +2199,9 @@ private File[] buildClassesDex(List<String> jars, File mainDexList) throws Exten
         context.put("jars", jars);
         context.put("engineJars", empty_list);
         context.put("mainDexList", mainDexList.getAbsolutePath());
+        // Always bound, also for engines that don't send it, so that a build.yml referencing
+        // '--min-api {{minAndroidSdkVersion}}' can never render the flag without its argument.
+        context.put("minAndroidSdkVersion", buildState.getMinAndroidSdkVersion());
 
         // replace parameter name because '--main-dex-list' is deprecated and reported as error
         // we can't change command format for older version of engine so replace parameter here.

server/src/main/java/com/defold/extender/ExtenderBuildState.java

@@ -8,6 +8,9 @@ public class ExtenderBuildState {
     static final String APPMANIFEST_BUILD_ARTIFACTS_KEYWORD = "buildArtifacts";
     static final String APPMANIFEST_JETIFIER_KEYWORD = "jetifier";
     static final String APPMANIFEST_DEBUG_SOURCE_PATH = "debugSourcePath";
+    static final String APPMANIFEST_MIN_ANDROID_SDK_VERSION_KEYWORD = "minAndroidSdkVersion";
+
+    static final int DEFAULT_MIN_ANDROID_SDK_VERSION = 21;
 
     File jobDir;
     File uploadDir;
@@ -19,6 +22,7 @@ public class ExtenderBuildState {
     String hostPlatform;
     private final String buildArtifacts;
     private final String debugSourcePath;
+    private final int minAndroidSdkVersion;
 
     private final Boolean withSymbols;
     private final Boolean useJetifier;
@@ -37,6 +41,7 @@ public class ExtenderBuildState {
         this.withSymbols = ExtenderUtil.getAppManifestContextBoolean(appManifest, APPMANIFEST_WITH_SYMBOLS_KEYWORD, true);
         this.buildArtifacts = ExtenderUtil.getAppManifestContextString(appManifest, APPMANIFEST_BUILD_ARTIFACTS_KEYWORD, "");
         this.debugSourcePath = ExtenderUtil.getAppManifestContextString(appManifest, APPMANIFEST_DEBUG_SOURCE_PATH, null);
+        this.minAndroidSdkVersion = ExtenderUtil.getAppManifestContextInteger(appManifest, APPMANIFEST_MIN_ANDROID_SDK_VERSION_KEYWORD, DEFAULT_MIN_ANDROID_SDK_VERSION);
         // assign configuration names started with upper letter because it used for cocoapods
         if (baseVariant != null && (baseVariant.equals("release") || baseVariant.equals("headless"))) {
             this.buildConfiguration = "Release";
@@ -101,6 +106,10 @@ public String getDebugSourcePath() {
         return debugSourcePath;
     }
 
+    public int getMinAndroidSdkVersion() {
+        return minAndroidSdkVersion;
+    }
+
     public Boolean isNeedSymbols() {
         return withSymbols;
     }

server/src/main/java/com/defold/extender/ExtenderUtil.java

@@ -411,6 +411,25 @@ static String getAppManifestContextString(AppManifestConfiguration manifest, Str
         return default_value;
     }
 
+    // The app manifest context is generated by bob without quoting, so an integer written there is
+    // parsed by snakeyaml as an Integer, while a quoted one arrives as a String. Accept both, and
+    // fall back to the default for anything else (including older clients that omit the key).
+    static Integer getAppManifestContextInteger(AppManifestConfiguration manifest, String name, Integer default_value) throws ExtenderException {
+        Object o = getAppManifestContextObject(manifest, name);
+        if (o instanceof Integer) {
+            return (Integer)o;
+        }
+        if (o instanceof String) {
+            try {
+                return Integer.valueOf(((String)o).trim());
+            } catch (NumberFormatException e) {
+                throw new ExtenderException(String.format(
+                        "Error in app.manifest: '%s' must be an integer, got '%s'.", name, o));
+            }
+        }
+        return default_value;
+    }
+
     static public boolean isListOfStrings(List<Object> list) {
         return list != null && list.stream().allMatch(o -> o instanceof String);
     }

server/src/main/java/com/defold/extender/ExtensionManifestValidator.java

@@ -21,6 +21,7 @@ class ExtensionManifestValidator {
 
     private static final Pattern VALID_INCLUDE_PATH = Pattern.compile("^[A-Za-z0-9._+\\-/]+$");
     private static final Pattern VALID_SYMBOL_IDENTIFIER = Pattern.compile("^[A-Za-z_][A-Za-z0-9_]*$");
+    private static final Pattern VALID_API_LEVEL = Pattern.compile("^[0-9]+$");
 
     ExtensionManifestValidator(WhitelistConfig whitelistConfig, List<String> allowedFlags, List<String> allowedSymbols) {
         this.allowedDefines.add(WhitelistConfig.compile(whitelistConfig.defineRe));
@@ -57,6 +58,16 @@ void validateAppManifestContext(Map<String, Object> appContext) throws ExtenderE
                         ExtenderBuildState.APPMANIFEST_DEBUG_SOURCE_PATH, s));
             }
         }
+
+        // Reaches the command line as the argument of d8 --min-api, so it must be a bare number.
+        Object minAndroidSdkVersion = appContext.get(ExtenderBuildState.APPMANIFEST_MIN_ANDROID_SDK_VERSION_KEYWORD);
+        if (minAndroidSdkVersion != null && !(minAndroidSdkVersion instanceof Integer)) {
+            if (!(minAndroidSdkVersion instanceof String) || !VALID_API_LEVEL.matcher((String) minAndroidSdkVersion).matches()) {
+                throw new ExtenderException(String.format(
+                        "Error in app.manifest: '%s' must be an integer, got '%s'.",
+                        ExtenderBuildState.APPMANIFEST_MIN_ANDROID_SDK_VERSION_KEYWORD, minAndroidSdkVersion));
+            }
+        }
     }
 
     private void validateIncludePaths(String extensionName, File extensionFolder, List<String> includes) throws ExtenderException {

server/src/test/java/com/defold/extender/ExtenderUtilTest.java

@@ -376,4 +376,28 @@ public void testSanitizeJavacCmdFlagElsewhere() {
         String cmd = "javac -source 11 -proc:none Foo.java";
         assertEquals(cmd, ExtenderUtil.sanitizeJavacCmd(cmd));
     }
+
+    @Test
+    public void testGetAppManifestContextInteger() throws ExtenderException {
+        // Exercised with the minAndroidSdkVersion key, the d8 --min-api value
+        AppManifestConfiguration manifest = new AppManifestConfiguration();
+
+        // Engines older than the --min-api change send no context at all, or a context without the
+        // key. Both must fall back to the default instead of failing the build.
+        assertEquals(Integer.valueOf(21), ExtenderUtil.getAppManifestContextInteger(manifest, "minAndroidSdkVersion", 21));
+        manifest.context = new HashMap<>();
+        assertEquals(Integer.valueOf(21), ExtenderUtil.getAppManifestContextInteger(manifest, "minAndroidSdkVersion", 21));
+
+        // bob writes the value unquoted, so snakeyaml parses it as an Integer
+        manifest.context.put("minAndroidSdkVersion", 24);
+        assertEquals(Integer.valueOf(24), ExtenderUtil.getAppManifestContextInteger(manifest, "minAndroidSdkVersion", 21));
+
+        // A quoted value arrives as a String
+        manifest.context.put("minAndroidSdkVersion", "24");
+        assertEquals(Integer.valueOf(24), ExtenderUtil.getAppManifestContextInteger(manifest, "minAndroidSdkVersion", 21));
+
+        manifest.context.put("minAndroidSdkVersion", "not-a-number");
+        assertThrows(ExtenderException.class,
+                () -> ExtenderUtil.getAppManifestContextInteger(manifest, "minAndroidSdkVersion", 21));
+    }
 }

server/src/test/java/com/defold/extender/ExtensionManifestValidatorTest.java

@@ -274,6 +274,44 @@ public void testValidateAppManifestContextDebugSourcePath() throws ExtenderExcep
         }
     }
 
+    @Test
+    public void testValidateAppManifestContextMinAndroidSdkVersion() throws ExtenderException {
+        List<String> empty = new ArrayList<>();
+        ExtensionManifestValidator validator = new ExtensionManifestValidator(new WhitelistConfig(), empty, empty);
+
+        // Missing key is accepted: engines older than the --min-api change never send it
+        assertDoesNotThrow(() -> validator.validateAppManifestContext(new HashMap<>()));
+
+        // bob writes the value unquoted, so snakeyaml hands us an Integer; a quoted one is a String
+        for (Object v : new Object[] { 21, 24, "21", "24" }) {
+            Map<String, Object> ctx = new HashMap<>();
+            ctx.put("minAndroidSdkVersion", v);
+            assertDoesNotThrow(() -> validator.validateAppManifestContext(ctx),
+                    "expected to accept minAndroidSdkVersion: " + v);
+        }
+
+        // The value ends up as the argument of d8 --min-api, and the rendered command is split on
+        // spaces, so anything that is not a bare number is argv injection
+        Object[] bad = new Object[] {
+                "24 --output /tmp/evil",
+                "24;rm -rf /",
+                "24$(whoami)",
+                "-1",
+                "",
+                "twentyfour",
+                true,
+        };
+        for (Object v : bad) {
+            Map<String, Object> ctx = new HashMap<>();
+            ctx.put("minAndroidSdkVersion", v);
+            ExtenderException exc = assertThrows(ExtenderException.class,
+                    () -> validator.validateAppManifestContext(ctx),
+                    "expected rejection of minAndroidSdkVersion: " + v);
+            assertTrue(exc.getMessage().contains("minAndroidSdkVersion"),
+                    "message should mention minAndroidSdkVersion, got: " + exc.getMessage());
+        }
+    }
+
     @Test
     public void testValidateSymbols() throws ExtenderException {
         List<String> empty = new ArrayList<>();

server/src/test/java/com/defold/extender/TemplateExecutorTest.java

@@ -19,4 +19,18 @@ public void templateVariablesShouldBeReplacedByContext() {
         assertThat(result).isEqualTo("Hello James!");
     }
 
+    @Test
+    public void minAndroidSdkVersionShouldRenderAsASingleArgumentToMinApi() {
+        // The rendered command is split on spaces, so an unbound or multi-token
+        // minAndroidSdkVersion would make d8 read the next flag as the value of --min-api.
+        TemplateExecutor templateExecutor = new TemplateExecutor();
+        String template = "d8 --min-api {{minAndroidSdkVersion}} --main-dex-rules {{mainDexList}}";
+        Map<String, Object> context = new HashMap<>();
+        context.put("minAndroidSdkVersion", 24);
+        context.put("mainDexList", "/tmp/main.rules");
+        String result = templateExecutor.execute(template, context);
+        assertThat(result).isEqualTo("d8 --min-api 24 --main-dex-rules /tmp/main.rules");
+        assertThat(result.split(" ")).hasSize(5);
+    }
+
 }