apache / cordova-android

Apache Cordova Android
https://cordova.apache.org/
Apache License 2.0
3.59k stars 1.52k forks source link

cdvMinSdkVersion seems to not work in 7.1.4 #599

Closed falcon8823 closed 4 years ago

falcon8823 commented 5 years ago

I specified this <preference name="android-minSdkVersion" value="23" /> into config.xml, but it seems not work in 7.1.4.

I think these PRs might cause this problem.

https://github.com/apache/cordova-android/blob/a68f9fd752a25bb680da96414a3b956357c2dfcb/bin/templates/project/app/build.gradle#L125

I quickly fix this line to ext.cdvMinSdkVersion = cdvMinSdkVersion == null ? null : Integer.parseInt('' + defaultMinSdkVersion). It seems to work well.

I think actual cdvMinSdkVersion is set on this line.

https://github.com/apache/cordova-android/blob/a68f9fd752a25bb680da96414a3b956357c2dfcb/bin/templates/project/app/build.gradle#L171

brodybits commented 5 years ago

Please verify what you see in platforms/android/app/src/main/AndroidManifest.xml. I get the following line near the end of platforms/android/app/src/main/AndroidManifest.xml when I try it with 7.1.4:

    <uses-sdk android:minSdkVersion="23" android:targetSdkVersion="27" />
disaacson commented 5 years ago

I just upgraded from 7.1.1 to 7.1.4 and had the same issue. I checked my platforms/android/app/src/main/AndroidManifest.xml and saw that it did include the minSdkVersion in that file, but it does not seem to make it into the apk.

dpogue commented 5 years ago

If the minSdkVersion is specified as part of the gradle build, it will override whatever is listed in AndroidManifest.xml

disaacson commented 5 years ago

It looks like a default value must have begun overriding that build parameter recently. I added a gradle.properties file with a cdvMinSdkVersion=23 line and copied it to the necessary folder in a before_build hook like it mentions here

brodybits commented 5 years ago

I just added the bug label, will need further investigation and maybe another patch fix.

dpogue commented 5 years ago

This problem is caused by https://github.com/apache/cordova-android/pull/495/files#diff-6e8bcecf29cf2fd3cbafdc8d406476fbR120 always setting a default minSdkVersion in gradle, which overrides whatever is specified in AndroidManifest.xml.

This is the same concern I raised in https://github.com/apache/cordova-android/issues/508

brodybits commented 5 years ago

We may have to revert 00134cf9f3148335d17aa47dcfed589ca1460fda (cherry-pick of PR #495) in 7.1.x. For next major release I think we should either resolve #508 or revert PR #495.

dpogue commented 5 years ago

We cannot fix #508 in a patch release, and maybe not even in a minor release.

brodybits commented 5 years ago

We cannot fix #508 in a patch release, and maybe not even in a minor release.

Right. By "next release" I had meant "next major release", just updated the comment to make it clear.

I am afraid we would have to revert the update from #495 in 7.1.x to resolve this issue in a patch release. Any better ideas?

dmastag commented 5 years ago

Would the Solution be to move back to 7.1.2 while this is fixed ?

redwolf2 commented 5 years ago

@dmastag Yes, that worked for me

brodybits commented 5 years ago

Would the Solution be to move back to 7.1.2 while this is fixed ?

I cannot promise this will ever be fixed in a patch release. I personally do not have any extra time to make such a patch release and I think not other committers will have the extra time either.

The only way I can think of to fix this in a patch release would be to revert 00134cf (cherry-pick of PR #495) in 7.1.x. This would be a breaking change for people who configure via Gradle as described in [1], which would be a step backward.

For future releases such as upcoming Cordova 9 (apache/cordova#10) I would say the minimum & target SDK should be configured as described in [1], not by setting the preferences in config.xml.

[1] https://cordova.apache.org/docs/en/latest/guide/platforms/android/#setting-gradle-properties

falcon8823 commented 5 years ago

FYI: The workaround is to specify gradleArg when you build.

cordova build android -- --gradleArg=-PcdvMinSdkVersion=23

This will override minSdkVersion. I confirmed it works on 7.1.4.

You can confirm whether it works well using aapt command.

aapt l -a path/to/apk/android-release.apk | grep 'minSdkVersion'

robations commented 5 years ago

I'm seeing something like this after upgrading to v8 from v7.x, not sure if that's expected.

Symptoms:

It seems like a minimum SDK < 19 is no longer supported so I changed my preference in config.xml:

    <platform name="android">
        <preference name="android-targetSdkVersion" value="26" />
        <preference name="android-minSdkVersion" value="19" />

When I build (cordova build android) I get the following warning (which eventually causes the build to fail):

[Gradle Properties] Detected Gradle property "cdvMinSdkVersion" with the value of "16", Cordova's recommended value is "19"

When I search through the AndroidManifest.xml files under platforms/android everything is saying 19, so the merge has worked but it seems like there is a gradle property sticking around somewhere. (FYI I know almost nothing about the internals of gradle or the Android build process :-/ .)

I'm going to try @falcon8823 's workaround for now.

Update from my notes:

Seems like at some point the minSdkVersion parameter got saved to platforms/android/gradle.properties and then always overrides the other config.

Just delete the line and rebuild.

eddskt commented 4 years ago

Another solution?

eddskt commented 4 years ago

I solved changing lines in file: platforms/android/gradle.properties

org.gradle.daemon=true
org.gradle.jvmargs=-Xmx2048m
android.useDeprecatedNdk=true
cdvMinSdkVersion=21

to

org.gradle.daemon=true
org.gradle.jvmargs=-Xmx2048m
android.useDeprecatedNdk=true
cdvMinSdkVersion=19
vinayhsorathiya commented 4 years ago

@eddskt Yes, that worked for me

breautek commented 4 years ago

Unable to reproduce this on master, I believe this was fixed by https://github.com/apache/cordova-android/pull/699

Using the android-minSdkVersion and checking the apk resulted in the expected min SDK to be stored.

aapt list -a app-debug.apk | grep SdkVersion
    A: android:compileSdkVersion(0x01010572)=(type 0x10)0x1c
    A: android:compileSdkVersionCodename(0x01010573)="9" (Raw: "9")
      A: android:minSdkVersion(0x0101020c)=(type 0x10)0x17
      A: android:targetSdkVersion(0x01010270)=(type 0x10)0x1c

Note: The value is in hex, A: android:minSdkVersion(0x0101020c)=(type 0x10)0x17 0x17 is the hex value for 23.