From 632d9b3622040a4b25d5f97d72d2222496987f16 Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Sun, 13 Feb 2022 16:29:46 +0100 Subject: [PATCH 01/12] cli: set default worker queue thread count to cpus/2 Signed-off-by: Patrik Oldsberg --- packages/cli/src/lib/parallel.ts | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/packages/cli/src/lib/parallel.ts b/packages/cli/src/lib/parallel.ts index 246020fcc3..15946bd70d 100644 --- a/packages/cli/src/lib/parallel.ts +++ b/packages/cli/src/lib/parallel.ts @@ -14,6 +14,7 @@ * limitations under the License. */ +import os from 'os'; import { ErrorLike } from '@backstage/errors'; import { Worker } from 'worker_threads'; @@ -137,7 +138,7 @@ export type WorkerQueueThreadsOptions = { workerFactory: (data: TData) => (item: TItem) => Promise; /** Data supplied to each worker factory */ workerData?: TData; - /** Number of threads, defaults to the environment parallelism */ + /** Number of threads, defaults to half of the number of available CPUs */ threadCount?: number; }; @@ -151,7 +152,7 @@ export async function runWorkerQueueThreads( const { workerFactory, workerData, - threadCount = getEnvironmentParallelism(), + threadCount = os.cpus().length / 2, } = options; const iterator = options.items[Symbol.iterator](); From 0fb1b0dbe39448f62230359300de9a56d6487980 Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Sun, 13 Feb 2022 16:30:54 +0100 Subject: [PATCH 02/12] cli: update worker queue to allow async factory function Signed-off-by: Patrik Oldsberg --- packages/cli/src/lib/parallel.ts | 54 +++++++++++++++++++------------- 1 file changed, 33 insertions(+), 21 deletions(-) diff --git a/packages/cli/src/lib/parallel.ts b/packages/cli/src/lib/parallel.ts index 15946bd70d..ec3d5493c0 100644 --- a/packages/cli/src/lib/parallel.ts +++ b/packages/cli/src/lib/parallel.ts @@ -135,7 +135,11 @@ export type WorkerQueueThreadsOptions = { * note that they are both copied by value into the worker thread, except for * types that are explicitly shareable across threads, such as `SharedArrayBuffer`. */ - workerFactory: (data: TData) => (item: TItem) => Promise; + workerFactory: ( + data: TData, + ) => + | ((item: TItem) => Promise) + | Promise<(item: TItem) => Promise>; /** Data supplied to each worker factory */ workerData?: TData; /** Number of threads, defaults to half of the number of available CPUs */ @@ -209,31 +213,39 @@ export async function runWorkerQueueThreads( } function workerQueueThread( - workerFuncFactory: (data: unknown) => (item: unknown) => Promise, + workerFuncFactory: ( + data: unknown, + ) => Promise<(item: unknown) => Promise>, ) { const { parentPort, workerData } = require('worker_threads'); - const workerFunc = workerFuncFactory(workerData); - parentPort.on('message', async (message: WorkerThreadMessage) => { - if (message.type === 'done') { - parentPort.close(); - return; - } - if (message.type === 'item') { - try { - const result = await workerFunc(message.item); - parentPort.postMessage({ - type: 'result', - index: message.index, - result, + Promise.resolve() + .then(() => workerFuncFactory(workerData)) + .then( + workerFunc => { + parentPort.on('message', async (message: WorkerThreadMessage) => { + if (message.type === 'done') { + parentPort.close(); + return; + } + if (message.type === 'item') { + try { + const result = await workerFunc(message.item); + parentPort.postMessage({ + type: 'result', + index: message.index, + result, + }); + } catch (error) { + parentPort.postMessage({ type: 'error', error }); + } + } }); - } catch (error) { - parentPort.postMessage({ type: 'error', error }); - } - } - }); - parentPort.postMessage({ type: 'start' }); + parentPort.postMessage({ type: 'start' }); + }, + error => parentPort.postMessage({ type: 'error', error }), + ); } export type WorkerThreadsOptions = { From 432bc564f49ad2f261dc4796f75a75c8b69ca182 Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Sun, 13 Feb 2022 16:32:16 +0100 Subject: [PATCH 03/12] cli: add new hidden repo lint command Signed-off-by: Patrik Oldsberg --- packages/cli/src/commands/index.ts | 11 +++ packages/cli/src/commands/repo/lint.ts | 94 ++++++++++++++++++++++++++ 2 files changed, 105 insertions(+) create mode 100644 packages/cli/src/commands/repo/lint.ts diff --git a/packages/cli/src/commands/index.ts b/packages/cli/src/commands/index.ts index 770d216ba2..b3c9bae6a4 100644 --- a/packages/cli/src/commands/index.ts +++ b/packages/cli/src/commands/index.ts @@ -42,6 +42,17 @@ export function registerRepoCommand(program: CommanderStatic) { 'Build all packages, including bundled app and backend packages.', ) .action(lazy(() => import('./repo/build').then(m => m.command))); + + command + .command('lint') + .description('Lint all packages in the project') + .option( + '--format ', + 'Lint report output format', + 'eslint-formatter-friendly', + ) + .option('--fix', 'Attempt to automatically fix violations') + .action(lazy(() => import('./repo/lint').then(m => m.command))); } export function registerScriptCommand(program: CommanderStatic) { diff --git a/packages/cli/src/commands/repo/lint.ts b/packages/cli/src/commands/repo/lint.ts new file mode 100644 index 0000000000..4c1b912d92 --- /dev/null +++ b/packages/cli/src/commands/repo/lint.ts @@ -0,0 +1,94 @@ +/* + * Copyright 2020 The Backstage Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import chalk from 'chalk'; +import { Command } from 'commander'; +import { relative as relativePath } from 'path'; +import { PackageGraph } from '../../lib/monorepo'; +import { runWorkerQueueThreads } from '../../lib/parallel'; +import { paths } from '../../lib/paths'; + +export async function command(cmd: Command): Promise { + const packages = await PackageGraph.listTargetPackages(); + + // This formatter uses the cwd to format file paths, so let's have that happen from the root instead + if (cmd.format === 'eslint-formatter-friendly') { + process.chdir(paths.targetRoot); + } + + // Make sure lint output is colored unless the user explicitly disabled it + if (!process.env.FORCE_COLOR) { + process.env.FORCE_COLOR = '1'; + } + + const resultsList = await runWorkerQueueThreads({ + items: packages.map(pkg => ({ + fullDir: pkg.dir, + relativeDir: relativePath(paths.targetRoot, pkg.dir), + })), + workerData: { + fix: Boolean(cmd.fix), + format: cmd.format as string | undefined, + }, + workerFactory: async ({ fix, format }) => { + const { ESLint } = require('eslint'); + + return async ({ + fullDir, + relativeDir, + }): Promise<{ relativeDir: string; resultText: string }> => { + // Bit of a hack to make file resolutions happen from the correct directory + // since some lint rules don't respect the cwd of ESLint + process.cwd = () => fullDir; + + const eslint = new ESLint({ + cwd: fullDir, + fix, + extensions: ['js', 'jsx', 'ts', 'tsx', 'mjs', 'cjs'], + }); + const formatter = await eslint.loadFormatter(format); + + const results = await eslint.lintFiles(['.']); + + const count = String(results.length).padStart(3); + console.log(`Checked ${count} files in ${relativeDir}`); + + if (fix) { + await ESLint.outputFixes(results); + } + + const resultText = formatter.format(results); + + return { relativeDir, resultText }; + }; + }, + }); + + let failed = false; + for (const { relativeDir, resultText } of resultsList) { + if (resultText) { + console.log(); + console.log(chalk.red(`Lint failed in ${relativeDir}:`)); + console.log(resultText.trimLeft()); + + failed = true; + } + } + + if (failed) { + process.exit(1); + } +} From fb39eb9fc421e669d0a9a362ab36612086592cd2 Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Sun, 13 Feb 2022 21:26:10 +0100 Subject: [PATCH 04/12] cli: added lib utilities for working with git Signed-off-by: Patrik Oldsberg --- packages/cli/src/lib/git.test.ts | 54 +++++++++++++++++++++++++++++ packages/cli/src/lib/git.ts | 59 ++++++++++++++++++++++++++++++++ 2 files changed, 113 insertions(+) create mode 100644 packages/cli/src/lib/git.test.ts create mode 100644 packages/cli/src/lib/git.ts diff --git a/packages/cli/src/lib/git.test.ts b/packages/cli/src/lib/git.test.ts new file mode 100644 index 0000000000..ecb234f79a --- /dev/null +++ b/packages/cli/src/lib/git.test.ts @@ -0,0 +1,54 @@ +/* + * Copyright 2022 The Backstage Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import { runGit, listChangedFiles } from './git'; + +describe('runGit', () => { + it('runs a git command', async () => { + await expect(runGit('log', 'HEAD..HEAD')).resolves.toEqual(['']); + }); + + it('fails for unknown commands', async () => { + await expect(runGit('ryckbegäran')).rejects.toThrow( + /^git ryckbegäran failed, git: 'ryckbegäran' is not a git command/, + ); + }); + + it('forwards failures', async () => { + await expect( + runGit( + 'show', + '--quiet', + '--pretty=format:%s', + '0000000000000000000000000000000000000000', + ), + ).rejects.toThrow( + 'git show failed, fatal: bad object 0000000000000000000000000000000000000000', + ); + }); +}); + +describe('listChangedFiles', () => { + it('requires a ref', async () => { + await expect(listChangedFiles('')).rejects.toThrow('ref is required'); + }); + + it('should return something', async () => { + await expect(listChangedFiles('origin/master')).resolves.toEqual( + expect.any(Array), + ); + }); +}); diff --git a/packages/cli/src/lib/git.ts b/packages/cli/src/lib/git.ts new file mode 100644 index 0000000000..f8783804d1 --- /dev/null +++ b/packages/cli/src/lib/git.ts @@ -0,0 +1,59 @@ +/* + * Copyright 2022 The Backstage Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import { assertError, ForwardedError } from '@backstage/errors'; +import { execFile as execFileCb } from 'child_process'; +import { promisify } from 'util'; +import { paths } from './paths'; + +const execFile = promisify(execFileCb); + +/** + * Run a git command, trimming the output splitting it into lines. + */ +export async function runGit(...args: string[]) { + try { + const { stdout } = await execFile('git', args, { + shell: true, + cwd: paths.targetRoot, + }); + return stdout.trim().split(/\r\n|\r|\n/); + } catch (error) { + assertError(error); + if (error.stderr || typeof error.code === 'number') { + const stderr = (error.stderr as undefined | Buffer)?.toString('utf8'); + const msg = stderr?.trim() ?? `with exit code ${error.code}`; + throw new Error(`git ${args[0]} failed, ${msg}`); + } + throw new ForwardedError('Unknown execution error', error); + } +} + +/** + * Returns a sorted list of all files that have changed since the merge base + * of the provided `ref` and HEAD, as well as all files that are not tracked by git. + */ +export async function listChangedFiles(ref: string) { + if (!ref) { + throw new Error('ref is required'); + } + const [base] = await runGit('merge-base', 'HEAD', ref); + + const tracked = await runGit('diff', '--name-only', base); + const untracked = await runGit('ls-files', '--others', '--exclude-standard'); + + return Array.from(new Set([...tracked, ...untracked])); +} From aa892fccc5ff61158ffa0e3898c008017afc50a4 Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Sun, 13 Feb 2022 21:27:01 +0100 Subject: [PATCH 05/12] cli: added package graph utility to list changed packages Signed-off-by: Patrik Oldsberg --- .../cli/src/lib/monorepo/PackageGraph.test.ts | 36 ++++++++++++++- packages/cli/src/lib/monorepo/PackageGraph.ts | 45 +++++++++++++++++++ 2 files changed, 79 insertions(+), 2 deletions(-) diff --git a/packages/cli/src/lib/monorepo/PackageGraph.test.ts b/packages/cli/src/lib/monorepo/PackageGraph.test.ts index e8e531bc93..06427a7313 100644 --- a/packages/cli/src/lib/monorepo/PackageGraph.test.ts +++ b/packages/cli/src/lib/monorepo/PackageGraph.test.ts @@ -15,8 +15,20 @@ */ import { getPackages } from '@manypkg/get-packages'; -import { paths } from '../paths'; import { PackageGraph } from './PackageGraph'; +import { listChangedFiles } from '../git'; + +jest.mock('../git'); + +const mockListChangedFiles = listChangedFiles as jest.MockedFunction< + typeof listChangedFiles +>; + +jest.mock('../paths', () => ({ + paths: { + targetRoot: '/', + }, +})); const testPackages = [ { @@ -53,7 +65,7 @@ const testPackages = [ describe('PackageGraph', () => { it('is able to construct a graph from this repo', async () => { - const { packages } = await getPackages(paths.ownDir); + const { packages } = await getPackages(__dirname); const graph = PackageGraph.fromPackages(packages); expect(graph.has('@backstage/cli')).toBe(true); }); @@ -124,4 +136,24 @@ describe('PackageGraph', () => { `Package 'unknown' not found`, ); }); + + it('lists changed packages', async () => { + const graph = PackageGraph.fromPackages(testPackages); + + mockListChangedFiles.mockResolvedValueOnce( + [ + 'README.md', + 'packages/a/src/foo.ts', + 'packages/a/src/bar.ts', + 'packages/b/package.json', + 'packages/f/src/foo.ts', + 'packages/f/README.md', + 'plugins/foo/src/index.ts', + ].sort(), + ); + + await expect( + graph.listChangedPackages({ ref: 'origin/master' }), + ).resolves.toEqual([graph.get('a'), graph.get('b')]); + }); }); diff --git a/packages/cli/src/lib/monorepo/PackageGraph.ts b/packages/cli/src/lib/monorepo/PackageGraph.ts index 89d5f79cd2..a1f77299df 100644 --- a/packages/cli/src/lib/monorepo/PackageGraph.ts +++ b/packages/cli/src/lib/monorepo/PackageGraph.ts @@ -14,9 +14,11 @@ * limitations under the License. */ +import { relative as relativePath, posix as posixPath } from 'path'; import { getPackages, Package } from '@manypkg/get-packages'; import { paths } from '../paths'; import { PackageRole } from '../role'; +import { listChangedFiles } from '../git'; type PackageJSON = Package['packageJson']; @@ -158,4 +160,47 @@ export class PackageGraph extends Map { return targets; } + + async listChangedPackages(options: { ref: string }) { + const changedFiles = await listChangedFiles(options.ref); + + const dirMap = new Map( + Array.from(this.values()).map(pkg => [ + posixPath.normalize(relativePath(paths.targetRoot, pkg.dir)) + + posixPath.sep, + pkg, + ]), + ); + const packageDirs = Array.from(dirMap.keys()); + + const result = new Array(); + let searchIndex = 0; + + changedFiles.sort(); + packageDirs.sort(); + + for (const packageDir of packageDirs) { + // Skip through changes that appear before our package dir + while ( + searchIndex < changedFiles.length && + changedFiles[searchIndex] < packageDir + ) { + searchIndex += 1; + } + + // Check if we arrived at a match, otherwise we move on to the next package dir + if (changedFiles[searchIndex]?.startsWith(packageDir)) { + searchIndex += 1; + + result.push(dirMap.get(packageDir)!); + + // Skip through the rest of the changed files for the same package + while (changedFiles[searchIndex]?.startsWith(packageDir)) { + searchIndex += 1; + } + } + } + + return result; + } } From 103278ecb9b08e65658a83faa3e82998b7750ba3 Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Sun, 13 Feb 2022 21:27:28 +0100 Subject: [PATCH 06/12] cli: add --since flag for repo lint command Signed-off-by: Patrik Oldsberg --- packages/cli/src/commands/index.ts | 4 ++++ packages/cli/src/commands/repo/lint.ts | 7 ++++++- 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/packages/cli/src/commands/index.ts b/packages/cli/src/commands/index.ts index b3c9bae6a4..6f534d3e62 100644 --- a/packages/cli/src/commands/index.ts +++ b/packages/cli/src/commands/index.ts @@ -51,6 +51,10 @@ export function registerRepoCommand(program: CommanderStatic) { 'Lint report output format', 'eslint-formatter-friendly', ) + .option( + '--since ', + 'Only lint packages that changed since the specified ref', + ) .option('--fix', 'Attempt to automatically fix violations') .action(lazy(() => import('./repo/lint').then(m => m.command))); } diff --git a/packages/cli/src/commands/repo/lint.ts b/packages/cli/src/commands/repo/lint.ts index 4c1b912d92..08d111712d 100644 --- a/packages/cli/src/commands/repo/lint.ts +++ b/packages/cli/src/commands/repo/lint.ts @@ -22,7 +22,12 @@ import { runWorkerQueueThreads } from '../../lib/parallel'; import { paths } from '../../lib/paths'; export async function command(cmd: Command): Promise { - const packages = await PackageGraph.listTargetPackages(); + let packages = await PackageGraph.listTargetPackages(); + + if (cmd.since) { + const graph = PackageGraph.fromPackages(packages); + packages = await graph.listChangedPackages({ ref: cmd.since }); + } // This formatter uses the cwd to format file paths, so let's have that happen from the root instead if (cmd.format === 'eslint-formatter-friendly') { From 20b135e24e9de9ed47f2f885a13e308ce1b996b1 Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Sun, 13 Feb 2022 21:32:22 +0100 Subject: [PATCH 07/12] cli: tweak worker queue to not spawn too many threads and use env Signed-off-by: Patrik Oldsberg --- packages/cli/src/lib/parallel.ts | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/packages/cli/src/lib/parallel.ts b/packages/cli/src/lib/parallel.ts index ec3d5493c0..d51e9b3b2d 100644 --- a/packages/cli/src/lib/parallel.ts +++ b/packages/cli/src/lib/parallel.ts @@ -153,13 +153,14 @@ export type WorkerQueueThreadsOptions = { export async function runWorkerQueueThreads( options: WorkerQueueThreadsOptions, ): Promise { + const items = Array.from(options.items); const { workerFactory, workerData, - threadCount = os.cpus().length / 2, + threadCount = Math.min(getEnvironmentParallelism(), items.length), } = options; - const iterator = options.items[Symbol.iterator](); + const iterator = items[Symbol.iterator](); const results = new Array(); let itemIndex = 0; From 92f90d552a49e48d737bb5b2635d0e0e05007d77 Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Sun, 13 Feb 2022 21:35:53 +0100 Subject: [PATCH 08/12] cli: set default parallelism to cpus/2 Signed-off-by: Patrik Oldsberg --- packages/cli/src/lib/parallel.test.ts | 16 ++++++++++------ packages/cli/src/lib/parallel.ts | 8 ++++---- 2 files changed, 14 insertions(+), 10 deletions(-) diff --git a/packages/cli/src/lib/parallel.test.ts b/packages/cli/src/lib/parallel.test.ts index 4fbcae2191..032d66afac 100644 --- a/packages/cli/src/lib/parallel.test.ts +++ b/packages/cli/src/lib/parallel.test.ts @@ -14,6 +14,7 @@ * limitations under the License. */ +import os from 'os'; import { parseParallelismOption, getEnvironmentParallelism, @@ -22,6 +23,8 @@ import { runWorkerThreads, } from './parallel'; +const defaultParallelism = Math.ceil(os.cpus().length / 2); + describe('parseParallelismOption', () => { it('coerces false no parallelism', () => { expect(parseParallelismOption(false)).toBe(1); @@ -29,10 +32,10 @@ describe('parseParallelismOption', () => { }); it('coerces true or undefined to default parallelism', () => { - expect(parseParallelismOption(true)).toBe(4); - expect(parseParallelismOption('true')).toBe(4); - expect(parseParallelismOption(undefined)).toBe(4); - expect(parseParallelismOption(null)).toBe(4); + expect(parseParallelismOption(true)).toBe(defaultParallelism); + expect(parseParallelismOption('true')).toBe(defaultParallelism); + expect(parseParallelismOption(undefined)).toBe(defaultParallelism); + expect(parseParallelismOption(null)).toBe(defaultParallelism); }); it('coerces number string to number', () => { @@ -52,13 +55,13 @@ describe('getEnvironmentParallelism', () => { expect(getEnvironmentParallelism()).toBe(2); process.env.BACKSTAGE_CLI_BUILD_PARALLEL = 'true'; - expect(getEnvironmentParallelism()).toBe(4); + expect(getEnvironmentParallelism()).toBe(defaultParallelism); process.env.BACKSTAGE_CLI_BUILD_PARALLEL = 'false'; expect(getEnvironmentParallelism()).toBe(1); delete process.env.BACKSTAGE_CLI_BUILD_PARALLEL; - expect(getEnvironmentParallelism()).toBe(4); + expect(getEnvironmentParallelism()).toBe(defaultParallelism); }); }); @@ -70,6 +73,7 @@ describe('runParallelWorkers', () => { const work = runParallelWorkers({ items: [0, 1, 2, 3, 4], + parallelismSetting: 4, parallelismFactor: 0.5, // 2 at a time worker: async item => { started.push(item); diff --git a/packages/cli/src/lib/parallel.ts b/packages/cli/src/lib/parallel.ts index d51e9b3b2d..7b4ae4609e 100644 --- a/packages/cli/src/lib/parallel.ts +++ b/packages/cli/src/lib/parallel.ts @@ -18,17 +18,17 @@ import os from 'os'; import { ErrorLike } from '@backstage/errors'; import { Worker } from 'worker_threads'; -export const DEFAULT_PARALLELISM = 4; +const defaultParallelism = Math.ceil(os.cpus().length / 2); -export const PARALLEL_ENV_VAR = 'BACKSTAGE_CLI_BUILD_PARALLEL'; +const PARALLEL_ENV_VAR = 'BACKSTAGE_CLI_BUILD_PARALLEL'; export type ParallelismOption = boolean | string | number | null | undefined; export function parseParallelismOption(parallel: ParallelismOption): number { if (parallel === undefined || parallel === null) { - return DEFAULT_PARALLELISM; + return defaultParallelism; } else if (typeof parallel === 'boolean') { - return parallel ? DEFAULT_PARALLELISM : 1; + return parallel ? defaultParallelism : 1; } else if (typeof parallel === 'number' && Number.isInteger(parallel)) { if (parallel < 1) { return 1; From 99d975cd3eafeae6336ee8a008b5d226bd5f54c1 Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Sun, 13 Feb 2022 22:05:18 +0100 Subject: [PATCH 09/12] cli: tiny bit of logic to optimize repo lint order Signed-off-by: Patrik Oldsberg --- packages/cli/src/commands/repo/lint.ts | 14 +++++++++++++- packages/cli/src/lib/monorepo/index.ts | 6 +++++- 2 files changed, 18 insertions(+), 2 deletions(-) diff --git a/packages/cli/src/commands/repo/lint.ts b/packages/cli/src/commands/repo/lint.ts index 08d111712d..1cd0f1ce63 100644 --- a/packages/cli/src/commands/repo/lint.ts +++ b/packages/cli/src/commands/repo/lint.ts @@ -17,10 +17,18 @@ import chalk from 'chalk'; import { Command } from 'commander'; import { relative as relativePath } from 'path'; -import { PackageGraph } from '../../lib/monorepo'; +import { PackageGraph, ExtendedPackageJSON } from '../../lib/monorepo'; import { runWorkerQueueThreads } from '../../lib/parallel'; import { paths } from '../../lib/paths'; +function depCount(pkg: ExtendedPackageJSON) { + const deps = pkg.dependencies ? Object.keys(pkg.dependencies).length : 0; + const devDeps = pkg.devDependencies + ? Object.keys(pkg.devDependencies).length + : 0; + return deps + devDeps; +} + export async function command(cmd: Command): Promise { let packages = await PackageGraph.listTargetPackages(); @@ -29,6 +37,10 @@ export async function command(cmd: Command): Promise { packages = await graph.listChangedPackages({ ref: cmd.since }); } + // Packages are ordered from most to least number of dependencies, as a + // very cheap way of guessing which packages are more likely to be larger. + packages.sort((a, b) => depCount(b.packageJson) - depCount(a.packageJson)); + // This formatter uses the cwd to format file paths, so let's have that happen from the root instead if (cmd.format === 'eslint-formatter-friendly') { process.chdir(paths.targetRoot); diff --git a/packages/cli/src/lib/monorepo/index.ts b/packages/cli/src/lib/monorepo/index.ts index aa12c63abc..7156975c07 100644 --- a/packages/cli/src/lib/monorepo/index.ts +++ b/packages/cli/src/lib/monorepo/index.ts @@ -15,4 +15,8 @@ */ export { PackageGraph } from './PackageGraph'; -export type { PackageGraphNode } from './PackageGraph'; +export type { + PackageGraphNode, + ExtendedPackage, + ExtendedPackageJSON, +} from './PackageGraph'; From 0b74c72987dcfc09322006d825d9aa48f43edd56 Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Sun, 13 Feb 2022 22:07:19 +0100 Subject: [PATCH 10/12] changesets: added changeset for repo lint addition Signed-off-by: Patrik Oldsberg --- .changeset/angry-bottles-mix.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/angry-bottles-mix.md diff --git a/.changeset/angry-bottles-mix.md b/.changeset/angry-bottles-mix.md new file mode 100644 index 0000000000..cd05ef6514 --- /dev/null +++ b/.changeset/angry-bottles-mix.md @@ -0,0 +1,5 @@ +--- +'@backstage/cli': patch +--- + +Added a new experimental and hidden `backstage-cli repo lint` command that can be used to lint all packages in the project, similar to `lerna run lint`. From 00208b83f4cd6321902365560f0dbe4bcfcc8f1f Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Sun, 13 Feb 2022 22:13:44 +0100 Subject: [PATCH 11/12] cli: add duration in repo lint output Signed-off-by: Patrik Oldsberg --- packages/cli/src/commands/repo/lint.ts | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/packages/cli/src/commands/repo/lint.ts b/packages/cli/src/commands/repo/lint.ts index 1cd0f1ce63..b054dabe34 100644 --- a/packages/cli/src/commands/repo/lint.ts +++ b/packages/cli/src/commands/repo/lint.ts @@ -71,6 +71,7 @@ export async function command(cmd: Command): Promise { // since some lint rules don't respect the cwd of ESLint process.cwd = () => fullDir; + const start = Date.now(); const eslint = new ESLint({ cwd: fullDir, fix, @@ -81,7 +82,8 @@ export async function command(cmd: Command): Promise { const results = await eslint.lintFiles(['.']); const count = String(results.length).padStart(3); - console.log(`Checked ${count} files in ${relativeDir}`); + const time = ((Date.now() - start) / 1000).toFixed(2); + console.log(`Checked ${count} files in ${relativeDir} ${time}s`); if (fix) { await ESLint.outputFixes(results); From 2badbd0054581d5c776392da1f06f81041cb891f Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Mon, 14 Feb 2022 12:09:41 +0100 Subject: [PATCH 12/12] cli: avoid requiring any particular ref to be present in git test Signed-off-by: Patrik Oldsberg --- packages/cli/src/lib/git.test.ts | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/packages/cli/src/lib/git.test.ts b/packages/cli/src/lib/git.test.ts index ecb234f79a..2e5d464796 100644 --- a/packages/cli/src/lib/git.test.ts +++ b/packages/cli/src/lib/git.test.ts @@ -47,8 +47,6 @@ describe('listChangedFiles', () => { }); it('should return something', async () => { - await expect(listChangedFiles('origin/master')).resolves.toEqual( - expect.any(Array), - ); + await expect(listChangedFiles('HEAD')).resolves.toEqual(expect.any(Array)); }); });