You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cordova.apache.org by an...@apache.org on 2016/07/25 19:04:33 UTC
[3/7] cordova-windows git commit: CB-11548 Fix issues where MSBuild
cannot be found
CB-11548 Fix issues where MSBuild cannot be found
This is due to VS installation change. Let Cordova Windows look for MSBuild under VSINSTALLDIR process environment variable. Also, make Cordova Windows more resilient to MSBuild version change. This commit contains work by Vladimir and Jason. Code is reviewed by Vladimir and Tim Barham. Commits are squashed.
Project: http://git-wip-us.apache.org/repos/asf/cordova-windows/repo
Commit: http://git-wip-us.apache.org/repos/asf/cordova-windows/commit/70adcc6f
Tree: http://git-wip-us.apache.org/repos/asf/cordova-windows/tree/70adcc6f
Diff: http://git-wip-us.apache.org/repos/asf/cordova-windows/diff/70adcc6f
Branch: refs/heads/4.4.x
Commit: 70adcc6fc807751e3fc0173c9dcd30c259600bab
Parents: 070a6f0
Author: Jason Wang (DEVDIV) <ji...@microsoft.com>
Authored: Thu Jul 21 13:47:49 2016 -0700
Committer: Vladimir Kotikov <v-...@microsoft.com>
Committed: Mon Jul 25 11:59:21 2016 +0300
----------------------------------------------------------------------
spec/unit/build.spec.js | 25 ++++++++++++--
template/cordova/lib/MSBuildTools.js | 23 ++++++++++---
template/cordova/lib/build.js | 57 ++++++++++++++++---------------
3 files changed, 71 insertions(+), 34 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/cordova-windows/blob/70adcc6f/spec/unit/build.spec.js
----------------------------------------------------------------------
diff --git a/spec/unit/build.spec.js b/spec/unit/build.spec.js
index dc48356..e8fd8d8 100644
--- a/spec/unit/build.spec.js
+++ b/spec/unit/build.spec.js
@@ -74,10 +74,12 @@ function createConfigParserMock(winVersion, phoneVersion) {
describe('run method', function() {
var findAvailableVersionOriginal,
+ findAllAvailableVersionsOriginal,
configParserOriginal;
beforeEach(function () {
findAvailableVersionOriginal = build.__get__('MSBuildTools.findAvailableVersion');
+ findAllAvailableVersionsOriginal = build.__get__('MSBuildTools.findAllAvailableVersions');
configParserOriginal = build.__get__('ConfigParser');
var originalBuildMethod = build.run;
@@ -101,6 +103,7 @@ describe('run method', function() {
afterEach(function() {
build.__set__('MSBuildTools.findAvailableVersion', findAvailableVersionOriginal);
+ build.__set__('MSBuildTools.findAllAvailableVersions', findAllAvailableVersionsOriginal);
build.__set__('ConfigParser', configParserOriginal);
});
@@ -336,7 +339,7 @@ describe('run method', function() {
it('spec.13 should be able to override target via --appx parameter', function(done) {
var buildSpy = jasmine.createSpy().andCallFake(function(solutionFile, buildType, buildArch) {
// check that we build Windows 10 and not Windows 8.1
- expect(solutionFile.toLowerCase().indexOf('cordovaapp.windows10.jsproj') >=0).toBe(true);
+ expect(solutionFile.toLowerCase()).toMatch('cordovaapp.windows10.jsproj');
});
createFindAllAvailableVersionsMock([{version: '14.0', buildProject: buildSpy, path: testPath }]);
@@ -352,6 +355,7 @@ describe('run method', function() {
it('spec.14 should use user-specified msbuild if VSINSTALLDIR variable is set', function (done) {
var customMSBuildPath = '/some/path';
+ var msBuildBinPath = path.join(customMSBuildPath, 'MSBuild/15.0/Bin');
var customMSBuildVersion = '15.0';
process.env.VSINSTALLDIR = customMSBuildPath;
@@ -368,9 +372,26 @@ describe('run method', function() {
.fail(fail)
.finally(function() {
expect(fail).not.toHaveBeenCalled();
- expect(MSBuildTools.getMSBuildToolsAt).toHaveBeenCalledWith(customMSBuildPath);
+ expect(MSBuildTools.getMSBuildToolsAt).toHaveBeenCalledWith(msBuildBinPath);
delete process.env.VSINSTALLDIR;
done();
});
});
+
+ it('spec.15 should choose latest version if there are multiple versions available with minor version difference', function(done) {
+ var fail = jasmine.createSpy('fail');
+ var buildTools14 = {version: '14.0', buildProject: jasmine.createSpy('buildTools14'), path: testPath };
+ var buildTools15 = {version: '15.0', buildProject: jasmine.createSpy('buildTools15'), path: testPath };
+ var buildTools151 = {version: '15.1', buildProject: jasmine.createSpy('buildTools151'), path: testPath };
+
+ createFindAllAvailableVersionsMock([buildTools14, buildTools15, buildTools151]);
+ // explicitly specify Windows 10 as target
+ build.run({argv: ['--appx=uap']})
+ .fail(fail)
+ .finally(function() {
+ expect(fail).not.toHaveBeenCalled();
+ expect(buildTools151.buildProject).toHaveBeenCalled();
+ done();
+ });
+ });
});
http://git-wip-us.apache.org/repos/asf/cordova-windows/blob/70adcc6f/template/cordova/lib/MSBuildTools.js
----------------------------------------------------------------------
diff --git a/template/cordova/lib/MSBuildTools.js b/template/cordova/lib/MSBuildTools.js
index 1296f32..7cc6794 100644
--- a/template/cordova/lib/MSBuildTools.js
+++ b/template/cordova/lib/MSBuildTools.js
@@ -84,7 +84,7 @@ module.exports.findAvailableVersion = function () {
});
};
-module.exports.findAllAvailableVersions = function () {
+function findAllAvailableVersionsFallBack() {
var versions = ['15.0', '14.0', '12.0', '4.0'];
events.emit('verbose', 'Searching for available MSBuild versions...');
@@ -93,6 +93,21 @@ module.exports.findAllAvailableVersions = function () {
return !!item;
});
});
+}
+
+module.exports.findAllAvailableVersions = function () {
+ // CB-11548 use VSINSTALLDIR environment if defined to find MSBuild. If VSINSTALLDIR
+ // is not specified or doesn't contain the MSBuild path we are looking for - fall back
+ // to default discovery mechanism.
+ if (process.env.VSINSTALLDIR) {
+ var msBuildPath = path.join(process.env.VSINSTALLDIR, 'MSBuild/15.0/Bin');
+ return module.exports.getMSBuildToolsAt(msBuildPath)
+ .then(function (msBuildTools) {
+ return [msBuildTools];
+ }).catch(findAllAvailableVersionsFallBack);
+ }
+
+ return findAllAvailableVersionsFallBack();
};
/**
@@ -101,7 +116,7 @@ module.exports.findAllAvailableVersions = function () {
* @param {String} location FS location where to search for MSBuild
* @returns Promise<MSBuildTools> The MSBuildTools instance at specified location
*/
-function getMSBuildToolsAt(location) {
+module.exports.getMSBuildToolsAt = function (location) {
var msbuildExe = path.resolve(location, 'msbuild');
// TODO: can we account on these params availability and printed version format?
@@ -111,9 +126,7 @@ function getMSBuildToolsAt(location) {
var version = output.match(/^(\d+\.\d+)/)[1];
return new MSBuildTools(version, location);
});
-}
-
-module.exports.getMSBuildToolsAt = getMSBuildToolsAt;
+};
function checkMSBuildVersion(version) {
return spawn('reg', ['query', 'HKLM\\SOFTWARE\\Microsoft\\MSBuild\\ToolsVersions\\' + version, '/v', 'MSBuildToolsPath'])
http://git-wip-us.apache.org/repos/asf/cordova-windows/blob/70adcc6f/template/cordova/lib/build.js
----------------------------------------------------------------------
diff --git a/template/cordova/lib/build.js b/template/cordova/lib/build.js
index 84dedd8..6c173a6 100644
--- a/template/cordova/lib/build.js
+++ b/template/cordova/lib/build.js
@@ -24,6 +24,7 @@ var shell = require('shelljs');
var utils = require('./utils');
var prepare = require('./prepare');
var package = require('./package');
+var Version = require('./Version');
var MSBuildTools = require('./MSBuildTools');
var AppxManifest = require('./AppxManifest');
var ConfigParser = require('./ConfigParser');
@@ -57,23 +58,7 @@ module.exports.run = function run (buildOptions) {
var buildConfig = parseAndValidateArgs(buildOptions);
- return Q().then(function () {
- // CB-something use VSINSTALLDIR environment if defined to find MSBuild
- // If VSINSTALLDIR is not specified use default discovery mechanism
- if (!process.env.VSINSTALLDIR) {
- return MSBuildTools.findAllAvailableVersions();
- }
-
- events.emit('log', 'Found VSINSTALLDIR environment variable. Attempting to build project using that version of MSBuild');
-
- return MSBuildTools.getMSBuildToolsAt(process.env.VSINSTALLDIR)
- .then(function (tools) { return [tools]; })
- .catch(function (err) {
- // If we failed to find msbuild at VSINSTALLDIR
- // location we fall back to default discovery
- return MSBuildTools.findAllAvailableVersions();
- });
- })
+ return MSBuildTools.findAllAvailableVersions()
.then(function(msbuildTools) {
// Apply build related configs
prepare.updateBuildConfig(buildConfig);
@@ -284,7 +269,7 @@ function updateManifestWithPublisher(allMsBuildVersions, config) {
if (!config.publisherId) return;
var selectedBuildTargets = getBuildTargets(config);
- var msbuild = getMsBuildForTargets(selectedBuildTargets, config, allMsBuildVersions);
+ var msbuild = getLatestMSBuild(allMsBuildVersions);
var myBuildTargets = filterSupportedTargets(selectedBuildTargets, msbuild);
var manifestFiles = myBuildTargets.map(function(proj) {
return projFilesToManifests[proj];
@@ -299,7 +284,7 @@ function updateManifestWithPublisher(allMsBuildVersions, config) {
function buildTargets(allMsBuildVersions, config) {
// filter targets to make sure they are supported on this development machine
var selectedBuildTargets = getBuildTargets(config);
- var msbuild = getMsBuildForTargets(selectedBuildTargets, config, allMsBuildVersions);
+ var msbuild = getLatestMSBuild(allMsBuildVersions);
if (!msbuild) {
return Q.reject(new CordovaError('No valid MSBuild was detected for the selected target.'));
}
@@ -490,14 +475,27 @@ function getBuildTargets(buildConfig) {
return targets;
}
-function getMsBuildForTargets(selectedTargets, buildConfig, allMsBuildVersions) {
+function getLatestMSBuild(allMsBuildVersions) {
var availableVersions = allMsBuildVersions
- .reduce(function(obj, msbuildVersion) {
- obj[msbuildVersion.version] = msbuildVersion;
- return obj;
- }, {});
+ .filter(function (buildTools) {
+ // Sanitize input - filter out tools w/ invalid versions
+ return Version.tryParse(buildTools.version);
+ })
+ .sort(function (a, b) {
+ // Sort tools list - use parsed Version objects for that
+ // to respect both major and minor versions segments
+ var parsedA = Version.fromString(a.version);
+ var parsedB = Version.fromString(b.version);
+
+ if (parsedA.gt(parsedB)) return -1;
+ if (parsedA.eq(parsedB)) return 0;
+ return 1;
+ });
- return availableVersions['15.0'] || availableVersions['14.0'] || availableVersions['12.0'];
+ if (availableVersions.length > 0) {
+ // After sorting the first item will be the highest version available
+ return availableVersions[0];
+ }
}
// TODO: Fix this so that it outlines supported versions based on version criteria:
@@ -524,10 +522,15 @@ function filterSupportedTargets (targets, msbuild) {
var targetFilters = {
'12.0': msBuild12TargetsFilter,
'14.0': msBuild14TargetsFilter,
- '15.0': msBuild15TargetsFilter
+ '15.x': msBuild15TargetsFilter,
+ get: function (version) {
+ // Apart from exact match also try to get filter for version range
+ // so we can find for example targets for version '15.1'
+ return this[version] || this[version.replace(/\.\d+$/, '.x')];
+ }
};
- var filter = targetFilters[msbuild.version];
+ var filter = targetFilters.get(msbuild.version);
if (!filter) {
events.emit('warn', 'MSBuild v' + msbuild.version + ' is not supported, aborting.');
return [];
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@cordova.apache.org
For additional commands, e-mail: commits-help@cordova.apache.org