Skip to content

Commit f6434f1

Browse files
authored
Merge pull request serverless#292 from bsamuel-ui/master
Allow 'strip' to be run via docker
2 parents 9934bf7 + 72e5565 commit f6434f1

File tree

3 files changed

+149
-110
lines changed

3 files changed

+149
-110
lines changed

lib/pip.js

+113-102
Original file line numberDiff line numberDiff line change
@@ -6,23 +6,42 @@ const set = require('lodash.set');
66
const { spawnSync } = require('child_process');
77
const { quote } = require('shell-quote');
88
const { buildImage, getBindPath, getDockerUid } = require('./docker');
9-
const { getStripCommand, deleteFiles } = require('./slim');
9+
const { getStripCommand, getStripMode, deleteFiles } = require('./slim');
1010
const {
1111
checkForAndDeleteMaxCacheVersions,
1212
md5Path,
1313
getRequirementsWorkingPath,
1414
getUserCachePath
1515
} = require('./shared');
1616

17-
function quote_single(quoteme) {
18-
return quote([quoteme]);
17+
/**
18+
* Omit empty commands.
19+
* In this context, a "command" is a list of arguments. An empty list or falsy value is ommitted.
20+
* @param {string[][]} many commands to merge.
21+
* @return {string[][]} a list of valid commands.
22+
*/
23+
function filterCommands(commands) {
24+
return commands.filter(cmd => Boolean(cmd) && cmd.length > 0);
1925
}
2026

21-
function quoteFroWin(quoteme) {
22-
if (process.platform === 'win32') {
23-
return `"${quoteme}"`;
27+
/**
28+
* Render zero or more commands as a single command for a Unix environment.
29+
* In this context, a "command" is a list of arguments. An empty list or falsy value is ommitted.
30+
*
31+
* @param {string[][]} many commands to merge.
32+
* @return {string[]} a single list of words.
33+
*/
34+
function mergeCommands(commands) {
35+
const cmds = filterCommands(commands);
36+
if (cmds.length === 0) {
37+
throw new Error('Expected at least one non-empty command');
38+
} else if (cmds.length === 1) {
39+
return cmds[0];
40+
} else {
41+
// Quote the arguments in each command and join them all using &&.
42+
const script = cmds.map(quote).join(' && ');
43+
return ['/bin/sh', '-c', script];
2444
}
25-
return quoteme;
2645
}
2746

2847
/**
@@ -58,6 +77,18 @@ function installRequirementsFile(
5877
}
5978
}
6079

80+
function pipAcceptsSystem(pythonBin) {
81+
// Check if pip has Debian's --system option and set it if so
82+
const pipTestRes = spawnSync(pythonBin, ['-m', 'pip', 'help', 'install']);
83+
if (pipTestRes.error) {
84+
if (pipTestRes.error.code === 'ENOENT') {
85+
throw new Error(`${pythonBin} not found! Try the pythonBin option.`);
86+
}
87+
throw pipTestRes.error;
88+
}
89+
return pipTestRes.stdout.toString().indexOf('--system') >= 0;
90+
}
91+
6192
/**
6293
* Install requirements described from requirements in the targetFolder into that same targetFolder
6394
* @param {string} targetFolder
@@ -72,15 +103,16 @@ function installRequirements(targetFolder, serverless, options) {
72103
`Installing requirements from ${targetRequirementsTxt} ...`
73104
);
74105

75-
let cmd;
76-
let cmdOptions;
77-
let pipCmd = [
106+
const dockerCmd = [];
107+
const pipCmd = [
78108
options.pythonBin,
79109
'-m',
80110
'pip',
81111
'install',
82112
...options.pipCmdExtraArgs
83113
];
114+
const pipCmds = [pipCmd];
115+
const postCmds = [];
84116
// Check if we're using the legacy --cache-dir command...
85117
if (options.pipCmdExtraArgs.indexOf('--cache-dir') > -1) {
86118
if (options.dockerizePip) {
@@ -101,8 +133,12 @@ function installRequirements(targetFolder, serverless, options) {
101133

102134
if (!options.dockerizePip) {
103135
// Push our local OS-specific paths for requirements and target directory
104-
pipCmd.push('-t', dockerPathForWin(options, targetFolder));
105-
pipCmd.push('-r', dockerPathForWin(options, targetRequirementsTxt));
136+
pipCmd.push(
137+
'-t',
138+
dockerPathForWin(targetFolder),
139+
'-r',
140+
dockerPathForWin(targetRequirementsTxt)
141+
);
106142
// If we want a download cache...
107143
if (options.useDownloadCache) {
108144
const downloadCacheDir = path.join(
@@ -111,35 +147,17 @@ function installRequirements(targetFolder, serverless, options) {
111147
);
112148
serverless.cli.log(`Using download cache directory ${downloadCacheDir}`);
113149
fse.ensureDirSync(downloadCacheDir);
114-
pipCmd.push('--cache-dir', quote_single(downloadCacheDir));
150+
pipCmd.push('--cache-dir', downloadCacheDir);
115151
}
116152

117-
// Check if pip has Debian's --system option and set it if so
118-
const pipTestRes = spawnSync(options.pythonBin, [
119-
'-m',
120-
'pip',
121-
'help',
122-
'install'
123-
]);
124-
if (pipTestRes.error) {
125-
if (pipTestRes.error.code === 'ENOENT') {
126-
throw new Error(
127-
`${options.pythonBin} not found! ` + 'Try the pythonBin option.'
128-
);
129-
}
130-
throw pipTestRes.error;
131-
}
132-
if (pipTestRes.stdout.toString().indexOf('--system') >= 0) {
153+
if (pipAcceptsSystem(options.pythonBin)) {
133154
pipCmd.push('--system');
134155
}
135156
}
136157
// If we are dockerizing pip
137158
if (options.dockerizePip) {
138-
cmd = 'docker';
139-
140159
// Push docker-specific paths for requirements and target directory
141-
pipCmd.push('-t', '/var/task/');
142-
pipCmd.push('-r', '/var/task/requirements.txt');
160+
pipCmd.push('-t', '/var/task/', '-r', '/var/task/requirements.txt');
143161

144162
// Build docker image if required
145163
let dockerImage;
@@ -154,29 +172,21 @@ function installRequirements(targetFolder, serverless, options) {
154172
serverless.cli.log(`Docker Image: ${dockerImage}`);
155173

156174
// Prepare bind path depending on os platform
157-
const bindPath = dockerPathForWin(
158-
options,
159-
getBindPath(serverless, targetFolder)
160-
);
175+
const bindPath = dockerPathForWin(getBindPath(serverless, targetFolder));
161176

162-
cmdOptions = ['run', '--rm', '-v', quoteFroWin(`${bindPath}:/var/task:z`)];
177+
dockerCmd.push('docker', 'run', '--rm', '-v', `${bindPath}:/var/task:z`);
163178
if (options.dockerSsh) {
164179
// Mount necessary ssh files to work with private repos
165-
cmdOptions.push(
180+
dockerCmd.push(
166181
'-v',
167-
quote_single(`${process.env.HOME}/.ssh/id_rsa:/root/.ssh/id_rsa:z`)
168-
);
169-
cmdOptions.push(
182+
`${process.env.HOME}/.ssh/id_rsa:/root/.ssh/id_rsa:z`,
170183
'-v',
171-
quote_single(
172-
`${process.env.HOME}/.ssh/known_hosts:/root/.ssh/known_hosts:z`
173-
)
174-
);
175-
cmdOptions.push(
184+
`${process.env.HOME}/.ssh/known_hosts:/root/.ssh/known_hosts:z`,
176185
'-v',
177-
quote_single(`${process.env.SSH_AUTH_SOCK}:/tmp/ssh_sock:z`)
186+
`${process.env.SSH_AUTH_SOCK}:/tmp/ssh_sock:z`,
187+
'-e',
188+
'SSH_AUTH_SOCK=/tmp/ssh_sock'
178189
);
179-
cmdOptions.push('-e', 'SSH_AUTH_SOCK=/tmp/ssh_sock');
180190
}
181191

182192
// If we want a download cache...
@@ -196,104 +206,105 @@ function installRequirements(targetFolder, serverless, options) {
196206
);
197207
const windowsized = getBindPath(serverless, downloadCacheDir);
198208
// And now push it to a volume mount and to pip...
199-
cmdOptions.push(
200-
'-v',
201-
quote_single(`${windowsized}:${dockerDownloadCacheDir}:z`)
202-
);
203-
pipCmd.push('--cache-dir', quote_single(dockerDownloadCacheDir));
209+
dockerCmd.push('-v', `${windowsized}:${dockerDownloadCacheDir}:z`);
210+
pipCmd.push('--cache-dir', dockerDownloadCacheDir);
204211
}
205212

206213
if (options.dockerEnv) {
207214
// Add environment variables to docker run cmd
208215
options.dockerEnv.forEach(function(item) {
209-
cmdOptions.push('-e', item);
216+
dockerCmd.push('-e', item);
210217
});
211218
}
212219

213220
if (process.platform === 'linux') {
214221
// Use same user so requirements folder is not root and so --cache-dir works
215-
var commands = [];
216222
if (options.useDownloadCache) {
217223
// Set the ownership of the download cache dir to root
218-
commands.push(quote(['chown', '-R', '0:0', dockerDownloadCacheDir]));
224+
pipCmds.unshift(['chown', '-R', '0:0', dockerDownloadCacheDir]);
219225
}
220226
// Install requirements with pip
221-
commands.push(pipCmd.join(' '));
222227
// Set the ownership of the current folder to user
223-
commands.push(
224-
quote([
228+
pipCmds.push([
229+
'chown',
230+
'-R',
231+
`${process.getuid()}:${process.getgid()}`,
232+
'/var/task'
233+
]);
234+
if (options.useDownloadCache) {
235+
// Set the ownership of the download cache dir back to user
236+
pipCmds.push([
225237
'chown',
226238
'-R',
227239
`${process.getuid()}:${process.getgid()}`,
228-
'/var/task'
229-
])
230-
);
231-
if (options.useDownloadCache) {
232-
// Set the ownership of the download cache dir back to user
233-
commands.push(
234-
quote([
235-
'chown',
236-
'-R',
237-
`${process.getuid()}:${process.getgid()}`,
238-
dockerDownloadCacheDir
239-
])
240-
);
240+
dockerDownloadCacheDir
241+
]);
241242
}
242-
pipCmd = ['/bin/bash', '-c', '"' + commands.join(' && ') + '"'];
243243
} else {
244244
// Use same user so --cache-dir works
245-
cmdOptions.push('-u', quote_single(getDockerUid(bindPath)));
245+
dockerCmd.push('-u', getDockerUid(bindPath));
246246
}
247-
cmdOptions.push(dockerImage);
248-
cmdOptions.push(...pipCmd);
249-
} else {
250-
cmd = pipCmd[0];
251-
cmdOptions = pipCmd.slice(1);
247+
dockerCmd.push(dockerImage);
252248
}
253249

254250
// If enabled slimming, strip so files
255-
if (options.slim === true || options.slim === 'true') {
256-
const preparedPath = dockerPathForWin(options, targetFolder);
257-
cmdOptions.push(getStripCommand(options, preparedPath));
251+
switch (getStripMode(options)) {
252+
case 'docker':
253+
pipCmds.push(getStripCommand(options, '/var/task'));
254+
break;
255+
case 'direct':
256+
postCmds.push(getStripCommand(options, dockerPathForWin(targetFolder)));
257+
break;
258258
}
259+
259260
let spawnArgs = { shell: true };
260261
if (process.env.SLS_DEBUG) {
261262
spawnArgs.stdio = 'inherit';
262263
}
263-
const res = spawnSync(cmd, cmdOptions, spawnArgs);
264-
if (res.error) {
265-
if (res.error.code === 'ENOENT') {
266-
if (options.dockerizePip) {
267-
throw new Error('docker not found! Please install it.');
264+
let mainCmds = [];
265+
if (dockerCmd.length) {
266+
dockerCmd.push(...mergeCommands(pipCmds));
267+
mainCmds = [dockerCmd];
268+
} else {
269+
mainCmds = pipCmds;
270+
}
271+
mainCmds.push(...postCmds);
272+
273+
serverless.cli.log(`Running ${quote(dockerCmd)}...`);
274+
275+
filterCommands(mainCmds).forEach(([cmd, ...args]) => {
276+
const res = spawnSync(cmd, args);
277+
if (res.error) {
278+
if (res.error.code === 'ENOENT') {
279+
const advice =
280+
cmd.indexOf('python') > -1
281+
? 'Try the pythonBin option'
282+
: 'Please install it';
283+
throw new Error(`${cmd} not found! ${advice}`);
268284
}
269-
throw new Error(
270-
`${options.pythonBin} not found! Try the pythonBin option.`
271-
);
285+
throw res.error;
272286
}
273-
throw res.error;
274-
}
275-
if (res.status !== 0) {
276-
throw new Error(res.stderr);
277-
}
287+
if (res.status !== 0) {
288+
throw new Error(res.stderr);
289+
}
290+
});
278291
// If enabled slimming, delete files in slimPatterns
279292
if (options.slim === true || options.slim === 'true') {
280293
deleteFiles(options, targetFolder);
281294
}
282295
}
283296

284297
/**
285-
* convert path from Windows style to Linux style, if needed
286-
* @param {Object} options
298+
* Convert path from Windows style to Linux style, if needed.
287299
* @param {string} path
288300
* @return {string}
289301
*/
290-
function dockerPathForWin(options, path) {
302+
function dockerPathForWin(path) {
291303
if (process.platform === 'win32') {
292-
return `${path.replace(/\\/g, '/')}`;
293-
} else if (process.platform === 'win32' && !options.dockerizePip) {
304+
return path.replace(/\\/g, '/');
305+
} else {
294306
return path;
295307
}
296-
return quote_single(path);
297308
}
298309

299310
/** create a filtered requirements.txt without anything from noDeploy

lib/slim.js

+26-4
Original file line numberDiff line numberDiff line change
@@ -2,10 +2,31 @@ const isWsl = require('is-wsl');
22
const glob = require('glob-all');
33
const fse = require('fs-extra');
44

5-
const getStripCommand = (options, folderPath) =>
6-
process.platform !== 'win32' || isWsl || options.dockerizePip
7-
? ` && find ${folderPath} -name "*.so" -exec strip {} ';'`
8-
: '';
5+
const getStripMode = options => {
6+
if (options.slim === false || options.slim === 'false') {
7+
return 'skip';
8+
} else if (options.dockerizePip) {
9+
return 'docker';
10+
} else if (
11+
(!isWsl && process.platform === 'win32') ||
12+
process.platform === 'darwin'
13+
) {
14+
return 'skip';
15+
} else {
16+
return 'direct';
17+
}
18+
};
19+
20+
const getStripCommand = (options, folderPath) => [
21+
'find',
22+
folderPath,
23+
'-name',
24+
'*.so',
25+
'-exec',
26+
'strip',
27+
'{}',
28+
';'
29+
];
930

1031
const deleteFiles = (options, folderPath) => {
1132
let patterns = ['**/*.py[c|o]', '**/__pycache__*', '**/*.dist-info*'];
@@ -27,6 +48,7 @@ const deleteFiles = (options, folderPath) => {
2748
};
2849

2950
module.exports = {
51+
getStripMode,
3052
getStripCommand,
3153
deleteFiles
3254
};

0 commit comments

Comments
 (0)