Skip to content

Commit c4ecd96

Browse files
authoredDec 14, 2021
fix(aws-lambda-nodejs): use closest lockfile when autodetecting (#16629)
fixes #15847 A bug in the automatic lockfile finding logic causes lockfiles higher in the directory tree to be used over lower/closer ones. This is because the code traverses the tree once per lockfile type in series, stopping when it finds one: https://github.com/aws/aws-cdk/blob/58fda9104ad884026d578dc0602f7d64dd533f6d/packages/%40aws-cdk/aws-lambda-nodejs/lib/function.ts#L137-L139 This updates the code to traverse the tree once looking for all the lockfile types at the same time and stop when one or more is found. If multiple are found at the same level, an error is thrown (per #15847 (comment)). ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
1 parent 2fc6895 commit c4ecd96

File tree

3 files changed

+75
-12
lines changed

3 files changed

+75
-12
lines changed
 

‎packages/@aws-cdk/aws-lambda-nodejs/lib/function.ts

+11-6
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ import { Architecture } from '@aws-cdk/aws-lambda';
55
import { Bundling } from './bundling';
66
import { PackageManager } from './package-manager';
77
import { BundlingOptions } from './types';
8-
import { callsites, findUp } from './util';
8+
import { callsites, findUpMultiple } from './util';
99

1010
// keep this import separate from other imports to reduce chance for merge conflicts with v2-main
1111
// eslint-disable-next-line no-duplicate-imports, import/order
@@ -137,15 +137,20 @@ function findLockFile(depsLockFilePath?: string): string {
137137
return path.resolve(depsLockFilePath);
138138
}
139139

140-
const lockFile = findUp(PackageManager.PNPM.lockFile)
141-
?? findUp(PackageManager.YARN.lockFile)
142-
?? findUp(PackageManager.NPM.lockFile);
140+
const lockFiles = findUpMultiple([
141+
PackageManager.PNPM.lockFile,
142+
PackageManager.YARN.lockFile,
143+
PackageManager.NPM.lockFile,
144+
]);
143145

144-
if (!lockFile) {
146+
if (lockFiles.length === 0) {
145147
throw new Error('Cannot find a package lock file (`pnpm-lock.yaml`, `yarn.lock` or `package-lock.json`). Please specify it with `depsFileLockPath`.');
146148
}
149+
if (lockFiles.length > 1) {
150+
throw new Error(`Multiple package lock files found: ${lockFiles.join(', ')}. Please specify the desired one with \`depsFileLockPath\`.`);
151+
}
147152

148-
return lockFile;
153+
return lockFiles[0];
149154
}
150155

151156
/**

‎packages/@aws-cdk/aws-lambda-nodejs/lib/util.ts

+20-5
Original file line numberDiff line numberDiff line change
@@ -35,19 +35,34 @@ export function callsites(): CallSite[] {
3535
* Find a file by walking up parent directories
3636
*/
3737
export function findUp(name: string, directory: string = process.cwd()): string | undefined {
38+
return findUpMultiple([name], directory)[0];
39+
}
40+
41+
/**
42+
* Find the lowest of multiple files by walking up parent directories. If
43+
* multiple files exist at the same level, they will all be returned.
44+
*/
45+
export function findUpMultiple(names: string[], directory: string = process.cwd()): string[] {
3846
const absoluteDirectory = path.resolve(directory);
3947

40-
const file = path.join(directory, name);
41-
if (fs.existsSync(file)) {
42-
return file;
48+
const files = [];
49+
for (const name of names) {
50+
const file = path.join(directory, name);
51+
if (fs.existsSync(file)) {
52+
files.push(file);
53+
}
54+
}
55+
56+
if (files.length > 0) {
57+
return files;
4358
}
4459

4560
const { root } = path.parse(absoluteDirectory);
4661
if (absoluteDirectory === root) {
47-
return undefined;
62+
return [];
4863
}
4964

50-
return findUp(name, path.dirname(absoluteDirectory));
65+
return findUpMultiple(names, path.dirname(absoluteDirectory));
5166
}
5267

5368
/**

‎packages/@aws-cdk/aws-lambda-nodejs/test/util.test.ts

+44-1
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
import * as child_process from 'child_process';
22
import * as fs from 'fs';
33
import * as path from 'path';
4-
import { callsites, exec, extractDependencies, findUp } from '../lib/util';
4+
import { callsites, exec, extractDependencies, findUp, findUpMultiple } from '../lib/util';
55

66
beforeEach(() => {
77
jest.clearAllMocks();
@@ -33,6 +33,49 @@ describe('findUp', () => {
3333
});
3434
});
3535

36+
describe('findUpMultiple', () => {
37+
test('Starting at process.cwd()', () => {
38+
const files = findUpMultiple(['README.md', 'package.json']);
39+
expect(files).toHaveLength(2);
40+
expect(files[0]).toMatch(/aws-lambda-nodejs\/README\.md$/);
41+
expect(files[1]).toMatch(/aws-lambda-nodejs\/package\.json$/);
42+
});
43+
44+
test('Non existing files', () => {
45+
expect(findUpMultiple(['non-existing-file.unknown', 'non-existing-file.unknown2'])).toEqual([]);
46+
});
47+
48+
test('Existing and non existing files', () => {
49+
const files = findUpMultiple(['non-existing-file.unknown', 'README.md']);
50+
expect(files).toHaveLength(1);
51+
expect(files[0]).toMatch(/aws-lambda-nodejs\/README\.md$/);
52+
});
53+
54+
test('Starting at a specific path', () => {
55+
const files = findUpMultiple(['util.test.ts', 'function.test.ts'], path.join(__dirname, 'integ-handlers'));
56+
expect(files).toHaveLength(2);
57+
expect(files[0]).toMatch(/aws-lambda-nodejs\/test\/util\.test\.ts$/);
58+
expect(files[1]).toMatch(/aws-lambda-nodejs\/test\/function\.test\.ts$/);
59+
});
60+
61+
test('Non existing files starting at a non existing relative path', () => {
62+
expect(findUpMultiple(['not-to-be-found.txt', 'not-to-be-found2.txt'], 'non-existing/relative/path')).toEqual([]);
63+
});
64+
65+
test('Starting at a relative path', () => {
66+
const files = findUpMultiple(['util.test.ts', 'function.test.ts'], 'test/integ-handlers');
67+
expect(files).toHaveLength(2);
68+
expect(files[0]).toMatch(/aws-lambda-nodejs\/test\/util\.test\.ts$/);
69+
expect(files[1]).toMatch(/aws-lambda-nodejs\/test\/function\.test\.ts$/);
70+
});
71+
72+
test('Files on multiple levels', () => {
73+
const files = findUpMultiple(['README.md', 'util.test.ts'], path.join(__dirname, 'integ-handlers'));
74+
expect(files).toHaveLength(1);
75+
expect(files[0]).toMatch(/aws-lambda-nodejs\/test\/util\.test\.ts$/);
76+
});
77+
});
78+
3679
describe('exec', () => {
3780
test('normal execution', () => {
3881
const spawnSyncMock = jest.spyOn(child_process, 'spawnSync').mockReturnValue({

0 commit comments

Comments
 (0)
Please sign in to comment.