From dacb0e5c721a7476b767261433fb96e7a7b6f088 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E3=82=A8=E3=83=AA=E3=82=B9?= Date: Wed, 11 Jul 2018 12:14:04 +0900 Subject: [PATCH] CB-14201: (android) Removes Gradle property in-line command arguments for gradle.properties --- .../cordova/lib/builders/ProjectBuilder.js | 13 +- .../lib/config/GradlePropertiesParser.js | 97 ++++++++++ bin/templates/cordova/lib/prepare.js | 5 + .../config/GradlePropertiesParser.spec.js | 167 ++++++++++++++++++ 4 files changed, 273 insertions(+), 9 deletions(-) create mode 100644 bin/templates/cordova/lib/config/GradlePropertiesParser.js create mode 100644 spec/unit/config/GradlePropertiesParser.spec.js diff --git a/bin/templates/cordova/lib/builders/ProjectBuilder.js b/bin/templates/cordova/lib/builders/ProjectBuilder.js index b000de3e07..24aef07cd0 100644 --- a/bin/templates/cordova/lib/builders/ProjectBuilder.js +++ b/bin/templates/cordova/lib/builders/ProjectBuilder.js @@ -46,20 +46,15 @@ class ProjectBuilder { } else if (cmd === 'debug') { cmd = 'cdvBuildDebug'; } - var args = [cmd, '-b', path.join(this.root, 'build.gradle')]; + + let args = [cmd, '-b', path.join(this.root, 'build.gradle')]; + if (opts.arch) { args.push('-PcdvBuildArch=' + opts.arch); } - // 10 seconds -> 6 seconds - args.push('-Dorg.gradle.daemon=true'); - // to allow dex in process - args.push('-Dorg.gradle.jvmargs=-Xmx2048m'); - // allow NDK to be used - required by Gradle 1.5 plugin - // args.push('-Pandroid.useDeprecatedNdk=true'); args.push.apply(args, opts.extraArgs); - // Shaves another 100ms, but produces a "try at own risk" warning. Not worth it (yet): - // args.push('-Dorg.gradle.parallel=true'); + return args; } diff --git a/bin/templates/cordova/lib/config/GradlePropertiesParser.js b/bin/templates/cordova/lib/config/GradlePropertiesParser.js new file mode 100644 index 0000000000..ac7f386346 --- /dev/null +++ b/bin/templates/cordova/lib/config/GradlePropertiesParser.js @@ -0,0 +1,97 @@ +/** + Licensed to the Apache Software Foundation (ASF) under one + or more contributor license agreements. See the NOTICE file + distributed with this work for additional information + regarding copyright ownership. The ASF licenses this file + to you under the Apache License, Version 2.0 (the + "License"); you may not use this file except in compliance + with the License. You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, + software distributed under the License is distributed on an + "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + KIND, either express or implied. See the License for the + specific language governing permissions and limitations + under the License. +*/ + +let fs = require('fs'); +let path = require('path'); +let propertiesParser = require('properties-parser'); +let events = require('cordova-common').events; + +class GradlePropertiesParser { + /** + * Loads and Edits Gradle Properties File. + * + * @param {String} platformDir is the path of the Android platform directory + */ + constructor (platformDir) { + this._defaults = { + // 10 seconds -> 6 seconds + 'org.gradle.daemon': 'true', + + // to allow dex in process + 'org.gradle.jvmargs': '-Xmx2048m', + + // allow NDK to be used - required by Gradle 1.5 plugin + 'android.useDeprecatedNdk': 'true' + + // Shaves another 100ms, but produces a "try at own risk" warning. Not worth it (yet): + // 'org.gradle.parallel': 'true' + }; + + this.gradleFilePath = path.join(platformDir, 'gradle.properties'); + } + + configure () { + events.emit('verbose', '[Gradle Properties] Preparing Configuration'); + + this._initializeEditor(); + this._configureDefaults(); + this._save(); + } + + /** + * Initialize the properties editor for parsing, setting, etc. + */ + _initializeEditor () { + // Touch empty gradle.properties file if missing. + if (!fs.existsSync(this.gradleFilePath)) { + events.emit('verbose', '[Gradle Properties] File missing, creating file with Cordova defaults.'); + fs.writeFileSync(this.gradleFilePath, '', 'utf-8'); + } + + // Create an editor for parsing, getting, and setting configurations. + this.gradleFile = propertiesParser.createEditor(this.gradleFilePath); + } + + /** + * Validate that defaults are set and set the missing defaults. + */ + _configureDefaults () { + // Loop though Cordova default properties and set only if missing. + Object.keys(this._defaults).forEach(key => { + let value = this.gradleFile.get(key); + + if (!value) { + events.emit('verbose', `[Gradle Properties] Appended missing default: ${key}=${this._defaults[key]}`); + this.gradleFile.set(key, this._defaults[key]); + } else if (value !== this._defaults[key]) { + events.emit('info', `[Gradle Properties] Detected Gradle property "${key}" with the value of "${value}", Cordova's recommended value is "${this._defaults[key]}"`); + } + }); + } + + /** + * Saves any changes that has been made to the properties file. + */ + _save () { + events.emit('verbose', '[Gradle Properties] Updating and Saving File'); + this.gradleFile.save(); + } +} + +module.exports = GradlePropertiesParser; diff --git a/bin/templates/cordova/lib/prepare.js b/bin/templates/cordova/lib/prepare.js index 9dc85ff3af..54aa97d1ed 100644 --- a/bin/templates/cordova/lib/prepare.js +++ b/bin/templates/cordova/lib/prepare.js @@ -33,6 +33,8 @@ var PlatformJson = require('cordova-common').PlatformJson; var PlatformMunger = require('cordova-common').ConfigChanges.PlatformMunger; var PluginInfoProvider = require('cordova-common').PluginInfoProvider; +const GradlePropertiesParser = require('./config/GradlePropertiesParser'); + module.exports.prepare = function (cordovaProject, options) { var self = this; @@ -41,6 +43,9 @@ module.exports.prepare = function (cordovaProject, options) { this._config = updateConfigFilesFrom(cordovaProject.projectConfig, munger, this.locations); + let gradlePropertiesParser = new GradlePropertiesParser(this.locations.root); + gradlePropertiesParser.configure(); + // Update own www dir with project's www assets and plugins' assets and js-files return Q.when(updateWww(cordovaProject, this.locations)).then(function () { // update project according to config.xml changes. diff --git a/spec/unit/config/GradlePropertiesParser.spec.js b/spec/unit/config/GradlePropertiesParser.spec.js new file mode 100644 index 0000000000..a9f78905f9 --- /dev/null +++ b/spec/unit/config/GradlePropertiesParser.spec.js @@ -0,0 +1,167 @@ +/** + Licensed to the Apache Software Foundation (ASF) under one + or more contributor license agreements. See the NOTICE file + distributed with this work for additional information + regarding copyright ownership. The ASF licenses this file + to you under the Apache License, Version 2.0 (the + "License"); you may not use this file except in compliance + with the License. You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, + software distributed under the License is distributed on an + "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + KIND, either express or implied. See the License for the + specific language governing permissions and limitations + under the License. +*/ + +const rewire = require('rewire'); +let GradlePropertiesParser = rewire('../../../bin/templates/cordova/lib/config/GradlePropertiesParser'); + +describe('Gradle Builder', () => { + describe('_initializeEditor method', () => { + let parser; + let writeFileSyncSpy; + let existsSyncSpy; + let createEditorSpy; + let emitSpy; + + beforeEach(() => { + createEditorSpy = jasmine.createSpy('createEditor'); + emitSpy = jasmine.createSpy('emit'); + + GradlePropertiesParser.__set__('propertiesParser', { + createEditor: createEditorSpy + }); + + GradlePropertiesParser.__set__('events', { + emit: emitSpy + }); + + parser = new GradlePropertiesParser('/root'); + }); + + it('should not detect an existing gradle.properties file and create new file', () => { + existsSyncSpy = jasmine.createSpy('existsSync').and.returnValue(false); + writeFileSyncSpy = jasmine.createSpy('writeFileSync'); + GradlePropertiesParser.__set__('fs', { + existsSync: existsSyncSpy, + writeFileSync: writeFileSyncSpy + }); + + parser._initializeEditor(); + + expect(emitSpy).toHaveBeenCalled(); + expect(emitSpy.calls.argsFor(0)[1]).toContain('File missing, creating file with Cordova defaults'); + expect(writeFileSyncSpy).toHaveBeenCalled(); + expect(createEditorSpy).toHaveBeenCalled(); + }); + + it('should detect an existing gradle.properties file', () => { + existsSyncSpy = jasmine.createSpy('existsSync').and.returnValue(true); + writeFileSyncSpy = jasmine.createSpy('writeFileSync'); + GradlePropertiesParser.__set__('fs', { + existsSync: existsSyncSpy, + writeFileSync: writeFileSyncSpy + }); + + parser._initializeEditor(); + + expect(writeFileSyncSpy).not.toHaveBeenCalled(); + expect(createEditorSpy).toHaveBeenCalled(); + }); + }); + + describe('_configureDefaults method', () => { + let parser; + let emitSpy; + + beforeEach(() => { + emitSpy = jasmine.createSpy('emit'); + + GradlePropertiesParser.__set__('events', { + emit: emitSpy + }); + + parser = new GradlePropertiesParser('/root'); + + parser._defaults = {'org.gradle.jvmargs': '-Xmx2048m'}; + }); + + it('should detect missing default property and sets the property.', () => { + let setSpy = jasmine.createSpy('set'); + let getSpy = jasmine.createSpy('get').and.returnValue(false); + + parser.gradleFile = { + set: setSpy, + get: getSpy + }; + + parser._configureDefaults(); + + expect(getSpy).toHaveBeenCalled(); + expect(setSpy).toHaveBeenCalled(); + expect(emitSpy.calls.argsFor(0)[1]).toContain('Appended missing default'); + }); + + it('should not detect missing defaults and not call set.', () => { + let setSpy = jasmine.createSpy('set'); + let getSpy = jasmine.createSpy('get').and.returnValue(true); + + parser.gradleFile = { + set: setSpy, + get: getSpy + }; + + parser._configureDefaults(); + + expect(getSpy).toHaveBeenCalled(); + expect(setSpy).not.toHaveBeenCalled(); + }); + + it('should detect default with changed value.', () => { + let setSpy = jasmine.createSpy('set'); + let getSpy = jasmine.createSpy('get').and.returnValue('-Xmx512m'); + + parser.gradleFile = { + set: setSpy, + get: getSpy + }; + + parser._configureDefaults(); + + expect(getSpy).toHaveBeenCalled(); + expect(setSpy).not.toHaveBeenCalled(); + expect(emitSpy.calls.argsFor(0)[1]).toContain('Cordova\'s recommended value is'); + }); + }); + + describe('_save method', () => { + let parser; + let emitSpy; + let saveSpy; + + beforeEach(() => { + emitSpy = jasmine.createSpy('emit'); + GradlePropertiesParser.__set__('events', { + emit: emitSpy + }); + + parser = new GradlePropertiesParser('/root'); + + saveSpy = jasmine.createSpy('save'); + parser.gradleFile = { + save: saveSpy + }; + }); + + it('should detect save being called.', () => { + parser._save(); + + expect(saveSpy).toHaveBeenCalled(); + expect(emitSpy.calls.argsFor(0)[1]).toContain('Updating and Saving File'); + }); + }); +});