CB-14201: (android) Removes Gradle property in-line command arguments…

[erisu]

Platforms affected

Android

What does this PR do?

This PR removes the Gradle properties in-line command arguments in favor of setting the gradle.properties file.

This PR will give users more flexibility to override defaults that Cordova defines.

What testing has been done on this change?

• Locally ran the unit test, eslint, and code coverage.
• Locally ran the Cordova prepare, run and build commands.
• https://travis-ci.org/erisu/cordova-android/builds/402490984
• https://ci.appveyor.com/project/erisu/cordova-android/build/1.0.41

Checklist

• Reported an issue in the JIRA database
• Commit message follows the format: "CB-3232: (android) Fix bug with resolving file paths", where CB-xxxx is the JIRA ID & "android" is the platform affected.
• Added automated test coverage as appropriate for this change.

[erisu]

@raphinesse

I created this PR to replace the previous PR #459.

As discusses in #459, it does feel better to remove the Gradle command line argument usage in-favor of the gradle.properties file. This also eliminates the idea or need for creating extra Cordova arguments for setting jvmargs or potential other Gradle property overrides.

The user can instead change the values in gradle.properties.

Please let me know what you think of this over the other PR.

Also, this PR is linked to a new ticket, https://issues.apache.org/jira/browse/CB-14201, as the previous ticket only focused on jvmargs. This PR will affect all Gradle properties that we current set. (org.gradle.daemon, org.gradle.jvmargs, android.useDeprecatedNdk)

[codecov-io]

Codecov Report

Merging #465 into master will increase coverage by 0.22%.
The diff coverage is 78.78%.

@@            Coverage Diff             @@
##           master     #465      +/-   ##
==========================================
+ Coverage   61.64%   61.87%   +0.22%     
==========================================
  Files          16       17       +1     
  Lines        1945     1975      +30     
  Branches      363      366       +3     
==========================================
+ Hits         1199     1222      +23     
- Misses        746      753       +7

Impacted Files	Coverage Δ
...n/templates/cordova/lib/builders/ProjectBuilder.js	44.55% <100%> (-0.57%)	⬇️
bin/templates/cordova/lib/prepare.js	41.59% <33.33%> (-0.08%)	⬇️
...lates/cordova/lib/config/GradlePropertiesParser.js	82.75% <82.75%> (ø)

---

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 23b2449...dacb0e5. Read the comment docs.

[janpio]

Nice.

Did I get this right:
You removed the previous hardcoded default config values and created a gradle.properties parser that uses the old defaults to create the file if it is missing, but can also read and supply lots of other configs that weren't there before. Correct?

[erisu]

Partial,

Yes, The hardcoded defaults were removed so they can now be overridden by the user from the gradle.properties.

Additional Note: Overridden defaults are accepted, not reverted, but the user will see an info message emitted with the Cordova's recommended value.

Yes, the old defaults are used for creating the file. Since the first step for a new project is cordova platform add, it will naturally run prepare right after. This will create the new gradle.properties file with the original defaults and values that were previously hard-coded.

The parser can read other non-default properties but is only used for the original defaults.

As for supplying a lot of other configs, it was already possible by creating this file manually since Gradle naturally detects and uses the properties file. This PR main goal was removing the three hardcoded command-line arguments so it won't be locked in.

Here are also some additional use cases:
Use Case 1: For existing projects that are missing the gradle.properties file, it will be created during the prepare step. Either triggered by cordova (prepare|run|build).

Use Case 2: Existing projects with existing gradle.properties file will check and add missing defaults.

And lastly, possible drawbacks, but not serious:
1. If the user deletes a default, it will be re-appended. In this case, the user would just need to negate its value or take the default value listed by Gradle's docs if they don't want it or modify it.

2. (Possibly a Very Small Edge Case) For existing projects, with no gradle.properties file, if only running cordova compile without preparing, builds will not run with any defaults. During local testing, with no gradle.properties file builds continue to build successfully.

[knight9999]

I feel that these changes are OK.

Even recently, I needed to be able to stop the daemon but because it was hardcoded it was not simple.

@dpogue Do you have any opinions on this? I would like to merge this in if there is nothing holding this up.

[dpogue]

👍 from me

[ebhsgit]

@erisu
Hi, I am looking to increase the JVM heap size by overwrite the default org.gradle.jvmargs value.

I understand that with this PR, the setting can now be changed in the gradle.properties file created as part of the cordova build process. And to change the default values, the user needs to create their own custom gradle.properties.

It is not clear to me where to put my custom gradle.properties file (in my source directory) so that the build process will pick up the file and overwrite the default file.