Skip to content

clean up slim code using glob-all to make it more cross platform #227

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 9 commits into from
Aug 28, 2018
Merged
1 change: 1 addition & 0 deletions index.js
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ class ServerlessPythonRequirements {
const options = Object.assign(
{
slim: false,
slimPatterns: false,
zip: false,
cleanupZipHelper: true,
invalidateCaches: false,
Expand Down
17 changes: 7 additions & 10 deletions lib/pip.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ const get = require('lodash.get');
const set = require('lodash.set');
const { spawnSync } = require('child_process');
const { buildImage, getBindPath, getDockerUid } = require('./docker');
const { getSlimPackageCommands } = require('./slim');
const { getStripCommand, deleteFiles } = require('./slim');

/**
* Install requirements described in requirementsPath to targetFolder
Expand Down Expand Up @@ -106,12 +106,6 @@ function installRequirements(
if (process.platform === 'linux') {
// Use same user so requirements folder is not root and so --cache-dir works
cmdOptions.push('-u', `${process.getuid()}`);
// const stripCmd = quote([
// 'find', targetRequirementsFolder,
// '-name', '"*.so"',
// '-exec', 'strip', '{}', '\;',
// ]);
// pipCmd = ['/bin/bash', '-c', '"' + pipCmd + ' && ' + stripCmd + ' && ' + chownCmd + '"'];
} else {
// Use same user so --cache-dir works
cmdOptions.push('-u', getDockerUid(bindPath));
Expand All @@ -123,11 +117,10 @@ function installRequirements(
cmdOptions = pipCmd.slice(1);
}

// If enabled slimming, strip out the caches, tests and dist-infos
// If enabled slimming, strip so files
if (options.slim === true || options.slim === 'true') {
const preparedPath = dockerPathForWin(options, targetRequirementsFolder);
const slimCmd = getSlimPackageCommands(options, preparedPath);
cmdOptions.push(...slimCmd);
cmdOptions.push(getStripCommand(options, preparedPath));
}

const res = spawnSync(cmd, cmdOptions, { cwd: servicePath, shell: true });
Expand All @@ -145,6 +138,10 @@ function installRequirements(
if (res.status !== 0) {
throw new Error(res.stderr);
}
// If enabled slimming, delete files in slimPatterns
if (options.slim === true || options.slim === 'true') {
deleteFiles(options, targetRequirementsFolder);
}
}

/**
Expand Down
68 changes: 17 additions & 51 deletions lib/slim.js
Original file line number Diff line number Diff line change
@@ -1,59 +1,25 @@
const isWsl = require('is-wsl');
const glob = require('glob-all');
const fse = require('fs-extra');

/**
* Get commands to slim the installed requirements
* only for non-windows platforms:
* works for docker builds and when run on UNIX platforms (wsl included)
* @param {Object} options
* @param {string} folderPath
* @return {Array.<String>}
*/
function getSlimPackageCommands(options, folderPath) {
let stripCmd = [];
const getStripCommand = (options, folderPath) =>
process.platform !== 'win32' || isWsl || options.dockerizePip
? ` && find ${folderPath} -name "*.so" -exec strip {} +`
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This again did not work on Win10 when I tried 😅

Serverless: Docker Image: lambci/lambda:build-python3.6
Error --------------------------------------------------
...
find: ‘strip’: No such file or directory

But now having changed it back to a semicolon with a different escape seems to work and supposedly should also be fine for *nix platforms... Could you try ` && find ${folderPath} -name "*.so" -exec strip {} ';' ` (yes, single quotes :D)
Sorry for making it so absurd to try different ways every time 😅

But it will call the strip command per each found .so file, whereas + should've processed as much files at once as possible (which might actually still be one per call)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's so odd. I'm fine with it being a strip call per .so. Just switched to ';'. Lets see if CI/*nix is happy. Windows should be happy per your experiments.

: '';

// Default stripping is done for non-windows environments
if (process.platform !== 'win32' || isWsl) {
stripCmd = getDefaultSLimOptions(folderPath);

// If specified any custom patterns to remove
if (options.slimPatterns instanceof Array) {
// Add the custom specified patterns to remove to the default commands
const customPatterns = options.slimPatterns.map(pattern => {
return getRemovalCommand(folderPath, pattern);
});
stripCmd = stripCmd.concat(customPatterns);
const deleteFiles = (options, folderPath) => {
let patterns = ['**/*.py[c|o]', '**/__pycache__*', '**/*.dist-info*'];
if (options.slimPatterns) {
patterns = patterns.concat(options.slimPatterns);
}
for (const pattern of patterns) {
for (const file of glob.sync(`${folderPath}/${pattern}`)) {
fse.removeSync(file);
}
}
return stripCmd;
}

/**
* Gets the commands to slim the default (safe) files:
* including removing caches, stripping compiled files, removing dist-infos
* @param {String} folderPath
* @return {Array}
*/
function getDefaultSLimOptions(folderPath) {
return [
`&& find ${folderPath} -name "*.so" -exec strip {} \\;`,
`&& find ${folderPath} -name "*.py[c|o]" -exec rm -rf {} +`,
`&& find ${folderPath} -type d -name "__pycache__*" -exec rm -rf {} +`,
`&& find ${folderPath} -type d -name "*.dist-info*" -exec rm -rf {} +`
];
}

/**
* Get the command created fromt he find and remove template:
* returns a string in form `&& find <folder> -name "<match>" -exec rm -rf {} +`
* @param {String} folderPath
* @param {String} removalMatch
* @return {String}
*/
function getRemovalCommand(folderPath, removalMatch) {
return `&& find ${folderPath} -type d -wholename "${removalMatch}" -exec rm -rf {} +`;
}
};

module.exports = {
getSlimPackageCommands,
getDefaultSLimOptions
getStripCommand,
deleteFiles
};
2 changes: 1 addition & 1 deletion tests/base/_slimPatterns.yml
Original file line number Diff line number Diff line change
@@ -1,2 +1,2 @@
slimPatterns:
- "*.egg-info*"
- "**/*.egg-info*"
2 changes: 1 addition & 1 deletion tests/pipenv/_slimPatterns.yml
Original file line number Diff line number Diff line change
@@ -1,2 +1,2 @@
slimPatterns:
- "*.egg-info*"
- "**/*.egg-info*"