[DRAFT] fix(check_reqs.get_target): do not infer project root from sc…

…ript location
apache · Jul 9, 2021 · aad74f3 · aad74f3
1 parent ac3df01
commit aad74f3
Show file tree

Hide file tree

Showing 10 changed files with 39 additions and 27 deletions.
diff --git a/bin/lib/create.js b/bin/lib/create.js
@@ -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)

diff --git a/bin/templates/cordova/Api.js b/bin/templates/cordova/Api.js
@@ -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);
     }
 
     /**

diff --git a/bin/templates/cordova/lib/builders/ProjectBuilder.js b/bin/templates/cordova/lib/builders/ProjectBuilder.js
@@ -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;
                 }

diff --git a/bin/templates/cordova/lib/check_reqs.js b/bin/templates/cordova/lib/check_reqs.js
@@ -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 semver = require('semver');
@@ -35,10 +34,11 @@ const EXPECTED_JAVA_VERSION = '1.8.x';
 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}.`);
@@ -47,10 +47,17 @@ 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 does not yet have the preferences when this is called
+    // during create.
+    const configFile = path.join(projectRoot, '../../config.xml');
+    console.log(configFile);
     if (!fs.existsSync(configFile)) return 0;
 
     const configParser = new ConfigParser(configFile);
@@ -246,13 +253,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;
@@ -297,9 +304,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'),
@@ -310,7 +318,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
     ];
 

diff --git a/bin/templates/cordova/lib/emulator.js b/bin/templates/cordova/lib/emulator.js
@@ -139,14 +139,14 @@ module.exports.list_images = function () {
  * or undefined if no avds exist.
  * Returns a promise.
  */
-module.exports.best_image = function () {
+module.exports.best_image = function (projectRoot) {
     return this.list_images().then(function (images) {
         // Just return undefined if there is no images
         if (images.length === 0) return;
 
         var closest = 9999;
         var best = images[0];
-        var project_target = parseInt(check_reqs.get_target().replace('android-', ''));
+        var project_target = parseInt(check_reqs.get_target(projectRoot).replace('android-', ''));
         for (var i in images) {
             var target = images[i].target;
             if (target && target.indexOf('API level') > -1) {

diff --git a/bin/templates/cordova/lib/run.js b/bin/templates/cordova/lib/run.js
@@ -61,7 +61,7 @@ module.exports.run = function (runOptions) {
     var self = this;
     const spec = buildTargetSpec(runOptions);
 
-    return target.resolve(spec).then(function (resolvedTarget) {
+    return target.resolve(spec, this.root).then(function (resolvedTarget) {
         events.emit('log', `Deploying to ${formatResolvedTarget(resolvedTarget)}`);
 
         return new Promise((resolve) => {

diff --git a/bin/templates/cordova/lib/target.js b/bin/templates/cordova/lib/target.js
@@ -72,16 +72,17 @@ async function isEmulatorName (name) {
 
 /**
  * @param {TargetSpec?} spec
+ * @param {string?} projectRoot
  * @return {Promise<Target>}
  */
-async function resolveToOfflineEmulator ({ id: avdName, type } = {}) {
+async function resolveToOfflineEmulator ({ id: avdName, type } = {}, projectRoot) {
     if (type === 'device') return null;
 
     if (avdName) {
         if (!await isEmulatorName(avdName)) return null;
     } else {
         events.emit('verbose', 'Looking for emulator image that best matches the target API');
-        const best = await emulator.best_image();
+        const best = await emulator.best_image(projectRoot);
         avdName = best && best.name;
         if (!avdName) return null;
     }
@@ -94,14 +95,15 @@ async function resolveToOfflineEmulator ({ id: avdName, type } = {}) {
 
 /**
  * @param {TargetSpec?} spec
+ * @param {string?} projectRoot
  * @return {Promise<Target & {arch: string}>}
  */
-exports.resolve = async (spec = {}) => {
+exports.resolve = async (spec = {}, projectRoot) => {
     events.emit('verbose', `Trying to find target matching ${inspect(spec)}`);
 
     const resolvedTarget =
         (await resolveToOnlineTarget(spec)) ||
-        (await resolveToOfflineEmulator(spec));
+        (await resolveToOfflineEmulator(spec, projectRoot));
 
     if (!resolvedTarget) {
         throw new CordovaError(`Could not find target matching ${inspect(spec)}`);

diff --git a/spec/unit/builders/ProjectBuilder.spec.js b/spec/unit/builders/ProjectBuilder.spec.js
@@ -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);
                 }
             );

diff --git a/spec/unit/check_reqs.spec.js b/spec/unit/check_reqs.spec.js
@@ -272,6 +272,7 @@ describe('check_reqs', function () {
     });
 
     describe('get_target', function () {
+        const projectRoot = 'fakeProjectRoot';
         var ConfigParser;
         var getPreferenceSpy;
         beforeEach(function () {
@@ -283,7 +284,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);
         });
@@ -292,7 +293,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));
@@ -302,7 +303,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);
@@ -315,7 +316,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);

diff --git a/spec/unit/target.spec.js b/spec/unit/target.spec.js
@@ -193,12 +193,13 @@ describe('target', () => {
 
         it('should delegate to resolveToOfflineEmulator if resolveToOnlineTarget fails', () => {
             const spec = { type: 'emulator' };
+            const projectRoot = 'fakeRoot';
             resolveToOfflineEmulator.and.resolveTo({ id: 'emu1', type: 'emulator' });
 
-            return target.resolve(spec).then(result => {
+            return target.resolve(spec, projectRoot).then(result => {
                 expect(result.id).toBe('emu1');
                 expect(resolveToOnlineTarget).toHaveBeenCalledWith(spec);
-                expect(resolveToOfflineEmulator).toHaveBeenCalledWith(spec);
+                expect(resolveToOfflineEmulator).toHaveBeenCalledWith(spec, projectRoot);
             });
         });