Fixes build & run related bugs from builder refactor PR #461 (#490)

- General Code Refactor - Removed builder type argument from getBuilder API - Removed any reference of conditional statements around builder type - Remove plugin handler install and uninstall option flag android_studio - Remove --gradle flag references - Fixed plugin handler install and uninstall pathing issues - Added parseBuildOptions export so run can get build related options. - Use the nobuild flag option to control the run build. - Updated test spec to reflect the changes.
apache · Sep 6, 2018 · 23b2449 · 23b2449
1 parent 7ab0cf1
commit 23b2449
Show file tree

Hide file tree

Showing 13 changed files with 81 additions and 192 deletions.
diff --git a/bin/lib/create.js b/bin/lib/create.js
@@ -141,9 +141,9 @@ function writeProjectProperties (projectPath, target_api) {
 }
 
 // This makes no sense, what if you're building with a different build system?
-function prepBuildFiles (projectPath, builder) {
+function prepBuildFiles (projectPath) {
     var buildModule = require(path.resolve(projectPath, 'cordova/lib/builders/builders'));
-    buildModule.getBuilder(builder).prepBuildFiles();
+    buildModule.getBuilder().prepBuildFiles();
 }
 
 function copyBuildRules (projectPath, isLegacy) {
@@ -329,7 +329,7 @@ exports.create = function (project_path, config, options, events) {
             });
             // Link it to local android install.
             exports.writeProjectProperties(project_path, target_api);
-            exports.prepBuildFiles(project_path, 'studio');
+            exports.prepBuildFiles(project_path);
             events.emit('log', generateDoneMessage('create', options.link));
         }).thenResolve(project_path);
 };

diff --git a/bin/templates/cordova/Api.js b/bin/templates/cordova/Api.js
@@ -55,23 +55,23 @@ function setupEvents (externalEventEmitter) {
 function Api (platform, platformRootDir, events) {
     this.platform = PLATFORM;
     this.root = path.resolve(__dirname, '..');
-    this.builder = 'studio';
 
     setupEvents(events);
 
-    var self = this;
+    const appMain = path.join(this.root, 'app', 'src', 'main');
+    const appRes = path.join(appMain, 'res');
 
     this.locations = {
-        root: self.root,
-        www: path.join(self.root, 'app/src/main/assets/www'),
-        res: path.join(self.root, 'app/src/main/res'),
-        platformWww: path.join(self.root, 'platform_www'),
-        configXml: path.join(self.root, 'app/src/main/res/xml/config.xml'),
-        defaultConfigXml: path.join(self.root, 'cordova/defaults.xml'),
-        strings: path.join(self.root, 'app/src/main/res/values/strings.xml'),
-        manifest: path.join(self.root, 'app/src/main/AndroidManifest.xml'),
-        build: path.join(self.root, 'build'),
-        javaSrc: path.join(self.root, 'app/src/main/java/'),
+        root: this.root,
+        www: path.join(appMain, 'assets', 'www'),
+        res: appRes,
+        platformWww: path.join(this.root, 'platform_www'),
+        configXml: path.join(appRes, 'xml', 'config.xml'),
+        defaultConfigXml: path.join(this.root, 'cordova', 'defaults.xml'),
+        strings: path.join(appRes, 'values', 'strings.xml'),
+        manifest: path.join(appMain, 'AndroidManifest.xml'),
+        build: path.join(this.root, 'build'),
+        javaSrc: path.join(appMain, 'java'),
         // NOTE: Due to platformApi spec we need to return relative paths here
         cordovaJs: 'bin/templates/project/assets/www/cordova.js',
         cordovaJsSrc: 'cordova-js-src'
@@ -208,17 +208,13 @@ Api.prototype.addPlugin = function (plugin, installOptions) {
         installOptions.variables.PACKAGE_NAME = project.getPackageName();
     }
 
-    if (this.android_studio === true) {
-        installOptions.android_studio = true;
-    }
-
     return Q().then(function () {
         return PluginManager.get(self.platform, self.locations, project).addPlugin(plugin, installOptions);
     }).then(function () {
         if (plugin.getFrameworks(this.platform).length === 0) return;
         selfEvents.emit('verbose', 'Updating build files since android plugin contained <framework>');
         // This should pick the correct builder, not just get gradle
-        require('./lib/builders/builders').getBuilder(this.builder).prepBuildFiles();
+        require('./lib/builders/builders').getBuilder().prepBuildFiles();
     }.bind(this))
         // CB-11022 Return truthy value to prevent running prepare after
         .thenResolve(true);
@@ -240,9 +236,8 @@ Api.prototype.addPlugin = function (plugin, installOptions) {
 Api.prototype.removePlugin = function (plugin, uninstallOptions) {
     var project = AndroidProject.getProjectFile(this.root);
 
-    if (uninstallOptions && uninstallOptions.usePlatformWww === true && this.android_studio === true) {
+    if (uninstallOptions && uninstallOptions.usePlatformWww === true) {
         uninstallOptions.usePlatformWww = false;
-        uninstallOptions.android_studio = true;
     }
 
     return PluginManager.get(this.platform, this.locations, project)
@@ -251,7 +246,7 @@ Api.prototype.removePlugin = function (plugin, uninstallOptions) {
             if (plugin.getFrameworks(this.platform).length === 0) return;
 
             selfEvents.emit('verbose', 'Updating build files since android plugin contained <framework>');
-            require('./lib/builders/builders').getBuilder(this.builder).prepBuildFiles();
+            require('./lib/builders/builders').getBuilder().prepBuildFiles();
         }.bind(this))
         // CB-11022 Return truthy value to prevent running prepare after
         .thenResolve(true);
@@ -304,9 +299,7 @@ Api.prototype.removePlugin = function (plugin, uninstallOptions) {
  */
 Api.prototype.build = function (buildOptions) {
     var self = this;
-    if (this.android_studio) {
-        buildOptions.studio = true;
-    }
+
     return require('./lib/check_reqs').run().then(function () {
         return require('./lib/build').run.call(self, buildOptions);
     }).then(function (buildResults) {
@@ -350,12 +343,9 @@ Api.prototype.run = function (runOptions) {
  */
 Api.prototype.clean = function (cleanOptions) {
     var self = this;
-    if (this.android_studio) {
-        // This will lint, checking for null won't
-        if (typeof cleanOptions === 'undefined') {
-            cleanOptions = {};
-        }
-        cleanOptions.studio = true;
+    // This will lint, checking for null won't
+    if (typeof cleanOptions === 'undefined') {
+        cleanOptions = {};
     }
 
     return require('./lib/check_reqs').run().then(function () {

diff --git a/bin/templates/cordova/lib/build.js b/bin/templates/cordova/lib/build.js
@@ -31,11 +31,10 @@ var events = require('cordova-common').events;
 var spawn = require('cordova-common').superspawn.spawn;
 var CordovaError = require('cordova-common').CordovaError;
 
+module.exports.parseBuildOptions = parseOpts;
 function parseOpts (options, resolvedTarget, projectRoot) {
     options = options || {};
     options.argv = nopt({
-        gradle: Boolean,
-        studio: Boolean,
         prepenv: Boolean,
         versionCode: String,
         minSdkVersion: String,
@@ -50,26 +49,13 @@ function parseOpts (options, resolvedTarget, projectRoot) {
     // Android Studio Build method is the default
     var ret = {
         buildType: options.release ? 'release' : 'debug',
-        buildMethod: process.env.ANDROID_BUILD || 'studio',
         prepEnv: options.argv.prepenv,
         arch: resolvedTarget && resolvedTarget.arch,
         extraArgs: []
     };
 
-    if (options.argv.gradle || options.argv.studio) {
-        ret.buildMethod = options.argv.studio ? 'studio' : 'gradle';
-    }
-
-    // This comes from cordova/run
-    if (options.studio) ret.buildMethod = 'studio';
-    if (options.gradle) ret.buildMethod = 'gradle';
-
-    if (options.nobuild) ret.buildMethod = 'none';
-
     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.gradleArg) {
         ret.extraArgs = ret.extraArgs.concat(options.argv.gradleArg);
     }
@@ -129,7 +115,8 @@ function parseOpts (options, resolvedTarget, projectRoot) {
  */
 module.exports.runClean = function (options) {
     var opts = parseOpts(options, null, this.root);
-    var builder = builders.getBuilder(opts.buildMethod);
+    var builder = builders.getBuilder();
+
     return builder.prepEnv(opts).then(function () {
         return builder.clean(opts);
     });
@@ -149,8 +136,8 @@ module.exports.runClean = function (options) {
  */
 module.exports.run = function (options, optResolvedTarget) {
     var opts = parseOpts(options, optResolvedTarget, this.root);
-    console.log(opts.buildMethod);
-    var builder = builders.getBuilder(opts.buildMethod);
+    var builder = builders.getBuilder();
+
     return builder.prepEnv(opts).then(function () {
         if (opts.prepEnv) {
             events.emit('verbose', 'Build file successfully prepared.');
@@ -161,8 +148,7 @@ module.exports.run = function (options, optResolvedTarget) {
             events.emit('log', 'Built the following apk(s): \n\t' + apkPaths.join('\n\t'));
             return {
                 apkPaths: apkPaths,
-                buildType: opts.buildType,
-                buildMethod: opts.buildMethod
+                buildType: opts.buildType
             };
         });
     });
@@ -276,12 +262,10 @@ module.exports.help = function () {
     console.log('Flags:');
     console.log('    \'--debug\': will build project in debug mode (default)');
     console.log('    \'--release\': will build project for release');
-    console.log('    \'--ant\': will build project with ant');
-    console.log('    \'--gradle\': will build project with gradle (default)');
     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. Requires --gradle.');
-    console.log('    \'--minSdkVersion=#\': Override minSdkVersion for this build. Useful for uploading multiple APKs. Requires --gradle.');
+    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('    \'--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) :');

diff --git a/bin/templates/cordova/lib/builders/ProjectBuilder.js b/bin/templates/cordova/lib/builders/ProjectBuilder.js
@@ -35,12 +35,9 @@ const TEMPLATE =
     '# Do not modify this file -- ' + MARKER + '\n';
 
 class ProjectBuilder {
-    constructor (projectRoot) {
-        this.root = projectRoot || path.resolve(__dirname, '../../..');
-        this.binDirs = {
-            studio: path.join(this.root, 'app', 'build', 'outputs', 'apk'),
-            gradle: path.join(this.root, 'app', 'build', 'outputs', 'apk')
-        };
+    constructor (rootDirectory) {
+        this.root = rootDirectory || path.resolve(__dirname, '../../..');
+        this.binDir = path.join(this.root, 'app', 'build', 'outputs', 'apk');
     }
 
     getArgs (cmd, opts) {
@@ -295,11 +292,14 @@ class ProjectBuilder {
     }
 
     findOutputApks (build_type, arch) {
-        var self = this;
-        return Object.keys(this.binDirs).reduce(function (result, builderName) {
-            var binDir = self.binDirs[builderName];
-            return result.concat(findOutputApksHelper(binDir, build_type, builderName === 'ant' ? null : arch));
-        }, []).sort(apkSorter);
+        return findOutputApksHelper(this.binDir, build_type, arch).sort(apkSorter);
+    }
+
+    fetchBuildResults (build_type, arch) {
+        return {
+            apkPaths: this.findOutputApks(build_type, arch),
+            buildType: build_type
+        };
     }
 }
 

diff --git a/bin/templates/cordova/lib/builders/builders.js b/bin/templates/cordova/lib/builders/builders.js
@@ -17,29 +17,18 @@
     under the License.
 */
 
-var CordovaError = require('cordova-common').CordovaError;
-
-var knownBuilders = {
-    gradle: 'ProjectBuilder',
-    studio: 'ProjectBuilder'
-};
+const CordovaError = require('cordova-common').CordovaError;
 
 /**
- * Helper method that instantiates and returns a builder for specified build
- *   type.
+ * Helper method that instantiates and returns a builder for specified build type.
  *
- * @param   {String}  builderType   Builder name to construct and return. Must
- *   be one of gradle' or 'studio'
- *
- * @return  {Builder}               A builder instance for specified build type.
+ * @return {Builder} A builder instance for specified build type.
  */
-module.exports.getBuilder = function (builderType, projectRoot) {
-    if (!knownBuilders[builderType]) { throw new CordovaError('Builder ' + builderType + ' is not supported.'); }
-
+module.exports.getBuilder = function () {
     try {
-        var Builder = require('./' + knownBuilders[builderType]);
-        return new Builder(projectRoot);
+        const Builder = require('./ProjectBuilder');
+        return new Builder();
     } catch (err) {
-        throw new CordovaError('Failed to instantiate ' + knownBuilders[builderType] + ' builder: ' + err);
+        throw new CordovaError('Failed to instantiate ProjectBuilder builder: ' + err);
     }
 };
diff --git a/bin/templates/cordova/lib/emulator.js b/bin/templates/cordova/lib/emulator.js
@@ -434,12 +434,9 @@ module.exports.install = function (givenTarget, buildResults) {
 
     var target;
     // We need to find the proper path to the Android Manifest
-    var manifestPath = path.join(__dirname, '..', '..', 'app', 'src', 'main', 'AndroidManifest.xml');
-    if (buildResults.buildMethod === 'gradle') {
-        manifestPath = path.join(__dirname, '../../AndroidManifest.xml');
-    }
-    var manifest = new AndroidManifest(manifestPath);
-    var pkgName = manifest.getPackageId();
+    const manifestPath = path.join(__dirname, '..', '..', 'app', 'src', 'main', 'AndroidManifest.xml');
+    const manifest = new AndroidManifest(manifestPath);
+    const pkgName = manifest.getPackageId();
 
     // resolve the target emulator
     return Promise.resolve().then(function () {

diff --git a/bin/templates/cordova/lib/pluginHandlers.js b/bin/templates/cordova/lib/pluginHandlers.js
@@ -26,14 +26,7 @@ var handlers = {
             if (!obj.src) throw new CordovaError(generateAttributeError('src', 'source-file', plugin.id));
             if (!obj.targetDir) throw new CordovaError(generateAttributeError('target-dir', 'source-file', plugin.id));
 
-            var dest = path.join(obj.targetDir, path.basename(obj.src));
-
-            // TODO: This code needs to be replaced, since the core plugins need to be re-mapped to a different location in
-            // a later plugins release.  This is for legacy plugins to work with Cordova.
-
-            if (options && options.android_studio === true) {
-                dest = studioPathRemap(obj);
-            }
+            var dest = studioPathRemap(obj);
 
             if (options && options.force) {
                 copyFile(plugin.dir, obj.src, project.projectDir, dest, !!(options && options.link));
@@ -42,11 +35,7 @@ var handlers = {
             }
         },
         uninstall: function (obj, plugin, project, options) {
-            var dest = path.join(obj.targetDir, path.basename(obj.src));
-
-            if (options && options.android_studio === true) {
-                dest = studioPathRemap(obj);
-            }
+            var dest = studioPathRemap(obj);
 
             // TODO: Add Koltin extension to uninstall, since they are handled like Java files
             if (obj.src.endsWith('java')) {
@@ -59,33 +48,21 @@ var handlers = {
     },
     'lib-file': {
         install: function (obj, plugin, project, options) {
-            var dest = path.join('libs', path.basename(obj.src));
-            if (options && options.android_studio === true) {
-                dest = path.join('app/libs', path.basename(obj.src));
-            }
+            var dest = path.join('app/libs', path.basename(obj.src));
             copyFile(plugin.dir, obj.src, project.projectDir, dest, !!(options && options.link));
         },
         uninstall: function (obj, plugin, project, options) {
-            var dest = path.join('libs', path.basename(obj.src));
-            if (options && options.android_studio === true) {
-                dest = path.join('app/libs', path.basename(obj.src));
-            }
+            var dest = path.join('app/libs', path.basename(obj.src));
             removeFile(project.projectDir, dest);
         }
     },
     'resource-file': {
         install: function (obj, plugin, project, options) {
-            var dest = path.normalize(obj.target);
-            if (options && options.android_studio === true) {
-                dest = path.join('app/src/main', dest);
-            }
+            var dest = path.join('app', 'src', 'main', obj.target);
             copyFile(plugin.dir, obj.src, project.projectDir, dest, !!(options && options.link));
         },
         uninstall: function (obj, plugin, project, options) {
-            var dest = path.normalize(obj.target);
-            if (options && options.android_studio === true) {
-                dest = path.join('app/src/main', dest);
-            }
+            var dest = path.join('app', 'src', 'main', obj.target);
             removeFile(project.projectDir, dest);
         }
     },