From 4d4586007e1ef0ff76edbc8b4fff9bb65edcb8d6 Mon Sep 17 00:00:00 2001 From: Amir Szekely Date: Tue, 27 Feb 2018 23:44:42 -0800 Subject: [PATCH 01/13] Inject files into package instead of creating symlinks - symlinks are still not working properly on windows (it fails deleting them because they're actually folders) - IDE indexes the symlinked folders while they're there which slows things down - any failure, like a file being in use, causes all symlinks to be left behind --- index.js | 7 +- lib/inject.js | 73 ++++++++++++++++++++ lib/link.js | 187 -------------------------------------------------- 3 files changed, 76 insertions(+), 191 deletions(-) create mode 100644 lib/inject.js delete mode 100644 lib/link.js diff --git a/index.js b/index.js index def09055..7bc0519a 100644 --- a/index.js +++ b/index.js @@ -4,9 +4,9 @@ const BbPromise = require('bluebird'); const fse = require('fs-extra'); const {addVendorHelper, removeVendorHelper, packRequirements} = require('./lib/zip'); +const {injectAllRequirements} = require('./lib/inject'); const {installAllRequirements} = require('./lib/pip'); const {pipfileToRequirements} = require('./lib/pipenv'); -const {linkAllRequirements, unlinkAllRequirements} = require('./lib/link'); const {cleanup} = require('./lib/clean'); BbPromise.promisifyAll(fse); @@ -101,12 +101,11 @@ class ServerlessPythonRequirements { .then(pipfileToRequirements) .then(addVendorHelper) .then(installAllRequirements) - .then(packRequirements) - .then(linkAllRequirements); + .then(packRequirements); const after = () => BbPromise.bind(this) .then(removeVendorHelper) - .then(unlinkAllRequirements); + .then(injectAllRequirements); const invalidateCaches = () => { if (this.options.invalidateCaches) { diff --git a/lib/inject.js b/lib/inject.js new file mode 100644 index 00000000..aea54742 --- /dev/null +++ b/lib/inject.js @@ -0,0 +1,73 @@ +const fse = require('fs-extra'); +const glob = require('glob-all'); +const get = require('lodash.get'); +const set = require('lodash.set'); +const path = require('path'); +const values = require('lodash.values'); +const zipper = require('zip-local'); + +/** + * inject requirements into packaged application + * @param {string} requirementsPath requirements folder path + * @param {string} packagePath target package path + * @param {Object} options our options object + */ +function injectRequirements(requirementsPath, packagePath, options) { + const noDeploy = new Set(options.noDeploy || []); + + const zip = zipper.sync.unzip(packagePath).lowLevel(); + + glob.sync([path.join(requirementsPath, '**')], {mark: true, dot: true}).forEach((file) => { + if (file.endsWith('/')) { + return; + } + + if (noDeploy.has(file.split(/[-\\\/]/, 1)[0])) { + return; + } + + zip.file(path.relative(requirementsPath, file), fse.readFileSync(file), { + date: new Date(0), // necessary to get the same hash when zipping the same content + }); + }); + + const buff = zip.generate({ + type: 'nodebuffer', + compression: 'DEFLATE', + }); + + fse.writeFileSync(packagePath, buff); +} + +/** + * inject requirements into packaged application + */ +function injectAllRequirements() { + this.serverless.cli.log('Injecting required Python packages to package...'); + + if (this.serverless.service.package.individually) { + let doneModules = []; + values(this.serverless.service.functions) + .forEach((f) => { + if (!get(f, 'module')) { + set(f, ['module'], '.'); + } + if (!doneModules.includes(f.module)) { + injectRequirements( + path.join('.serverless', f.module, 'requirements'), + f.package.artifact, + this.options + ); + doneModules.push(f.module); + } + }); + } else { + injectRequirements( + path.join('.serverless', 'requirements'), + this.serverless.service.package.artifact, + this.options + ); + } +} + +module.exports = {injectAllRequirements}; diff --git a/lib/link.js b/lib/link.js deleted file mode 100644 index 9cbf52fa..00000000 --- a/lib/link.js +++ /dev/null @@ -1,187 +0,0 @@ -const fse = require('fs-extra'); -const path = require('path'); -const get = require('lodash.get'); -const set = require('lodash.set'); -const values = require('lodash.values'); -const rimraf = require('rimraf'); -const zipper = require('zip-local'); - -/** - * Make a symlink - * @param {string} source - * @param {string} target - * @return {undefined} - */ -function makeSymlink(source, target) { - try { - fse.symlinkSync(source, target); - } catch (exception) { - if (exception.code === 'EPERM' && process.platform === 'win32') { - fse.copySync(source, target); - } else { - let linkDest = null; - try { - linkDest = fse.readlinkSync(target); - } catch (e) { - } - if (linkDest !== source) { - throw exception; - } - } - } -} - -/** - * Link requirements installed in sourceFolder to targetFolder - * @param {string} sourceFolder - * @param {string} targetFolder - * @param {boolean} isPython - * @param {Object} package - * @param {Object} serverless - * @param {string} servicePath - * @param {Object} options - * @return {undefined} - */ -function linkRequirements( - sourceFolder, targetFolder, isPython, package, serverless, servicePath, options -) { - const requirementsDir = path.join(servicePath, sourceFolder); - if (fse.existsSync('__pycache__')) { - rimraf.sync('__pycache__'); - } - if (!options.zip && fse.existsSync(requirementsDir)) { - serverless.cli.log(`Linking required Python packages to ${targetFolder}...`); - const noDeploy = new Set(options.noDeploy || []); - fse.readdirSync(requirementsDir).map((file) => { - if (noDeploy.has(file.split(/\.(py|pyc|dist-info\/?)$/)[0])) { - return; - } - - // don't include python deps in non-python functions - if (isPython) { - if (!package.exclude.includes(file)) { - package.include.push(file); - package.include.push(`${file}/**`); - makeSymlink(`${requirementsDir}/${file}`, `${targetFolder}/${file}`); - } else { - package.exclude.push(`${file}/**`); - } - } - }); - } -} - -/** - * Link all requirements files - * @return {undefined} - */ -function linkAllRequirements() { - if (this.serverless.service.package.individually) { - values(this.serverless.service.functions) - .forEach((f) => { - // Initialize include and exclude arrays - if (!get(f, 'package.include')) { - set(f, ['package', 'include'], []); - } - if (!get(f, 'package.exclude')) { - set(f, ['package', 'exclude'], []); - } - if (!get(f, 'module')) { - set(f, ['module'], '.'); - } - // Update the include and exclude arrays and build symlinks - linkRequirements( - path.join('.serverless', f.module, 'requirements'), - f.module, - (f.runtime || this.serverless.service.provider.runtime).match(/^python.*/), - f.package, - this.serverless, - this.servicePath, - this.options - ); - if (f.module !== '.') { - const artifactPath = `.serverless/${f.module}.zip`; - f.package.artifact = artifactPath; - zipper.sync.zip(f.module).compress().save(artifactPath); - } - }); - } else { - // Initialize include and exclude arrays - if (!get(this.serverless.service.package, 'include')) { - set(this.serverless.service.package, ['include'], []); - } - if (!get(this.serverless.service.package, 'exclude')) { - set(this.serverless.service.package, ['exclude'], []); - } - // Update the include and exclude arrays and build symlinks - linkRequirements( - '.serverless/requirements', - './', - this.serverless.service.provider.runtime.match(/^python.*/), - this.serverless.service.package, - this.serverless, - this.servicePath, - this.options - ); - } -} - -/** - * Unlink requirements installed from targetFolder - * @param {string} sourceFolder - * @param {string} targetFolder - * @param {Object} serverless - * @param {string} servicePath - * @param {Object} options - * @return {undefined} - */ -function unlinkRequirements(sourceFolder, targetFolder, serverless, servicePath, options) { - const requirementsDir = path.join(servicePath, sourceFolder); - if (!options.zip && fse.existsSync(requirementsDir)) { - serverless.cli.log(`Unlinking required Python packages from ${targetFolder}...`); - const noDeploy = new Set(options.noDeploy || []); - fse.readdirSync(requirementsDir).map((file) => { - if (noDeploy.has(file.split(/\.(py|pyc|dist-info\/?)$/)[0])) { - return; - } - let targetFile = path.join(targetFolder, file); - if (fse.existsSync(targetFile)) { - fse.unlinkSync(targetFile); - } - }); - } -} - - -/** - * Unlink all the requirements files - * @return {undefined} - */ -function unlinkAllRequirements() { - if (this.serverless.service.package.individually) { - let doneModules = []; - values(this.serverless.service.functions) - .forEach((f) => { - if (!doneModules.includes(f.module)) { - unlinkRequirements( - `.serverless/${f.module}/requirements`, - `${f.module}`, - this.serverless, - this.servicePath, - this.options - ); - doneModules.push(f.module); - } - }); - } else { - unlinkRequirements( - '.serverless/requirements', - './', - this.serverless, - this.servicePath, - this.options - ); - } -} - -module.exports = {linkAllRequirements, unlinkAllRequirements}; From d0a89c06c7466594e7d2daeeaf97509d9f28762a Mon Sep 17 00:00:00 2001 From: Amir Szekely Date: Wed, 28 Feb 2018 00:26:01 -0800 Subject: [PATCH 02/13] fix noDeploy #146 already takes care of this, but we need proper noDeploy here again in case a white-listed package is dependent on a black-listed package --- lib/inject.js | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/lib/inject.js b/lib/inject.js index aea54742..e2cb695e 100644 --- a/lib/inject.js +++ b/lib/inject.js @@ -21,12 +21,14 @@ function injectRequirements(requirementsPath, packagePath, options) { if (file.endsWith('/')) { return; } + + const relativeFile = path.relative(requirementsPath, file); - if (noDeploy.has(file.split(/[-\\\/]/, 1)[0])) { + if (noDeploy.has(relativeFile.split(/[-\\\/]/, 1)[0])) { return; } - zip.file(path.relative(requirementsPath, file), fse.readFileSync(file), { + zip.file(relativeFile, fse.readFileSync(file), { date: new Date(0), // necessary to get the same hash when zipping the same content }); }); From 67a9eaeb2b219b4762f4bccb33fa454db65b793b Mon Sep 17 00:00:00 2001 From: Amir Szekely Date: Wed, 28 Feb 2018 00:41:58 -0800 Subject: [PATCH 03/13] Fix lint issue --- lib/inject.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/inject.js b/lib/inject.js index e2cb695e..b9bacce6 100644 --- a/lib/inject.js +++ b/lib/inject.js @@ -21,7 +21,7 @@ function injectRequirements(requirementsPath, packagePath, options) { if (file.endsWith('/')) { return; } - + const relativeFile = path.relative(requirementsPath, file); if (noDeploy.has(relativeFile.split(/[-\\\/]/, 1)[0])) { From 346db42c78cedd75baf00eecd7e42d1ac27ca129 Mon Sep 17 00:00:00 2001 From: Amir Szekely Date: Wed, 28 Feb 2018 01:14:42 -0800 Subject: [PATCH 04/13] Exclude .py too --- lib/inject.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/inject.js b/lib/inject.js index b9bacce6..9c9c6855 100644 --- a/lib/inject.js +++ b/lib/inject.js @@ -24,7 +24,7 @@ function injectRequirements(requirementsPath, packagePath, options) { const relativeFile = path.relative(requirementsPath, file); - if (noDeploy.has(relativeFile.split(/[-\\\/]/, 1)[0])) { + if (noDeploy.has(relativeFile.split(/([-\\\/]|\.py$|\.pyc$)/, 1)[0])) { return; } From 3e3ddb9348ac7cdaefcde33db18620da6e2ebeb5 Mon Sep 17 00:00:00 2001 From: Amir Szekely Date: Wed, 28 Feb 2018 09:35:20 -0800 Subject: [PATCH 05/13] Include requirements into every package, even when it's in the root --- lib/inject.js | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-) diff --git a/lib/inject.js b/lib/inject.js index 9c9c6855..34c416b9 100644 --- a/lib/inject.js +++ b/lib/inject.js @@ -54,14 +54,11 @@ function injectAllRequirements() { if (!get(f, 'module')) { set(f, ['module'], '.'); } - if (!doneModules.includes(f.module)) { - injectRequirements( - path.join('.serverless', f.module, 'requirements'), - f.package.artifact, - this.options - ); - doneModules.push(f.module); - } + injectRequirements( + path.join('.serverless', f.module, 'requirements'), + f.package.artifact, + this.options + ); }); } else { injectRequirements( From 0941fd2d30fdb9e87ad3b11427c7f68f14ca4e49 Mon Sep 17 00:00:00 2001 From: Amir Szekely Date: Wed, 28 Feb 2018 09:36:03 -0800 Subject: [PATCH 06/13] Fix lint issue --- lib/inject.js | 1 - 1 file changed, 1 deletion(-) diff --git a/lib/inject.js b/lib/inject.js index 34c416b9..c61e287d 100644 --- a/lib/inject.js +++ b/lib/inject.js @@ -48,7 +48,6 @@ function injectAllRequirements() { this.serverless.cli.log('Injecting required Python packages to package...'); if (this.serverless.service.package.individually) { - let doneModules = []; values(this.serverless.service.functions) .forEach((f) => { if (!get(f, 'module')) { From c2a7360d71ab5a57ea35e3420b5b7740db072f27 Mon Sep 17 00:00:00 2001 From: Amir Szekely Date: Wed, 28 Feb 2018 10:14:46 -0800 Subject: [PATCH 07/13] Handle runtime that's not Python --- lib/inject.js | 3 +++ 1 file changed, 3 insertions(+) diff --git a/lib/inject.js b/lib/inject.js index c61e287d..3db6e08a 100644 --- a/lib/inject.js +++ b/lib/inject.js @@ -50,6 +50,9 @@ function injectAllRequirements() { if (this.serverless.service.package.individually) { values(this.serverless.service.functions) .forEach((f) => { + if (!(f.runtime || this.serverless.service.provider.runtime).match(/^python.*/)) { + return; + } if (!get(f, 'module')) { set(f, ['module'], '.'); } From e43dcc6d387d70d4757dd0e722e3db4c567fda2e Mon Sep 17 00:00:00 2001 From: Amir Szekely Date: Wed, 28 Feb 2018 10:36:43 -0800 Subject: [PATCH 08/13] Use function.module to name package --- lib/inject.js | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/lib/inject.js b/lib/inject.js index 3db6e08a..40e76f58 100644 --- a/lib/inject.js +++ b/lib/inject.js @@ -61,6 +61,11 @@ function injectAllRequirements() { f.package.artifact, this.options ); + if (f.module !== '.') { + const artifactPath = path.join('.serverless', `${f.module}.zip`); + fse.moveSync(f.package.artifact, artifactPath, {overwrite: true}); + f.package.artifact = artifactPath; + } }); } else { injectRequirements( From 28120224558849d3ab62dcf07b7baf0cf68d497d Mon Sep 17 00:00:00 2001 From: Amir Szekely Date: Wed, 28 Feb 2018 11:52:16 -0800 Subject: [PATCH 09/13] Filter files not in function.module with individually: true --- lib/inject.js | 64 +++++++++++++++++++++++++++++++++++++++++---------- 1 file changed, 52 insertions(+), 12 deletions(-) diff --git a/lib/inject.js b/lib/inject.js index 40e76f58..824d8c61 100644 --- a/lib/inject.js +++ b/lib/inject.js @@ -5,6 +5,33 @@ const set = require('lodash.set'); const path = require('path'); const values = require('lodash.values'); const zipper = require('zip-local'); +const JSZip = require('jszip'); + +/** + * write zip contents to a file + * @param {Object} zip + * @param {string} path + */ +function writeZip(zip, path) { + const buff = zip.generate({ + type: 'nodebuffer', + compression: 'DEFLATE', + }); + + fse.writeFileSync(path, buff); +} + +/** + * add a new file to a zip file from a buffer + * @param {Object} zip + * @param {string} path path to put in zip + * @param {string} buffer file contents + */ +function zipFile(zip, path, buffer) { + zip.file(path, buffer, { + date: new Date(0), // necessary to get the same hash when zipping the same content + }); +} /** * inject requirements into packaged application @@ -28,17 +55,30 @@ function injectRequirements(requirementsPath, packagePath, options) { return; } - zip.file(relativeFile, fse.readFileSync(file), { - date: new Date(0), // necessary to get the same hash when zipping the same content - }); + zipFile(zip, relativeFile, fse.readFileSync(file)); }); - const buff = zip.generate({ - type: 'nodebuffer', - compression: 'DEFLATE', + writeZip(zip, packagePath); +} + +/** + * remove all modules but the selected module from a package + * @param {string} source original package + * @param {string} target result package + * @param {string} module module to keep + */ +function moveModuleUp(source, target, module) { + const sourceZip = zipper.sync.unzip(source).memory(); + const targetZip = JSZip.make(); + + sourceZip.contents().forEach((file) => { + if (!file.startsWith(module + '/')) { + return; + } + zipFile(targetZip, file.replace(module + '/', ''), sourceZip.read(file, 'buffer')); }); - fse.writeFileSync(packagePath, buff); + writeZip(targetZip, target); } /** @@ -56,16 +96,16 @@ function injectAllRequirements() { if (!get(f, 'module')) { set(f, ['module'], '.'); } + if (f.module !== '.') { + const artifactPath = path.join('.serverless', `${f.module}.zip`); + moveModuleUp(f.package.artifact, artifactPath, f.module); + f.package.artifact = artifactPath; + } injectRequirements( path.join('.serverless', f.module, 'requirements'), f.package.artifact, this.options ); - if (f.module !== '.') { - const artifactPath = path.join('.serverless', `${f.module}.zip`); - fse.moveSync(f.package.artifact, artifactPath, {overwrite: true}); - f.package.artifact = artifactPath; - } }); } else { injectRequirements( From 18656a2a53bbd7295b20bb28b7d803518d0c1862 Mon Sep 17 00:00:00 2001 From: Amir Szekely Date: Wed, 28 Feb 2018 13:10:09 -0800 Subject: [PATCH 10/13] Don't inject requirements when zip: true is used Also avoid including __pycache__ where compiled noDeploy packages might remain --- lib/inject.js | 7 +++++++ test.bats | 2 ++ 2 files changed, 9 insertions(+) diff --git a/lib/inject.js b/lib/inject.js index 824d8c61..115b6940 100644 --- a/lib/inject.js +++ b/lib/inject.js @@ -51,6 +51,9 @@ function injectRequirements(requirementsPath, packagePath, options) { const relativeFile = path.relative(requirementsPath, file); + if (relativeFile.match(/^__pycache__[\\\/]/)) { + return; + } if (noDeploy.has(relativeFile.split(/([-\\\/]|\.py$|\.pyc$)/, 1)[0])) { return; } @@ -87,6 +90,10 @@ function moveModuleUp(source, target, module) { function injectAllRequirements() { this.serverless.cli.log('Injecting required Python packages to package...'); + if (this.options.zip) { + return; + } + if (this.serverless.service.package.individually) { values(this.serverless.service.functions) .forEach((f) => { diff --git a/test.bats b/test.bats index daf944a9..2c104cff 100755 --- a/test.bats +++ b/test.bats @@ -27,6 +27,7 @@ teardown() { sls --zip=true package unzip .serverless/sls-py-req-test.zip -d puck ls puck/.requirements.zip puck/unzip_requirements.py + ! ls puck/flask } @test "py3.6 doesn't package boto3 by default" { @@ -40,6 +41,7 @@ teardown() { sls package unzip .serverless/sls-py-req-test.zip -d puck ! ls puck/bottle.py + ! ls puck/__pycache__/bottle.cpython-36.pyc } @test "py3.6 can package flask with zip & dockerizePip option" { From bcd305920efdfcbbaf81609698fec38e7dff5e4f Mon Sep 17 00:00:00 2001 From: Amir Szekely Date: Fri, 2 Mar 2018 15:42:32 -0800 Subject: [PATCH 11/13] Remove dependency --- package.json | 1 - 1 file changed, 1 deletion(-) diff --git a/package.json b/package.json index 6d1b7c40..7b1b8bd0 100644 --- a/package.json +++ b/package.json @@ -55,7 +55,6 @@ "lodash.get": "^4.4.2", "lodash.set": "^4.3.2", "lodash.values": "^4.3.0", - "rimraf": "^2.6.2", "shell-quote": "^1.6.1", "zip-local": "^0.3.4" } From cb590f521917ae6c5342051baf8c0268a5fed0e3 Mon Sep 17 00:00:00 2001 From: Amir Szekely Date: Mon, 5 Mar 2018 09:38:57 -0800 Subject: [PATCH 12/13] Specifically add jszip --- package.json | 1 + 1 file changed, 1 insertion(+) diff --git a/package.json b/package.json index 6e32ef26..251d966c 100644 --- a/package.json +++ b/package.json @@ -50,6 +50,7 @@ "fs-extra": "^3.0.1", "glob-all": "^3.1.0", "is-wsl": "^1.1.0", + "jszip": "^3.1.5", "lodash.get": "^4.4.2", "lodash.set": "^4.3.2", "lodash.values": "^4.3.0", From bb7d493ebc17b60994afb983a8aed032ac0d5cbd Mon Sep 17 00:00:00 2001 From: Amir Szekely Date: Mon, 5 Mar 2018 09:57:12 -0800 Subject: [PATCH 13/13] We need an older version --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index 251d966c..d2f008d9 100644 --- a/package.json +++ b/package.json @@ -50,7 +50,7 @@ "fs-extra": "^3.0.1", "glob-all": "^3.1.0", "is-wsl": "^1.1.0", - "jszip": "^3.1.5", + "jszip": "^2.5.0", "lodash.get": "^4.4.2", "lodash.set": "^4.3.2", "lodash.values": "^4.3.0",