You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@cordova.apache.org by vladimir-kotikov <gi...@git.apache.org> on 2015/05/22 11:18:06 UTC
[GitHub] cordova-windows pull request: CB-8954 Adds support for `requiremen...
GitHub user vladimir-kotikov opened a pull request:
https://github.com/apache/cordova-windows/pull/83
CB-8954 Adds support for `requirements` command
Implementation for https://issues.apache.org/jira/browse/CB-8954
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/MSOpenTech/cordova-windows CB-8954
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/cordova-windows/pull/83.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #83
----
commit 8b6b50212dbfc6aacbf0cdda861c63ff81555295
Author: Vladimir Kotikov <v-...@microsoft.com>
Date: 2015-05-05T10:05:55Z
Adds `requirements` command support to check_reqs module
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org
[GitHub] cordova-windows pull request: CB-8954 Adds support for `requiremen...
Posted by TimBarham <gi...@git.apache.org>.
Github user TimBarham commented on a diff in the pull request:
https://github.com/apache/cordova-windows/pull/83#discussion_r31070257
--- Diff: bin/lib/check_reqs.js ---
@@ -17,17 +17,101 @@
under the License.
*/
-var Q = require('Q'),
- os = require('os'),
+/*jshint node:true*/
+
+var Q = require('Q');
+
+var MSBuildTools;
+try {
MSBuildTools = require('../../template/cordova/lib/MSBuildTools');
+} catch (ex) {
+ // If previous import fails, we probably running this script
+ // from installed platform and the module location is different.
+ MSBuildTools = require('./MSBuildTools');
+}
+
+/**
+ * Check if current OS is supports building windows platform
+ * @return {Promise} Promise either fullfilled or rejected with error message.
+ */
+var checkOS = function () {
+ var platform = process.platform;
+ return (platform === 'win32') ?
+ Q.resolve(platform):
+ // Build Universal windows apps available for windows platform only, so we reject on others platforms
+ Q.reject('Cordova tooling for Windows requires Windows OS to build project');
+};
+
+/**
+ * Checks if MSBuild tools is available.
+ * @return {Promise} Promise either fullfilled with MSBuild version
+ * or rejected with error message.
+ */
+var checkMSBuild = function () {
+ return MSBuildTools.findAvailableVersion()
+ .then(function (msbuildTools) {
+ // return Q.resolve('MSBuild tools v.' + msbuildTools.version + ' found at ' + msbuildTools.path);
--- End diff --
Remove commented line.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org
[GitHub] cordova-windows pull request: CB-8954 Adds support for `requiremen...
Posted by TimBarham <gi...@git.apache.org>.
Github user TimBarham commented on a diff in the pull request:
https://github.com/apache/cordova-windows/pull/83#discussion_r31070612
--- Diff: bin/lib/check_reqs.js ---
@@ -17,17 +17,101 @@
under the License.
*/
-var Q = require('Q'),
- os = require('os'),
+/*jshint node:true*/
+
+var Q = require('Q');
+
+var MSBuildTools;
+try {
MSBuildTools = require('../../template/cordova/lib/MSBuildTools');
+} catch (ex) {
+ // If previous import fails, we probably running this script
+ // from installed platform and the module location is different.
+ MSBuildTools = require('./MSBuildTools');
+}
+
+/**
+ * Check if current OS is supports building windows platform
+ * @return {Promise} Promise either fullfilled or rejected with error message.
+ */
+var checkOS = function () {
+ var platform = process.platform;
+ return (platform === 'win32') ?
+ Q.resolve(platform):
+ // Build Universal windows apps available for windows platform only, so we reject on others platforms
+ Q.reject('Cordova tooling for Windows requires Windows OS to build project');
+};
+
+/**
+ * Checks if MSBuild tools is available.
+ * @return {Promise} Promise either fullfilled with MSBuild version
+ * or rejected with error message.
+ */
+var checkMSBuild = function () {
+ return MSBuildTools.findAvailableVersion()
+ .then(function (msbuildTools) {
+ // return Q.resolve('MSBuild tools v.' + msbuildTools.version + ' found at ' + msbuildTools.path);
+ return Q.resolve(msbuildTools.version);
+ }, function () {
+ return Q.reject('MSBuild tools not found. Please install MSBuild tools or VS 2013 from ' +
+ 'https://www.visualstudio.com/downloads/download-visual-studio-vs');
+ });
+};
module.exports.run = function () {
- if (os.platform() != 'win32'){
- // Build Universal windows apps available for windows platform only, so we reject on others platforms
- return Q.reject('ERROR: Cordova tooling for Windows requires Windows OS');
- }
- // Check whther MSBuild Tools are available
- return MSBuildTools.findAvailableVersion();
+ return checkOS().then(function () {
+ return MSBuildTools.findAvailableVersion();
+ });
+};
+
+/**
+ * Object thar represents one of requirements for current platform.
+ * @param {String} id The unique identifier for this requirements.
+ * @param {String} name The name of requirements. Human-readable field.
+ * @param {String} version The version of requirement installed. In some cases could be an array of strings
+ * (for example, check_android_target returns an array of android targets installed)
+ * @param {Boolean} installed Indicates whether the reuirement is installed or not
+ */
+var Requirement = function (id, name, version, installed) {
+ this.id = id;
+ this.name = name;
+ this.installed = installed || false;
+ this.metadata = {
+ version: version,
+ };
+};
+
+/**
+ * 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
+ *
+ * @return Promise<Requirement[]> Array of requirements. Due to implementation, promise is always fulfilled.
+ */
+module.exports.check_all = function() {
+
+ var requirements = [
+ new Requirement('os', 'Windows OS'),
+ new Requirement('msbuild', 'MSBuild Tools')
+ ];
+
+ // Define list of checks needs to be performed
+ var checkFns = [checkOS, checkMSBuild];
+ // Then execute them one-by-one
+ return checkFns.reduce(function (promise, checkFn, idx) {
+ // Update each requirement with results
+ var requirement = requirements[idx];
+ return promise.then(checkFn)
+ .then(function (version) {
+ requirement.installed = true;
+ requirement.metadata.version = version;
+ }, function (err) {
+ requirement.metadata.reason = err;
+ });
+ }, Q())
+ .then(function () {
+ // When chain is completed, return requirements array to upstream API
+ return requirements;
+ });
--- End diff --
I know it's not a lot of code, but since it will be duplicated for every platform... it would be nice if the definition of `Requirement`, and the `check_all` logic could be in its own module that the platforms take as a dependency. A pain to set up now, but easier maintenance later on.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org
[GitHub] cordova-windows pull request: CB-8954 Adds support for `requiremen...
Posted by TimBarham <gi...@git.apache.org>.
Github user TimBarham commented on a diff in the pull request:
https://github.com/apache/cordova-windows/pull/83#discussion_r31070922
--- Diff: bin/lib/check_reqs.js ---
@@ -17,17 +17,101 @@
under the License.
*/
-var Q = require('Q'),
- os = require('os'),
+/*jshint node:true*/
+
+var Q = require('Q');
+
+var MSBuildTools;
+try {
MSBuildTools = require('../../template/cordova/lib/MSBuildTools');
+} catch (ex) {
+ // If previous import fails, we probably running this script
--- End diff --
Typo: "we" should be "we're" or "we are".
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org
[GitHub] cordova-windows pull request: CB-8954 Adds support for `requiremen...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/cordova-windows/pull/83
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org