Fyne package missing some Android options

[andydotxyz]

Describe the bug:

Android manifest is scanned in "fyne package" and unrecognised tags create errors. Notably the following (at least) is not understood:

• usesCleartextTraffic

To Reproduce:

Steps to reproduce the behaviour:

1. Create a custom AndroidManifest.xml
2. Add android:usesCleartextTraffic="true" to <application ...> tag
3. Fyne package and see error:

failed to find table ref by "attr/usesCleartextTraffic" 

Device (please complete the following information):

• OS: macOS
• Version: Catalina
• Go version: 1.12
• Fyne version: 1.3.0

[david-massey]

(I understand this issue is a year and a half old. Not trying to raise the dead, but I saw the issue is still open)

I've run into something similar trying to use more modern AndroidManifest attributes. The binres SDKTable is linked to Android API 15 (Android 4.0.3/4). This is found in cmd/fyne/internal/mobile/binres/sdk.go Lines 14

const MinSDK = 15

and Line 62

// 55-64
func apiResourcesPath() (string, error) {
    // TODO(elias.naur): use the logic from gomobile's androidAPIPath and use the any installed version of the
    // Android SDK instead. Currently, the binres_test.go tests fail on anything newer than android-15.
    sdkdir := os.Getenv("ANDROID_HOME")
    if sdkdir == "" {
        return "", fmt.Errorf("ANDROID_HOME env var not set")
    }
 -> platform := fmt.Sprintf("android-%v", MinSDK)
    return path.Join(sdkdir, "platforms", platform, "android.jar"), nil
}

This specific issue is looking for an attribute added in API 23. Given the minimum API is 15, the package can't be built using attributes not available to API 15. The good news is in my [limited] testing I can safely put MinSDK up to 25, but starting at 26 it starts generating invalid binary manifests that fail to install.

> adb install gui/my_fyne_app.apk
Performing Streamed Install
adb: failed to install gui/my_fyne_app.apk: Failure [INSTALL_PARSE_FAILED_MANIFEST_MALFORMED: Failed parse during installPackageLI: /data/app/vmdl664305682.tmp/base.apk (at Binary XML file line #23): <activity> does not specify android:name]

I am not sure if there are any reasons why the minimum API can't be bumped up to 24 or 25. I am not yet familiar enough with generating and linking these binary resources to try to fix minimum APIs > 25 (e.g. Android 10—API 29).

[andydotxyz]

I think you may be confusing minSDK and targetSDK... The minimum is what it will run on - if we update this to a higher value the app drop support for older devices. The targetSDK is used to make APIs available that your code must check the existence of before using (assuming Min < Target). It is possible that we are not passing target correctly in, but changing minSDK does not seem like the right solution.

[david-massey]

I have no misunderstandings about the theoretical difference between the minimum SDK and the target SDK, but I thank you for diligently checking. I will often refer to this as "API", as android documentation talks about when various features were introduced, deprecated, or removed by API version number (e.g. 15 for Android 4.0).

TLDR

In other Android compilers/packagers like Android Studio, trying to use manifest attributes that are not found in the minimum SDK generates warnings about compatibility or unused attributes. GoMobile (where almost all of this code comes from) throws an error if you try to use attributes not found in the minimum SDK.

Why the error is thrown

As stated in my original post, sdk.go uses minSDK rather than the target API for its apiResourcesPath. If it wanted to use the target SDK (targeted by API version number), it would need to generate several files at the time a project is packaged—greatly increasing packaging time and modifying files installed to get the CLI to work properly.

Expanding on this, it uses it to load resources.arsc from its minSDK:

// Boilerplate error checking removed for simplicity
func apiResources() ([]byte, error) {
    apiResPath, err := apiResourcesPath()
    zr, err := zip.OpenReader(apiResPath)
    defer zr.Close()

    buf := new(bytes.Buffer)
    for _, f := range zr.File {
        if f.Name == "resources.arsc" {
            rc, err := f.Open()
            _, err = io.Copy(buf, rc)
            err = rc.Close()
            break
        }
    }
    return buf.Bytes(), nil
}

This is in turn called in table.go to load resources.arsc into a Table struct:

// Boilerplate error checking removed for simplicity
func OpenSDKTable() (*Table, error) {
    bin, err := apiResources()
    tbl := new(Table)
    return tbl, nil
}

And back to sdk.go:

func PackResources() ([]byte, error) {
    tbl, err := OpenSDKTable()

    ... // Remove unnecessary entries, save only those needed. Makes it more memory-friendly

    bin, err := tbl.MarshalBinary()

    buf := new(bytes.Buffer)

    zw := gzip.NewWriter(buf)
    zw.Write(bin)
    zw.Flush()
    zw.Close()
    return buf.Bytes(), nil

Which is used by genarsc.go to generate arsc.go with the binres.arsc variable:

func main() {
    arsc, err := binres.PackResources()

Later, after generating or loading and then parsing and modifying the XML version of the manifest, it uses this pre-generated arsc variable to go through every attribute in the manifest. It either finds the correct binary (reference? I am a little fuzzy on this part) for it or throws an error if it's unknown.

// table.go
// OpenTable decodes the prepacked resources.arsc for the supported sdk platform.
func OpenTable() (*Table, error) {
    zr, err := gzip.NewReader(bytes.NewReader(arsc))

// binres.go
func buildXML(q []ltoken) (*XML, error) {
    // temporary pool to resolve real poolref later
    pool := new(Pool)

    tbl, err := OpenTable() 

tbl is then passed around in a bunch of functions, but more importantly, the source of the error this issue is about:

// table.go
func (tbl *Table) RefByName(name string) (TableRef, error) {
    pp, pkg, tt, spec, err := tbl.SpecByName(name)
    if err != nil {
        return 0, err
    }

    q := strings.Split(name, "/")
    if len(q) != 2 {
        return 0, fmt.Errorf("invalid entry format, missing forward-slash: %q", name)
    }
    n := q[1]

    for _, t := range spec.types {
        for eeee, nt := range t.entries {
            if nt == nil { // NoEntry
                continue
            }
            if n == pkg.keyPool.strings[nt.key] {
                return TableRef(uint32(eeee) | uint32(tt+1)<<16 | uint32(pp+1)<<24), nil
            }
        }
    }
    return 0, fmt.Errorf("failed to find table ref by %q", name)
}

Finally

changing minSDK does not seem like the right solution

I also have no opinion on whether or not backwards compatibility for an OS version released for hardware from a decade ago is important to a project I am not part of.

[andydotxyz]

Thanks for your info, it is very helpful. Just to clarify where my point was coming from, I was responding to the:

I am not sure if there are any reasons why the minimum API can't be bumped up to 24 or 25.

I hope it did not sound too critical of your knowledge.

As to the correct fix, it sounds like our packaging is erroring too early and should in fact allow attributes that are not in the min SDK. (which I suppose is indeed relating to the main topic of the ticket). We did look at allowing developers to change the min SDK to work around this but, as you have observed, it randomly breaks things if the toolkit is not matching the manifest in its expectations.

[david-massey]

Oh, there are enough gaps in my knowledge that I don't take any offense about misunderstandings or not knowing something. There is always someone that knows something I do not or knows a topic better than I, and I make mistakes. No worries.

What I mean by "I am not sure if there are any reasons why the minimum API can't be bumped up to 24 or 25." is that I have not fully traced everywhere tbl is passed around and if that breaks anything anywhere else. I have only done spot tests to check APIs other than 15—largely "does it compile, install, and run without immediately crashing?" Without doing a deep audit, it appears that MinSDK as found in sdk.go is only used to generate the binary/packaged version of the manifest, for there are a few other places that appear to specify the minimum SDK.

(tbl *Table) RefByName is used when adding attributes to the binary manifest. I am not sure if silently ignoring attributes is the correct answer, which is effectively what would happen if you do not throw an error for an unknown manifest attribute; not finding an attribute would be the same as not adding an attribute in its current implementation. Arbitrarily changing the minimum SDK does have the ability to randomly break things.

For my use, I only care about Android versions >= 10 (soon to be >= 11). It gets really tedious writing if (Build.VERSION.SDK_INT < Build.VERSION_CODES.O) {} But, this is what forks are for. Until I fix minimum SDKs > 25, I must still use SDK_INT checks.