This is an automated email from the ASF dual-hosted git repository.
normanbreau pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/cordova-android.git
The following commit(s) were added to refs/heads/master by this push:
new 3343c3bb fix: Gradle Args parsing (#1606)
3343c3bb is described below
commit 3343c3bb3490563d96740df84c60fa06d3662970
Author: Norman Breau <[email protected]>
AuthorDate: Sat Apr 22 17:00:51 2023 -0300
fix: Gradle Args parsing (#1606)
* fix: Gradle Args parsing
* refactor: Applied ARGVParser.parseArgsStringToArgv ->
parseArgsStringToArgv suggestion
* test: Added deeper testing for gradle argument parsing
---
lib/build.js | 8 ++-
lib/builders/ProjectBuilder.js | 16 +++--
package-lock.json | 14 ++++
package.json | 1 +
spec/unit/build.spec.js | 116 ++++++++++++++++++++++++++++++
spec/unit/builders/ProjectBuilder.spec.js | 18 ++---
6 files changed, 158 insertions(+), 15 deletions(-)
diff --git a/lib/build.js b/lib/build.js
index ad71093b..7e42f280 100644
--- a/lib/build.js
+++ b/lib/build.js
@@ -21,6 +21,7 @@ const path = require('path');
const fs = require('fs');
const nopt = require('nopt');
const untildify = require('untildify');
+const { parseArgsStringToArgv } = require('string-argv');
const Adb = require('./Adb');
@@ -36,7 +37,11 @@ function parseOpts (options, resolvedTarget, projectRoot) {
minSdkVersion: String,
maxSdkVersion: String,
targetSdkVersion: String,
+
+ // This needs to be an array since nopts will parse its entries as
further options for this process
+ // It will be an array of 1 string: [ "string args" ]
gradleArg: [String, Array],
+
keystore: path,
alias: String,
storePassword: String,
@@ -58,7 +63,8 @@ function parseOpts (options, resolvedTarget, projectRoot) {
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);
+ const gradleArgs = parseArgsStringToArgv(options.argv.gradleArg[0]);
+ ret.extraArgs = ret.extraArgs.concat(gradleArgs);
}
const packageArgs = {};
diff --git a/lib/builders/ProjectBuilder.js b/lib/builders/ProjectBuilder.js
index 4410cd5b..815d08f3 100644
--- a/lib/builders/ProjectBuilder.js
+++ b/lib/builders/ProjectBuilder.js
@@ -85,7 +85,13 @@ class ProjectBuilder {
}
getArgs (cmd, opts) {
- let args;
+ let args = [
+ '-b', path.join(this.root, 'build.gradle')
+ ];
+ if (opts.extraArgs) {
+ args = args.concat(opts.extraArgs);
+ }
+
let buildCmd = cmd;
if (opts.packageType === PackageType.BUNDLE) {
if (cmd === 'release') {
@@ -93,8 +99,6 @@ class ProjectBuilder {
} else if (cmd === 'debug') {
buildCmd = ':app:bundleDebug';
}
-
- args = [buildCmd, '-b', path.join(this.root, 'build.gradle')];
} else {
if (cmd === 'release') {
buildCmd = 'cdvBuildRelease';
@@ -102,14 +106,12 @@ class ProjectBuilder {
buildCmd = 'cdvBuildDebug';
}
- args = [buildCmd, '-b', path.join(this.root, 'build.gradle')];
-
if (opts.arch) {
args.push('-PcdvBuildArch=' + opts.arch);
}
}
- args.push.apply(args, opts.extraArgs);
+ args.push(buildCmd);
return args;
}
@@ -318,6 +320,8 @@ class ProjectBuilder {
const wrapper = path.join(this.root, 'gradlew');
const args = this.getArgs(opts.buildType === 'debug' ? 'debug' :
'release', opts);
+ events.emit('verbose', `Running Gradle Build: ${wrapper} ${args.join('
')}`);
+
try {
return await execa(wrapper, args, { stdio: 'inherit', cwd:
path.resolve(this.root) });
} catch (error) {
diff --git a/package-lock.json b/package-lock.json
index 380b0067..a8f9873d 100644
--- a/package-lock.json
+++ b/package-lock.json
@@ -18,6 +18,7 @@
"nopt": "^7.1.0",
"properties-parser": "^0.3.1",
"semver": "^7.3.8",
+ "string-argv": "^0.3.1",
"untildify": "^4.0.0",
"which": "^3.0.0"
},
@@ -4358,6 +4359,14 @@
"integrity":
"sha512-D9cPgkvLlV3t3IzL0D0YLvGA9Ahk4PcvVwUbN0dSGr1aP0Nrt4AEnTUbuGvquEC0mA64Gqt1fzirlRs5ibXx8g==",
"dev": true
},
+ "node_modules/string-argv": {
+ "version": "0.3.1",
+ "resolved":
"https://registry.npmjs.org/string-argv/-/string-argv-0.3.1.tgz";,
+ "integrity":
"sha512-a1uQGz7IyVy9YwhqjZIZu1c8JO8dNIe20xBmSS6qu9kv++k3JGzCVmprbNN5Kn+BgzD5E7YYwg1CcjuJMRNsvg==",
+ "engines": {
+ "node": ">=0.6.19"
+ }
+ },
"node_modules/string-width": {
"version": "4.2.3",
"resolved":
"https://registry.npmjs.org/string-width/-/string-width-4.2.3.tgz";,
@@ -8118,6 +8127,11 @@
"integrity":
"sha512-D9cPgkvLlV3t3IzL0D0YLvGA9Ahk4PcvVwUbN0dSGr1aP0Nrt4AEnTUbuGvquEC0mA64Gqt1fzirlRs5ibXx8g==",
"dev": true
},
+ "string-argv": {
+ "version": "0.3.1",
+ "resolved":
"https://registry.npmjs.org/string-argv/-/string-argv-0.3.1.tgz";,
+ "integrity":
"sha512-a1uQGz7IyVy9YwhqjZIZu1c8JO8dNIe20xBmSS6qu9kv++k3JGzCVmprbNN5Kn+BgzD5E7YYwg1CcjuJMRNsvg=="
+ },
"string-width": {
"version": "4.2.3",
"resolved":
"https://registry.npmjs.org/string-width/-/string-width-4.2.3.tgz";,
diff --git a/package.json b/package.json
index 699450de..c73e1d88 100644
--- a/package.json
+++ b/package.json
@@ -34,6 +34,7 @@
"nopt": "^7.1.0",
"properties-parser": "^0.3.1",
"semver": "^7.3.8",
+ "string-argv": "^0.3.1",
"untildify": "^4.0.0",
"which": "^3.0.0"
},
diff --git a/spec/unit/build.spec.js b/spec/unit/build.spec.js
new file mode 100644
index 00000000..9e249eca
--- /dev/null
+++ b/spec/unit/build.spec.js
@@ -0,0 +1,116 @@
+/*
+ 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');
+const builders = require('../../lib/builders/builders');
+
+describe('build', () => {
+ let build;
+ const builder = builders.getBuilder('FakeRootPath');
+
+ beforeEach(() => {
+ build = rewire('../../lib/build');
+ build.__set__({
+ events: jasmine.createSpyObj('eventsSpy', ['emit'])
+ });
+
+ // run needs `this` to behave like an Api instance
+ build.run = build.run.bind({
+ _builder: builder
+ });
+
+ spyOn(builder, 'build').and.returnValue(Promise.resolve({
+ paths: ['fake.apk'],
+ buildtype: 'debug'
+ }));
+ });
+
+ describe('argument parsing', () => {
+ let prepEnvSpy;
+
+ beforeEach(() => {
+ prepEnvSpy = spyOn(builder,
'prepEnv').and.returnValue(Promise.resolve());
+ });
+
+ describe('gradleArg', () => {
+ const baseOptions = {
+ packageType: 'apk',
+ arch: undefined,
+ prepEnv: undefined,
+ buildType: 'debug'
+ };
+
+ it('can parse single gradle argument', async () => {
+ await build.run({
+ argv: [
+ 'node',
+ '--gradleArg=--stacktrace'
+ ]
+ });
+
+ expect(prepEnvSpy).toHaveBeenCalledWith({
+ ...baseOptions,
+ extraArgs: ['--stacktrace']
+ });
+ });
+
+ it('can parse multiple gradle arguments', async () => {
+ await build.run({
+ argv: [
+ 'node',
+ '--gradleArg=--stacktrace --info'
+ ]
+ });
+
+ expect(prepEnvSpy).toHaveBeenCalledWith({
+ ...baseOptions,
+ extraArgs: ['--stacktrace', '--info']
+ });
+ });
+
+ it('can parse multiple gradle arguments with strings', async () =>
{
+ await build.run({
+ argv: [
+ 'node',
+ '--gradleArg=--testArg="hello world"'
+ ]
+ });
+
+ expect(prepEnvSpy).toHaveBeenCalledWith({
+ ...baseOptions,
+ extraArgs: ['--testArg="hello world"']
+ });
+ });
+
+ it('gradle args will split when necessary', async () => {
+ await build.run({
+ argv: [
+ 'node',
+ '--gradleArg=--warning-mode all'
+ ]
+ });
+
+ expect(prepEnvSpy).toHaveBeenCalledWith({
+ ...baseOptions,
+ extraArgs: ['--warning-mode', 'all']
+ });
+ });
+ });
+ });
+});
diff --git a/spec/unit/builders/ProjectBuilder.spec.js
b/spec/unit/builders/ProjectBuilder.spec.js
index 7d954910..f2cb633a 100644
--- a/spec/unit/builders/ProjectBuilder.spec.js
+++ b/spec/unit/builders/ProjectBuilder.spec.js
@@ -53,40 +53,40 @@ describe('ProjectBuilder', () => {
it('should set release argument', () => {
const args = builder.getArgs('release', {});
- expect(args[0]).toBe('cdvBuildRelease');
+ expect(args[args.length - 1]).toBe('cdvBuildRelease');
});
it('should set debug argument', () => {
const args = builder.getArgs('debug', {});
- expect(args[0]).toBe('cdvBuildDebug');
+ expect(args[args.length - 1]).toBe('cdvBuildDebug');
});
it('should set apk release', () => {
const args = builder.getArgs('release', {
packageType: 'apk'
});
- expect(args[0]).withContext(args).toBe('cdvBuildRelease');
+ expect(args[args.length -
1]).withContext(args).toBe('cdvBuildRelease');
});
it('should set apk debug', () => {
const args = builder.getArgs('debug', {
packageType: 'apk'
});
- expect(args[0]).withContext(args).toBe('cdvBuildDebug');
+ expect(args[args.length -
1]).withContext(args).toBe('cdvBuildDebug');
});
it('should set bundle release', () => {
const args = builder.getArgs('release', {
packageType: 'bundle'
});
- expect(args[0]).withContext(args).toBe(':app:bundleRelease');
+ expect(args[args.length -
1]).withContext(args).toBe(':app:bundleRelease');
});
it('should set bundle debug', () => {
const args = builder.getArgs('debug', {
packageType: 'bundle'
});
- expect(args[0]).withContext(args).toBe(':app:bundleDebug');
+ expect(args[args.length -
1]).withContext(args).toBe(':app:bundleDebug');
});
it('should add architecture if it is passed', () => {
@@ -100,14 +100,14 @@ describe('ProjectBuilder', () => {
const args = builder.getArgs('clean', {
packageType: 'apk'
});
- expect(args[0]).toBe('clean');
+ expect(args[args.length - 1]).toBe('clean');
});
it('should clean bundle', () => {
const args = builder.getArgs('clean', {
packageType: 'bundle'
});
- expect(args[0]).toBe('clean');
+ expect(args[args.length - 1]).toBe('clean');
});
describe('should accept extra arguments', () => {
@@ -176,6 +176,7 @@ describe('ProjectBuilder', () => {
it('should reject if the spawn fails', () => {
const errorMessage = 'Test error';
execaSpy.and.rejectWith(new Error(errorMessage));
+ builder.getArgs.and.returnValue([]);
return builder.build({}).then(
() => fail('Unexpectedly resolved'),
@@ -192,6 +193,7 @@ describe('ProjectBuilder', () => {
ProjectBuilder.__set__('check_reqs', checkReqsSpy);
checkReqsSpy.check_android_target.and.resolveTo();
execaSpy.and.rejectWith(testError);
+ builder.getArgs.and.returnValue([]);
return builder.build({}).then(
() => fail('Unexpectedly resolved'),
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
