Skip to content
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

fix: allow source folder copy task to dereference symlinks #424

Open
wants to merge 5 commits into
base: develop
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 11 additions & 4 deletions lib/builtins/build-flows/nodejs-npm.js
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,9 @@ class NodeJsNpmBuildFlow extends AbstractBuildFlow {
* @param {String} options.buildFile full path for zip file to generate
* @param {Boolean} options.doDebug debug flag
*/
constructor({ cwd, src, buildFile, doDebug }) {
constructor({ cwd, src, buildFile, doDebug, noNPMInstall }) {
super({ cwd, src, buildFile, doDebug });
this.noNPMInstall = noNPMInstall;
}

/**
Expand All @@ -37,9 +38,15 @@ class NodeJsNpmBuildFlow extends AbstractBuildFlow {
execute(callback) {
const installCmd = this._hasLockFile() ? 'ci' : 'install';
const quietFlag = this.doDebug ? '' : ' --quiet';
this.env.NODE_ENV = 'production';
this.debug(`Installing NodeJS dependencies based on the ${NodeJsNpmBuildFlow.manifest}.`);
this.execCommand(`npm ${installCmd}${quietFlag}`);

if (this.noNPMInstall) {
this.debug('noNPMInstall flag is true. Skipping npm install.');
} else {
this.env.NODE_ENV = 'production';
this.debug(`Installing NodeJS dependencies based on the ${NodeJsNpmBuildFlow.manifest}.`);
this.execCommand(`npm ${installCmd}${quietFlag}`);
}

this.createZip(callback);
}

Expand Down
9 changes: 5 additions & 4 deletions lib/commands/deploy/helper.js
Original file line number Diff line number Diff line change
Expand Up @@ -51,11 +51,12 @@ function deploySkillMetadata(options, callback) {
* Deploy skill code by calling SkillCodeController
* @param {String} profile ask-cli profile
* @param {Boolean} doDebug
* @param {Boolean} shouldDereferenceSymlinks If true, will copy symlinked files and folders into final build
* @param {*} callback (error, uniqueCodeList)
* @param {*} callback.uniqueCodeList [{ src, build{file, folder}}, buildFlow, regionsList }]
*/
function buildSkillCode(profile, doDebug, callback) {
const skillCodeController = new SkillCodeController({ profile, doDebug });
function buildSkillCode({ profile, doDebug, shouldDereferenceSymlinks, noNPMInstall }, callback) {
const skillCodeController = new SkillCodeController({ profile, doDebug, shouldDereferenceSymlinks, noNPMInstall });
skillCodeController.buildCode((buildErr, uniqueCodeList) => {
if (buildErr) {
return callback(buildErr);
Expand All @@ -71,7 +72,7 @@ function buildSkillCode(profile, doDebug, callback) {
* @param {Boolean} ignoreHash
* @param {*} callback (error)
*/
function deploySkillInfrastructure(profile, doDebug, ignoreHash, callback) {
function deploySkillInfrastructure({ profile, doDebug, ignoreHash }, callback) {
const skillInfraController = new SkillInfrastructureController({ profile, doDebug, ignoreHash });
skillInfraController.deployInfrastructure((deployError) => {
if (deployError) {
Expand All @@ -87,7 +88,7 @@ function deploySkillInfrastructure(profile, doDebug, ignoreHash, callback) {
* @param {Boolean} doDebug The flag of debug or not
* @param {Function} callback
*/
function enableSkill(profile, doDebug, callback) {
function enableSkill({ profile, doDebug }, callback) {
const skillMetaController = new SkillMetadataController({ profile, doDebug });
Messenger.getInstance().info('\n==================== Enable Skill ====================');
try {
Expand Down
20 changes: 14 additions & 6 deletions lib/commands/deploy/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ class DeployCommand extends AbstractCommand {
}

optionalOptions() {
return ['ignore-hash', 'target', 'profile', 'debug'];
return ['ignore-hash', 'target', 'profile', 'debug', 'dereference-symlinks', 'no-npm-install'];
}

handle(cmd, cb) {
Expand Down Expand Up @@ -62,7 +62,14 @@ class DeployCommand extends AbstractCommand {
return cb(new CliError(errMessage));
}

const options = { profile, doDebug: cmd.debug, ignoreHash: cmd.ignoreHash, target: cmd.target };
const options = {
profile,
doDebug: cmd.debug,
ignoreHash: cmd.ignoreHash,
target: cmd.target,
shouldDereferenceSymlinks: !!cmd.dereferenceSymlinks,
noNPMInstall: !cmd.npmInstall
};
deployResources(options, (deployErr) => {
// Write updates back to resources file
if (deployErr) {
Expand All @@ -80,7 +87,7 @@ class DeployCommand extends AbstractCommand {
}
// Post deploy logic
// call smapiClient to enable skill
helper.enableSkill(profile, cmd.debug, (enableError) => {
helper.enableSkill({ profile, doDebug: cmd.debug }, (enableError) => {
if (enableError instanceof CliWarn) {
Messenger.getInstance().warn(enableError);
return cb();
Expand Down Expand Up @@ -123,10 +130,11 @@ class DeployCommand extends AbstractCommand {
* @param {String} profile The profile name
* @param {Boolean} doDebug The flag of debug or not
* @param {Boolean} ignoreHash The flag to ignore difference between local and remote version
* @param {Boolean} shouldDereferenceSymlinks If true, will copy symlinked files and folders into final build
* @param {Function} callback
*/
function deployResources(options, callback) {
const { profile, doDebug, target, ignoreHash } = options;
const { profile, doDebug, target, ignoreHash, shouldDereferenceSymlinks, noNPMInstall } = options;
_deploySkillMetadata(options, (err) => {
if (err) {
return callback(err);
Expand All @@ -148,7 +156,7 @@ function deployResources(options, callback) {
return callback();
}
Messenger.getInstance().info('\n==================== Build Skill Code ====================');
helper.buildSkillCode(profile, doDebug, (buildErr, uniqueCodeList) => {
helper.buildSkillCode({ profile, doDebug, shouldDereferenceSymlinks, noNPMInstall }, (buildErr, uniqueCodeList) => {
if (buildErr) {
return callback(buildErr);
}
Expand All @@ -165,7 +173,7 @@ with build flow ${codeProperty.buildFlow}.`);
return callback();
}
Messenger.getInstance().info('\n==================== Deploy Skill Infrastructure ====================');
helper.deploySkillInfrastructure(profile, doDebug, ignoreHash, (infraErr) => {
helper.deploySkillInfrastructure({ profile, doDebug, ignoreHash }, (infraErr) => {
if (infraErr) {
return callback(infraErr);
}
Expand Down
10 changes: 10 additions & 0 deletions lib/commands/option-model.json
Original file line number Diff line number Diff line change
Expand Up @@ -200,5 +200,15 @@
"name": "watch",
"description": "Uses nodemon to monitor changes and automatically restart the run session.",
"stringInput": "NONE"
},
"dereference-symlinks": {
"name": "dereference-symlinks",
"description": "Copies symlinked files from code source as actual files instead of symlinks",
"stringInput": "NONE"
},
"no-npm-install": {
"name": "no-npm-install",
"description": "Skips running `npm install` before deploying code",
"stringInput": "NONE"
}
}
13 changes: 9 additions & 4 deletions lib/controllers/skill-code-controller/code-builder.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,12 +19,14 @@ const BUILD_FLOWS = [
class CodeBuilder {
/**
* Constructor for CodeBuilder
* @param {Object} config { src, build, doDebug }, where build = { folder, file }.
* @param {Object} config { src, build, shouldDereferenceSymlinks, noNPMInstall, doDebug }, where build = { folder, file }.
*/
constructor(config) {
const { src, build, doDebug } = config;
const { src, build, shouldDereferenceSymlinks, noNPMInstall, doDebug } = config;
this.src = src;
this.build = build;
this.shouldDereferenceSymlinks = shouldDereferenceSymlinks || false;
this.noNPMInstall = noNPMInstall || false;
this.doDebug = doDebug;
this.BuildFlowClass = {};
this._selectBuildFlowClass();
Expand All @@ -40,15 +42,18 @@ class CodeBuilder {
} catch (fsErr) {
return process.nextTick(callback(fsErr));
}
const options = { cwd: this.build.folder, src: this.src, buildFile: this.build.file, doDebug: this.doDebug };
const options = { cwd: this.build.folder, src: this.src, buildFile: this.build.file, doDebug: this.doDebug, noNPMInstall: this.noNPMInstall };
const builder = new this.BuildFlowClass(options);
builder.execute((error) => callback(error));
}

_setUpBuildFolder() {
fs.ensureDirSync(this.build.folder);
fs.emptyDirSync(this.build.folder);
fs.copySync(path.resolve(this.src), this.build.folder, { filter: src => !src.includes(this.build.folder) });
fs.copySync(path.resolve(this.src), this.build.folder, {
filter: src => !src.includes(this.build.folder),
dereference: this.shouldDereferenceSymlinks
});
}

_selectBuildFlowClass() {
Expand Down
6 changes: 5 additions & 1 deletion lib/controllers/skill-code-controller/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,10 @@ module.exports = class SkillCodeController {
* @param {Object} configuration { profile, doDebug }
*/
constructor(configuration) {
const { profile, doDebug } = configuration;
const { profile, doDebug, shouldDereferenceSymlinks, noNPMInstall } = configuration;
this.profile = profile;
this.shouldDereferenceSymlinks = shouldDereferenceSymlinks;
this.noNPMInstall = noNPMInstall;
this.doDebug = doDebug;
}

Expand All @@ -33,6 +35,8 @@ module.exports = class SkillCodeController {
const codeBuilder = new CodeBuilder({
src: codeProperty.src,
build: codeProperty.build,
shouldDereferenceSymlinks: this.shouldDereferenceSymlinks,
noNPMInstall: this.noNPMInstall,
doDebug: this.doDebug
});
codeProperty.buildFlow = codeBuilder.BuildFlowClass.name;
Expand Down
47 changes: 47 additions & 0 deletions test/integration/code-builder/code-builder-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,53 @@ describe('code builder test', () => {
});
});

it('| should dereference symlinked files into build', (done) => {
const sourceDirName = 'with-symlinks';
const config = setUpTempFolder(sourceDirName);
const sourceDir = path.join(fixturesDirectory, sourceDirName);
sinon.stub(process, 'cwd').returns(sourceDir);

config.shouldDereferenceSymlinks = true;
const codeBuilder = new CodeBuilder(config);

codeBuilder.execute((err, res) => {
expect(err).eql(null);
expect(res).eql(undefined);
expect(fs.lstatSync(path.join(config.build.folder, 'symlinked-folder', 'symlinked-file.js')).isSymbolicLink()).eq(false);
done();
});
});

it('| should NOT dereference symlinked files into build', (done) => {
const sourceDirName = 'with-symlinks';
const config = setUpTempFolder(sourceDirName);
const sourceDir = path.join(fixturesDirectory, sourceDirName);
sinon.stub(process, 'cwd').returns(sourceDir);

const codeBuilder = new CodeBuilder(config);

codeBuilder.execute((err, res) => {
expect(err).eql(null);
expect(res).eql(undefined);
expect(fs.lstatSync(path.join(config.build.folder, 'symlinked-folder', 'symlinked-file.js')).isSymbolicLink()).eq(false);
done();
});
});

it('| should NOT run NPM install if this is an NPM project and noNPMInstall is true', (done) => {
const config = setUpTempFolder('nodejs-npm');

config.noNPMInstall = true;
const codeBuilder = new CodeBuilder(config);

codeBuilder.execute((err, res) => {
expect(err).eql(null);
expect(res).eql(undefined);
expect(() => fs.statSync(path.join(config.build.folder, 'node_modules'))).to.throw();
done();
});
});

afterEach(() => {
sinon.restore();
});
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
console.log('symlinked file');
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
console.log('node lambda code');
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
{
"name": "hello-world",
"version": "1.1.0",
"description": "test",
"main": "index.js",
"scripts": {
"test": "echo \"Error: no test specified\" && exit 1"
},
"author": "Amazon Alexa",
"license": "ISC",
"dependencies": {
"ask-sdk-core": "^2.6.0"
}
}
26 changes: 19 additions & 7 deletions test/unit/commands/deploy/helper-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@ describe('Commands deploy test - helper test', () => {
const TEST_IGNORE_HASH = false;
const TEST_VENDOR_ID = 'vendor';
const TEST_DO_DEBUG = false;
const TEST_SHOULD_DEREFERENCE_SYMLINKS = true;
const TEST_NO_NPM_INSTALL = false;
const TEST_OPTIONS = { profile: TEST_PROFILE, doDebug: TEST_DO_DEBUG, ignoreHash: TEST_IGNORE_HASH };

describe('# test helper method - confirmProfile', () => {
Expand Down Expand Up @@ -97,7 +99,12 @@ describe('Commands deploy test - helper test', () => {
// setup
sinon.stub(SkillCodeController.prototype, 'buildCode').callsArgWith(0, 'error');
// call
helper.buildSkillCode(TEST_PROFILE, TEST_DO_DEBUG, (err, res) => {
helper.buildSkillCode({
profile: TEST_PROFILE,
doDebug: TEST_DO_DEBUG,
shouldDereferenceSymlinks: TEST_SHOULD_DEREFERENCE_SYMLINKS,
noNPMInstall: TEST_NO_NPM_INSTALL
}, (err, res) => {
// verify
expect(err).equal('error');
expect(res).equal(undefined);
Expand All @@ -109,7 +116,12 @@ describe('Commands deploy test - helper test', () => {
// setup
sinon.stub(SkillCodeController.prototype, 'buildCode').callsArgWith(0);
// call
helper.buildSkillCode(TEST_PROFILE, TEST_DO_DEBUG, (err, res) => {
helper.buildSkillCode({
profile: TEST_PROFILE,
doDebug: TEST_DO_DEBUG,
shouldDereferenceSymlinks: TEST_SHOULD_DEREFERENCE_SYMLINKS,
noNPMInstall: TEST_NO_NPM_INSTALL
}, (err, res) => {
// verify
expect(err).equal(null);
expect(res).equal(undefined);
Expand All @@ -127,7 +139,7 @@ describe('Commands deploy test - helper test', () => {
// setup
sinon.stub(SkillInfrastructureController.prototype, 'deployInfrastructure').callsArgWith(0, 'error');
// call
helper.deploySkillInfrastructure(TEST_PROFILE, TEST_DO_DEBUG, TEST_IGNORE_HASH, (err, res) => {
helper.deploySkillInfrastructure({ profile: TEST_PROFILE, doDebug: TEST_DO_DEBUG, ignoreHash: TEST_IGNORE_HASH }, (err, res) => {
// verify
expect(err).equal('error');
expect(res).equal(undefined);
Expand All @@ -139,7 +151,7 @@ describe('Commands deploy test - helper test', () => {
// setup
sinon.stub(SkillInfrastructureController.prototype, 'deployInfrastructure').callsArgWith(0);
// call
helper.deploySkillInfrastructure(TEST_PROFILE, TEST_DO_DEBUG, TEST_IGNORE_HASH, (err, res) => {
helper.deploySkillInfrastructure({ profile: TEST_PROFILE, doDebug: TEST_DO_DEBUG, ignoreHash: TEST_IGNORE_HASH }, (err, res) => {
// verify
expect(err).equal(undefined);
expect(res).equal(undefined);
Expand All @@ -165,7 +177,7 @@ describe('Commands deploy test - helper test', () => {
// setup
sinon.stub(SkillMetadataController.prototype, 'enableSkill').callsArgWith(0, 'error');
// call
helper.enableSkill(TEST_PROFILE, TEST_DO_DEBUG, (err) => {
helper.enableSkill({ profile: TEST_PROFILE, doDebug: TEST_DO_DEBUG }, (err) => {
expect(err).equal('error');
expect(infoStub.args[0][0]).equal('\n==================== Enable Skill ====================');
done();
Expand All @@ -176,7 +188,7 @@ describe('Commands deploy test - helper test', () => {
// setup
sinon.stub(SkillMetadataController.prototype, 'validateDomain').throws('test-error');
// call
helper.enableSkill(TEST_PROFILE, TEST_DO_DEBUG, (err) => {
helper.enableSkill({ profile: TEST_PROFILE, doDebug: TEST_DO_DEBUG }, (err) => {
expect(err.name).equal('test-error');
expect(infoStub.args[0][0]).equal('\n==================== Enable Skill ====================');
done();
Expand All @@ -187,7 +199,7 @@ describe('Commands deploy test - helper test', () => {
// setup
sinon.stub(SkillMetadataController.prototype, 'enableSkill').callsArgWith(0);
// call
helper.enableSkill(TEST_PROFILE, TEST_DO_DEBUG, (err, res) => {
helper.enableSkill({ profile: TEST_PROFILE, doDebug: TEST_DO_DEBUG }, (err, res) => {
// verify
expect(err).equal(undefined);
expect(res).equal(undefined);
Expand Down
Loading