Merge pull request #26986 from backstage/rugvip/lint-cache

cli: add --successCache option for repo lint

.changeset/calm-owls-move.md

@@ -0,0 +1,5 @@
+---
+'@backstage/cli-node': patch
+---
+
+Added new `lockfile.getDependencyTreeHash(name)` utility.

.changeset/small-donkeys-attack.md

@@ -0,0 +1,7 @@
+---
+'@backstage/cli': patch
+---
+
+Added a new `--successCache` option to the `backstage-cli repo lint` command. The cache keeps track of successful lint runs and avoids re-running linting of individual packages if they haven't changed. This option is primarily intended to be used in CI.
+
+In addition a `--successCacheDir` option has also been added to be able to override the default cache directory.

.github/workflows/ci.yml

@@ -234,7 +234,7 @@ jobs:
         run: node scripts/verify-release.js
 
       - name: lint changed packages
-        run: yarn backstage-cli repo lint --since origin/master
+        run: yarn backstage-cli repo lint --since origin/master --successCache
 
       - name: test changed packages
         run: yarn backstage-cli repo test --maxWorkers=3 --workerIdleMemoryLimit=1300M --since origin/master

.github/workflows/deploy_packages.yml

@@ -94,7 +94,7 @@ jobs:
         run: yarn backstage-cli config:check --lax
 
       - name: lint
-        run: yarn backstage-cli repo lint
+        run: yarn backstage-cli repo lint --successCache
 
       - name: type checking and declarations
         run: yarn tsc:full

.github/workflows/verify_windows.yml

@@ -46,7 +46,7 @@ jobs:
         run: yarn install --immutable
 
       - name: lint
-        run: yarn backstage-cli repo lint
+        run: yarn backstage-cli repo lint --successCache
 
       - name: type checking and declarations
         run: yarn tsc:full

docs/tooling/cli/02-build-system.md

@@ -559,6 +559,14 @@ The overrides in a single `package.json` may for example look like this:
   },
 ```
 
+## Caching
+
+Caching is used sparingly throughout the Backstage build system. It is always used as a way to squeeze out a little bit of extra performance, rather than requirement to keep things fast. The following is a list of places where optional caching is available:
+
+- **TypeScript** - The default `tsconfig.json` used by Backstage projects has `incremental` set to `true`, which enables local caching of type checking results. It is however generally not recommended in CI, where `yarn tsc:full` is preferred, which sets `--incremental false`.
+- **Linting** - The `backstage-cli repo lint` command has a `--successCache` flag that enables caching of successful linting results. This is done at the package level, meaning that if a package has not been changed since the last lint run and it was successful, the linting will be skipped. This is recommended to be used in CI, but not during local development.
+- **Webpack** - It is possible to enable experimental caching of frontend package builds using the `BACKSTAGE_CLI_EXPERIMENTAL_BUILD_CACHE` environment variable. This will enable the Webpack filesystem cache.
+
 ### Debugging Jest Tests
 
 For your productivity working with unit tests it's quite essential to have your debugging configured in IDE. It will help you to identify the root cause of the issue faster.

packages/cli-node/report.api.md

@@ -92,6 +92,7 @@ export function isMonoRepo(): Promise<boolean>;
 export class Lockfile {
   createSimplifiedDependencyGraph(): Map<string, Set<string>>;
   diff(otherLockfile: Lockfile): LockfileDiff;
+  getDependencyTreeHash(startName: string): string;
   static load(path: string): Promise<Lockfile>;
   static parse(content: string): Lockfile;
 }

packages/cli-node/src/monorepo/Lockfile.test.ts

@@ -493,4 +493,95 @@ d@^1:
       );
     });
   });
+
+  describe('getDependencyTreeHash', () => {
+    const content = `${MODERN_HEADER}
+"a@npm:^1":
+  version: "1.0.0"
+  checksum: sha512-a-1
+  dependencies:
+    b: "^2"
+
+"b@npm:2.0.x, b@npm:^2":
+  version: "2.0.0"
+  checksum: sha512-b-1
+
+"b@npm:4":
+  version: "3.0.0"
+  checksum: sha512-b-2
+
+"c@npm:^1":
+  version: "4.0.0"
+  checksum: sha512-c-1
+`;
+    const lockfile = Lockfile.parse(content);
+
+    const hashA = lockfile.getDependencyTreeHash('a');
+    const hashB = lockfile.getDependencyTreeHash('b');
+    const hashC = lockfile.getDependencyTreeHash('c');
+
+    it('should generate stable dependency hashes', () => {
+      expect(hashA).toMatchInlineSnapshot(
+        `"2d1d4c1c577c291e815e87779c72fc78a78e56cc"`,
+      );
+      expect(hashB).toMatchInlineSnapshot(
+        `"7e46d0c7337540179b442c87a7c5555543798f15"`,
+      );
+      expect(hashC).toMatchInlineSnapshot(
+        `"e65103abd217954bad40e2f834b990a5e6fa4054"`,
+      );
+    });
+
+    it('should generate different hashes for different versions', () => {
+      const lockfileNewA = Lockfile.parse(content.replace('1.0.0', '1.0.1'));
+      expect(lockfileNewA.getDependencyTreeHash('a')).not.toBe(hashA);
+      expect(lockfileNewA.getDependencyTreeHash('b')).toBe(hashB);
+      expect(lockfileNewA.getDependencyTreeHash('c')).toBe(hashC);
+
+      const lockfileNewB1 = Lockfile.parse(content.replace('2.0.0', '2.0.1'));
+      expect(lockfileNewB1.getDependencyTreeHash('a')).not.toBe(hashA);
+      expect(lockfileNewB1.getDependencyTreeHash('b')).not.toBe(hashB);
+      expect(lockfileNewB1.getDependencyTreeHash('c')).toBe(hashC);
+
+      const lockfileNewB2 = Lockfile.parse(content.replace('3.0.0', '3.0.1'));
+      expect(lockfileNewB2.getDependencyTreeHash('a')).not.toBe(hashA);
+      expect(lockfileNewB2.getDependencyTreeHash('b')).not.toBe(hashB);
+      expect(lockfileNewB2.getDependencyTreeHash('c')).toBe(hashC);
+
+      const lockfileNewC = Lockfile.parse(content.replace('4.0.0', '4.0.1'));
+      expect(lockfileNewC.getDependencyTreeHash('a')).toBe(hashA);
+      expect(lockfileNewC.getDependencyTreeHash('b')).toBe(hashB);
+      expect(lockfileNewC.getDependencyTreeHash('c')).not.toBe(hashC);
+    });
+
+    it('should generate different hashes for different checksums', () => {
+      const lockfileNewA = Lockfile.parse(
+        content.replace('sha512-a-1', 'sha512-a-1-new'),
+      );
+      expect(lockfileNewA.getDependencyTreeHash('a')).not.toBe(hashA);
+      expect(lockfileNewA.getDependencyTreeHash('b')).toBe(hashB);
+      expect(lockfileNewA.getDependencyTreeHash('c')).toBe(hashC);
+
+      const lockfileNewB1 = Lockfile.parse(
+        content.replace('sha512-b-1', 'sha512-b-1-new'),
+      );
+      expect(lockfileNewB1.getDependencyTreeHash('a')).not.toBe(hashA);
+      expect(lockfileNewB1.getDependencyTreeHash('b')).not.toBe(hashB);
+      expect(lockfileNewB1.getDependencyTreeHash('c')).toBe(hashC);
+
+      const lockfileNewB2 = Lockfile.parse(
+        content.replace('sha512-b-2', 'sha512-b-2-new'),
+      );
+      expect(lockfileNewB2.getDependencyTreeHash('a')).not.toBe(hashA);
+      expect(lockfileNewB2.getDependencyTreeHash('b')).not.toBe(hashB);
+      expect(lockfileNewB2.getDependencyTreeHash('c')).toBe(hashC);
+
+      const lockfileNewC = Lockfile.parse(
+        content.replace('sha512-c-1', 'sha512-c-1-new'),
+      );
+      expect(lockfileNewC.getDependencyTreeHash('a')).toBe(hashA);
+      expect(lockfileNewC.getDependencyTreeHash('b')).toBe(hashB);
+      expect(lockfileNewC.getDependencyTreeHash('c')).not.toBe(hashC);
+    });
+  });
 });

packages/cli-node/src/monorepo/Lockfile.ts

@@ -15,6 +15,7 @@
  */
 
 import { parseSyml } from '@yarnpkg/parsers';
+import crypto from 'node:crypto';
 import fs from 'fs-extra';
 
 const ENTRY_PATTERN = /^((?:@[^/]+\/)?[^@/]+)@(.+)$/;
@@ -215,4 +216,63 @@ export class Lockfile {
 
     return diff;
   }
+
+  /**
+   * Generates a sha1 hex hash of the dependency graph for a package.
+   */
+  getDependencyTreeHash(startName: string): string {
+    if (!this.packages.has(startName)) {
+      throw new Error(`Package '${startName}' not found in lockfile`);
+    }
+
+    const hash = crypto.createHash('sha1');
+
+    const queue = [startName];
+    const seen = new Set<string>();
+
+    while (queue.length > 0) {
+      const name = queue.pop()!;
+
+      if (seen.has(name)) {
+        continue;
+      }
+      seen.add(name);
+
+      const entries = this.packages.get(name);
+      if (!entries) {
+        continue; // In case of missing optional peer dependencies
+      }
+
+      hash.update(`pkg:${name}`);
+      hash.update('\0');
+
+      // TODO(Rugvip): This uses the same simplified lookup as createSimplifiedDependencyGraph()
+      //               we could match version queries to make the resulting tree a bit smaller.
+      const deps = new Array<string>();
+      for (const entry of entries) {
+        // We're not being particular about stable ordering here. If the lockfile ordering changes, so will likely hash.
+        hash.update(entry.version);
+
+        const data = this.data[entry.dataKey];
+        if (!data) {
+          continue;
+        }
+
+        const checksum = data.checksum || data.integrity;
+        if (checksum) {
+          hash.update('#');
+          hash.update(checksum);
+        }
+
+        hash.update(' ');
+
+        deps.push(...Object.keys(data.dependencies ?? {}));
+        deps.push(...Object.keys(data.peerDependencies ?? {}));
+      }
+
+      queue.push(...new Set(deps));
+    }
+
+    return hash.digest('hex');
+  }
 }

packages/cli/cli-report.md

@@ -446,6 +446,8 @@ Usage: backstage-cli repo lint [options]
 Options:
   --format <format>
   --since <ref>
+  --successCache
+  --successCacheDir <path>
   --fix
   -h, --help
 ```

packages/cli/src/commands/index.ts

@@ -61,6 +61,14 @@ export function registerRepoCommand(program: Command) {
       '--since <ref>',
       'Only lint packages that changed since the specified ref',
     )
+    .option(
+      '--successCache',
+      'Enable success caching, which skips running tests for unchanged packages that were successful in the previous run',
+    )
+    .option(
+      '--successCacheDir <path>',
+      'Set the success cache location, (default: node_modules/.cache/backstage-cli)',
+    )
     .option('--fix', 'Attempt to automatically fix violations')
     .action(lazy(() => import('./repo/lint').then(m => m.command)));
 

packages/cli/src/commands/repo/lint.ts

@@ -16,8 +16,14 @@
 
 import chalk from 'chalk';
 import { Command, OptionValues } from 'commander';
-import { relative as relativePath } from 'path';
-import { PackageGraph, BackstagePackageJson } from '@backstage/cli-node';
+import fs from 'fs-extra';
+import { createHash } from 'crypto';
+import { relative as relativePath, resolve as resolvePath } from 'path';
+import {
+  PackageGraph,
+  BackstagePackageJson,
+  Lockfile,
+} from '@backstage/cli-node';
 import { paths } from '../../lib/paths';
 import { runWorkerQueueThreads } from '../../lib/parallel';
 import { createScriptOptionsParser } from './optionsParser';
@@ -30,9 +36,43 @@ function depCount(pkg: BackstagePackageJson) {
   return deps + devDeps;
 }
 
+const CACHE_FILE_NAME = 'lint-cache.json';
+
+type Cache = string[];
+
+async function readCache(dir: string): Promise<Cache | undefined> {
+  try {
+    const data = await fs.readJson(resolvePath(dir, CACHE_FILE_NAME));
+    if (!Array.isArray(data)) {
+      return undefined;
+    }
+    if (data.some(x => typeof x !== 'string')) {
+      return undefined;
+    }
+    return data as Cache;
+  } catch {
+    return undefined;
+  }
+}
+
+async function writeCache(dir: string, cache: Cache) {
+  await fs.mkdirp(dir);
+  await fs.writeJson(resolvePath(dir, CACHE_FILE_NAME), cache, { spaces: 2 });
+}
+
 export async function command(opts: OptionValues, cmd: Command): Promise<void> {
   let packages = await PackageGraph.listTargetPackages();
 
+  const cacheDir = resolvePath(
+    opts.successCacheDir ?? 'node_modules/.cache/backstage-cli',
+  );
+  const cacheContext = opts.successCache
+    ? {
+        cache: await readCache(cacheDir),
+        lockfile: await Lockfile.load(paths.resolveTargetRoot('yarn.lock')),
+      }
+    : undefined;
+
   if (opts.since) {
     const graph = PackageGraph.fromPackages(packages);
     packages = await graph.listChangedPackages({
@@ -57,26 +97,66 @@ export async function command(opts: OptionValues, cmd: Command): Promise<void> {
 
   const parseLintScript = createScriptOptionsParser(cmd, ['package', 'lint']);
 
+  const items = await Promise.all(
+    packages.map(async pkg => {
+      const lintOptions = parseLintScript(pkg.packageJson.scripts?.lint);
+      const base = {
+        fullDir: pkg.dir,
+        relativeDir: relativePath(paths.targetRoot, pkg.dir),
+        lintOptions,
+        parentHash: undefined,
+      };
+
+      if (!cacheContext) {
+        return base;
+      }
+
+      const hash = createHash('sha1');
+
+      hash.update(
+        cacheContext.lockfile.getDependencyTreeHash(pkg.packageJson.name),
+      );
+      hash.update('\0');
+      hash.update(JSON.stringify(lintOptions));
+      hash.update('\0');
+      hash.update(process.version); // Node.js version
+      hash.update('\0');
+      hash.update('v1'); // The version of this implementation
+
+      return {
+        ...base,
+        parentHash: hash.digest('hex'),
+      };
+    }),
+  );
+
   const resultsList = await runWorkerQueueThreads({
-    items: packages.map(pkg => ({
-      fullDir: pkg.dir,
-      relativeDir: relativePath(paths.targetRoot, pkg.dir),
-      lintOptions: parseLintScript(pkg.packageJson.scripts?.lint),
-    })),
+    items,
     workerData: {
       fix: Boolean(opts.fix),
       format: opts.format as string | undefined,
+      shouldCache: Boolean(cacheContext),
+      successCache: cacheContext?.cache,
     },
-    workerFactory: async ({ fix, format }) => {
+    workerFactory: async ({ fix, format, shouldCache, successCache }) => {
       const { ESLint } = require('eslint') as typeof import('eslint');
+      const crypto = require('crypto') as typeof import('crypto');
+      const recursiveReadDir =
+        require('recursive-readdir') as typeof import('recursive-readdir');
+      const { readFile } =
+        require('fs/promises') as typeof import('fs/promises');
+      const { relative: workerRelativePath } =
+        require('path') as typeof import('path');
 
       return async ({
         fullDir,
         relativeDir,
         lintOptions,
+        parentHash,
       }): Promise<{
         relativeDir: string;
-        resultText: string;
+        sha?: string;
+        resultText?: string;
         failed: boolean;
       }> => {
         // Bit of a hack to make file resolutions happen from the correct directory
@@ -89,6 +169,35 @@ export async function command(opts: OptionValues, cmd: Command): Promise<void> {
           fix,
           extensions: ['js', 'jsx', 'ts', 'tsx', 'mjs', 'cjs'],
         });
+
+        let sha: string | undefined = undefined;
+        if (shouldCache) {
+          const result = await recursiveReadDir(fullDir);
+
+          const hash = crypto.createHash('sha1');
+          hash.update(parentHash!);
+          hash.update('\0');
+
+          for (const path of result.sort()) {
+            if (await eslint.isPathIgnored(path)) {
+              continue;
+            }
+            hash.update(workerRelativePath(fullDir, path));
+            hash.update('\0');
+            hash.update(await readFile(path));
+            hash.update('\0');
+            hash.update(
+              JSON.stringify(await eslint.calculateConfigForFile(path)),
+            );
+            hash.update('\0');
+          }
+          sha = await hash.digest('hex');
+          if (successCache?.includes(sha)) {
+            console.log(`Skipped ${relativeDir} due to cache hit`);
+            return { relativeDir, sha, failed: false };
+          }
+        }
+
         const formatter = await eslint.loadFormatter(format);
 
         const results = await eslint.lintFiles(['.']);
@@ -112,13 +221,21 @@ export async function command(opts: OptionValues, cmd: Command): Promise<void> {
           relativeDir,
           resultText,
           failed,
+          sha,
         };
       };
     },
   });
 
+  const outputSuccessCache = [];
+
   let failed = false;
-  for (const { relativeDir, resultText, failed: runFailed } of resultsList) {
+  for (const {
+    relativeDir,
+    resultText,
+    failed: runFailed,
+    sha,
+  } of resultsList) {
     if (runFailed) {
       console.log(chalk.red(`Lint failed in ${relativeDir}`));
       failed = true;
@@ -129,9 +246,15 @@ export async function command(opts: OptionValues, cmd: Command): Promise<void> {
         console.log();
         console.log(resultText.trimStart());
       }
+    } else if (sha) {
+      outputSuccessCache.push(sha);
     }
   }
 
+  if (cacheContext) {
+    await writeCache(cacheDir, outputSuccessCache);
+  }
+
   if (failed) {
     process.exit(1);
   }

packages/cli/src/lib/new/factories/backendModule.test.ts

@@ -90,7 +90,7 @@ describe('backendModule factory', () => {
       `availability  plugins${sep}test-backend-module-tester-two`,
       'creating      temp dir',
       'Executing Template:',
-      'copying       .eslintrc.js',
+      'templating    .eslintrc.js.hbs',
       'templating    README.md.hbs',
       'templating    package.json.hbs',
       'templating    index.ts.hbs',

packages/cli/src/lib/new/factories/backendPlugin.test.ts

@@ -89,7 +89,7 @@ describe('backendPlugin factory', () => {
       `availability  plugins${sep}test-backend`,
       'creating      temp dir',
       'Executing Template:',
-      'copying       .eslintrc.js',
+      'templating    .eslintrc.js.hbs',
       'templating    README.md.hbs',
       'templating    index.ts.hbs',
       'templating    package.json.hbs',

packages/cli/src/lib/new/factories/frontendPlugin.test.ts

@@ -91,7 +91,7 @@ describe('frontendPlugin factory', () => {
       `availability  plugins${sep}test`,
       'creating      temp dir',
       'Executing Template:',
-      'copying       .eslintrc.js',
+      'templating    .eslintrc.js.hbs',
       'templating    README.md.hbs',
       'templating    package.json.hbs',
       'templating    index.tsx.hbs',

packages/cli/src/lib/new/factories/nodeLibraryPackage.test.ts

@@ -75,7 +75,7 @@ describe('nodeLibraryPackage factory', () => {
       `availability  ${joinPath('packages', expectedNodeLibraryPackageName)}`,
       'creating      temp dir',
       'Executing Template:',
-      'copying       .eslintrc.js',
+      'templating    .eslintrc.js.hbs',
       'templating    README.md.hbs',
       'templating    package.json.hbs',
       'templating    index.ts.hbs',

packages/cli/src/lib/new/factories/pluginCommon.test.ts

@@ -73,7 +73,7 @@ describe('pluginCommon factory', () => {
       `availability  plugins${sep}test-common`,
       'creating      temp dir',
       'Executing Template:',
-      'copying       .eslintrc.js',
+      'templating    .eslintrc.js.hbs',
       'templating    README.md.hbs',
       'templating    package.json.hbs',
       'templating    index.ts.hbs',

packages/cli/src/lib/new/factories/pluginNode.test.ts

@@ -73,7 +73,7 @@ describe('pluginNode factory', () => {
       `availability  plugins${sep}test-node`,
       'creating      temp dir',
       'Executing Template:',
-      'copying       .eslintrc.js',
+      'templating    .eslintrc.js.hbs',
       'templating    README.md.hbs',
       'templating    package.json.hbs',
       'templating    index.ts.hbs',

packages/cli/src/lib/new/factories/pluginWeb.test.ts

@@ -73,7 +73,7 @@ describe('pluginWeb factory', () => {
       `availability  plugins${sep}test-react`,
       'creating      temp dir',
       'Executing Template:',
-      'copying       .eslintrc.js',
+      'templating    .eslintrc.js.hbs',
       'templating    README.md.hbs',
       'templating    package.json.hbs',
       'templating    index.ts.hbs',

packages/cli/src/lib/new/factories/scaffolderModule.test.ts

@@ -89,7 +89,7 @@ describe('scaffolderModule factory', () => {
       `availability  plugins${sep}scaffolder-backend-module-test`,
       'creating      temp dir',
       'Executing Template:',
-      'copying       .eslintrc.js',
+      'templating    .eslintrc.js.hbs',
       'templating    README.md.hbs',
       'templating    package.json.hbs',
       'templating    index.ts.hbs',

packages/cli/src/lib/new/factories/webLibraryPackage.test.ts

@@ -75,7 +75,7 @@ describe('webLibraryPackage factory', () => {
       `availability  ${joinPath('packages', expectedwebLibraryPackageName)}`,
       'creating      temp dir',
       'Executing Template:',
-      'copying       .eslintrc.js',
+      'templating    .eslintrc.js.hbs',
       'templating    README.md.hbs',
       'templating    package.json.hbs',
       'templating    index.ts.hbs',