Skip to content

Commit

Permalink
fix(Packaging): Add exec bit for packaged files on Windows (#8615)
Browse files Browse the repository at this point in the history
  • Loading branch information
scadu committed Dec 14, 2020
1 parent d0beed4 commit c864fbd
Show file tree
Hide file tree
Showing 4 changed files with 11 additions and 94 deletions.
30 changes: 2 additions & 28 deletions lib/plugins/package/lib/packageService.js
Original file line number Diff line number Diff line change
Expand Up @@ -93,28 +93,9 @@ module.exports = {

packageAll() {
const zipFileName = `${this.serverless.service.service}.zip`;
/*
* crosscompiled GoLang binaries on windows don't have their execute bit set correctly.
* This is nearly impossible to actually set on a windows machine, so find all the Go handler
* files and pass them into zipFiles as files to add with the execute bit in the zip file
*/
const filesToChmodPlusX =
process.platform !== 'win32'
? []
: Object.values(this.serverless.service.functions)
.map(f =>
Object.assign(
{
runtime: this.getRuntime(),
},
f
)
)
.filter(f => f.runtime && f.runtime.startsWith('go'))
.map(f => path.normalize(f.handler));

return this.resolveFilePathsAll().then(filePaths =>
this.zipFiles(filePaths, zipFileName, undefined, filesToChmodPlusX).then(filePath => {
this.zipFiles(filePaths, zipFileName).then(filePath => {
// only set the default artifact for backward-compatibility
// when no explicit artifact is defined
if (!this.serverless.service.package.artifact) {
Expand Down Expand Up @@ -151,16 +132,9 @@ module.exports = {
}

const zipFileName = `${functionName}.zip`;
const filesToChmodPlusX = [];
if (process.platform === 'win32') {
const runtime = this.getRuntime(functionName.runtime);
if (runtime.startsWith('go')) {
filesToChmodPlusX.push(functionObject.handler);
}
}

const filePaths = await this.resolveFilePathsFunction(functionName);
const artifactPath = await this.zipFiles(filePaths, zipFileName, undefined, filesToChmodPlusX);
const artifactPath = await this.zipFiles(filePaths, zipFileName);
funcPackageConfig.artifact = artifactPath;
return artifactPath;
},
Expand Down
12 changes: 2 additions & 10 deletions lib/plugins/package/lib/zipService.js
Original file line number Diff line number Diff line change
Expand Up @@ -54,10 +54,8 @@ module.exports = {
* @param files - an Array of filenames
* @param zipFiles - the filename to save the zip at
* @param prefix - a prefix to strip from the file names. use for layers support
* @param filesToChmodPlusX - an array of files to add the execute bit to.
* used for golang support on windows.
*/
zipFiles(files, zipFileName, prefix, filesToChmodPlusX) {
zipFiles(files, zipFileName, prefix) {
if (files.length === 0) {
const error = new this.serverless.classes.Error('No files to package');
return BbPromise.reject(error);
Expand All @@ -84,8 +82,6 @@ module.exports = {

// normalize both maps to avoid problems with e.g. Path Separators in different shells
const normalizedFiles = _.uniq(files.map(file => path.normalize(file)));
const normalizedFilesToChmodPlusX =
filesToChmodPlusX && _.uniq(filesToChmodPlusX.map(file => path.normalize(file)));

return BbPromise.all(normalizedFiles.map(this.getFileContentAndStat.bind(this)))
.then(contents => {
Expand All @@ -95,11 +91,7 @@ module.exports = {
const name = file.filePath.slice(prefix ? `${prefix}${path.sep}`.length : 0);
// Ensure file is executable if it is locally executable or
// it's forced (via normalizedFilesToChmodPlusX) to be executable
const mode =
file.stat.mode & 0o100 ||
(normalizedFilesToChmodPlusX && normalizedFilesToChmodPlusX.includes(name))
? 0o755
: 0o644;
const mode = file.stat.mode & 0o100 || process.platform === 'win32' ? 0o755 : 0o644;
zip.append(file.data, {
name,
mode,
Expand Down
53 changes: 2 additions & 51 deletions test/unit/lib/plugins/package/lib/packageService.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -271,69 +271,20 @@ describe('#packageService()', () => {
});

(process.platfrom === 'win32' ? it : it.skip)(
'should call zipService with settings & binaries to chmod for GoLang on win32',
'should call zipService with settings & binaries to chmod on win32',
() => {
const servicePath = 'test';
const zipFileName = `${serverless.service.service}.zip`;

serverless.config.servicePath = servicePath;
serverless.service.provider.runtime = 'go1.x';

return expect(packagePlugin.packageService()).to.be.fulfilled.then(() =>
BbPromise.all([
expect(getExcludesStub).to.be.calledOnce,
expect(getIncludesStub).to.be.calledOnce,
expect(resolveFilePathsFromPatternsStub).to.be.calledOnce,
expect(zipFilesStub).to.be.calledOnce,
expect(zipFilesStub).to.have.been.calledWithExactly(files, zipFileName, undefined, [
'foo',
]),
])
);
}
);

(process.platfrom === 'win32' ? it : it.skip)(
'should call zipService with settings & no binaries to chmod for non-go on win32',
() => {
const servicePath = 'test';
const zipFileName = `${serverless.service.service}.zip`;

serverless.config.servicePath = servicePath;

return expect(packagePlugin.packageService()).to.be.fulfilled.then(() =>
BbPromise.all([
expect(getExcludesStub).to.be.calledOnce,
expect(getIncludesStub).to.be.calledOnce,
expect(resolveFilePathsFromPatternsStub).to.be.calledOnce,
expect(zipFilesStub).to.be.calledOnce,
expect(zipFilesStub).to.have.been.calledWithExactly(files, zipFileName, undefined, []),
])
);
}
);

(process.platfrom === 'win32' ? it : it.skip)(
'should normalize the handler path for go runtimes on win32',
() => {
serverless.service.functions.first.handler = 'foo/bar//foo\\bar\\\\foo';
serverless.service.provider.runtime = 'go1.x';

const servicePath = 'test';
const zipFileName = `${serverless.service.service}.zip`;

serverless.config.servicePath = servicePath;

return expect(packagePlugin.packageService()).to.be.fulfilled.then(() =>
BbPromise.all([
expect(getExcludesStub).to.be.calledOnce,
expect(getIncludesStub).to.be.calledOnce,
expect(serverless.service.functions.first),
expect(resolveFilePathsFromPatternsStub).to.be.calledOnce,
expect(zipFilesStub).to.be.calledOnce,
expect(zipFilesStub).to.have.been.calledWithExactly(files, zipFileName, undefined, [
path.normalize(serverless.service.functions.first.handler),
]),
expect(zipFilesStub).to.have.been.calledWithExactly(files, zipFileName, []),
])
);
}
Expand Down
10 changes: 5 additions & 5 deletions test/unit/lib/plugins/package/lib/zipService.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -818,12 +818,12 @@ describe('zipService', () => {
expect(unzippedFileData['bin/binary-777'].unixPermissions).to.equal(
Math.pow(2, 15) + 0o755
);
}

// read only file is set with chmod of 444
expect(unzippedFileData['bin/binary-444'].unixPermissions).to.equal(
Math.pow(2, 15) + 0o644
);
// read only file is set with chmod of 444
expect(unzippedFileData['bin/binary-444'].unixPermissions).to.equal(
Math.pow(2, 15) + 0o644
);
}
});
});

Expand Down

0 comments on commit c864fbd

Please sign in to comment.