-
Notifications
You must be signed in to change notification settings - Fork 293
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
Changes from 3 commits
c7cd20b
e96ed4c
c25a855
f374288
7f5d861
2ef8cae
8886f21
b87a3fc
4423205
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,59 +1,30 @@ | ||
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 {} \\;` | ||
: ''; | ||
|
||
// 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 = [ | ||
'**/*.so', | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Also deleteing the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. DoH! Good catch. I didn't mean to delete those. that's what |
||
'**/*.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 | ||
}; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,2 +1,2 @@ | ||
slimPatterns: | ||
- "*.egg-info*" | ||
- "**/*.egg-info*" |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,2 +1,2 @@ | ||
slimPatterns: | ||
- "*.egg-info*" | ||
- "**/*.egg-info*" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have noticed that again changing
` && find ${folderPath} -name "*.so" -exec strip {} \\; `
toeither
` && find ${folderPath} -name "*.so" -exec strip {} \;`
or
` && find ${folderPath} -name "*.so" -exec strip {} ;`
makes it run as expected on Win10.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Interesting. pushing a commit with out the
\\
to see how it fares in circle. It might have to be a platform dependent thing. I'd assume WSL should be treated like a *nix tho because it's running in as *nix shell.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@dschep
Indeed, I was rather surprised too..
If it fails, could be interesting to also replace
;
operator with+
.I did not have a good reason to use a semicolon, but it seems that
+
should also work and it does not generally need an escape: as mentioned hereThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good suggestion! Works great since
strip
supports getting multiple arguments.