Improve Gradle Build Arguments (#699)

* Remove `uses-sdk` from AndroidManifest
* Remove dependency `elementtree`
* Updated Build Command Help Menu Printout
  * Cleanup `minSdkVersion` printout
  * Added  `maxSdkVersion`, but not recommended to set.
  * Added `targetSdkVersion`
* Update the `GradlePropertiesParser` & Test Spec
  * Always Set Overriding Changes
  * Update existing properties
  * Update configure method
This commit is contained in:
エリス 2019-04-06 13:28:25 +09:00 committed by GitHub
parent 516c3411aa
commit 485e6e0e4d
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
15 changed files with 104 additions and 127 deletions

View File

@ -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');

View File

@ -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);
}

View File

@ -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);
}
@ -265,7 +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(' \'--minSdkVersion=#\': Override minSdkVersion for this build.');
console.log(' \'--maxSdkVersion=#\': Override maxSdkVersion for this build. (Not Recommended)');
console.log(' \'--targetSdkVersion=#\': Override targetSdkVersion for this build.');
console.log(' \'--gradleArg=<gradle command line arg>\': 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) :');

View File

@ -84,10 +84,20 @@ 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 (value !== properties[key]) {
events.emit('info', `[Gradle Properties] Detected Gradle property "${key}" with the value of "${value}", Cordova's recommended value is "${properties[key]}"`);
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]);
}
});
}

View File

@ -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

View File

@ -44,6 +44,4 @@
</intent-filter>
</activity>
</application>
<uses-sdk android:minSdkVersion="19" android:targetSdkVersion="__APILEVEL__"/>
</manifest>

View File

@ -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 {

View File

@ -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 {

View File

@ -19,5 +19,4 @@
-->
<manifest xmlns:android="http://schemas.android.com/apk/res/android"
package="org.apache.cordova" android:versionName="1.0" android:versionCode="1">
<uses-sdk android:minSdkVersion="19" />
</manifest>

View File

@ -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'

View File

@ -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",

View File

@ -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 = `<?xml version='1.0' encoding='utf-8'?>
<manifest android:hardwareAccelerated="true" android:versionCode="${VERSION_CODE}" android:versionName="${VERSION_NAME}"
@ -51,7 +48,6 @@ describe('AndroidManifest', () => {
</intent-filter>
</activity>
</application>
<uses-sdk android:minSdkVersion="${MIN_SDK_VERSION}" android:maxSdkVersion="${MAX_SDK_VERSION}" android:targetSdkVersion="${TARGET_SDK_VERSION}" />
</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);

View File

@ -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 to match default and set.', () => {
let setSpy = jasmine.createSpy('set');
let getSpy = jasmine.createSpy('get').and.returnValue('-Xmx512m');
@ -133,7 +133,23 @@ describe('Gradle Builder', () => {
parser._configureProperties(parser._defaults);
expect(getSpy).toHaveBeenCalled();
expect(setSpy).not.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');
});
});

View File

@ -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());

View File

@ -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:/));