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

Enhanced TurboSnap: Trace dependency changes instead of bailing out #683

Merged
merged 29 commits into from
Dec 19, 2022
Merged
Show file tree
Hide file tree
Changes from 14 commits
Commits
Show all changes
29 commits
Select commit Hold shift + click to select a range
d7b1b4f
Lockfile parser added
skitterm Oct 5, 2022
8cf0080
BaselineCommits used so we can diff the lockfiles
skitterm Oct 5, 2022
d102861
Yarn files diffed
skitterm Oct 5, 2022
68f3e2e
Merge branch 'package-jason' into lockfiles
skitterm Oct 17, 2022
d46e4cb
lockfile explorations
skitterm Nov 10, 2022
7581a11
Merge branch 'main' into lockfiles
ghengeveld Nov 17, 2022
e27319e
Refactored a bunch of logic and implemented proper behavior
ghengeveld Nov 25, 2022
feb1b5e
Fix tests
ghengeveld Nov 25, 2022
0fb8ec5
Use temp file for git.checkoutFile
ghengeveld Dec 1, 2022
b673232
Use replacementBuild commit if set
ghengeveld Dec 12, 2022
2ad7785
Find dependency changes in subpackages
ghengeveld Dec 12, 2022
f5f856c
Merge branch 'main' into lockfiles
ghengeveld Dec 15, 2022
85fe475
Add test for when package.json has changed but lockfile hasn't
ghengeveld Dec 15, 2022
6b716ec
Merge branch 'main' into lockfiles
ghengeveld Dec 15, 2022
e009164
Fix mock response
ghengeveld Dec 15, 2022
da6452f
6.14.0-canary.0
ghengeveld Dec 16, 2022
30d2afc
Append filename to temp filename
ghengeveld Dec 16, 2022
2a7ccef
Track changedDependencyNames on context for diagnostics, and fix debu…
ghengeveld Dec 16, 2022
3cf6d17
Skip snapshots when there are no actual tests to run
ghengeveld Dec 16, 2022
3c25400
Properly list changed files and dependencies
ghengeveld Dec 16, 2022
e30b39a
Wait for build even when theres zero tests to run
ghengeveld Dec 16, 2022
2df121f
Pull dependency retrieval out to separate file
ghengeveld Dec 16, 2022
15fa827
Split files and add tests
ghengeveld Dec 16, 2022
9ff8583
Don't rely on hasYarn, but look for either lockfile
ghengeveld Dec 16, 2022
91cfbab
Limit concurrency on creating temp files, and add caching for when we…
ghengeveld Dec 16, 2022
b6aa5f4
Add some debug logging for when we fail to retrieve the list of depen…
ghengeveld Dec 16, 2022
be75e97
Allow skipping dependency tracing using --untraced
ghengeveld Dec 16, 2022
6dd5a4a
6.14.0-next.0
ghengeveld Dec 19, 2022
e885e9d
6.14.0
ghengeveld Dec 19, 2022
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
12 changes: 12 additions & 0 deletions bin-src/git/git.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import execa from 'execa';
import { EOL } from 'os';
import { file as tmpFile } from 'tmp-promise';

import { Context } from '../types';

Expand Down Expand Up @@ -211,6 +212,12 @@ export async function checkout(ref: string) {
return execGitCommand(`git checkout ${ref}`);
}

export async function checkoutFile(ref: string, fileName: string) {
const { path: targetFileName } = await tmpFile();
await execGitCommand(`git show ${ref}:${fileName} > ${targetFileName}`);
return targetFileName;
}

export async function checkoutPrevious() {
return execGitCommand(`git checkout -`);
}
Expand All @@ -222,3 +229,8 @@ export async function discardChanges() {
export async function getRepositoryRoot() {
return execGitCommand(`git rev-parse --show-toplevel`);
}

export async function findFiles(pattern: string) {
const files = await execGitCommand(`git ls-files -z '${pattern}'`);
return files.split('\0').filter(Boolean);
}
2 changes: 1 addition & 1 deletion bin-src/lib/checkForUpdates.ts
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,6 @@ export default async function checkForUpdates(ctx: Context) {
}

if (semver.major(ctx.pkg.version) < semver.major(latestVersion)) {
ctx.log.warn(outdatedPackage(ctx, latestVersion, hasYarn));
ctx.log.warn(outdatedPackage(ctx, latestVersion, hasYarn()));
}
}
215 changes: 215 additions & 0 deletions bin-src/lib/findChangedDependencies.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,215 @@
import { buildDepTreeFromFiles } from 'snyk-nodejs-lockfile-parser';
import { findChangedDependencies } from './findChangedDependencies';
import * as git from '../git/git';

jest.mock('snyk-nodejs-lockfile-parser');
jest.mock('yarn-or-npm');
jest.mock('../git/git');

const getRepositoryRoot = <jest.MockedFunction<typeof git.getRepositoryRoot>>git.getRepositoryRoot;
const checkoutFile = <jest.MockedFunction<typeof git.checkoutFile>>git.checkoutFile;
const findFiles = <jest.MockedFunction<typeof git.findFiles>>git.findFiles;
const buildDepTree = <jest.MockedFunction<typeof buildDepTreeFromFiles>>buildDepTreeFromFiles;

beforeEach(() => {
getRepositoryRoot.mockResolvedValue('/root');
findFiles.mockImplementation((file) => Promise.resolve(file.startsWith('**') ? [] : [file]));
});
afterEach(() => {
getRepositoryRoot.mockReset();
checkoutFile.mockReset();
findFiles.mockReset();
buildDepTree.mockReset();
});

describe('findChangedDependencies', () => {
it('returns nothing given no baselines', async () => {
buildDepTree.mockResolvedValue({ dependencies: {} });

await expect(findChangedDependencies([])).resolves.toEqual([]);
});

it('returns nothing when dependency tree is empty', async () => {
checkoutFile.mockResolvedValueOnce('A.package.json');
checkoutFile.mockResolvedValueOnce('A.package-lock.json');
buildDepTree.mockResolvedValue({ dependencies: {} });

await expect(findChangedDependencies(['A'])).resolves.toEqual([]);
});

it('returns nothing when dependency tree is unchanged', async () => {
checkoutFile.mockResolvedValueOnce('A.package.json');
checkoutFile.mockResolvedValueOnce('A.package-lock.json');
buildDepTree.mockResolvedValue({
dependencies: {
react: { name: 'react', version: '18.2.0', dependencies: {} },
},
});

await expect(findChangedDependencies(['A'])).resolves.toEqual([]);
});

it('returns updated dependencies', async () => {
// HEAD
buildDepTree.mockResolvedValueOnce({
dependencies: { react: { name: 'react', version: '18.2.0', dependencies: {} } },
});

// Baseline A
checkoutFile.mockResolvedValueOnce('A.package.json');
checkoutFile.mockResolvedValueOnce('A.package-lock.json');
buildDepTree.mockResolvedValueOnce({
dependencies: { react: { name: 'react', version: '18.3.0', dependencies: {} } },
});

await expect(findChangedDependencies(['A'])).resolves.toEqual(['react']);
});

it('returns added/removed dependencies', async () => {
// HEAD
buildDepTree.mockResolvedValueOnce({
dependencies: { react: { name: 'react', version: '18.2.0', dependencies: {} } },
});

// Baseline A
checkoutFile.mockResolvedValueOnce('A.package.json');
checkoutFile.mockResolvedValueOnce('A.package-lock.json');
buildDepTree.mockResolvedValueOnce({
dependencies: { vue: { name: 'vue', version: '3.2.0', dependencies: {} } },
});

await expect(findChangedDependencies(['A'])).resolves.toEqual(['vue', 'react']);
});

it('finds updated transient dependencies', async () => {
// HEAD
buildDepTree.mockResolvedValueOnce({
dependencies: {
react: {
name: 'react',
version: '18.2.0',
dependencies: {
'loose-envify': { name: 'loose-envify', version: '1.3.1', dependencies: {} },
},
},
},
});

// Baseline A
checkoutFile.mockResolvedValueOnce('A.package.json');
checkoutFile.mockResolvedValueOnce('A.package-lock.json');
buildDepTree.mockResolvedValueOnce({
dependencies: {
react: {
name: 'react',
version: '18.2.0',
dependencies: {
'loose-envify': { name: 'loose-envify', version: '1.4.0', dependencies: {} },
},
},
},
});

await expect(findChangedDependencies(['A'])).resolves.toEqual(['loose-envify']);
});

it('combines and dedupes changes for multiple baselines', async () => {
// HEAD
buildDepTree.mockResolvedValueOnce({
dependencies: {
react: { name: 'react', version: '18.2.0', dependencies: {} },
lodash: { name: 'lodash', version: '4.17.21', dependencies: {} },
},
});

// Baseline A
checkoutFile.mockResolvedValueOnce('A.package.json');
checkoutFile.mockResolvedValueOnce('A.package-lock.json');
buildDepTree.mockResolvedValueOnce({
dependencies: {
react: { name: 'react', version: '18.3.0', dependencies: {} },
lodash: { name: 'lodash', version: '4.17.21', dependencies: {} },
},
});

// Baseline B
checkoutFile.mockResolvedValueOnce('B.package.json');
checkoutFile.mockResolvedValueOnce('B.package-lock.json');
buildDepTree.mockResolvedValueOnce({
dependencies: {
react: { name: 'react', version: '18.3.0', dependencies: {} },
lodash: { name: 'lodash', version: '4.18.0', dependencies: {} },
},
});

await expect(findChangedDependencies(['A', 'B'])).resolves.toEqual(['react', 'lodash']);
});

it('looks for manifest and lock files in subpackages', async () => {
findFiles.mockImplementation((file) =>
Promise.resolve(file.startsWith('**') ? [file.replace('**', 'subdir')] : [file])
);

// HEAD root
buildDepTree.mockResolvedValueOnce({
dependencies: { react: { name: 'react', version: '18.2.0', dependencies: {} } },
});
// HEAD subdir
buildDepTree.mockResolvedValueOnce({
dependencies: { lodash: { name: 'lodash', version: '4.17.21', dependencies: {} } },
});

// Baseline A
checkoutFile.mockImplementation((commit, file) => Promise.resolve(`${commit}.${file}`));
buildDepTree.mockResolvedValueOnce({
dependencies: { react: { name: 'react', version: '18.3.0', dependencies: {} } },
});
buildDepTree.mockResolvedValueOnce({
dependencies: { lodash: { name: 'lodash', version: '4.18.0', dependencies: {} } },
});

await expect(findChangedDependencies(['A'])).resolves.toEqual(['react', 'lodash']);

// Root manifest and lock files are checked
expect(buildDepTree).toHaveBeenCalledWith('/root', 'package.json', 'package-lock.json', true);
expect(buildDepTree).toHaveBeenCalledWith(
'/root',
'A.package.json',
'A.package-lock.json',
true
);

// Subpackage manifest and lock files are checked
expect(buildDepTree).toHaveBeenCalledWith(
'/root',
'subdir/package.json',
'subdir/package-lock.json',
true
);
expect(buildDepTree).toHaveBeenCalledWith(
'/root',
'A.subdir/package.json',
'A.subdir/package-lock.json',
true
);
});

it('uses root lockfile when subpackage lockfile is missing', async () => {
findFiles.mockImplementation((file) => {
if (file === 'subdir/package-lock.json') return Promise.resolve([]);
return Promise.resolve(file.startsWith('**') ? [file.replace('**', 'subdir')] : [file]);
});

checkoutFile.mockImplementation((commit, file) => Promise.resolve(`${commit}.${file}`));
buildDepTree.mockResolvedValue({ dependencies: {} });

await expect(findChangedDependencies(['A'])).resolves.toEqual([]);

expect(buildDepTree).toHaveBeenCalledWith(
'/root',
'A.subdir/package.json',
'A.package-lock.json', // root lockfile
true
);
});
});
77 changes: 77 additions & 0 deletions bin-src/lib/findChangedDependencies.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,77 @@
import path from 'path';
import { buildDepTreeFromFiles, PkgTree } from 'snyk-nodejs-lockfile-parser';
import { hasYarn } from 'yarn-or-npm';
import { checkoutFile, findFiles, getRepositoryRoot } from '../git/git';

const flattenDependencyTree = (
tree: PkgTree['dependencies'],
results = new Set<string>()
): Set<string> =>
Object.values(tree).reduce((acc, dep) => {
acc.add(`${dep.name}@@${dep.version}`);
return flattenDependencyTree(dep.dependencies || {}, acc);
}, results);

// Retrieve a set of values which is in either set, but not both.
const xor = <T>(left: Set<T>, right: Set<T>) =>
Array.from(right.values()).reduce((acc, value) => {
if (acc.has(value)) acc.delete(value);
else acc.add(value);
return acc;
}, new Set(left));

// Yields a list of dependency names which have changed since the baseline.
// E.g. ['react', 'react-dom', '@storybook/react']
export const findChangedDependencies = async (baselineCommits: string[]) => {
const manifestName = 'package.json';
const lockfileName = hasYarn() ? 'yarn.lock' : 'package-lock.json';
tmeasday marked this conversation as resolved.
Show resolved Hide resolved

const rootPath = await getRepositoryRoot();
const [rootManifestPath] = await findFiles(manifestName);
const [rootLockfilePath] = await findFiles(lockfileName);
if (!rootManifestPath || !rootLockfilePath) {
throw new Error(`Could not find ${manifestName} or ${lockfileName} in the repository`);
}

// Handle monorepos with multiple package.json files.
const nestedManifestPaths = await findFiles(`**/${manifestName}`);
const pathPairs = await Promise.all(
nestedManifestPaths.map(async (manifestPath) => {
const dirname = path.dirname(manifestPath);
const [lockfilePath] = await findFiles(`${dirname}/${lockfileName}`);
// Fall back to the root lockfile if we can't find one in the same directory.
return [manifestPath, lockfilePath || rootLockfilePath];
})
);

// Use a Set so we only keep distinct package names.
const changedDependencyNames = new Set<string>();

await Promise.all(
[[rootManifestPath, rootLockfilePath], ...pathPairs].map(
async ([manifestPath, lockfilePath]) => {
const headTree = await buildDepTreeFromFiles(rootPath, manifestPath, lockfilePath, true);
const headDependencies = flattenDependencyTree(headTree.dependencies);

// Retrieve the union of dependencies which changed compared to each baseline.
// A change means either the version number is different or the dependency was added/removed.
// If a manifest or lockfile is missing on the baseline, this throws and we'll end up bailing.
await Promise.all(
baselineCommits.map(async (commit) => {
const manifest = await checkoutFile(commit, manifestPath);
const lockfile = await checkoutFile(commit, lockfilePath);
const baselineTree = await buildDepTreeFromFiles(rootPath, manifest, lockfile, true);
const baselineDependencies = flattenDependencyTree(baselineTree.dependencies);
// eslint-disable-next-line no-restricted-syntax
for (const dependency of xor(baselineDependencies, headDependencies)) {
// Strip the version number so we get a set of package names.
changedDependencyNames.add(dependency.split('@@')[0]);
}
})
);
}
)
);

return Array.from(changedDependencyNames);
};
Loading