From 56e32b3de6db8c2ef542258010ce5e8647cca960 Mon Sep 17 00:00:00 2001 From: Piotr Grzesik Date: Wed, 24 Nov 2021 11:42:35 +0100 Subject: [PATCH] refactor: Adapt to `async` version of `spawn` --- lib/docker.js | 37 ++++++------- lib/pip.js | 147 +++++++++++++++++++++++++++----------------------- lib/pipenv.js | 28 +++++----- lib/poetry.js | 47 ++++++++-------- package.json | 1 + 5 files changed, 136 insertions(+), 124 deletions(-) diff --git a/lib/docker.js b/lib/docker.js index 328e3088..94229b21 100644 --- a/lib/docker.js +++ b/lib/docker.js @@ -1,4 +1,4 @@ -const { spawnSync } = require('child_process'); +const spawn = require('child-process-ext/spawn'); const isWsl = require('is-wsl'); const fse = require('fs-extra'); const path = require('path'); @@ -8,18 +8,19 @@ const path = require('path'); * @param {string[]} options * @return {Object} */ -function dockerCommand(options) { +async function dockerCommand(options) { const cmd = 'docker'; - const ps = spawnSync(cmd, options, { encoding: 'utf-8' }); - if (ps.error) { - if (ps.error.code === 'ENOENT') { + try { + return await spawn(cmd, options, { encoding: 'utf-8' }); + } catch (e) { + if ( + e.stderrBuffer && + e.stderrBuffer.toString().includes('command not found') + ) { throw new Error('docker not found! Please install it.'); } - throw new Error(ps.error); - } else if (ps.status !== 0) { - throw new Error(ps.stderr); + throw e; } - return ps; } /** @@ -28,7 +29,7 @@ function dockerCommand(options) { * @param {string[]} extraArgs * @return {string} The name of the built docker image. */ -function buildImage(dockerFile, extraArgs) { +async function buildImage(dockerFile, extraArgs) { const imageName = 'sls-py-reqs-custom'; const options = ['build', '-f', dockerFile, '-t', imageName]; @@ -40,7 +41,7 @@ function buildImage(dockerFile, extraArgs) { options.push('.'); - dockerCommand(options); + await dockerCommand(options); return imageName; } @@ -72,7 +73,7 @@ function findTestFile(servicePath) { * @param {string} bindPath * @return {boolean} */ -function tryBindPath(serverless, bindPath, testFile) { +async function tryBindPath(serverless, bindPath, testFile) { const debug = process.env.SLS_DEBUG; const options = [ 'run', @@ -85,7 +86,7 @@ function tryBindPath(serverless, bindPath, testFile) { ]; try { if (debug) serverless.cli.log(`Trying bindPath ${bindPath} (${options})`); - const ps = dockerCommand(options); + const ps = await dockerCommand(options); if (debug) serverless.cli.log(ps.stdout.trim()); return ps.stdout.trim() === `/test/${testFile}`; } catch (err) { @@ -100,14 +101,14 @@ function tryBindPath(serverless, bindPath, testFile) { * @param {string} servicePath * @return {string} The bind path. */ -function getBindPath(serverless, servicePath) { +async function getBindPath(serverless, servicePath) { // Determine bind path if (process.platform !== 'win32' && !isWsl) { return servicePath; } // test docker is available - dockerCommand(['version']); + await dockerCommand(['version']); // find good bind path for Windows let bindPaths = []; @@ -144,7 +145,7 @@ function getBindPath(serverless, servicePath) { for (let i = 0; i < bindPaths.length; i++) { const bindPath = bindPaths[i]; - if (tryBindPath(serverless, bindPath, testFile)) { + if (await tryBindPath(serverless, bindPath, testFile)) { return bindPath; } } @@ -157,7 +158,7 @@ function getBindPath(serverless, servicePath) { * @param {string} bindPath * @return {boolean} */ -function getDockerUid(bindPath) { +async function getDockerUid(bindPath) { const options = [ 'run', '--rm', @@ -169,7 +170,7 @@ function getDockerUid(bindPath) { '%u', '/bin/sh', ]; - const ps = dockerCommand(options); + const ps = await dockerCommand(options); return ps.stdout.trim(); } diff --git a/lib/pip.js b/lib/pip.js index 244010c8..78af2e20 100644 --- a/lib/pip.js +++ b/lib/pip.js @@ -3,7 +3,7 @@ const rimraf = require('rimraf'); const path = require('path'); const get = require('lodash.get'); const set = require('lodash.set'); -const { spawnSync } = require('child_process'); +const spawn = require('child-process-ext/spawn'); const { quote } = require('shell-quote'); const { buildImage, getBindPath, getDockerUid } = require('./docker'); const { getStripCommand, getStripMode, deleteFiles } = require('./slim'); @@ -96,16 +96,23 @@ function generateRequirementsFile( } } -function pipAcceptsSystem(pythonBin) { +async function pipAcceptsSystem(pythonBin) { // Check if pip has Debian's --system option and set it if so - const pipTestRes = spawnSync(pythonBin, ['-m', 'pip', 'help', 'install']); - if (pipTestRes.error) { - if (pipTestRes.error.code === 'ENOENT') { + try { + const pipTestRes = await spawn(pythonBin, ['-m', 'pip', 'help', 'install']); + return ( + pipTestRes.stdoutBuffer && + pipTestRes.stdoutBuffer.toString().indexOf('--system') >= 0 + ); + } catch (e) { + if ( + e.stderrBuffer && + e.stderrBuffer.toString().includes('command not found') + ) { throw new Error(`${pythonBin} not found! Try the pythonBin option.`); } - throw pipTestRes.error; + throw e; } - return pipTestRes.stdout.toString().indexOf('--system') >= 0; } /** @@ -115,7 +122,7 @@ function pipAcceptsSystem(pythonBin) { * @param {Object} options * @return {undefined} */ -function installRequirements(targetFolder, serverless, options) { +async function installRequirements(targetFolder, serverless, options) { const targetRequirementsTxt = path.join(targetFolder, 'requirements.txt'); serverless.cli.log( @@ -176,7 +183,7 @@ function installRequirements(targetFolder, serverless, options) { pipCmd.push('--cache-dir', downloadCacheDir); } - if (pipAcceptsSystem(options.pythonBin)) { + if (await pipAcceptsSystem(options.pythonBin)) { pipCmd.push('--system'); } } @@ -191,7 +198,7 @@ function installRequirements(targetFolder, serverless, options) { serverless.cli.log( `Building custom docker image from ${options.dockerFile}...` ); - dockerImage = buildImage( + dockerImage = await buildImage( options.dockerFile, options.dockerBuildCmdExtraArgs ); @@ -201,7 +208,9 @@ function installRequirements(targetFolder, serverless, options) { serverless.cli.log(`Docker Image: ${dockerImage}`); // Prepare bind path depending on os platform - const bindPath = dockerPathForWin(getBindPath(serverless, targetFolder)); + const bindPath = dockerPathForWin( + await getBindPath(serverless, targetFolder) + ); dockerCmd.push('docker', 'run', '--rm', '-v', `${bindPath}:/var/task:z`); if (options.dockerSsh) { @@ -233,7 +242,7 @@ function installRequirements(targetFolder, serverless, options) { fse.closeSync( fse.openSync(path.join(downloadCacheDir, 'requirements.txt'), 'w') ); - const windowsized = getBindPath(serverless, downloadCacheDir); + const windowsized = await getBindPath(serverless, downloadCacheDir); // And now push it to a volume mount and to pip... dockerCmd.push('-v', `${windowsized}:${dockerDownloadCacheDir}:z`); pipCmd.push('--cache-dir', dockerDownloadCacheDir); @@ -262,7 +271,7 @@ function installRequirements(targetFolder, serverless, options) { ]); } else { // Use same user so --cache-dir works - dockerCmd.push('-u', getDockerUid(bindPath)); + dockerCmd.push('-u', await getDockerUid(bindPath)); } for (let path of options.dockerExtraFiles) { @@ -315,22 +324,23 @@ function installRequirements(targetFolder, serverless, options) { serverless.cli.log(`Running ${quote(dockerCmd)}...`); - filterCommands(mainCmds).forEach(([cmd, ...args]) => { - const res = spawnSync(cmd, args); - if (res.error) { - if (res.error.code === 'ENOENT') { + for (const [cmd, ...args] of mainCmds) { + try { + await spawn(cmd, args); + } catch (e) { + if ( + e.stderrBuffer && + e.stderrBuffer.toString().includes('command not found') + ) { const advice = cmd.indexOf('python') > -1 ? 'Try the pythonBin option' : 'Please install it'; throw new Error(`${cmd} not found! ${advice}`); } - throw res.error; - } - if (res.status !== 0) { - throw new Error(`STDOUT: ${res.stdout}\n\nSTDERR: ${res.stderr}`); + throw e; } - }); + } // If enabled slimming, delete files in slimPatterns if (options.slim === true || options.slim === 'true') { deleteFiles(options, targetFolder); @@ -489,7 +499,7 @@ function requirementsFileExists(servicePath, options, fileName) { * @param {Object} serverless * @return {string} */ -function installRequirementsIfNeeded( +async function installRequirementsIfNeeded( servicePath, modulePath, options, @@ -573,7 +583,7 @@ function installRequirementsIfNeeded( fse.copySync(slsReqsTxt, path.join(workingReqsFolder, 'requirements.txt')); // Then install our requirements from this folder - installRequirements(workingReqsFolder, serverless, options); + await installRequirements(workingReqsFolder, serverless, options); // Copy vendor libraries to requirements folder if (options.vendor) { @@ -596,7 +606,7 @@ function installRequirementsIfNeeded( * pip install the requirements to the requirements directory * @return {undefined} */ -function installAllRequirements() { +async function installAllRequirements() { // fse.ensureDirSync(path.join(this.servicePath, '.serverless')); // First, check and delete cache versions, if enabled checkForAndDeleteMaxCacheVersions(this.options, this.serverless); @@ -604,55 +614,56 @@ function installAllRequirements() { // Then if we're going to package functions individually... if (this.serverless.service.package.individually) { let doneModules = []; - this.targetFuncs - .filter((func) => - (func.runtime || this.serverless.service.provider.runtime).match( - /^python.*/ - ) + const filteredFuncs = this.targetFuncs.filter((func) => + (func.runtime || this.serverless.service.provider.runtime).match( + /^python.*/ ) - .map((f) => { - if (!get(f, 'module')) { - set(f, ['module'], '.'); - } - // If we didn't already process a module (functions can re-use modules) - if (!doneModules.includes(f.module)) { - const reqsInstalledAt = installRequirementsIfNeeded( - this.servicePath, - f.module, - this.options, - f, - this.serverless - ); - // Add modulePath into .serverless for each module so it's easier for injecting and for users to see where reqs are - let modulePath = path.join( - this.servicePath, - '.serverless', - `${f.module}`, - 'requirements' - ); - // Only do if we didn't already do it - if ( - reqsInstalledAt && - !fse.existsSync(modulePath) && - reqsInstalledAt != modulePath - ) { - if (this.options.useStaticCache) { - // Windows can't symlink so we have to copy on Windows, - // it's not as fast, but at least it works - if (process.platform == 'win32') { - fse.copySync(reqsInstalledAt, modulePath); - } else { - fse.symlink(reqsInstalledAt, modulePath); - } + ); + + for (const f of filteredFuncs) { + if (!get(f, 'module')) { + set(f, ['module'], '.'); + } + + // If we didn't already process a module (functions can re-use modules) + if (!doneModules.includes(f.module)) { + const reqsInstalledAt = await installRequirementsIfNeeded( + this.servicePath, + f.module, + this.options, + f, + this.serverless + ); + // Add modulePath into .serverless for each module so it's easier for injecting and for users to see where reqs are + let modulePath = path.join( + this.servicePath, + '.serverless', + `${f.module}`, + 'requirements' + ); + // Only do if we didn't already do it + if ( + reqsInstalledAt && + !fse.existsSync(modulePath) && + reqsInstalledAt != modulePath + ) { + if (this.options.useStaticCache) { + // Windows can't symlink so we have to copy on Windows, + // it's not as fast, but at least it works + if (process.platform == 'win32') { + fse.copySync(reqsInstalledAt, modulePath); } else { - fse.rename(reqsInstalledAt, modulePath); + fse.symlink(reqsInstalledAt, modulePath); } + } else { + fse.rename(reqsInstalledAt, modulePath); } - doneModules.push(f.module); } - }); + doneModules.push(f.module); + } + } } else { - const reqsInstalledAt = installRequirementsIfNeeded( + const reqsInstalledAt = await installRequirementsIfNeeded( this.servicePath, '', this.options, diff --git a/lib/pipenv.js b/lib/pipenv.js index 063fb5d8..e5731aaf 100644 --- a/lib/pipenv.js +++ b/lib/pipenv.js @@ -1,12 +1,12 @@ const fse = require('fs-extra'); const path = require('path'); -const { spawnSync } = require('child_process'); +const spawn = require('child-process-ext/spawn'); const { EOL } = require('os'); /** * pipenv install */ -function pipfileToRequirements() { +async function pipfileToRequirements() { if ( !this.options.usePipenv || !fse.existsSync(path.join(this.servicePath, 'Pipfile')) @@ -16,28 +16,26 @@ function pipfileToRequirements() { this.serverless.cli.log('Generating requirements.txt from Pipfile...'); - const res = spawnSync( - 'pipenv', - ['lock', '--requirements', '--keep-outdated'], - { + let res; + try { + res = await spawn('pipenv', ['lock', '--requirements', '--keep-outdated'], { cwd: this.servicePath, - } - ); - if (res.error) { - if (res.error.code === 'ENOENT') { + }); + } catch (e) { + if ( + e.stderrBuffer && + e.stderrBuffer.toString().includes('command not found') + ) { throw new Error( `pipenv not found! Install it with 'pip install pipenv'.` ); } - throw new Error(res.error); - } - if (res.status !== 0) { - throw new Error(res.stderr); + throw e; } fse.ensureDirSync(path.join(this.servicePath, '.serverless')); fse.writeFileSync( path.join(this.servicePath, '.serverless/requirements.txt'), - removeEditableFlagFromRequirementsString(res.stdout) + removeEditableFlagFromRequirementsString(res.stdoutBuffer) ); } diff --git a/lib/poetry.js b/lib/poetry.js index 553a1392..55f83289 100644 --- a/lib/poetry.js +++ b/lib/poetry.js @@ -1,44 +1,45 @@ const fs = require('fs'); const fse = require('fs-extra'); const path = require('path'); -const { spawnSync } = require('child_process'); +const spawn = require('child-process-ext/spawn'); const tomlParse = require('@iarna/toml/parse-string'); /** * poetry install */ -function pyprojectTomlToRequirements() { +async function pyprojectTomlToRequirements() { if (!this.options.usePoetry || !isPoetryProject(this.servicePath)) { return; } this.serverless.cli.log('Generating requirements.txt from pyproject.toml...'); - const res = spawnSync( - 'poetry', - [ - 'export', - '--without-hashes', - '-f', - 'requirements.txt', - '-o', - 'requirements.txt', - '--with-credentials', - ], - { - cwd: this.servicePath, - } - ); - if (res.error) { - if (res.error.code === 'ENOENT') { + try { + await spawn( + 'poetry', + [ + 'export', + '--without-hashes', + '-f', + 'requirements.txt', + '-o', + 'requirements.txt', + '--with-credentials', + ], + { + cwd: this.servicePath, + } + ); + } catch (e) { + if ( + e.stderrBuffer && + e.stderrBuffer.toString().includes('command not found') + ) { throw new Error( `poetry not found! Install it according to the poetry docs.` ); } - throw new Error(res.error); - } - if (res.status !== 0) { - throw new Error(res.stderr); + throw e; } const editableFlag = new RegExp(/^-e /gm); diff --git a/package.json b/package.json index 1fed4c39..c9d247a3 100644 --- a/package.json +++ b/package.json @@ -55,6 +55,7 @@ "@iarna/toml": "^2.2.5", "appdirectory": "^0.1.0", "bluebird": "^3.7.2", + "child-process-ext": "^2.1.1", "fs-extra": "^9.1.0", "glob-all": "^3.2.1", "is-wsl": "^2.2.0",