From c95b0a1aa7a615ad88d541b7f60bef869093a160 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E3=82=A8=E3=83=AA=E3=82=B9?= Date: Wed, 27 Mar 2019 16:27:21 +0900 Subject: [PATCH 1/8] Remove uses-sdk from AndroidManifest --- bin/lib/create.js | 1 - bin/templates/cordova/lib/AndroidManifest.js | 30 -------- bin/templates/cordova/lib/build.js | 6 ++ bin/templates/cordova/lib/prepare.js | 7 +- bin/templates/project/AndroidManifest.xml | 2 - bin/templates/project/app/build.gradle | 35 +++++++-- bin/templates/project/legacy/build.gradle | 23 ++++++ framework/AndroidManifest.xml | 1 - framework/build.gradle | 5 ++ spec/unit/AndroidManifest.spec.js | 76 -------------------- spec/unit/create.spec.js | 3 +- spec/unit/run.spec.js | 3 +- 12 files changed, 72 insertions(+), 120 deletions(-) diff --git a/bin/lib/create.js b/bin/lib/create.js index d6451a7568..3f5bcb1fdd 100755 --- a/bin/lib/create.js +++ b/bin/lib/create.js @@ -321,7 +321,6 @@ exports.create = function (project_path, config, options, events) { var manifest = new AndroidManifest(path.join(project_template_dir, 'AndroidManifest.xml')); manifest.setPackageId(package_name) - .setTargetSdkVersion(target_api.split('-')[1]) .getActivity().setName(safe_activity_name); var manifest_path = path.join(app_path, 'AndroidManifest.xml'); diff --git a/bin/templates/cordova/lib/AndroidManifest.js b/bin/templates/cordova/lib/AndroidManifest.js index 4fe1c2b13f..a4489f1f89 100644 --- a/bin/templates/cordova/lib/AndroidManifest.js +++ b/bin/templates/cordova/lib/AndroidManifest.js @@ -18,7 +18,6 @@ */ var fs = require('fs'); -var et = require('elementtree'); var xml = require('cordova-common').xmlHelpers; var DEFAULT_ORIENTATION = 'default'; @@ -98,31 +97,6 @@ AndroidManifest.prototype.getActivity = function () { }; }; -['minSdkVersion', 'maxSdkVersion', 'targetSdkVersion'].forEach(function (sdkPrefName) { - // Copy variable reference to avoid closure issues - var prefName = sdkPrefName; - - AndroidManifest.prototype['get' + capitalize(prefName)] = function () { - var usesSdk = this.doc.getroot().find('./uses-sdk'); - return usesSdk && usesSdk.attrib['android:' + prefName]; - }; - - AndroidManifest.prototype['set' + capitalize(prefName)] = function (prefValue) { - var usesSdk = this.doc.getroot().find('./uses-sdk'); - - if (!usesSdk && prefValue) { // if there is no required uses-sdk element, we should create it first - usesSdk = new et.Element('uses-sdk'); - this.doc.getroot().append(usesSdk); - } - - if (prefValue) { - usesSdk.attrib['android:' + prefName] = prefValue; - } - - return this; - }; -}); - AndroidManifest.prototype.getDebuggable = function () { return this.doc.getroot().find('./application').attrib['android:debuggable'] === 'true'; }; @@ -150,7 +124,3 @@ AndroidManifest.prototype.write = function (destPath) { }; module.exports = AndroidManifest; - -function capitalize (str) { - return str.charAt(0).toUpperCase() + str.slice(1); -} diff --git a/bin/templates/cordova/lib/build.js b/bin/templates/cordova/lib/build.js index 2f0ba690ed..89ba5b1416 100644 --- a/bin/templates/cordova/lib/build.js +++ b/bin/templates/cordova/lib/build.js @@ -38,6 +38,8 @@ function parseOpts (options, resolvedTarget, projectRoot) { prepenv: Boolean, versionCode: String, minSdkVersion: String, + maxSdkVersion: String, + targetSdkVersion: String, gradleArg: [String, Array], keystore: path, alias: String, @@ -56,6 +58,8 @@ function parseOpts (options, resolvedTarget, projectRoot) { if (options.argv.versionCode) { ret.extraArgs.push('-PcdvVersionCode=' + options.argv.versionCode); } if (options.argv.minSdkVersion) { ret.extraArgs.push('-PcdvMinSdkVersion=' + options.argv.minSdkVersion); } + if (options.argv.maxSdkVersion) { ret.extraArgs.push('-PcdvMaxSdkVersion=' + options.argv.maxSdkVersion); } + if (options.argv.targetSdkVersion) { ret.extraArgs.push('-PcdvTargetSdkVersion=' + options.argv.targetSdkVersion); } if (options.argv.gradleArg) { ret.extraArgs = ret.extraArgs.concat(options.argv.gradleArg); } @@ -266,6 +270,8 @@ module.exports.help = function () { console.log(' \'--prepenv\': don\'t build, but copy in build scripts where necessary'); console.log(' \'--versionCode=#\': Override versionCode for this build. Useful for uploading multiple APKs.'); console.log(' \'--minSdkVersion=#\': Override minSdkVersion for this build. Useful for uploading multiple APKs.'); + console.log(' \'--maxSdkVersion=#\': Override maxSdkVersion for this build. Useful for uploading multiple APKs.'); + console.log(' \'--targetSdkVersion=#\': Override targetSdkVersion for this build. Useful for uploading multiple APKs.'); console.log(' \'--gradleArg=\': Extra args to pass to the gradle command. Use one flag per arg. Ex. --gradleArg=-PcdvBuildMultipleApks=true'); console.log(''); console.log('Signed APK flags (overwrites debug/release-signing.proprties) :'); diff --git a/bin/templates/cordova/lib/prepare.js b/bin/templates/cordova/lib/prepare.js index 49e0a7d7c3..64970e8c05 100644 --- a/bin/templates/cordova/lib/prepare.js +++ b/bin/templates/cordova/lib/prepare.js @@ -45,9 +45,13 @@ module.exports.prepare = function (cordovaProject, options) { // Get the min SDK version from config.xml const minSdkVersion = this._config.getPreference('android-minSdkVersion', 'android'); + const maxSdkVersion = this._config.getPreference('android-maxSdkVersion', 'android'); + const targetSdkVersion = this._config.getPreference('android-targetSdkVersion', 'android'); let gradlePropertiesUserConfig = {}; if (minSdkVersion) gradlePropertiesUserConfig.cdvMinSdkVersion = minSdkVersion; + if (maxSdkVersion) gradlePropertiesUserConfig.cdvMaxSdkVersion = maxSdkVersion; + if (targetSdkVersion) gradlePropertiesUserConfig.cdvTargetSdkVersion = targetSdkVersion; let gradlePropertiesParser = new GradlePropertiesParser(this.locations.root); gradlePropertiesParser.configure(gradlePropertiesUserConfig); @@ -205,9 +209,6 @@ function updateProjectAccordingTo (platformConfig, locations) { manifest.setVersionName(platformConfig.version()) .setVersionCode(platformConfig.android_versionCode() || default_versionCode(platformConfig.version())) .setPackageId(androidPkgName) - .setMinSdkVersion(platformConfig.getPreference('android-minSdkVersion', 'android')) - .setMaxSdkVersion(platformConfig.getPreference('android-maxSdkVersion', 'android')) - .setTargetSdkVersion(platformConfig.getPreference('android-targetSdkVersion', 'android')) .write(); // Java file paths shouldn't be hard coded diff --git a/bin/templates/project/AndroidManifest.xml b/bin/templates/project/AndroidManifest.xml index 9c36963a9e..9e259506d5 100644 --- a/bin/templates/project/AndroidManifest.xml +++ b/bin/templates/project/AndroidManifest.xml @@ -44,6 +44,4 @@ - - diff --git a/bin/templates/project/app/build.gradle b/bin/templates/project/app/build.gradle index bfad374a4c..ea26195fc6 100644 --- a/bin/templates/project/app/build.gradle +++ b/bin/templates/project/app/build.gradle @@ -64,6 +64,14 @@ ext { if (!project.hasProperty('cdvMinSdkVersion')) { cdvMinSdkVersion = null } + // Sets the maxSdkVersion to the given value. + if (!project.hasProperty('cdvMaxSdkVersion')) { + cdvMaxSdkVersion = null + } + // The value for android.targetSdkVersion. + if (!project.hasProperty('cdvTargetSdkVersion')) { + cdvTargetSdkVersion = null; + } // Whether to build architecture-specific APKs. if (!project.hasProperty('cdvBuildMultipleApks')) { cdvBuildMultipleApks = null @@ -103,10 +111,12 @@ if (hasBuildExtras2) { } // Set property defaults after extension .gradle files. -if (ext.cdvCompileSdkVersion == null) { - ext.cdvCompileSdkVersion = privateHelpers.getProjectTarget() - //ext.cdvCompileSdkVersion = project.ext.defaultCompileSdkVersion -} +ext.cdvCompileSdkVersion = cdvCompileSdkVersion == null ? ( + defaultCompileSdkVersion == null + ? privateHelpers.getProjectTarget() + : defaultCompileSdkVersion +) : Integer.parseInt('' + cdvCompileSdkVersion); + if (ext.cdvBuildToolsVersion == null) { ext.cdvBuildToolsVersion = privateHelpers.findLatestInstalledBuildTools() //ext.cdvBuildToolsVersion = project.ext.defaultBuildToolsVersion @@ -121,7 +131,14 @@ if (ext.cdvReleaseSigningPropertiesFile == null && file('../release-signing.prop // Cast to appropriate types. ext.cdvBuildMultipleApks = cdvBuildMultipleApks == null ? false : cdvBuildMultipleApks.toBoolean(); ext.cdvVersionCodeForceAbiDigit = cdvVersionCodeForceAbiDigit == null ? false : cdvVersionCodeForceAbiDigit.toBoolean(); + +// minSdkVersion, maxSdkVersion and targetSdkVersion ext.cdvMinSdkVersion = cdvMinSdkVersion == null ? defaultMinSdkVersion : Integer.parseInt('' + cdvMinSdkVersion) +if (cdvMaxSdkVersion != null) { + ext.cdvMaxSdkVersion = Integer.parseInt('' + cdvMaxSdkVersion) +} +ext.cdvTargetSdkVersion = cdvTargetSdkVersion == null ? defaultTargetSdkVersion : Integer.parseInt('' + cdvTargetSdkVersion) + ext.cdvVersionCode = cdvVersionCode == null ? null : Integer.parseInt('' + cdvVersionCode) def computeBuildTargetName(debugBuild) { @@ -151,6 +168,8 @@ task cdvPrintProps { println('cdvVersionCode=' + cdvVersionCode) println('cdvVersionCodeForceAbiDigit=' + cdvVersionCodeForceAbiDigit) println('cdvMinSdkVersion=' + cdvMinSdkVersion) + println('cdvMaxSdkVersion=' + cdvMaxSdkVersion) + println('cdvTargetSdkVersion=' + cdvTargetSdkVersion) println('cdvBuildMultipleApks=' + cdvBuildMultipleApks) println('cdvReleaseSigningPropertiesFile=' + cdvReleaseSigningPropertiesFile) println('cdvDebugSigningPropertiesFile=' + cdvDebugSigningPropertiesFile) @@ -170,6 +189,14 @@ android { if (cdvMinSdkVersion != null) { minSdkVersion cdvMinSdkVersion } + + if (cdvMaxSdkVersion != null) { + maxSdkVersion cdvMaxSdkVersion + } + + if(cdvTargetSdkVersion != null) { + targetSdkVersion cdvTargetSdkVersion + } } lintOptions { diff --git a/bin/templates/project/legacy/build.gradle b/bin/templates/project/legacy/build.gradle index 2b48a75520..f97c734606 100644 --- a/bin/templates/project/legacy/build.gradle +++ b/bin/templates/project/legacy/build.gradle @@ -66,6 +66,14 @@ ext { if (!project.hasProperty('cdvMinSdkVersion')) { cdvMinSdkVersion = null } + // Sets the maxSdkVersion to the given value. + if (!project.hasProperty('cdvMaxSdkVersion')) { + cdvMaxSdkVersion = null + } + // The value for android.targetSdkVersion. + if (!project.hasProperty('cdvTargetSdkVersion')) { + cdvTargetSdkVersion = null; + } // Whether to build architecture-specific APKs. if (!project.hasProperty('cdvBuildMultipleApks')) { cdvBuildMultipleApks = null @@ -112,6 +120,11 @@ if (ext.cdvReleaseSigningPropertiesFile == null && file('release-signing.propert // Cast to appropriate types. ext.cdvBuildMultipleApks = cdvBuildMultipleApks == null ? false : cdvBuildMultipleApks.toBoolean(); ext.cdvMinSdkVersion = cdvMinSdkVersion == null ? null : Integer.parseInt('' + cdvMinSdkVersion) + +if(cdvMaxSdkVersion != null) { + ext.cdvMaxSdkVersion = Integer.parseInt('' + cdvMaxSdkVersion) +} + ext.cdvVersionCode = cdvVersionCode == null ? null : Integer.parseInt('' + cdvVersionCode) def computeBuildTargetName(debugBuild) { @@ -139,6 +152,8 @@ task cdvPrintProps << { println('cdvBuildToolsVersion=' + cdvBuildToolsVersion) println('cdvVersionCode=' + cdvVersionCode) println('cdvMinSdkVersion=' + cdvMinSdkVersion) + println('cdvMaxSdkVersion=' + cdvMaxSdkVersion) + println('cdvTargetSdkVersion=' + cdvTargetSdkVersion) println('cdvBuildMultipleApks=' + cdvBuildMultipleApks) println('cdvReleaseSigningPropertiesFile=' + cdvReleaseSigningPropertiesFile) println('cdvDebugSigningPropertiesFile=' + cdvDebugSigningPropertiesFile) @@ -170,6 +185,14 @@ android { if (cdvMinSdkVersion != null) { minSdkVersion cdvMinSdkVersion } + + if (cdvMaxSdkVersion != null) { + maxSdkVersion cdvMaxSdkVersion + } + + if(cdvTargetSdkVersion != null) { + targetSdkVersion cdvTargetSdkVersion + } } lintOptions { diff --git a/framework/AndroidManifest.xml b/framework/AndroidManifest.xml index 1625b896bd..320c2538ec 100755 --- a/framework/AndroidManifest.xml +++ b/framework/AndroidManifest.xml @@ -19,5 +19,4 @@ --> - diff --git a/framework/build.gradle b/framework/build.gradle index 09b51ef4e3..c09fc3e39f 100644 --- a/framework/build.gradle +++ b/framework/build.gradle @@ -53,6 +53,11 @@ android { targetCompatibility JavaVersion.VERSION_1_8 } + // For the Android Cordova Lib, we will hardcode the minSdkVersion and not allow changes. + defaultConfig { + minSdkVersion 19 + } + sourceSets { main { manifest.srcFile 'AndroidManifest.xml' diff --git a/spec/unit/AndroidManifest.spec.js b/spec/unit/AndroidManifest.spec.js index 84c577b16f..aa083455ae 100644 --- a/spec/unit/AndroidManifest.spec.js +++ b/spec/unit/AndroidManifest.spec.js @@ -29,9 +29,6 @@ describe('AndroidManifest', () => { const ACTIVITY_LAUNCH_MODE = 'singleTop'; const ACTIVITY_NAME = 'MainActivity'; const ACTIVITY_ORIENTATION = 'portrait'; - const MIN_SDK_VERSION = '12'; - const MAX_SDK_VERSION = '88'; - const TARGET_SDK_VERSION = '27'; const DEFAULT_MANIFEST = ` { - `; const manifestPath = path.join(os.tmpdir(), `AndroidManifest${Date.now()}.xml`); @@ -190,78 +186,6 @@ describe('AndroidManifest', () => { }); }); - describe('minSdkVersion', () => { - it('should get minSdkVersion', () => { - expect(manifest.getMinSdkVersion()).toBe(MIN_SDK_VERSION); - }); - - it('should set minSdkVersion', () => { - const newMinSdkVersion = `${MIN_SDK_VERSION}111`; - manifest.setMinSdkVersion(newMinSdkVersion); - expect(manifest.getMinSdkVersion()).toBe(newMinSdkVersion); - }); - - it('should create the uses-sdk node if it does not exist when setting minSdkVersion', () => { - const root = manifest.doc.getroot(); - root.remove(root.find('./uses-sdk')); - - expect(root.find('./uses-sdk')).toBe(null); - - manifest.setMinSdkVersion(1); - - expect(root.find('./uses-sdk')).not.toBe(null); - expect(manifest.getMinSdkVersion()).toBe(1); - }); - }); - - describe('maxSdkVersion', () => { - it('should get maxSdkVersion', () => { - expect(manifest.getMaxSdkVersion()).toBe(MAX_SDK_VERSION); - }); - - it('should set maxSdkVersion', () => { - const newMaxSdkVersion = `${MAX_SDK_VERSION}999`; - manifest.setMaxSdkVersion(newMaxSdkVersion); - expect(manifest.getMaxSdkVersion()).toBe(newMaxSdkVersion); - }); - - it('should create the uses-sdk node if it does not exist when setting maxSdkVersion', () => { - const root = manifest.doc.getroot(); - root.remove(root.find('./uses-sdk')); - - expect(root.find('./uses-sdk')).toBe(null); - - manifest.setMaxSdkVersion(1); - - expect(root.find('./uses-sdk')).not.toBe(null); - expect(manifest.getMaxSdkVersion()).toBe(1); - }); - }); - - describe('targetSdkVersion', () => { - it('should get targetSdkVersion', () => { - expect(manifest.getTargetSdkVersion()).toBe(TARGET_SDK_VERSION); - }); - - it('should set targetSdkVersion', () => { - const newTargetSdkVersion = `${TARGET_SDK_VERSION}555`; - manifest.setTargetSdkVersion(newTargetSdkVersion); - expect(manifest.getTargetSdkVersion()).toBe(newTargetSdkVersion); - }); - - it('should create the uses-sdk node if it does not exist when setting targetSdkVersion', () => { - const root = manifest.doc.getroot(); - root.remove(root.find('./uses-sdk')); - - expect(root.find('./uses-sdk')).toBe(null); - - manifest.setTargetSdkVersion(1); - - expect(root.find('./uses-sdk')).not.toBe(null); - expect(manifest.getTargetSdkVersion()).toBe(1); - }); - }); - describe('debuggable', () => { it('should get debuggable', () => { expect(manifest.getDebuggable()).toBe(true); diff --git a/spec/unit/create.spec.js b/spec/unit/create.spec.js index 5b330588ad..661b2c4f77 100644 --- a/spec/unit/create.spec.js +++ b/spec/unit/create.spec.js @@ -131,9 +131,8 @@ describe('create', function () { var default_templates = path.join(__dirname, '..', '..', 'bin', 'templates', 'project'); var fake_android_target = 'android-1337'; beforeEach(function () { - Manifest_mock.prototype = jasmine.createSpyObj('AndroidManifest instance mock', ['setPackageId', 'setTargetSdkVersion', 'getActivity', 'setName', 'write']); + Manifest_mock.prototype = jasmine.createSpyObj('AndroidManifest instance mock', ['setPackageId', 'getActivity', 'setName', 'write']); Manifest_mock.prototype.setPackageId.and.returnValue(new Manifest_mock()); - Manifest_mock.prototype.setTargetSdkVersion.and.returnValue(new Manifest_mock()); Manifest_mock.prototype.getActivity.and.returnValue(new Manifest_mock()); Manifest_mock.prototype.setName.and.returnValue(new Manifest_mock()); spyOn(create, 'validatePackageName').and.returnValue(Q()); diff --git a/spec/unit/run.spec.js b/spec/unit/run.spec.js index 8b5108a3c7..836b5b2bcf 100644 --- a/spec/unit/run.spec.js +++ b/spec/unit/run.spec.js @@ -201,8 +201,9 @@ describe('run', () => { describe('help', () => { it('should print out usage and help', () => { const logSpy = jasmine.createSpy(); + const errorSpy = jasmine.createSpy(); const procStub = { exit: _ => null, cwd: _ => '', argv: ['', ''] }; - run.__set__({ console: { log: logSpy }, process: procStub }); + run.__set__({ console: { log: logSpy, error: errorSpy }, process: procStub }); run.help(); expect(logSpy).toHaveBeenCalledWith(jasmine.stringMatching(/^Usage:/)); From 42d3b4f2d1e0fe30064880a7a44537e4d51b20be Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E3=82=A8=E3=83=AA=E3=82=B9?= Date: Wed, 27 Mar 2019 20:11:25 +0900 Subject: [PATCH 2/8] Remove dependency elementtree --- package.json | 1 - 1 file changed, 1 deletion(-) diff --git a/package.json b/package.json index c77ca13085..5900e05b17 100644 --- a/package.json +++ b/package.json @@ -31,7 +31,6 @@ "dependencies": { "android-versions": "^1.3.0", "cordova-common": "^3.1.0", - "elementtree": "^0.1.7", "nopt": "^4.0.1", "properties-parser": "^0.3.1", "q": "^1.4.1", From ac1bc60de249473885b17b3758e83554c828b712 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E3=82=A8=E3=83=AA=E3=82=B9?= Date: Fri, 29 Mar 2019 11:14:11 +0900 Subject: [PATCH 3/8] Updated Build Command Help Menu Printout --- bin/templates/cordova/lib/build.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/bin/templates/cordova/lib/build.js b/bin/templates/cordova/lib/build.js index 89ba5b1416..ace9ed9f44 100644 --- a/bin/templates/cordova/lib/build.js +++ b/bin/templates/cordova/lib/build.js @@ -269,9 +269,9 @@ module.exports.help = function () { console.log(' \'--nobuild\': will skip build process (useful when using run command)'); console.log(' \'--prepenv\': don\'t build, but copy in build scripts where necessary'); console.log(' \'--versionCode=#\': Override versionCode for this build. Useful for uploading multiple APKs.'); - console.log(' \'--minSdkVersion=#\': Override minSdkVersion for this build. Useful for uploading multiple APKs.'); - console.log(' \'--maxSdkVersion=#\': Override maxSdkVersion for this build. Useful for uploading multiple APKs.'); - console.log(' \'--targetSdkVersion=#\': Override targetSdkVersion for this build. Useful for uploading multiple APKs.'); + console.log(' \'--minSdkVersion=#\': Override minSdkVersion for this build.'); + console.log(' \'--maxSdkVersion=#\': Override maxSdkVersion for this build.'); + console.log(' \'--targetSdkVersion=#\': Override targetSdkVersion for this build.'); console.log(' \'--gradleArg=\': Extra args to pass to the gradle command. Use one flag per arg. Ex. --gradleArg=-PcdvBuildMultipleApks=true'); console.log(''); console.log('Signed APK flags (overwrites debug/release-signing.proprties) :'); From ccf648b60cea5a67e07555623fa7dc9f2de6683e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E3=82=A8=E3=83=AA=E3=82=B9?= Date: Sun, 31 Mar 2019 18:41:42 +0900 Subject: [PATCH 4/8] Update Gradle Properties Parser to Always Set Overriding Changes --- .../cordova/lib/config/GradlePropertiesParser.js | 5 +++-- spec/unit/config/GradlePropertiesParser.spec.js | 8 ++++---- 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/bin/templates/cordova/lib/config/GradlePropertiesParser.js b/bin/templates/cordova/lib/config/GradlePropertiesParser.js index b45c0a674f..5536508cc6 100644 --- a/bin/templates/cordova/lib/config/GradlePropertiesParser.js +++ b/bin/templates/cordova/lib/config/GradlePropertiesParser.js @@ -86,8 +86,9 @@ class GradlePropertiesParser { if (!value) { events.emit('verbose', `[Gradle Properties] Appending configuration item: ${key}=${properties[key]}`); this.gradleFile.set(key, properties[key]); - } else if (value !== properties[key]) { - events.emit('info', `[Gradle Properties] Detected Gradle property "${key}" with the value of "${value}", Cordova's recommended value is "${properties[key]}"`); + } else if (this._defaults[key] && value !== properties[key]) { + events.emit('info', `[Gradle Properties] Detected Gradle property "${key}" with the value of "${value}", Cordova's recommended value is "${this._defaults[key]}"`); + this.gradleFile.set(key, properties[key]); } }); } diff --git a/spec/unit/config/GradlePropertiesParser.spec.js b/spec/unit/config/GradlePropertiesParser.spec.js index 4f4b9cbb02..eeb3023ece 100644 --- a/spec/unit/config/GradlePropertiesParser.spec.js +++ b/spec/unit/config/GradlePropertiesParser.spec.js @@ -106,7 +106,7 @@ describe('Gradle Builder', () => { expect(emitSpy.calls.argsFor(0)[1]).toContain('Appending configuration item'); }); - it('should not detect missing defaults and not call set.', () => { + it('should not detect missing defaults and call set.', () => { let setSpy = jasmine.createSpy('set'); let getSpy = jasmine.createSpy('get').and.returnValue(true); @@ -118,10 +118,10 @@ describe('Gradle Builder', () => { parser._configureProperties(parser._defaults); expect(getSpy).toHaveBeenCalled(); - expect(setSpy).not.toHaveBeenCalled(); + expect(setSpy).toHaveBeenCalled(); }); - it('should detect default with changed value.', () => { + it('should detect default with changed value and set.', () => { let setSpy = jasmine.createSpy('set'); let getSpy = jasmine.createSpy('get').and.returnValue('-Xmx512m'); @@ -133,7 +133,7 @@ describe('Gradle Builder', () => { parser._configureProperties(parser._defaults); expect(getSpy).toHaveBeenCalled(); - expect(setSpy).not.toHaveBeenCalled(); + expect(setSpy).toHaveBeenCalled(); expect(emitSpy.calls.argsFor(0)[1]).toContain('Cordova\'s recommended value is'); }); }); From 842851c4fee42c261cd57e163fef1572fd8f23cc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E3=82=A8=E3=83=AA=E3=82=B9?= Date: Mon, 1 Apr 2019 12:28:10 +0900 Subject: [PATCH 5/8] Add Update Exisiting Gradle Property Use Case --- .../cordova/lib/config/GradlePropertiesParser.js | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/bin/templates/cordova/lib/config/GradlePropertiesParser.js b/bin/templates/cordova/lib/config/GradlePropertiesParser.js index 5536508cc6..cefb3703a8 100644 --- a/bin/templates/cordova/lib/config/GradlePropertiesParser.js +++ b/bin/templates/cordova/lib/config/GradlePropertiesParser.js @@ -84,11 +84,17 @@ class GradlePropertiesParser { let value = this.gradleFile.get(key); if (!value) { + // Handles the case of adding missing defaults or new properties that are missing. events.emit('verbose', `[Gradle Properties] Appending configuration item: ${key}=${properties[key]}`); this.gradleFile.set(key, properties[key]); - } else if (this._defaults[key] && value !== properties[key]) { + } else if (this._defaults[key] && value !== this._defaults[key]) { + // This case will notify that the value of the property does not match Cordova's recommended value but still being set. events.emit('info', `[Gradle Properties] Detected Gradle property "${key}" with the value of "${value}", Cordova's recommended value is "${this._defaults[key]}"`); - this.gradleFile.set(key, properties[key]); + this.gradleFile.set(key, value); + } else if (value !== properties[key]) { + // When the current value exists but does not match the new value, the new value it set. + events.emit('verbose', `[Gradle Properties] Updating Gradle property "${key}" with the value of "${value}"`); + this.gradleFile.set(key, value); } }); } From 7f074b9b4300ca64eb665f167ef355153b491e2d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E3=82=A8=E3=83=AA=E3=82=B9?= Date: Mon, 1 Apr 2019 13:01:27 +0900 Subject: [PATCH 6/8] Update Gradle Properties Configure Method --- .../lib/config/GradlePropertiesParser.js | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/bin/templates/cordova/lib/config/GradlePropertiesParser.js b/bin/templates/cordova/lib/config/GradlePropertiesParser.js index cefb3703a8..59d9eac3a9 100644 --- a/bin/templates/cordova/lib/config/GradlePropertiesParser.js +++ b/bin/templates/cordova/lib/config/GradlePropertiesParser.js @@ -87,14 +87,17 @@ class GradlePropertiesParser { // Handles the case of adding missing defaults or new properties that are missing. events.emit('verbose', `[Gradle Properties] Appending configuration item: ${key}=${properties[key]}`); this.gradleFile.set(key, properties[key]); - } else if (this._defaults[key] && value !== this._defaults[key]) { - // This case will notify that the value of the property does not match Cordova's recommended value but still being set. - events.emit('info', `[Gradle Properties] Detected Gradle property "${key}" with the value of "${value}", Cordova's recommended value is "${this._defaults[key]}"`); - this.gradleFile.set(key, value); } else if (value !== properties[key]) { - // When the current value exists but does not match the new value, the new value it set. - events.emit('verbose', `[Gradle Properties] Updating Gradle property "${key}" with the value of "${value}"`); - this.gradleFile.set(key, value); + if (this._defaults[key] && this._defaults[key] !== properties[key]) { + // Since the value does not match default, we will notify the discrepancy with Cordova's recommended value. + events.emit('info', `[Gradle Properties] Detected Gradle property "${key}" with the value of "${properties[key]}", Cordova's recommended value is "${this._defaults[key]}"`); + } else { + // When the current value exists but does not match the new value or does matches the default key value, the new value it set. + events.emit('verbose', `[Gradle Properties] Updating Gradle property "${key}" with the value of "${properties[key]}"`); + } + + // We will set the new value in either case. + this.gradleFile.set(key, properties[key]); } }); } From fcc2ce6a7e6ca6973cda0fb7ddc50f2d363eb282 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E3=82=A8=E3=83=AA=E3=82=B9?= Date: Mon, 1 Apr 2019 14:18:22 +0900 Subject: [PATCH 7/8] Update GradlePropertiesParser Test Spec --- .../unit/config/GradlePropertiesParser.spec.js | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-) diff --git a/spec/unit/config/GradlePropertiesParser.spec.js b/spec/unit/config/GradlePropertiesParser.spec.js index eeb3023ece..0ce2df8b57 100644 --- a/spec/unit/config/GradlePropertiesParser.spec.js +++ b/spec/unit/config/GradlePropertiesParser.spec.js @@ -121,7 +121,7 @@ describe('Gradle Builder', () => { expect(setSpy).toHaveBeenCalled(); }); - it('should detect default with changed value and set.', () => { + it('should detect default with changed value to match default and set.', () => { let setSpy = jasmine.createSpy('set'); let getSpy = jasmine.createSpy('get').and.returnValue('-Xmx512m'); @@ -132,6 +132,22 @@ describe('Gradle Builder', () => { parser._configureProperties(parser._defaults); + expect(getSpy).toHaveBeenCalled(); + expect(setSpy).toHaveBeenCalled(); + expect(emitSpy.calls.argsFor(0)[1]).toContain('Updating Gradle property'); + }); + + it('should detect default with changed value different from default and set.', () => { + let setSpy = jasmine.createSpy('set'); + let getSpy = jasmine.createSpy('get').and.returnValue('-Xmx2048m'); + + parser.gradleFile = { + set: setSpy, + get: getSpy + }; + + parser._configureProperties({ 'org.gradle.jvmargs': '-Xmx512m' }); + expect(getSpy).toHaveBeenCalled(); expect(setSpy).toHaveBeenCalled(); expect(emitSpy.calls.argsFor(0)[1]).toContain('Cordova\'s recommended value is'); From 9f67d1f0b233d99634f984140989523d79e75576 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E3=82=A8=E3=83=AA=E3=82=B9?= Date: Fri, 5 Apr 2019 08:35:42 +0900 Subject: [PATCH 8/8] Update Build Help Printout for maxSdkVersion Flag For flexibility, `maxSdkVersion` is an available option to users but not recommended to set. --- bin/templates/cordova/lib/build.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bin/templates/cordova/lib/build.js b/bin/templates/cordova/lib/build.js index ace9ed9f44..dec218ea00 100644 --- a/bin/templates/cordova/lib/build.js +++ b/bin/templates/cordova/lib/build.js @@ -270,7 +270,7 @@ module.exports.help = function () { console.log(' \'--prepenv\': don\'t build, but copy in build scripts where necessary'); console.log(' \'--versionCode=#\': Override versionCode for this build. Useful for uploading multiple APKs.'); console.log(' \'--minSdkVersion=#\': Override minSdkVersion for this build.'); - console.log(' \'--maxSdkVersion=#\': Override maxSdkVersion for this build.'); + console.log(' \'--maxSdkVersion=#\': Override maxSdkVersion for this build. (Not Recommended)'); console.log(' \'--targetSdkVersion=#\': Override targetSdkVersion for this build.'); console.log(' \'--gradleArg=\': Extra args to pass to the gradle command. Use one flag per arg. Ex. --gradleArg=-PcdvBuildMultipleApks=true'); console.log('');