You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cordova.apache.org by er...@apache.org on 2020/01/24 01:54:00 UTC

[cordova-android] branch master updated: fix: cordova requirements consider the android-targetSdkVersion (#849)

This is an automated email from the ASF dual-hosted git repository.

erisu 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 92268b2  fix: cordova requirements consider the android-targetSdkVersion (#849)
92268b2 is described below

commit 92268b2e76f10db7fb5e8d85b3bc2b42dbb70661
Author: Norman Breau <no...@normanbreau.com>
AuthorDate: Thu Jan 23 21:53:50 2020 -0400

    fix: cordova requirements consider the android-targetSdkVersion (#849)
    
    * Made cordova requirements consider the android-targetSdkVersion preference
    * refator: get_target method
    Added comments.
    Added JSDoc block
    Reduced error exit point to one spot
    
    Co-authored-by: エリス <er...@users.noreply.github.com>
---
 bin/templates/cordova/lib/check_reqs.js | 55 +++++++++++++++-------
 spec/unit/check_reqs.spec.js            | 83 ++++++++++++++++++++++++++++++++-
 2 files changed, 118 insertions(+), 20 deletions(-)

diff --git a/bin/templates/cordova/lib/check_reqs.js b/bin/templates/cordova/lib/check_reqs.js
index e8c0804..de0a9ea 100644
--- a/bin/templates/cordova/lib/check_reqs.js
+++ b/bin/templates/cordova/lib/check_reqs.js
@@ -26,8 +26,9 @@ var fs = require('fs');
 var os = require('os');
 var REPO_ROOT = path.join(__dirname, '..', '..', '..', '..');
 var PROJECT_ROOT = path.join(__dirname, '..', '..');
-var CordovaError = require('cordova-common').CordovaError;
+const { CordovaError, ConfigParser, events } = require('cordova-common');
 var android_sdk = require('./android_sdk');
+const { createEditor } = require('properties-parser');
 
 function forgivingWhichSync (cmd) {
     try {
@@ -45,26 +46,44 @@ module.exports.isDarwin = function () {
     return (os.platform() === 'darwin');
 };
 
-// Get valid target from framework/project.properties if run from this repo
-// Otherwise get target from project.properties file within a generated cordova-android project
+/**
+ * @description Get valid target from framework/project.properties if run from this repo
+ *              Otherwise get target from project.properties file within a generated cordova-android project
+ * @returns {string} The android target in format "android-${target}"
+ */
 module.exports.get_target = function () {
-    function extractFromFile (filePath) {
-        var target = shelljs.grep(/\btarget=/, filePath);
-        if (!target) {
-            throw new Error('Could not find android target within: ' + filePath);
-        }
-        return target.split('=')[1].trim();
-    }
-    var repo_file = path.join(REPO_ROOT, 'framework', 'project.properties');
-    if (fs.existsSync(repo_file)) {
-        return extractFromFile(repo_file);
+    const projectPropertiesPaths = [
+        path.join(REPO_ROOT, 'framework', 'project.properties'),
+        path.join(PROJECT_ROOT, 'project.properties')
+    ];
+
+    // Get the minimum required target API from the framework.
+    let target = projectPropertiesPaths.filter(filePath => fs.existsSync(filePath))
+        .map(filePath => createEditor(filePath).get('target'))
+        .pop();
+
+    if (!target) {
+        throw new Error(`We could not locate the target from the "project.properties" at either "${projectPropertiesPaths.join('", "')}".`);
     }
-    var project_file = path.join(PROJECT_ROOT, 'project.properties');
-    if (fs.existsSync(project_file)) {
-        // if no target found, we're probably in a project and project.properties is in PROJECT_ROOT.
-        return extractFromFile(project_file);
+
+    // If the repo config.xml file exists, find the desired targetSdkVersion.
+    const configFile = path.join(REPO_ROOT, 'config.xml');
+    if (!fs.existsSync(configFile)) return target;
+
+    const configParser = new ConfigParser(configFile);
+    const desiredAPI = parseInt(configParser.getPreference('android-targetSdkVersion', 'android'), 10);
+
+    if (!isNaN(desiredAPI)) {
+        const minimumAPI = parseInt(target.split('-').pop(), 10);
+
+        if (desiredAPI >= minimumAPI) {
+            target = `android-${desiredAPI}`;
+        } else {
+            events.emit('warn', `android-targetSdkVersion should be greater than or equal to ${minimumAPI}.`);
+        }
     }
-    throw new Error('Could not find android target in either ' + repo_file + ' nor ' + project_file);
+
+    return target;
 };
 
 // Returns a promise. Called only by build and clean commands.
diff --git a/spec/unit/check_reqs.spec.js b/spec/unit/check_reqs.spec.js
index 885e19b..b70ce75 100644
--- a/spec/unit/check_reqs.spec.js
+++ b/spec/unit/check_reqs.spec.js
@@ -17,11 +17,16 @@
     under the License.
 */
 
-var check_reqs = require('../../bin/templates/cordova/lib/check_reqs');
+var rewire = require('rewire');
+var check_reqs = rewire('../../bin/templates/cordova/lib/check_reqs');
 var android_sdk = require('../../bin/templates/cordova/lib/android_sdk');
 var shelljs = require('shelljs');
 var fs = require('fs');
 var path = require('path');
+var events = require('cordova-common').events;
+
+// This should match /bin/templates/project/build.gradle
+const DEFAULT_TARGET_API = 29;
 
 describe('check_reqs', function () {
     var original_env;
@@ -176,13 +181,87 @@ describe('check_reqs', function () {
             });
         });
     });
+
     describe('get_target', function () {
+        var ConfigParser;
+        var getPreferenceSpy;
+        beforeEach(function () {
+            getPreferenceSpy = jasmine.createSpy();
+            ConfigParser = jasmine.createSpy().and.returnValue({
+                getPreference: getPreferenceSpy
+            });
+            check_reqs.__set__('ConfigParser', ConfigParser);
+        });
+
         it('should retrieve target from framework project.properties file', function () {
             var target = check_reqs.get_target();
             expect(target).toBeDefined();
-            expect(target).toContain('android-');
+            expect(target).toContain('android-' + DEFAULT_TARGET_API);
+        });
+
+        it('should throw error if target cannot be found', function () {
+            spyOn(fs, 'existsSync').and.returnValue(false);
+            expect(function () {
+                check_reqs.get_target();
+            }).toThrow();
+        });
+
+        it('should override target from config.xml preference', () => {
+            var realExistsSync = fs.existsSync;
+            spyOn(fs, 'existsSync').and.callFake(function (path) {
+                if (path.indexOf('config.xml') > -1) {
+                    return true;
+                } else {
+                    return realExistsSync.call(fs, path);
+                }
+            });
+
+            getPreferenceSpy.and.returnValue(DEFAULT_TARGET_API + 1);
+
+            var target = check_reqs.get_target();
+
+            expect(getPreferenceSpy).toHaveBeenCalledWith('android-targetSdkVersion', 'android');
+            expect(target).toBe('android-' + (DEFAULT_TARGET_API + 1));
+        });
+
+        it('should fallback to default target if config.xml has invalid preference', () => {
+            var realExistsSync = fs.existsSync;
+            spyOn(fs, 'existsSync').and.callFake(function (path) {
+                if (path.indexOf('config.xml') > -1) {
+                    return true;
+                } else {
+                    return realExistsSync.call(fs, path);
+                }
+            });
+
+            getPreferenceSpy.and.returnValue(NaN);
+
+            var target = check_reqs.get_target();
+
+            expect(getPreferenceSpy).toHaveBeenCalledWith('android-targetSdkVersion', 'android');
+            expect(target).toBe('android-' + DEFAULT_TARGET_API);
+        });
+
+        it('should warn if target sdk preference is lower than the minimum required target SDK', () => {
+            var realExistsSync = fs.existsSync;
+            spyOn(fs, 'existsSync').and.callFake(function (path) {
+                if (path.indexOf('config.xml') > -1) {
+                    return true;
+                } else {
+                    return realExistsSync.call(fs, path);
+                }
+            });
+
+            getPreferenceSpy.and.returnValue(DEFAULT_TARGET_API - 1);
+
+            var target = check_reqs.get_target();
+
+            expect(getPreferenceSpy).toHaveBeenCalledWith('android-targetSdkVersion', 'android');
+            expect(target).toBe('android-' + DEFAULT_TARGET_API);
+            expect(events.emit).toHaveBeenCalledWith('warn', 'android-targetSdkVersion should be greater than or equal to ' + DEFAULT_TARGET_API + '.');
         });
     });
+
     describe('check_android_target', function () {
         it('should should return full list of supported targets if there is a match to ideal api level', () => {
             var fake_targets = ['you are my fire', 'my one desire'];


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@cordova.apache.org
For additional commands, e-mail: commits-help@cordova.apache.org