Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 90e4363c authored by Jared Duke's avatar Jared Duke
Browse files

Refine systemfeature codegen behavior 

Tweak the semantics of codegen tool feature version args, such that a
`--feature=$FEATURE:$VERSION` arg has the following $VERSION semantics:
  * ""            == undefined (runtime API)
  * valid int     == enabled   (compile-time API)
  * "UNAVAILABLE" == disabled  (compile-time API)

This will simplify how feature args get collected from build flags, and
also aligns with terminology used elsewhere for explicitly disabling
features via the "<unavailable-feature />" tag.

Bug: 203143243
Test: atest systemfeatures-gen-tests systemfeatures-gen-golden-tests
Flag: EXEMPT refactor
Change-Id: I502d455effbe4b81794a5f529406b8b68ac2b372
parent bc7a8588

tools/systemfeatures/Android.bp

+2 −2
Original line number Diff line number Diff line
@@ -30,8 +30,8 @@ genrule { 
    name: "systemfeatures-gen-tests-srcs",

    cmd: "$(location systemfeatures-gen-tool) com.android.systemfeatures.RwNoFeatures --readonly=false > $(location RwNoFeatures.java) && " +

        "$(location systemfeatures-gen-tool) com.android.systemfeatures.RoNoFeatures --readonly=true --feature-apis=WATCH > $(location RoNoFeatures.java) && " +

        "$(location systemfeatures-gen-tool) com.android.systemfeatures.RwFeatures --readonly=false --feature=WATCH:1 --feature=WIFI:0 --feature=VULKAN:-1 --feature=AUTO: > $(location RwFeatures.java) && " +

        "$(location systemfeatures-gen-tool) com.android.systemfeatures.RoFeatures --readonly=true --feature=WATCH:1 --feature=WIFI:0 --feature=VULKAN:-1 --feature=AUTO: --feature-apis=WATCH,PC > $(location RoFeatures.java)",

        "$(location systemfeatures-gen-tool) com.android.systemfeatures.RwFeatures --readonly=false --feature=WATCH:1 --feature=WIFI:0 --feature=VULKAN:UNAVAILABLE --feature=AUTO: > $(location RwFeatures.java) && " +

        "$(location systemfeatures-gen-tool) com.android.systemfeatures.RoFeatures --readonly=true --feature=WATCH:1 --feature=WIFI:0 --feature=VULKAN:UNAVAILABLE --feature=AUTO: --feature-apis=WATCH,PC > $(location RoFeatures.java)",

    out: [

        "RwNoFeatures.java",

        "RoNoFeatures.java",

tools/systemfeatures/src/com/android/systemfeatures/SystemFeaturesGenerator.kt

+17 −12
Original line number Diff line number Diff line
@@ -31,7 +31,7 @@ import javax.lang.model.element.Modifier 
 *

 * <pre>

 *   <cmd> com.foo.RoSystemFeatures --readonly=true \

 *           --feature=WATCH:0 --feature=AUTOMOTIVE: --feature=VULKAN:9348

 *           --feature=WATCH:0 --feature=AUTOMOTIVE: --feature=VULKAN:9348 --feature=PC:UNAVAILABLE

 *           --feature-apis=WATCH,PC,LEANBACK

 * </pre>

 *
@@ -43,10 +43,10 @@ import javax.lang.model.element.Modifier 
 *     @AssumeTrueForR8

 *     public static boolean hasFeatureWatch(Context context);

 *     @AssumeFalseForR8

 *     public static boolean hasFeatureAutomotive(Context context);

 *     public static boolean hasFeaturePc(Context context);

 *     @AssumeTrueForR8

 *     public static boolean hasFeatureVulkan(Context context);

 *     public static boolean hasFeaturePc(Context context);

 *     public static boolean hasFeatureAutomotive(Context context);

 *     public static boolean hasFeatureLeanback(Context context);

 *     public static Boolean maybeHasFeature(String feature, int version);

 * }
@@ -67,7 +67,10 @@ object SystemFeaturesGenerator { 
        println("Usage: SystemFeaturesGenerator <outputClassName> [options]")

        println(" Options:")

        println("  --readonly=true|false    Whether to encode features as build-time constants")

        println("  --feature=\$NAME:\$VER   A feature+version pair (blank version == disabled)")

        println("  --feature=\$NAME:\$VER   A feature+version pair, where \$VER can be:")

        println("                             * blank/empty == undefined (variable API)")

        println("                             * valid int   == enabled   (constant API)")

        println("                             * UNAVAILABLE == disabled  (constant API)")

        println("                           This will always generate associated query APIs,")

        println("                           adding to or replacing those from `--feature-apis=`.")

        println("  --feature-apis=\$NAME_1,\$NAME_2")
@@ -89,7 +92,7 @@ object SystemFeaturesGenerator { 


        var readonly = false

        var outputClassName: ClassName? = null

        val featureArgs = mutableListOf<FeatureArg>()

        val featureArgs = mutableListOf<FeatureInfo>()

        // We could just as easily hardcode this list, as the static API surface should change

        // somewhat infrequently, but this decouples the codegen from the framework completely.

        val featureApiArgs = mutableSetOf<String>()
@@ -122,7 +125,7 @@ object SystemFeaturesGenerator { 
        featureArgs.associateByTo(

            features,

            { it.name },

            { FeatureInfo(it.name, it.version, readonly) },

            { FeatureInfo(it.name, it.version, it.readonly && readonly) },

        )



        outputClassName
@@ -154,13 +157,17 @@ object SystemFeaturesGenerator { 
     * Parses a feature argument of the form "--feature=$NAME:$VER", where "$VER" is optional.

     *   * "--feature=WATCH:0" -> Feature enabled w/ version 0 (default version when enabled)

     *   * "--feature=WATCH:7" -> Feature enabled w/ version 7

     *   * "--feature=WATCH:"  -> Feature disabled

     *   * "--feature=WATCH:"  -> Feature status undefined, runtime API generated

     *   * "--feature=WATCH:UNAVAILABLE"  -> Feature disabled

     */

    private fun parseFeatureArg(arg: String): FeatureArg {

    private fun parseFeatureArg(arg: String): FeatureInfo {

        val featureArgs = arg.substring(FEATURE_ARG.length).split(":")

        val name = parseFeatureName(featureArgs[0])

        val version = featureArgs.getOrNull(1)?.toIntOrNull()

        return FeatureArg(name, version)

        return when (featureArgs.getOrNull(1)) {

            null, "" -> FeatureInfo(name, null, readonly = false)

            "UNAVAILABLE" -> FeatureInfo(name, null, readonly = true)

            else -> FeatureInfo(name, featureArgs[1].toIntOrNull(), readonly = true)

        }

    }



    private fun parseFeatureName(name: String): String =
@@ -267,7 +274,5 @@ object SystemFeaturesGenerator { 
        builder.addMethod(methodBuilder.build())

    }



    private data class FeatureArg(val name: String, val version: Int?)



    private data class FeatureInfo(val name: String, val version: Int?, val readonly: Boolean)

}

tools/systemfeatures/tests/golden/RoFeatures.java.gen

+3 −5
Original line number Diff line number Diff line
@@ -3,7 +3,7 @@ 
//            --readonly=true \

//            --feature=WATCH:1 \

//            --feature=WIFI:0 \

//            --feature=VULKAN:-1 \

//            --feature=VULKAN:UNAVAILABLE \

//            --feature=AUTO: \

//            --feature-apis=WATCH,PC

package com.android.systemfeatures;
@@ -62,9 +62,8 @@ public final class RoFeatures { 
     *

     * @hide

     */

    @AssumeFalseForR8

    public static boolean hasFeatureAuto(Context context) {

        return false;

        return hasFeatureFallback(context, PackageManager.FEATURE_AUTO);

    }



    private static boolean hasFeatureFallback(Context context, String featureName) {
@@ -79,8 +78,7 @@ public final class RoFeatures { 
        switch (featureName) {

            case PackageManager.FEATURE_WATCH: return 1 >= version;

            case PackageManager.FEATURE_WIFI: return 0 >= version;

            case PackageManager.FEATURE_VULKAN: return -1 >= version;

            case PackageManager.FEATURE_AUTO: return false;

            case PackageManager.FEATURE_VULKAN: return false;

            default: break;

        }

        return null;

tools/systemfeatures/tests/golden/RwFeatures.java.gen

+1 −1
Original line number Diff line number Diff line
@@ -3,7 +3,7 @@ 
//            --readonly=false \

//            --feature=WATCH:1 \

//            --feature=WIFI:0 \

//            --feature=VULKAN:-1 \

//            --feature=VULKAN:UNAVAILABLE \

//            --feature=AUTO:

package com.android.systemfeatures;

tools/systemfeatures/tests/src/SystemFeaturesGeneratorTest.java

+16 −8
Original line number Diff line number Diff line
@@ -110,10 +110,13 @@ public class SystemFeaturesGeneratorTest { 
        assertThat(RoFeatures.hasFeatureWatch(mContext)).isTrue();

        assertThat(RoFeatures.hasFeatureWifi(mContext)).isTrue();

        assertThat(RoFeatures.hasFeatureVulkan(mContext)).isFalse();

        assertThat(RoFeatures.hasFeatureAuto(mContext)).isFalse();

        verify(mPackageManager, never()).hasSystemFeature(anyString(), anyInt());



        // For defined feature types, conditional queries should reflect the build-time versions.

        // For defined feature types, conditional queries should reflect either:

        //  * Enabled if the feature version is specified

        //  * Disabled if UNAVAILABLE is specified

        //  * Unknown if no version value is provided



        // VERSION=1

        assertThat(RoFeatures.maybeHasFeature(PackageManager.FEATURE_WATCH, -1)).isTrue();

        assertThat(RoFeatures.maybeHasFeature(PackageManager.FEATURE_WATCH, 0)).isTrue();
@@ -124,15 +127,19 @@ public class SystemFeaturesGeneratorTest { 
        assertThat(RoFeatures.maybeHasFeature(PackageManager.FEATURE_WIFI, 0)).isTrue();

        assertThat(RoFeatures.maybeHasFeature(PackageManager.FEATURE_WIFI, 100)).isFalse();



        // VERSION=-1

        assertThat(RoFeatures.maybeHasFeature(PackageManager.FEATURE_VULKAN, -1)).isTrue();

        // VERSION=UNAVAILABLE

        assertThat(RoFeatures.maybeHasFeature(PackageManager.FEATURE_VULKAN, -1)).isFalse();

        assertThat(RoFeatures.maybeHasFeature(PackageManager.FEATURE_VULKAN, 0)).isFalse();

        assertThat(RoFeatures.maybeHasFeature(PackageManager.FEATURE_VULKAN, 100)).isFalse();



        // DISABLED

        assertThat(RoFeatures.maybeHasFeature(PackageManager.FEATURE_AUTO, -1)).isFalse();

        assertThat(RoFeatures.maybeHasFeature(PackageManager.FEATURE_AUTO, 0)).isFalse();

        assertThat(RoFeatures.maybeHasFeature(PackageManager.FEATURE_AUTO, 100)).isFalse();

        // VERSION=

        when(mPackageManager.hasSystemFeature(PackageManager.FEATURE_AUTO, 0)).thenReturn(false);

        assertThat(RoFeatures.hasFeatureAuto(mContext)).isFalse();

        when(mPackageManager.hasSystemFeature(PackageManager.FEATURE_AUTO, 0)).thenReturn(true);

        assertThat(RoFeatures.hasFeatureAuto(mContext)).isTrue();

        assertThat(RoFeatures.maybeHasFeature(PackageManager.FEATURE_AUTO, -1)).isNull();

        assertThat(RoFeatures.maybeHasFeature(PackageManager.FEATURE_AUTO, 0)).isNull();

        assertThat(RoFeatures.maybeHasFeature(PackageManager.FEATURE_AUTO, 100)).isNull();



        // For feature APIs without an associated feature definition, conditional queries should

        // report null, and explicit queries should report runtime-defined versions.
@@ -148,5 +155,6 @@ public class SystemFeaturesGeneratorTest { 
        assertThat(RoFeatures.maybeHasFeature("com.arbitrary.feature", -1)).isNull();

        assertThat(RoFeatures.maybeHasFeature("com.arbitrary.feature", 0)).isNull();

        assertThat(RoFeatures.maybeHasFeature("com.arbitrary.feature", 100)).isNull();

        assertThat(RoFeatures.maybeHasFeature("", 0)).isNull();

    }

}