Repository: cordova-windows Updated Branches: refs/heads/master 58080c4c4 -> 2d7d219fc
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/2d7d219f Tree: http://git-wip-us.apache.org/repos/asf/cordova-windows/tree/2d7d219f Diff: http://git-wip-us.apache.org/repos/asf/cordova-windows/diff/2d7d219f Branch: refs/heads/master Commit: 2d7d219fc1864fa22f87cebadac173fd41091351 Parents: 58080c4 Author: Jason Wang (DEVDIV) <jic...@microsoft.com> Authored: Thu Jul 21 13:47:49 2016 -0700 Committer: TimBarham <tim.bar...@microsoft.com> Committed: Fri Jul 22 10:51:43 2016 -0700 ---------------------------------------------------------------------- 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/2d7d219f/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/2d7d219f/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/2d7d219f/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-unsubscr...@cordova.apache.org For additional commands, e-mail: commits-h...@cordova.apache.org
