Skip to content

Commit 50c2850

Browse files
pgrzesikpgrzesik
committed
refactor: Adapt to async version of spawn 
1 parent ea38234 commit 50c2850

File tree

5 files changed

+136
-124
lines changed

5 files changed

+136
-124
lines changed

‎lib/docker.js

Lines changed: 19 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
const { spawnSync } = require('child_process');
1+
const spawn = require('child-process-ext/spawn');
22
const isWsl = require('is-wsl');
33
const fse = require('fs-extra');
44
const path = require('path');
@@ -8,18 +8,19 @@ const path = require('path');
88
* @param {string[]} options
99
* @return {Object}
1010
*/
11-
function dockerCommand(options) {
11+
async function dockerCommand(options) {
1212
const cmd = 'docker';
13-
const ps = spawnSync(cmd, options, { encoding: 'utf-8' });
14-
if (ps.error) {
15-
if (ps.error.code === 'ENOENT') {
13+
try {
14+
return await spawn(cmd, options, { encoding: 'utf-8' });
15+
} catch (e) {
16+
if (
17+
e.stderrBuffer &&
18+
e.stderrBuffer.toString().includes('command not found')
19+
) {
1620
throw new Error('docker not found! Please install it.');
1721
}
18-
throw new Error(ps.error);
19-
} else if (ps.status !== 0) {
20-
throw new Error(ps.stderr);
22+
throw e;
2123
}
22-
return ps;
2324
}
2425

2526
/**
@@ -28,7 +29,7 @@ function dockerCommand(options) {
2829
* @param {string[]} extraArgs
2930
* @return {string} The name of the built docker image.
3031
*/
31-
function buildImage(dockerFile, extraArgs) {
32+
async function buildImage(dockerFile, extraArgs) {
3233
const imageName = 'sls-py-reqs-custom';
3334
const options = ['build', '-f', dockerFile, '-t', imageName];
3435

@@ -40,7 +41,7 @@ function buildImage(dockerFile, extraArgs) {
4041

4142
options.push('.');
4243

43-
dockerCommand(options);
44+
await dockerCommand(options);
4445
return imageName;
4546
}
4647

@@ -72,7 +73,7 @@ function findTestFile(servicePath) {
7273
* @param {string} bindPath
7374
* @return {boolean}
7475
*/
75-
function tryBindPath(serverless, bindPath, testFile) {
76+
async function tryBindPath(serverless, bindPath, testFile) {
7677
const debug = process.env.SLS_DEBUG;
7778
const options = [
7879
'run',
@@ -85,7 +86,7 @@ function tryBindPath(serverless, bindPath, testFile) {
8586
];
8687
try {
8788
if (debug) serverless.cli.log(`Trying bindPath ${bindPath} (${options})`);
88-
const ps = dockerCommand(options);
89+
const ps = await dockerCommand(options);
8990
if (debug) serverless.cli.log(ps.stdout.trim());
9091
return ps.stdout.trim() === `/test/${testFile}`;
9192
} catch (err) {
@@ -100,14 +101,14 @@ function tryBindPath(serverless, bindPath, testFile) {
100101
* @param {string} servicePath
101102
* @return {string} The bind path.
102103
*/
103-
function getBindPath(serverless, servicePath) {
104+
async function getBindPath(serverless, servicePath) {
104105
// Determine bind path
105106
if (process.platform !== 'win32' && !isWsl) {
106107
return servicePath;
107108
}
108109

109110
// test docker is available
110-
dockerCommand(['version']);
111+
await dockerCommand(['version']);
111112

112113
// find good bind path for Windows
113114
let bindPaths = [];
@@ -144,7 +145,7 @@ function getBindPath(serverless, servicePath) {
144145

145146
for (let i = 0; i < bindPaths.length; i++) {
146147
const bindPath = bindPaths[i];
147-
if (tryBindPath(serverless, bindPath, testFile)) {
148+
if (await tryBindPath(serverless, bindPath, testFile)) {
148149
return bindPath;
149150
}
150151
}
@@ -157,7 +158,7 @@ function getBindPath(serverless, servicePath) {
157158
* @param {string} bindPath
158159
* @return {boolean}
159160
*/
160-
function getDockerUid(bindPath) {
161+
async function getDockerUid(bindPath) {
161162
const options = [
162163
'run',
163164
'--rm',
@@ -169,7 +170,7 @@ function getDockerUid(bindPath) {
169170
'%u',
170171
'/bin/sh',
171172
];
172-
const ps = dockerCommand(options);
173+
const ps = await dockerCommand(options);
173174
return ps.stdout.trim();
174175
}
175176

‎lib/pip.js

Lines changed: 79 additions & 68 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ const rimraf = require('rimraf');
33
const path = require('path');
44
const get = require('lodash.get');
55
const set = require('lodash.set');
6-
const { spawnSync } = require('child_process');
6+
const spawn = require('child-process-ext/spawn');
77
const { quote } = require('shell-quote');
88
const { buildImage, getBindPath, getDockerUid } = require('./docker');
99
const { getStripCommand, getStripMode, deleteFiles } = require('./slim');
@@ -96,16 +96,23 @@ function generateRequirementsFile(
9696
}
9797
}
9898

99-
function pipAcceptsSystem(pythonBin) {
99+
async function pipAcceptsSystem(pythonBin) {
100100
// Check if pip has Debian's --system option and set it if so
101-
const pipTestRes = spawnSync(pythonBin, ['-m', 'pip', 'help', 'install']);
102-
if (pipTestRes.error) {
103-
if (pipTestRes.error.code === 'ENOENT') {
101+
try {
102+
const pipTestRes = await spawn(pythonBin, ['-m', 'pip', 'help', 'install']);
103+
return (
104+
pipTestRes.stdoutBuffer &&
105+
pipTestRes.stdoutBuffer.toString().indexOf('--system') >= 0
106+
);
107+
} catch (e) {
108+
if (
109+
e.stderrBuffer &&
110+
e.stderrBuffer.toString().includes('command not found')
111+
) {
104112
throw new Error(`${pythonBin} not found! Try the pythonBin option.`);
105113
}
106-
throw pipTestRes.error;
114+
throw e;
107115
}
108-
return pipTestRes.stdout.toString().indexOf('--system') >= 0;
109116
}
110117

111118
/**
@@ -115,7 +122,7 @@ function pipAcceptsSystem(pythonBin) {
115122
* @param {Object} options
116123
* @return {undefined}
117124
*/
118-
function installRequirements(targetFolder, serverless, options) {
125+
async function installRequirements(targetFolder, serverless, options) {
119126
const targetRequirementsTxt = path.join(targetFolder, 'requirements.txt');
120127

121128
serverless.cli.log(
@@ -176,7 +183,7 @@ function installRequirements(targetFolder, serverless, options) {
176183
pipCmd.push('--cache-dir', downloadCacheDir);
177184
}
178185

179-
if (pipAcceptsSystem(options.pythonBin)) {
186+
if (await pipAcceptsSystem(options.pythonBin)) {
180187
pipCmd.push('--system');
181188
}
182189
}
@@ -191,7 +198,7 @@ function installRequirements(targetFolder, serverless, options) {
191198
serverless.cli.log(
192199
`Building custom docker image from ${options.dockerFile}...`
193200
);
194-
dockerImage = buildImage(
201+
dockerImage = await buildImage(
195202
options.dockerFile,
196203
options.dockerBuildCmdExtraArgs
197204
);
@@ -201,7 +208,9 @@ function installRequirements(targetFolder, serverless, options) {
201208
serverless.cli.log(`Docker Image: ${dockerImage}`);
202209

203210
// Prepare bind path depending on os platform
204-
const bindPath = dockerPathForWin(getBindPath(serverless, targetFolder));
211+
const bindPath = dockerPathForWin(
212+
await getBindPath(serverless, targetFolder)
213+
);
205214

206215
dockerCmd.push('docker', 'run', '--rm', '-v', `${bindPath}:/var/task:z`);
207216
if (options.dockerSsh) {
@@ -233,7 +242,7 @@ function installRequirements(targetFolder, serverless, options) {
233242
fse.closeSync(
234243
fse.openSync(path.join(downloadCacheDir, 'requirements.txt'), 'w')
235244
);
236-
const windowsized = getBindPath(serverless, downloadCacheDir);
245+
const windowsized = await getBindPath(serverless, downloadCacheDir);
237246
// And now push it to a volume mount and to pip...
238247
dockerCmd.push('-v', `${windowsized}:${dockerDownloadCacheDir}:z`);
239248
pipCmd.push('--cache-dir', dockerDownloadCacheDir);
@@ -262,7 +271,7 @@ function installRequirements(targetFolder, serverless, options) {
262271
]);
263272
} else {
264273
// Use same user so --cache-dir works
265-
dockerCmd.push('-u', getDockerUid(bindPath));
274+
dockerCmd.push('-u', await getDockerUid(bindPath));
266275
}
267276

268277
for (let path of options.dockerExtraFiles) {
@@ -315,22 +324,23 @@ function installRequirements(targetFolder, serverless, options) {
315324

316325
serverless.cli.log(`Running ${quote(dockerCmd)}...`);
317326

318-
filterCommands(mainCmds).forEach(([cmd, ...args]) => {
319-
const res = spawnSync(cmd, args);
320-
if (res.error) {
321-
if (res.error.code === 'ENOENT') {
327+
for (const [cmd, ...args] of mainCmds) {
328+
try {
329+
await spawn(cmd, args);
330+
} catch (e) {
331+
if (
332+
e.stderrBuffer &&
333+
e.stderrBuffer.toString().includes('command not found')
334+
) {
322335
const advice =
323336
cmd.indexOf('python') > -1
324337
? 'Try the pythonBin option'
325338
: 'Please install it';
326339
throw new Error(`${cmd} not found! ${advice}`);
327340
}
328-
throw res.error;
329-
}
330-
if (res.status !== 0) {
331-
throw new Error(`STDOUT: ${res.stdout}\n\nSTDERR: ${res.stderr}`);
341+
throw e;
332342
}
333-
});
343+
}
334344
// If enabled slimming, delete files in slimPatterns
335345
if (options.slim === true || options.slim === 'true') {
336346
deleteFiles(options, targetFolder);
@@ -489,7 +499,7 @@ function requirementsFileExists(servicePath, options, fileName) {
489499
* @param {Object} serverless
490500
* @return {string}
491501
*/
492-
function installRequirementsIfNeeded(
502+
async function installRequirementsIfNeeded(
493503
servicePath,
494504
modulePath,
495505
options,
@@ -573,7 +583,7 @@ function installRequirementsIfNeeded(
573583
fse.copySync(slsReqsTxt, path.join(workingReqsFolder, 'requirements.txt'));
574584

575585
// Then install our requirements from this folder
576-
installRequirements(workingReqsFolder, serverless, options);
586+
await installRequirements(workingReqsFolder, serverless, options);
577587

578588
// Copy vendor libraries to requirements folder
579589
if (options.vendor) {
@@ -596,63 +606,64 @@ function installRequirementsIfNeeded(
596606
* pip install the requirements to the requirements directory
597607
* @return {undefined}
598608
*/
599-
function installAllRequirements() {
609+
async function installAllRequirements() {
600610
// fse.ensureDirSync(path.join(this.servicePath, '.serverless'));
601611
// First, check and delete cache versions, if enabled
602612
checkForAndDeleteMaxCacheVersions(this.options, this.serverless);
603613

604614
// Then if we're going to package functions individually...
605615
if (this.serverless.service.package.individually) {
606616
let doneModules = [];
607-
this.targetFuncs
608-
.filter((func) =>
609-
(func.runtime || this.serverless.service.provider.runtime).match(
610-
/^python.*/
611-
)
617+
const filteredFuncs = this.targetFuncs.filter((func) =>
618+
(func.runtime || this.serverless.service.provider.runtime).match(
619+
/^python.*/
612620
)
613-
.map((f) => {
614-
if (!get(f, 'module')) {
615-
set(f, ['module'], '.');
616-
}
617-
// If we didn't already process a module (functions can re-use modules)
618-
if (!doneModules.includes(f.module)) {
619-
const reqsInstalledAt = installRequirementsIfNeeded(
620-
this.servicePath,
621-
f.module,
622-
this.options,
623-
f,
624-
this.serverless
625-
);
626-
// Add modulePath into .serverless for each module so it's easier for injecting and for users to see where reqs are
627-
let modulePath = path.join(
628-
this.servicePath,
629-
'.serverless',
630-
`${f.module}`,
631-
'requirements'
632-
);
633-
// Only do if we didn't already do it
634-
if (
635-
reqsInstalledAt &&
636-
!fse.existsSync(modulePath) &&
637-
reqsInstalledAt != modulePath
638-
) {
639-
if (this.options.useStaticCache) {
640-
// Windows can't symlink so we have to copy on Windows,
641-
// it's not as fast, but at least it works
642-
if (process.platform == 'win32') {
643-
fse.copySync(reqsInstalledAt, modulePath);
644-
} else {
645-
fse.symlink(reqsInstalledAt, modulePath);
646-
}
621+
);
622+
623+
for (const f of filteredFuncs) {
624+
if (!get(f, 'module')) {
625+
set(f, ['module'], '.');
626+
}
627+
628+
// If we didn't already process a module (functions can re-use modules)
629+
if (!doneModules.includes(f.module)) {
630+
const reqsInstalledAt = await installRequirementsIfNeeded(
631+
this.servicePath,
632+
f.module,
633+
this.options,
634+
f,
635+
this.serverless
636+
);
637+
// Add modulePath into .serverless for each module so it's easier for injecting and for users to see where reqs are
638+
let modulePath = path.join(
639+
this.servicePath,
640+
'.serverless',
641+
`${f.module}`,
642+
'requirements'
643+
);
644+
// Only do if we didn't already do it
645+
if (
646+
reqsInstalledAt &&
647+
!fse.existsSync(modulePath) &&
648+
reqsInstalledAt != modulePath
649+
) {
650+
if (this.options.useStaticCache) {
651+
// Windows can't symlink so we have to copy on Windows,
652+
// it's not as fast, but at least it works
653+
if (process.platform == 'win32') {
654+
fse.copySync(reqsInstalledAt, modulePath);
647655
} else {
648-
fse.rename(reqsInstalledAt, modulePath);
656+
fse.symlink(reqsInstalledAt, modulePath);
649657
}
658+
} else {
659+
fse.rename(reqsInstalledAt, modulePath);
650660
}
651-
doneModules.push(f.module);
652661
}
653-
});
662+
doneModules.push(f.module);
663+
}
664+
}
654665
} else {
655-
const reqsInstalledAt = installRequirementsIfNeeded(
666+
const reqsInstalledAt = await installRequirementsIfNeeded(
656667
this.servicePath,
657668
'',
658669
this.options,

0 commit comments

Comments
 (0)