[WIP] Quick fix for inconsistent SDK versions

[brodycj]

Resolves #508 - Inconsistent handling of min/target SDK values

This is a quick fix to use the minimum & target SDK version values from AndroidManifest.xml.

I have tested it in a newly generated Cordova project from the command line.

This proposal also resolves #631 - Target SDK version not consistent with defaults in root build.gradle bug

TODO items:

• update unit tests to succeed with the new code change
• update unit tests to cover extracting minimum & target SDK version values from AndroidManifest.xml

Unfortunately this does NOT resolve the issue reported in #629 (errors encountered on Android Studio 3.2 & 3.3).

I really hope this kind of a solution will be part of the upcoming major release for Cordova 9 (apache/cordova#10).

[brodycj]

Updated as follows:

• Updated ProjectBuilder.spec.js according to changes, with additional tests to cover reading of android:minSdkVersion and android:targetSdkVersion values from AndroidManifest.xml
• use parseInt on android:minSdkVersion and android:targetSdkVersion values
• fix use of wrong variable in an if statement, as discussed in review comments
• add missing "find" in a couple error message, as discussed in review comments

[erisu]

@brodybits Can you take a look into the AppVeyor build failure?

[codecov-io]

Codecov Report

Merging #633 into master will decrease coverage by 0.2%.
The diff coverage is 43.75%.

@@            Coverage Diff             @@
##           master     #633      +/-   ##
==========================================
- Coverage   64.66%   64.46%   -0.21%     
==========================================
  Files          18       18              
  Lines        1817     1829      +12     
==========================================
+ Hits         1175     1179       +4     
- Misses        642      650       +8

Impacted Files	Coverage Δ
...n/templates/cordova/lib/builders/ProjectBuilder.js	42.22% <43.75%> (-0.64%)	⬇️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 715ce2f...dec5926. Read the comment docs.

[brodycj]

Now green after an empty commit

[erisu]

Overall
LGTM

Recommendation:
If this is considered the final fix and you are not expecting to revisit, please remove the comments Quick fix from the files.

If you are planning additional work, can you replace the comments with

// @todo What is the desired task to preform other than the word quick fix. This is so if you are unable to revisit the area and others or I have time, we might be able to help out in this area as we would be able to understand what was the desired/expected task.

The above recommendations are just my opinions.

My Test Steps

$ npx cordova@nightly create androidTest com.foobar.androidTest && cd androidTest
$ npx cordova@nightly platform add github:brodybits/cordova-android\#quick-fix-for-inconsistent-sdk-versions
$ npx cordova@nightly build android

======================
build.gradle RESULTS
======================
defaultBuildToolsVersion="28.0.3"
defaultMinSdkVersion=19
defaultTargetSdkVersion=27
defaultCompileSdkVersion=27

$ vi config.xml

// ADDED
<preference name="android-minSdkVersion" value="21" />
<preference name="android-targetSdkVersion" value="28" />

$ npx cordova@nightly build android

======================
build.gradle RESULTS
======================
defaultBuildToolsVersion="28.0.3"
defaultMinSdkVersion=21
defaultTargetSdkVersion=28
defaultCompileSdkVersion=28

[brodycj]

Thanks @erisu for the review. I would consider this to be a stopgap or makeshift solution since it does not solve #508 and #631 the best way possible.

I would much rather find a way to resolve #508 and #631 without injecting the minimum & default SDK values into AndroidManifest.xml as a side effect. I think injecting the values into AndroidManifest.xml implies that the settings in AndroidManifest.xml are still relevant, which is not true if these settings are otherwise configured in Gradle.

An additional problem was already reported in #629: if we inject these SDK values in AndroidManifest.xml, recent versions of Android Studio would complain that these values should not be defined in a manifest file and offer to update manifest and Gradle files. This is behavior we would rather that our users do not see.

There was discussion in #508 about using GradlePropertiesParser.js to set the values in Gradle based on the settings in config.xml. I wonder if there would be an easy way to do this?

In case we do continue with this PR, I would definitely favor adding the @todo comment and changing "quick fix" to "stopgap" in all places including title, commit message, and comments.

The motivation for this stopgap solution was to solve the problem without blocking the new release for Cordova 9.

I just marked this proposal as [WIP] again, pending discussion whether to continue or find a better way to solve the problem.

[brodycj]

Closing in favor of alternatives discussed in #508 (comment) and #508 (comment)