Skip to content

Commit

Permalink
fix(check_reqs): do not infer project root from script location
Browse files Browse the repository at this point in the history
  • Loading branch information
raphinesse committed Jul 12, 2021
1 parent b08c7a7 commit 0f33ee5
Show file tree
Hide file tree
Showing 6 changed files with 26 additions and 18 deletions.
2 changes: 1 addition & 1 deletion bin/lib/create.js
Original file line number Diff line number Diff line change
Expand Up @@ -230,7 +230,7 @@ exports.create = function (project_path, config, options, events) {
? config.name().replace(/[^\w.]/g, '_') : 'CordovaExample';

var safe_activity_name = config.android_activityName() || options.activityName || 'MainActivity';
var target_api = check_reqs.get_target();
var target_api = check_reqs.get_target(project_path);

// Make the package conform to Java package types
return exports.validatePackageName(package_name)
Expand Down
2 changes: 1 addition & 1 deletion bin/templates/cordova/Api.js
Original file line number Diff line number Diff line change
Expand Up @@ -313,7 +313,7 @@ class Api {
* objects for current platform.
*/
requirements () {
return require('./lib/check_reqs').check_all();
return require('./lib/check_reqs').check_all(this.root);
}

/**
Expand Down
2 changes: 1 addition & 1 deletion bin/templates/cordova/lib/builders/ProjectBuilder.js
Original file line number Diff line number Diff line change
Expand Up @@ -310,7 +310,7 @@ class ProjectBuilder {
if (error.toString().includes('failed to find target with hash string')) {
// Add hint from check_android_target to error message
try {
await check_reqs.check_android_target();
await check_reqs.check_android_target(this.root);
} catch (checkAndroidTargetError) {
error.message += '\n' + checkAndroidTargetError.message;
}
Expand Down
27 changes: 17 additions & 10 deletions bin/templates/cordova/lib/check_reqs.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@ var path = require('path');
var fs = require('fs-extra');
const { forgivingWhichSync, isWindows, isDarwin } = require('./utils');
const java = require('./env/java');
var REPO_ROOT = path.join(__dirname, '..', '..', '..', '..');
const { CordovaError, ConfigParser, events } = require('cordova-common');
var android_sdk = require('./android_sdk');
const { SDK_VERSION } = require('./gradle-config-defaults');
Expand All @@ -32,10 +31,11 @@ const { SDK_VERSION } = require('./gradle-config-defaults');
Object.assign(module.exports, { isWindows, isDarwin });

/**
* @param {string} projectRoot
* @returns {string} The android target in format "android-${target}"
*/
module.exports.get_target = function () {
const userTargetSdkVersion = getUserTargetSdkVersion();
module.exports.get_target = function (projectRoot) {
const userTargetSdkVersion = getUserTargetSdkVersion(projectRoot);

if (userTargetSdkVersion && userTargetSdkVersion < SDK_VERSION) {
events.emit('warn', `android-targetSdkVersion should be greater than or equal to ${SDK_VERSION}.`);
Expand All @@ -44,10 +44,16 @@ module.exports.get_target = function () {
return `android-${Math.max(userTargetSdkVersion, SDK_VERSION)}`;
};

/** @returns {number} target sdk or 0 if undefined */
function getUserTargetSdkVersion () {
/**
* @param {string} projectRoot
* @returns {number} target sdk or 0 if undefined
*/
function getUserTargetSdkVersion (projectRoot) {
// If the repo config.xml file exists, find the desired targetSdkVersion.
const configFile = path.join(REPO_ROOT, 'config.xml');
// We need to use the cordova project's config.xml here, since the platform
// project's config.xml does not yet have the user's preferences when this
// function is called during `Api.createPlatform`.
const configFile = path.join(projectRoot, '../../config.xml');
if (!fs.existsSync(configFile)) return 0;

const configParser = new ConfigParser(configFile);
Expand Down Expand Up @@ -235,13 +241,13 @@ module.exports.check_android = function () {
});
};

module.exports.check_android_target = function () {
module.exports.check_android_target = function (projectRoot) {
// valid_target can look like:
// android-19
// android-L
// Google Inc.:Google APIs:20
// Google Inc.:Glass Development Kit Preview:20
var desired_api_level = module.exports.get_target();
var desired_api_level = module.exports.get_target(projectRoot);
return android_sdk.list_targets().then(function (targets) {
if (targets.indexOf(desired_api_level) >= 0) {
return targets;
Expand Down Expand Up @@ -286,9 +292,10 @@ var Requirement = function (id, name, version, installed) {
* Methods that runs all checks one by one and returns a result of checks
* as an array of Requirement objects. This method intended to be used by cordova-lib check_reqs method
*
* @param {string} projectRoot
* @return Promise<Requirement[]> Array of requirements. Due to implementation, promise is always fulfilled.
*/
module.exports.check_all = function () {
module.exports.check_all = function (projectRoot) {
var requirements = [
new Requirement('java', 'Java JDK'),
new Requirement('androidSdk', 'Android SDK'),
Expand All @@ -299,7 +306,7 @@ module.exports.check_all = function () {
var checkFns = [
this.check_java,
this.check_android,
this.check_android_target,
this.check_android_target.bind(this, projectRoot),
this.check_gradle
];

Expand Down
2 changes: 1 addition & 1 deletion spec/unit/builders/ProjectBuilder.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -222,7 +222,7 @@ describe('ProjectBuilder', () => {
return builder.build({}).then(
() => fail('Unexpectedly resolved'),
error => {
expect(checkReqsSpy.check_android_target).toHaveBeenCalled();
expect(checkReqsSpy.check_android_target).toHaveBeenCalledWith(rootDir);
expect(error).toBe(testError);
}
);
Expand Down
9 changes: 5 additions & 4 deletions spec/unit/check_reqs.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -262,6 +262,7 @@ describe('check_reqs', function () {
});

describe('get_target', function () {
const projectRoot = 'fakeProjectRoot';
var ConfigParser;
var getPreferenceSpy;
beforeEach(function () {
Expand All @@ -273,7 +274,7 @@ describe('check_reqs', function () {
});

it('should retrieve DEFAULT_TARGET_API', function () {
var target = check_reqs.get_target();
var target = check_reqs.get_target(projectRoot);
expect(target).toBeDefined();
expect(target).toContain('android-' + DEFAULT_TARGET_API);
});
Expand All @@ -282,7 +283,7 @@ describe('check_reqs', function () {
spyOn(fs, 'existsSync').and.returnValue(true);
getPreferenceSpy.and.returnValue(String(DEFAULT_TARGET_API + 1));

var target = check_reqs.get_target();
var target = check_reqs.get_target(projectRoot);

expect(getPreferenceSpy).toHaveBeenCalledWith('android-targetSdkVersion', 'android');
expect(target).toBe('android-' + (DEFAULT_TARGET_API + 1));
Expand All @@ -292,7 +293,7 @@ describe('check_reqs', function () {
spyOn(fs, 'existsSync').and.returnValue(true);
getPreferenceSpy.and.returnValue('android-99');

var target = check_reqs.get_target();
var target = check_reqs.get_target(projectRoot);

expect(getPreferenceSpy).toHaveBeenCalledWith('android-targetSdkVersion', 'android');
expect(target).toBe('android-' + DEFAULT_TARGET_API);
Expand All @@ -305,7 +306,7 @@ describe('check_reqs', function () {

getPreferenceSpy.and.returnValue(String(DEFAULT_TARGET_API - 1));

var target = check_reqs.get_target();
var target = check_reqs.get_target(projectRoot);

expect(getPreferenceSpy).toHaveBeenCalledWith('android-targetSdkVersion', 'android');
expect(target).toBe('android-' + DEFAULT_TARGET_API);
Expand Down

0 comments on commit 0f33ee5

Please sign in to comment.