From 97fa5c03e152b94d89a559f5576af571a25defa7 Mon Sep 17 00:00:00 2001 From: Alexander Sklar Date: Wed, 8 Apr 2020 19:16:29 -0700 Subject: [PATCH 1/5] Improve inner loop and error reporting (local CLI) --- vnext/local-cli/runWindows/runWindows.js | 16 +++++- .../runWindows/utils/WindowsStoreAppUtils.ps1 | 2 +- vnext/local-cli/runWindows/utils/build.js | 26 +++++++--- .../runWindows/utils/commandWithProgress.js | 31 +++++++++++- vnext/local-cli/runWindows/utils/deploy.js | 49 +++++++------------ .../runWindows/utils/msbuildtools.js | 9 ++-- 6 files changed, 84 insertions(+), 49 deletions(-) diff --git a/vnext/local-cli/runWindows/runWindows.js b/vnext/local-cli/runWindows/runWindows.js index 3a4c9e40bea..847b39f4915 100644 --- a/vnext/local-cli/runWindows/runWindows.js +++ b/vnext/local-cli/runWindows/runWindows.js @@ -12,6 +12,17 @@ const {newError, newInfo} = require('./utils/commandWithProgress'); const info = require('./utils/info'); const msbuildtools = require('./utils/msbuildtools'); const autolink = require('./utils/autolink'); +const fs = require('fs'); + +function unique(arr) { + let arrOut = []; + for (let i = 0; i < arr.length; i++) { + if (!arrOut.find(x => x === arr[i])) { + arrOut.push(arr[i]); + } + } + return arrOut; +} async function runWindows(config, args, options) { const verbose = options.logging; @@ -70,6 +81,9 @@ async function runWindows(config, args, options) { verbose, ); } catch (e) { + if (!e) { + e = unique(fs.readFileSync('msbuild.err').toString().split('\n')); + } newError( `Build failed with message ${e}. Check your build configuration.`, ); @@ -89,7 +103,7 @@ async function runWindows(config, args, options) { await deploy.deployToDesktop(options, verbose); } } catch (e) { - newError(`Failed to deploy: ${e.message}`); + newError(`Failed to deploy${e ? `: ${e.message}` : ''}`); process.exit(1); } } else { diff --git a/vnext/local-cli/runWindows/utils/WindowsStoreAppUtils.ps1 b/vnext/local-cli/runWindows/utils/WindowsStoreAppUtils.ps1 index 68a76ce1874..2a9ac5f0038 100644 --- a/vnext/local-cli/runWindows/utils/WindowsStoreAppUtils.ps1 +++ b/vnext/local-cli/runWindows/utils/WindowsStoreAppUtils.ps1 @@ -88,7 +88,7 @@ function EnableDevmode { New-Item -Path $RegistryKeyPath -ItemType Directory -Force } - Set-ItemProperty -Path $RegistryKeyPath -Name AllowDevelopmentWithoutDevLicense -Value 1 + Set-ItemProperty -Path $RegistryKeyPath -Name AllowDevelopmentWithoutDevLicense -Value 1 -ErrorAction Stop } # diff --git a/vnext/local-cli/runWindows/utils/build.js b/vnext/local-cli/runWindows/utils/build.js index 4d4ad38e431..5d89f805657 100644 --- a/vnext/local-cli/runWindows/utils/build.js +++ b/vnext/local-cli/runWindows/utils/build.js @@ -42,7 +42,7 @@ async function buildSolution( } async function nugetRestore(nugetPath, slnFile, verbose, msbuildVersion) { - const text = 'Restoring NuGets'; + const text = 'Restoring NuGet packages '; const spinner = newSpinner(text); console.log(nugetPath); await commandWithProgress( @@ -74,7 +74,7 @@ async function restoreNuGetPackages(options, slnFile, verbose) { ensureNugetSpinner, dlNugetText, 'powershell', - `Invoke-WebRequest https://dist.nuget.org/win-x86-commandline/v4.9.2/nuget.exe -outfile ${nugetPath}`.split( + `$progressPreference = [System.Management.Automation.ActionPreference]::SilentlyContinue; Invoke-WebRequest https://dist.nuget.org/win-x86-commandline/v4.9.2/nuget.exe -outfile ${nugetPath}`.split( ' ', ), verbose, @@ -83,12 +83,22 @@ async function restoreNuGetPackages(options, slnFile, verbose) { ensureNugetSpinner.succeed('Found NuGet Binary'); const msbuildTools = MSBuildTools.findAvailableVersion('x86', verbose); - await nugetRestore( - nugetPath, - slnFile, - verbose, - msbuildTools.installationVersion, - ); + try { + await nugetRestore( + nugetPath, + slnFile, + verbose, + msbuildTools.installationVersion, + ); + } catch (e) { + if (!options.isRetryingNuget) { + options.isRetryingNuget = true; + fs.unlinkSync(nugetPath); + await restoreNuGetPackages(options, slnFile, verbose); + return; + } + throw e; + } } function getSolutionFile(options) { diff --git a/vnext/local-cli/runWindows/utils/commandWithProgress.js b/vnext/local-cli/runWindows/utils/commandWithProgress.js index 75f43714ef3..219dd045caf 100644 --- a/vnext/local-cli/runWindows/utils/commandWithProgress.js +++ b/vnext/local-cli/runWindows/utils/commandWithProgress.js @@ -41,6 +41,27 @@ function newSpinner(text) { return ora(options).start(); } +async function runPowerShellScriptFunction(text, script, funcName, verbose) { + try { + await commandWithProgress( + newSpinner(text), + text, + 'powershell', + [ + '-NoProfile', + '-ExecutionPolicy', + 'RemoteSigned', + `Import-Module "${script}"; ${funcName} -ErrorAction Stop; exit $LASTEXITCODE`, + ], + verbose, + ); + } catch { + // The error output from the process will be shown if verbose is set. + // We don't capture the process output if verbose is set, but at least we have the task name in text, so throw that. + throw new Error(text); + } +} + function commandWithProgress(spinner, taskDoingName, command, args, verbose) { return new Promise(function(resolve, reject) { const spawnOptions = verbose ? {stdio: 'inherit'} : {}; @@ -50,7 +71,7 @@ function commandWithProgress(spinner, taskDoingName, command, args, verbose) { } const cp = child_process.spawn(command, args, spawnOptions); - + var error = null; if (!verbose) { cp.stdout.on('data', chunk => { const text = chunk.toString(); @@ -58,10 +79,15 @@ function commandWithProgress(spinner, taskDoingName, command, args, verbose) { }); cp.stderr.on('data', chunk => { const text = chunk.toString(); + if (!error) { + error = text; + } if (verbose) { console.error(chalk.red(text)); } - setSpinnerText(spinner, taskDoingName + ': ERROR: ', text); + if (text) { + setSpinnerText(spinner, taskDoingName + ': ERROR: ', error); + } }); } cp.on('error', e => { @@ -105,4 +131,5 @@ module.exports = { newSpinner, newSuccess, newWarn, + runPowerShellScriptFunction, }; diff --git a/vnext/local-cli/runWindows/utils/deploy.js b/vnext/local-cli/runWindows/utils/deploy.js index 5bddc239f59..3edfeac27ba 100644 --- a/vnext/local-cli/runWindows/utils/deploy.js +++ b/vnext/local-cli/runWindows/utils/deploy.js @@ -20,6 +20,7 @@ const { newWarn, newSpinner, commandWithProgress, + runPowerShellScriptFunction, } = require('./commandWithProgress'); function pushd(pathArg) { @@ -166,36 +167,24 @@ async function deployToDesktop(options, verbose) { const popd = pushd(options.root); - const removingText = 'Removing old version of the app'; - await commandWithProgress( - newSpinner(removingText), - removingText, - 'powershell', - `-NoProfile -ExecutionPolicy RemoteSigned Import-Module "${windowsStoreAppUtils}" ; Uninstall-App ${appName}`.split( - ' ', - ), + await runPowerShellScriptFunction( + 'Removing old version of the app', + windowsStoreAppUtils, + `Uninstall-App ${appName}`, verbose, ); - const devmodeText = 'Enabling Developer Mode'; - const devmodeEnable = `-NoProfile -ExecutionPolicy RemoteSigned Import-Module "${windowsStoreAppUtils}"; EnableDevmode "${script}"`; - - await commandWithProgress( - newSpinner(devmodeText), - devmodeText, - 'powershell', - devmodeEnable.split(' '), + await runPowerShellScriptFunction( + 'Enabling Developer Mode', + windowsStoreAppUtils, + `EnableDevMode "${script}"`, verbose, ); - const installingText = 'Installing new version of the app'; - const installApp = `-NoProfile -ExecutionPolicy RemoteSigned Import-Module "${windowsStoreAppUtils}"; Install-App "${script}" -Force`; - - await commandWithProgress( - newSpinner(installingText), - installingText, - 'powershell', - installApp.split(' '), + await runPowerShellScriptFunction( + 'Installing new version of the app', + windowsStoreAppUtils, + `Install-App "${script}" -Force`, verbose, ); @@ -223,14 +212,10 @@ async function deployToDesktop(options, verbose) { ); if (shouldLaunchApp(options)) { - const startingText = 'Starting the app'; - await commandWithProgress( - newSpinner(startingText), - startingText, - 'powershell', - `-ExecutionPolicy RemoteSigned Import-Module "${windowsStoreAppUtils}"; Start-Locally ${appName} ${args}`.split( - ' ', - ), + await runPowerShellScriptFunction( + 'Starting the app', + windowsStoreAppUtils, + `Start-Locally ${appName} ${args}`, verbose, ); } else { diff --git a/vnext/local-cli/runWindows/utils/msbuildtools.js b/vnext/local-cli/runWindows/utils/msbuildtools.js index 00e8d616b5e..cdacefaf6c3 100644 --- a/vnext/local-cli/runWindows/utils/msbuildtools.js +++ b/vnext/local-cli/runWindows/utils/msbuildtools.js @@ -51,18 +51,17 @@ class MSBuildTools { newInfo(`Build configuration: ${buildType}`); newInfo(`Build platform: ${buildArch}`); - //const verbosityOption = verbose ? 'normal' : 'quiet'; - const verbosityOption = 'normal'; + const verbosityOption = verbose ? 'normal' : 'quiet'; const args = [ `/clp:NoSummary;NoItemAndPropertyList;Verbosity=${verbosityOption}`, '/nologo', `/p:Configuration=${buildType}`, `/p:Platform=${buildArch}`, '/p:AppxBundle=Never', + '/bl', + '/flp1:errorsonly;logfile=msbuild.err' ]; - args.push('/bl'); - if (msBuildProps) { Object.keys(msBuildProps).forEach(function(key) { args.push(`/p:${key}=${msBuildProps[key]}`); @@ -73,7 +72,7 @@ class MSBuildTools { checkRequirements.isWinSdkPresent('10.0'); } catch (e) { newError(e.message); - return; + throw e; } console.log(`Running MSBuild with args ${args.join(' ')}`); From 18949ce7b75b4381fe49478923d6412a99075412 Mon Sep 17 00:00:00 2001 From: Alexander Sklar Date: Wed, 8 Apr 2020 19:30:11 -0700 Subject: [PATCH 2/5] Change files --- ...-native-windows-2020-04-08-19-30-11-fork-localCLI.json | 8 ++++++++ 1 file changed, 8 insertions(+) create mode 100644 change/react-native-windows-2020-04-08-19-30-11-fork-localCLI.json diff --git a/change/react-native-windows-2020-04-08-19-30-11-fork-localCLI.json b/change/react-native-windows-2020-04-08-19-30-11-fork-localCLI.json new file mode 100644 index 00000000000..4ca1e6568e3 --- /dev/null +++ b/change/react-native-windows-2020-04-08-19-30-11-fork-localCLI.json @@ -0,0 +1,8 @@ +{ + "type": "prerelease", + "comment": "Improve inner loop and error reporting (local CLI)", + "packageName": "react-native-windows", + "email": "asklar@microsoft.com", + "dependentChangeType": "patch", + "date": "2020-04-09T02:30:11.182Z" +} \ No newline at end of file From 4a72d4a3d9bdcb3a2363b426157567296c3ec4b5 Mon Sep 17 00:00:00 2001 From: Alexander Sklar Date: Wed, 8 Apr 2020 22:18:52 -0700 Subject: [PATCH 3/5] PR changes --- vnext/local-cli/runWindows/runWindows.js | 18 ++------ vnext/local-cli/runWindows/utils/build.js | 5 +- .../runWindows/utils/commandWithProgress.js | 21 +++++---- .../runWindows/utils/msbuildtools.js | 46 +++++++++++++++---- 4 files changed, 56 insertions(+), 34 deletions(-) diff --git a/vnext/local-cli/runWindows/runWindows.js b/vnext/local-cli/runWindows/runWindows.js index 847b39f4915..0c8188bb917 100644 --- a/vnext/local-cli/runWindows/runWindows.js +++ b/vnext/local-cli/runWindows/runWindows.js @@ -12,17 +12,6 @@ const {newError, newInfo} = require('./utils/commandWithProgress'); const info = require('./utils/info'); const msbuildtools = require('./utils/msbuildtools'); const autolink = require('./utils/autolink'); -const fs = require('fs'); - -function unique(arr) { - let arrOut = []; - for (let i = 0; i < arr.length; i++) { - if (!arrOut.find(x => x === arr[i])) { - arrOut.push(arr[i]); - } - } - return arrOut; -} async function runWindows(config, args, options) { const verbose = options.logging; @@ -81,11 +70,10 @@ async function runWindows(config, args, options) { verbose, ); } catch (e) { - if (!e) { - e = unique(fs.readFileSync('msbuild.err').toString().split('\n')); - } newError( - `Build failed with message ${e}. Check your build configuration.`, + `Build failed with message ${ + e.message + }. Check your build configuration.`, ); process.exit(1); } diff --git a/vnext/local-cli/runWindows/utils/build.js b/vnext/local-cli/runWindows/utils/build.js index 5d89f805657..620bd2aaaa5 100644 --- a/vnext/local-cli/runWindows/utils/build.js +++ b/vnext/local-cli/runWindows/utils/build.js @@ -92,10 +92,9 @@ async function restoreNuGetPackages(options, slnFile, verbose) { ); } catch (e) { if (!options.isRetryingNuget) { - options.isRetryingNuget = true; + const retryOptions = Object.assign({isRetryingNuget: true}, options); fs.unlinkSync(nugetPath); - await restoreNuGetPackages(options, slnFile, verbose); - return; + return restoreNuGetPackages(retryOptions, slnFile, verbose); } throw e; } diff --git a/vnext/local-cli/runWindows/utils/commandWithProgress.js b/vnext/local-cli/runWindows/utils/commandWithProgress.js index 219dd045caf..be19f0ad790 100644 --- a/vnext/local-cli/runWindows/utils/commandWithProgress.js +++ b/vnext/local-cli/runWindows/utils/commandWithProgress.js @@ -41,11 +41,16 @@ function newSpinner(text) { return ora(options).start(); } -async function runPowerShellScriptFunction(text, script, funcName, verbose) { +async function runPowerShellScriptFunction( + taskDescription, + script, + funcName, + verbose, +) { try { await commandWithProgress( - newSpinner(text), - text, + newSpinner(taskDescription), + taskDescription, 'powershell', [ '-NoProfile', @@ -58,7 +63,7 @@ async function runPowerShellScriptFunction(text, script, funcName, verbose) { } catch { // The error output from the process will be shown if verbose is set. // We don't capture the process output if verbose is set, but at least we have the task name in text, so throw that. - throw new Error(text); + throw new Error(taskDescription); } } @@ -71,7 +76,7 @@ function commandWithProgress(spinner, taskDoingName, command, args, verbose) { } const cp = child_process.spawn(command, args, spawnOptions); - var error = null; + let firstErrorLine = null; if (!verbose) { cp.stdout.on('data', chunk => { const text = chunk.toString(); @@ -79,14 +84,14 @@ function commandWithProgress(spinner, taskDoingName, command, args, verbose) { }); cp.stderr.on('data', chunk => { const text = chunk.toString(); - if (!error) { - error = text; + if (!firstErrorLine) { + firstErrorLine = text; } if (verbose) { console.error(chalk.red(text)); } if (text) { - setSpinnerText(spinner, taskDoingName + ': ERROR: ', error); + setSpinnerText(spinner, taskDoingName + ': ERROR: ', firstErrorLine); } }); } diff --git a/vnext/local-cli/runWindows/utils/msbuildtools.js b/vnext/local-cli/runWindows/utils/msbuildtools.js index cdacefaf6c3..44177f93818 100644 --- a/vnext/local-cli/runWindows/utils/msbuildtools.js +++ b/vnext/local-cli/runWindows/utils/msbuildtools.js @@ -22,6 +22,16 @@ const { } = require('./commandWithProgress'); const execSync = require('child_process').execSync; +function unique(arr) { + let arrOut = []; + for (let i = 0; i < arr.length; i++) { + if (!arrOut.find(x => x === arr[i])) { + arrOut.push(arr[i]); + } + } + return arrOut; +} + const MSBUILD_VERSIONS = ['16.0']; class MSBuildTools { @@ -52,6 +62,7 @@ class MSBuildTools { newInfo(`Build platform: ${buildArch}`); const verbosityOption = verbose ? 'normal' : 'quiet'; + const errorLog = path.join(process.env.temp, `msbuild_${process.pid}.err`); const args = [ `/clp:NoSummary;NoItemAndPropertyList;Verbosity=${verbosityOption}`, '/nologo', @@ -59,7 +70,7 @@ class MSBuildTools { `/p:Platform=${buildArch}`, '/p:AppxBundle=Never', '/bl', - '/flp1:errorsonly;logfile=msbuild.err' + `/flp1:errorsonly;logfile=${errorLog}`, ]; if (msBuildProps) { @@ -79,13 +90,32 @@ class MSBuildTools { const progressName = 'Building Solution'; const spinner = newSpinner(progressName); - await commandWithProgress( - spinner, - progressName, - path.join(this.path, 'msbuild.exe'), - [slnFile].concat(args), - verbose, - ); + try { + await commandWithProgress( + spinner, + progressName, + path.join(this.path, 'msbuild.exe'), + [slnFile].concat(args), + verbose, + ); + } catch (e) { + let error = e; + if (!e) { + error = new Error( + unique( + fs + .readFileSync(errorLog) + .toString() + .split('\n'), + )[0], + ); + } + throw error; + } + // If we have no errors, delete the error log when we're done + if (fs.statSync(errorLog).size === 0) { + fs.unlinkSync(errorLog); + } } } From 9cd08b790b4020551650d9468f416fdec46ff018 Mon Sep 17 00:00:00 2001 From: Alexander Sklar Date: Wed, 8 Apr 2020 23:20:47 -0700 Subject: [PATCH 4/5] simplify --- vnext/local-cli/runWindows/runWindows.js | 4 +++ .../runWindows/utils/msbuildtools.js | 27 +++++++------------ 2 files changed, 13 insertions(+), 18 deletions(-) diff --git a/vnext/local-cli/runWindows/runWindows.js b/vnext/local-cli/runWindows/runWindows.js index 0c8188bb917..8b951b663be 100644 --- a/vnext/local-cli/runWindows/runWindows.js +++ b/vnext/local-cli/runWindows/runWindows.js @@ -12,6 +12,7 @@ const {newError, newInfo} = require('./utils/commandWithProgress'); const info = require('./utils/info'); const msbuildtools = require('./utils/msbuildtools'); const autolink = require('./utils/autolink'); +const chalk = require('chalk'); async function runWindows(config, args, options) { const verbose = options.logging; @@ -75,6 +76,9 @@ async function runWindows(config, args, options) { e.message }. Check your build configuration.`, ); + if (e.logfile) { + console.log('See', chalk.bold(e.logfile)); + } process.exit(1); } } else { diff --git a/vnext/local-cli/runWindows/utils/msbuildtools.js b/vnext/local-cli/runWindows/utils/msbuildtools.js index 44177f93818..abe67b82dbf 100644 --- a/vnext/local-cli/runWindows/utils/msbuildtools.js +++ b/vnext/local-cli/runWindows/utils/msbuildtools.js @@ -22,16 +22,6 @@ const { } = require('./commandWithProgress'); const execSync = require('child_process').execSync; -function unique(arr) { - let arrOut = []; - for (let i = 0; i < arr.length; i++) { - if (!arrOut.find(x => x === arr[i])) { - arrOut.push(arr[i]); - } - } - return arrOut; -} - const MSBUILD_VERSIONS = ['16.0']; class MSBuildTools { @@ -61,7 +51,7 @@ class MSBuildTools { newInfo(`Build configuration: ${buildType}`); newInfo(`Build platform: ${buildArch}`); - const verbosityOption = verbose ? 'normal' : 'quiet'; + const verbosityOption = verbose ? 'normal' : 'minimal'; const errorLog = path.join(process.env.temp, `msbuild_${process.pid}.err`); const args = [ `/clp:NoSummary;NoItemAndPropertyList;Verbosity=${verbosityOption}`, @@ -86,7 +76,9 @@ class MSBuildTools { throw e; } - console.log(`Running MSBuild with args ${args.join(' ')}`); + if (verbose) { + console.log(`Running MSBuild with args ${args.join(' ')}`); + } const progressName = 'Building Solution'; const spinner = newSpinner(progressName); @@ -102,13 +94,12 @@ class MSBuildTools { let error = e; if (!e) { error = new Error( - unique( - fs - .readFileSync(errorLog) - .toString() - .split('\n'), - )[0], + fs + .readFileSync(errorLog) + .toString() + .split(EOL)[0], ); + error.logfile = errorLog; } throw error; } From 44603465662f73ad07f0ccaf997d5505ac57284c Mon Sep 17 00:00:00 2001 From: Alexander Sklar Date: Thu, 9 Apr 2020 19:35:39 -0700 Subject: [PATCH 5/5] call promise versions of fs methods --- .gitignore | 1 + vnext/local-cli/runWindows/utils/msbuildtools.js | 14 ++++++-------- 2 files changed, 7 insertions(+), 8 deletions(-) diff --git a/.gitignore b/.gitignore index 55964d8e227..2dfa77403f6 100644 --- a/.gitignore +++ b/.gitignore @@ -122,6 +122,7 @@ packages/ #Other files *.DotSettings .vs/ +.vscode/ *project.lock.json jsconfig.json package-lock.json diff --git a/vnext/local-cli/runWindows/utils/msbuildtools.js b/vnext/local-cli/runWindows/utils/msbuildtools.js index abe67b82dbf..a6ad327a507 100644 --- a/vnext/local-cli/runWindows/utils/msbuildtools.js +++ b/vnext/local-cli/runWindows/utils/msbuildtools.js @@ -93,19 +93,17 @@ class MSBuildTools { } catch (e) { let error = e; if (!e) { - error = new Error( - fs - .readFileSync(errorLog) - .toString() - .split(EOL)[0], - ); + const firstMessage = (await fs.promises.readFile(errorLog)) + .toString() + .split(EOL)[0]; + error = new Error(firstMessage); error.logfile = errorLog; } throw error; } // If we have no errors, delete the error log when we're done - if (fs.statSync(errorLog).size === 0) { - fs.unlinkSync(errorLog); + if ((await fs.promises.stat(errorLog)).size === 0) { + await fs.promises.unlink(errorLog); } } }