Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve Gradle Build Arguments #699

Merged
merged 8 commits into from
Apr 6, 2019
Merged
Show file tree
Hide file tree
Changes from 7 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion bin/lib/create.js
Original file line number Diff line number Diff line change
Expand Up @@ -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');
Expand Down
30 changes: 0 additions & 30 deletions bin/templates/cordova/lib/AndroidManifest.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@
*/

var fs = require('fs');
var et = require('elementtree');
var xml = require('cordova-common').xmlHelpers;

var DEFAULT_ORIENTATION = 'default';
Expand Down Expand Up @@ -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';
};
Expand Down Expand Up @@ -150,7 +124,3 @@ AndroidManifest.prototype.write = function (destPath) {
};

module.exports = AndroidManifest;

function capitalize (str) {
return str.charAt(0).toUpperCase() + str.slice(1);
}
8 changes: 7 additions & 1 deletion bin/templates/cordova/lib/build.js
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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);
}
Expand Down Expand Up @@ -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.');
erisu marked this conversation as resolved.
Show resolved Hide resolved
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) :');
Expand Down
12 changes: 11 additions & 1 deletion bin/templates/cordova/lib/config/GradlePropertiesParser.js
Original file line number Diff line number Diff line change
Expand Up @@ -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]);
}
});
}
Expand Down
7 changes: 4 additions & 3 deletions bin/templates/cordova/lib/prepare.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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
Expand Down
2 changes: 0 additions & 2 deletions bin/templates/project/AndroidManifest.xml
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,4 @@
</intent-filter>
</activity>
</application>

<uses-sdk android:minSdkVersion="19" android:targetSdkVersion="__APILEVEL__"/>
</manifest>
35 changes: 31 additions & 4 deletions bin/templates/project/app/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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 ? (
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

compileSdkVersion should probably default to targetSdkVersion, unless it is explicitly set

defaultCompileSdkVersion == null
? privateHelpers.getProjectTarget()
: defaultCompileSdkVersion
) : Integer.parseInt('' + cdvCompileSdkVersion);

if (ext.cdvBuildToolsVersion == null) {
ext.cdvBuildToolsVersion = privateHelpers.findLatestInstalledBuildTools()
//ext.cdvBuildToolsVersion = project.ext.defaultBuildToolsVersion
Expand All @@ -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) {
Expand Down Expand Up @@ -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)
Expand All @@ -170,6 +189,14 @@ android {
if (cdvMinSdkVersion != null) {
minSdkVersion cdvMinSdkVersion
}

if (cdvMaxSdkVersion != null) {
maxSdkVersion cdvMaxSdkVersion
}

if(cdvTargetSdkVersion != null) {
targetSdkVersion cdvTargetSdkVersion
}
}

lintOptions {
Expand Down
23 changes: 23 additions & 0 deletions bin/templates/project/legacy/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -170,6 +185,14 @@ android {
if (cdvMinSdkVersion != null) {
minSdkVersion cdvMinSdkVersion
}

if (cdvMaxSdkVersion != null) {
maxSdkVersion cdvMaxSdkVersion
}

if(cdvTargetSdkVersion != null) {
targetSdkVersion cdvTargetSdkVersion
}
}

lintOptions {
Expand Down
1 change: 0 additions & 1 deletion framework/AndroidManifest.xml
Original file line number Diff line number Diff line change
Expand Up @@ -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>
5 changes: 5 additions & 0 deletions framework/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Expand Down
1 change: 0 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
76 changes: 0 additions & 76 deletions spec/unit/AndroidManifest.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -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}"
Expand All @@ -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`);
Expand Down Expand Up @@ -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);
Expand Down
Loading