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

[lockfile-explorer] Add version validation capability (reopened) #4779

Merged
merged 53 commits into from
Jun 24, 2024
Merged
Show file tree
Hide file tree
Changes from 20 commits
Commits
Show all changes
53 commits
Select commit Hold shift + click to select a range
ce81faf
feat: add version validation capability
L-Qun May 15, 2024
7f00302
chore: remove unit test file
L-Qun May 27, 2024
c2e86bf
chore: update shrinkwrap hash
L-Qun Jun 5, 2024
024991f
chore: use lflint instead
L-Qun Jun 6, 2024
c57a379
chore: organize folder
L-Qun Jun 6, 2024
f679d5d
chore: update lockfile-explorer example usage
L-Qun Jun 6, 2024
b76e991
chore: update lint logic
L-Qun Jun 6, 2024
086ed9e
chore: add unit test for lint
L-Qun Jun 6, 2024
45ea1ce
chore: revert
L-Qun Jun 6, 2024
aca2678
Clarify change log
octogonz Jun 12, 2024
1e9a4d5
Fix incorrect command name for "lflint --help" and clarify unit tests
octogonz Jun 12, 2024
7286123
Merge remote-tracking branch 'remotes/origin/main'
octogonz Jun 12, 2024
ebbd0e9
Clarify that lint.ts is an entry point by renaming it to start-lint.ts
octogonz Jun 12, 2024
d8f9d65
Add a template file
octogonz Jun 12, 2024
145fc35
chore: add a new field 'commonLockfileExplorerConfigFolder' for rushc…
L-Qun Jun 12, 2024
465550d
rush change
L-Qun Jun 12, 2024
72d1c07
feat: add a new subcommand "init\for lflint
L-Qun Jun 12, 2024
7b9ee9d
feat: update lint command logic base on the schema
L-Qun Jun 12, 2024
1181731
update schema
L-Qun Jun 12, 2024
d93ded1
fix ci
L-Qun Jun 12, 2024
c5e2558
Update apps/lockfile-explorer/src/commands/init.ts
L-Qun Jun 13, 2024
e8c330e
Update apps/lockfile-explorer/src/commands/init.ts
L-Qun Jun 13, 2024
f81bc2f
Update apps/lockfile-explorer/src/commands/init.ts
L-Qun Jun 13, 2024
afd250e
Update apps/lockfile-explorer/src/constants/common.ts
L-Qun Jun 13, 2024
84e55e8
Update apps/lockfile-explorer/src/commands/lint.ts
L-Qun Jun 13, 2024
9230d3b
Update apps/lockfile-explorer/src/commands/lint.ts
L-Qun Jun 13, 2024
2ff2dbf
Update apps/lockfile-explorer/src/commands/lint.ts
L-Qun Jun 13, 2024
fa72086
Update apps/lockfile-explorer/src/commands/lint.ts
L-Qun Jun 13, 2024
cf8c73e
Update apps/lockfile-explorer/src/commands/lint.ts
L-Qun Jun 13, 2024
7bb89ab
fix: use rush sdk instead of rush lib
L-Qun Jun 13, 2024
9ee2f68
fix: add a util to splice name with version
L-Qun Jun 13, 2024
1fa898f
chore: move schema template json to assets folder
L-Qun Jun 13, 2024
2f08459
fix: remove lockfile filed from rushconfiguration
L-Qun Jun 13, 2024
ff67723
chore: use terminal instead of console
L-Qun Jun 13, 2024
a746dac
add lockfile-lint template for rush project
L-Qun Jun 13, 2024
e1e693c
remove yargs and add ts-command-line dependency for lockfile-explorer
L-Qun Jun 13, 2024
f8ca49c
remove yargs and add ts-command-line dependency for lockfile-explorer
L-Qun Jun 13, 2024
e11978a
migrate init to action
L-Qun Jun 13, 2024
2602025
migrate lint to action
L-Qun Jun 13, 2024
9f1f293
migrate explorer to action
L-Qun Jun 13, 2024
7b85ca7
fix ci
L-Qun Jun 13, 2024
ec9a99c
c# hore: cache lockfile data
L-Qun Jun 13, 2024
797868e
Merge branch 'microsoft:main' into main
L-Qun Jun 13, 2024
d63f320
chore: fix ci
L-Qun Jun 13, 2024
126e8ee
Update apps/lockfile-explorer/src/cli/explorer/actions/StartAction.ts
L-Qun Jun 18, 2024
3a647f6
Update apps/lockfile-explorer/src/cli/explorer/ExplorerCommandLinePar…
L-Qun Jun 18, 2024
b20466c
chore: resolve comments
L-Qun Jun 18, 2024
5477bbc
Merge branch 'main' into main
L-Qun Jun 18, 2024
689d243
chore: update shrinkwrap hash
L-Qun Jun 18, 2024
8094f14
fix: lint
L-Qun Jun 18, 2024
8e08d5c
Update apps/lockfile-explorer/src/cli/explorer/actions/StartAction.ts
L-Qun Jun 19, 2024
f0cb469
Update apps/lockfile-explorer/src/cli/explorer/actions/StartAction.ts
L-Qun Jun 19, 2024
9f571ee
chore: use IRequiredCommandLineStringParameter instead of CommandLine…
L-Qun Jun 19, 2024
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
2 changes: 1 addition & 1 deletion apps/lockfile-explorer/bin/lockfile-explorer
Original file line number Diff line number Diff line change
@@ -1,2 +1,2 @@
#!/usr/bin/env node
require('../lib/start.js');
require('../lib/start-explorer.js');
2 changes: 2 additions & 0 deletions apps/lockfile-explorer/bin/lockfile-lint
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
#!/usr/bin/env node
require('../lib/start-lint.js');
12 changes: 9 additions & 3 deletions apps/lockfile-explorer/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,9 @@
"license": "MIT",
"bin": {
"lockfile-explorer": "./bin/lockfile-explorer",
"lfx": "./bin/lockfile-explorer"
"lfx": "./bin/lockfile-explorer",
"lockfile-lint": "./bin/lockfile-lint",
"lflint": "./bin/lockfile-lint"
},
"scripts": {
"build": "heft build --clean",
Expand All @@ -52,7 +54,9 @@
"@types/js-yaml": "3.12.1",
"@types/update-notifier": "~6.0.1",
"local-node-rig": "workspace:*",
"@pnpm/lockfile-types": "~6.0.0"
"@pnpm/lockfile-types": "^5.1.5",
"@types/yargs": "~17.0.32",
"@types/semver": "7.5.0"
},
"dependencies": {
"@microsoft/rush-lib": "workspace:*",
Expand All @@ -63,6 +67,8 @@
"js-yaml": "~3.13.1",
"open": "~8.4.0",
"update-notifier": "~5.1.0",
"@pnpm/dependency-path": "~2.1.2"
"@pnpm/dependency-path": "~2.1.2",
"yargs": "~17.7.2",
L-Qun marked this conversation as resolved.
Show resolved Hide resolved
"semver": "~7.5.4"
}
}
23 changes: 0 additions & 23 deletions apps/lockfile-explorer/src/commandLine.test.ts

This file was deleted.

64 changes: 0 additions & 64 deletions apps/lockfile-explorer/src/commandLine.ts

This file was deleted.

46 changes: 46 additions & 0 deletions apps/lockfile-explorer/src/commands/init.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
// Copyright (c) Microsoft Corporation. All rights reserved. Licensed under the MIT license.
// See LICENSE in the project root for license information.

import { RushConfiguration } from '@microsoft/rush-lib';
L-Qun marked this conversation as resolved.
Show resolved Hide resolved
import { FileSystem } from '@rushstack/node-core-library';
import { Colorize } from '@rushstack/terminal';
import type { CommandModule } from 'yargs';
import * as path from 'path';
import { LockfileExplorerConfig } from '../constants/common';

// Example usage: lflint init
// Example usage: lockfile-lint init
export const initCommand: CommandModule = {
command: 'init',
describe: `Create ${LockfileExplorerConfig.FileName} config file`,
handler: () => {
L-Qun marked this conversation as resolved.
Show resolved Hide resolved
try {
const rushConfiguration: RushConfiguration | undefined = RushConfiguration.tryLoadFromDefaultLocation();
if (!rushConfiguration) {
throw new Error(
'The "lockfile-explorer check" must be executed in a folder that is under a Rush workspace folder'
);
}
const inputFilePath: string = path.resolve(__dirname, '../schemas/lockfile-lint-template.json');
const outputFilePath: string = path.resolve(
rushConfiguration.commonLockfileExplorerConfigFolder,
LockfileExplorerConfig.FileName
);

if (FileSystem.exists(outputFilePath)) {
L-Qun marked this conversation as resolved.
Show resolved Hide resolved
console.log(Colorize.red('The output file already exists:'));
console.log('\n ' + outputFilePath + '\n');
throw new Error('Unable to write output file');
}

console.log(Colorize.green('Writing file: ') + outputFilePath);
L-Qun marked this conversation as resolved.
Show resolved Hide resolved
FileSystem.copyFile({
sourcePath: inputFilePath,
destinationPath: outputFilePath
});
L-Qun marked this conversation as resolved.
Show resolved Hide resolved
} catch (error) {
console.error(Colorize.red('ERROR: ' + error.message));
process.exit(1);
}
}
};
167 changes: 167 additions & 0 deletions apps/lockfile-explorer/src/commands/lint.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,167 @@
// Copyright (c) Microsoft Corporation. All rights reserved. Licensed under the MIT license.
// See LICENSE in the project root for license information.

import type { Lockfile, LockfileV6 } from '@pnpm/lockfile-types';
import path from 'path';
import yaml from 'js-yaml';
import { RushConfiguration } from '@microsoft/rush-lib/lib/api/RushConfiguration';
import type { Subspace } from '@microsoft/rush-lib/lib/api/Subspace';
import type { RushConfigurationProject } from '@microsoft/rush-lib/lib/api/RushConfigurationProject';
L-Qun marked this conversation as resolved.
Show resolved Hide resolved
import { FileSystem, JsonFile, JsonSchema } from '@rushstack/node-core-library';
import type { CommandModule } from 'yargs';
import { Colorize } from '@rushstack/terminal';
import semver from 'semver';

import lockfileLintSchema from '../schemas/lockfile-lint.schema.json';
import { getShrinkwrapFileMajorVersion, parseDependencyPath } from '../utils/shrinkwrap';
import { LockfileExplorerConfig } from '../constants/common';

export interface ILintRule {
rule: 'restrict-versions';
project: string;
requiredVersions: Record<string, string>;
}

export interface ILockfileLint {
rules: ILintRule[];
}

async function checkVersionCompatibility(
shrinkwrapFileMajorVersion: number,
packages: Lockfile['packages'],
dependencyPath: string,
requiredVersions: Record<string, string>,
checkedDependencyPaths: Set<string>
): Promise<void> {
if (packages && packages[dependencyPath] && !checkedDependencyPaths.has(dependencyPath)) {
checkedDependencyPaths.add(dependencyPath);
const { name, version } = parseDependencyPath(shrinkwrapFileMajorVersion, dependencyPath);
if (name in requiredVersions && !semver.satisfies(version, requiredVersions[name])) {
throw new Error(`ERROR: Detected inconsistent version numbers in package '${name}': '${version}'!`);
}

for (const [dependencyPackageName, dependencyPackageVersion] of Object.entries(
packages[dependencyPath].dependencies ?? {}
)) {
L-Qun marked this conversation as resolved.
Show resolved Hide resolved
await checkVersionCompatibility(
shrinkwrapFileMajorVersion,
packages,
`/${dependencyPackageName}${shrinkwrapFileMajorVersion === 6 ? '@' : '/'}${dependencyPackageVersion}`,
L-Qun marked this conversation as resolved.
Show resolved Hide resolved
requiredVersions,
checkedDependencyPaths
);
}
}
}

async function searchAndValidateDependencies(
L-Qun marked this conversation as resolved.
Show resolved Hide resolved
rushConfiguration: RushConfiguration,
checkedProjects: Set<RushConfigurationProject>,
project: RushConfigurationProject,
requiredVersions: Record<string, string>
): Promise<void> {
console.log(`Checking the project: ${project.packageName}.`);

const projectFolder: string | undefined = project.projectFolder;
L-Qun marked this conversation as resolved.
Show resolved Hide resolved
const subspace: Subspace | undefined = project.subspace;
L-Qun marked this conversation as resolved.
Show resolved Hide resolved
if (subspace && projectFolder) {
const shrinkwrapFilename: string = subspace.getCommittedShrinkwrapFilename();
const pnpmLockfileText: string = await FileSystem.readFileAsync(shrinkwrapFilename);
const doc = yaml.load(pnpmLockfileText) as Lockfile | LockfileV6;
L-Qun marked this conversation as resolved.
Show resolved Hide resolved
const { importers, lockfileVersion, packages } = doc;
const shrinkwrapFileMajorVersion: number = getShrinkwrapFileMajorVersion(lockfileVersion);
const checkedDependencyPaths: Set<string> = new Set<string>();
for (const [relativePath, { dependencies }] of Object.entries(importers)) {
if (path.resolve(projectFolder, relativePath) === projectFolder) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it possible to make this comparison without using path.resolve? Functions on path are somewhat slow.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it would be better to use path.resolve here, using other methods might be a bit hacky

const dependenciesEntries = Object.entries(dependencies ?? {});
for (const [dependencyName, dependencyValue] of dependenciesEntries) {
const fullDependencyPath = `/${dependencyName}${shrinkwrapFileMajorVersion === 6 ? '@' : '/'}${
typeof dependencyValue === 'string'
? dependencyValue
: (
dependencyValue as {
version: string;
specifier: string;
}
).version
}`;
if (fullDependencyPath.includes('link:')) {
const dependencyProject: RushConfigurationProject | undefined =
rushConfiguration.getProjectByName(dependencyName);
if (dependencyProject && !checkedProjects.has(dependencyProject)) {
checkedProjects.add(project);
await searchAndValidateDependencies(
rushConfiguration,
checkedProjects,
dependencyProject,
requiredVersions
);
L-Qun marked this conversation as resolved.
Show resolved Hide resolved
}
} else {
await checkVersionCompatibility(
shrinkwrapFileMajorVersion,
packages,
fullDependencyPath,
requiredVersions,
checkedDependencyPaths
);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems like these results should be cached, as multiple projects can point at the same dependencies.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The same dependency is never checked twice

}
}
}
}
}
}

async function performVersionRestrictionCheck(
L-Qun marked this conversation as resolved.
Show resolved Hide resolved
rushConfiguration: RushConfiguration,
requiredVersions: Record<string, string>,
projectName: string
): Promise<void> {
const project: RushConfigurationProject | undefined = rushConfiguration?.getProjectByName(projectName);
if (!project) {
throw new Error(`Cannot found project name: ${projectName}`);
L-Qun marked this conversation as resolved.
Show resolved Hide resolved
L-Qun marked this conversation as resolved.
Show resolved Hide resolved
}
const checkedProjects: Set<RushConfigurationProject> = new Set<RushConfigurationProject>([project]);
await searchAndValidateDependencies(rushConfiguration, checkedProjects, project, requiredVersions);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems like this should collect all of the projects that are to be validated by this rule and then run them all together.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can specify multiple constrained versions for the same project and analyze these dependencies together
"rules": [
{
"rule": "restrict-versions",
"project": "A",
"requiredVersions": {
"react-router": "^4",
"react": "18.2.0"
}
}
]

}

// Example usage: lflint
// Example usage: lockfile-lint
export const lintCommand: CommandModule = {
command: '$0',
describe: 'Check if the specified package has a inconsistent package versions in target project',
// eslint-disable-next-line @typescript-eslint/naming-convention
handler: async () => {
try {
const rushConfiguration: RushConfiguration | undefined = RushConfiguration.tryLoadFromDefaultLocation();
if (!rushConfiguration) {
throw new Error(
'The "lockfile-explorer check" must be executed in a folder that is under a Rush workspace folder'
);
}
const lintingFile: string = path.resolve(
rushConfiguration.commonLockfileExplorerConfigFolder,
LockfileExplorerConfig.FileName
);
const { rules }: ILockfileLint = JsonFile.loadAndValidate(
L-Qun marked this conversation as resolved.
Show resolved Hide resolved
lintingFile,
JsonSchema.fromLoadedObject(lockfileLintSchema)
);
for (const { requiredVersions, project, rule } of rules) {
switch (rule) {
case 'restrict-versions': {
await performVersionRestrictionCheck(rushConfiguration, requiredVersions, project);
break;
}
default: {
throw new Error('Unsupported rule name: ' + rule);
}
}
}
L-Qun marked this conversation as resolved.
Show resolved Hide resolved
console.log(Colorize.green('Check passed!'));
} catch (error) {
console.error(Colorize.red('ERROR: ' + error.message));
process.exit(1);
}
}
};
Loading
Loading