From efb3fd98c665d4b45ab35963af1a8e7932a9d7c6 Mon Sep 17 00:00:00 2001 From: David Sanders Date: Wed, 18 Feb 2026 20:45:44 -0800 Subject: [PATCH] build: lint commits on Chromium roller branches (#49655) * build: lint commits on Chromium roller branches Assisted-By: Claude Opus 4.5 * chore: add ability to skip linting CLs by adding #nolint * chore: only exit with non-zero exit code in CI --- package.json | 6 +- script/lib/utils.js | 21 ++ script/lint-roller-chromium-changes.mjs | 287 ++++++++++++++++++++++++ script/release/notes/notes.ts | 22 +- 4 files changed, 314 insertions(+), 22 deletions(-) create mode 100644 script/lint-roller-chromium-changes.mjs diff --git a/package.json b/package.json index aef67c7abc..a463688ca9 100644 --- a/package.json +++ b/package.json @@ -65,7 +65,7 @@ "scripts": { "asar": "asar", "generate-version-json": "node script/generate-version-json.js", - "lint": "node ./script/lint.js && npm run lint:docs", + "lint": "node ./script/lint.js && npm run lint:docs && npm run lint:chromium-roller", "lint:js": "node ./script/lint.js --js", "lint:clang-format": "python3 script/run-clang-format.py -r -c shell/ || (echo \"\\nCode not formatted correctly.\" && exit 1)", "lint:clang-tidy": "ts-node ./script/run-clang-tidy.ts", @@ -80,6 +80,7 @@ "lint:ts-check-js-in-markdown": "lint-roller-markdown-ts-check --root docs \"**/*.md\" --ignore \"breaking-changes.md\"", "lint:js-in-markdown": "lint-roller-markdown-standard --root docs \"**/*.md\"", "lint:api-history": "lint-roller-markdown-api-history --root \"./docs/api/\" --schema \"./docs/api-history.schema.json\" --breaking-changes-file \"./docs/breaking-changes.md\" --check-placement --check-strings \"*.md\"", + "lint:chromium-roller": "node ./script/lint-roller-chromium-changes.mjs", "create-api-json": "node script/create-api-json.mjs", "create-typescript-definitions": "npm run create-api-json && electron-typescript-definitions --api=electron-api.json && node spec/ts-smoke/runner.js", "gn-typescript-definitions": "npm run create-typescript-definitions && node script/cp.mjs electron.d.ts", @@ -103,6 +104,9 @@ "electron" ], "lint-staged": { + "*": [ + "npm run lint:chromium-roller" + ], "*.{js,ts}": [ "node script/lint.js --js --fix --only --" ], diff --git a/script/lib/utils.js b/script/lib/utils.js index 140850fc87..7e469dc9d0 100644 --- a/script/lib/utils.js +++ b/script/lib/utils.js @@ -142,8 +142,29 @@ async function findMatchingFiles (top, test) { }); } +function compareVersions (v1, v2) { + const [split1, split2] = [v1.split('.'), v2.split('.')]; + + if (split1.length !== split2.length) { + throw new Error( + `Expected version strings to have same number of sections: ${split1} and ${split2}` + ); + } + for (let i = 0; i < split1.length; i++) { + const p1 = parseInt(split1[i], 10); + const p2 = parseInt(split2[i], 10); + + if (p1 > p2) return 1; + else if (p1 < p2) return -1; + // Continue checking the value if this portion is equal + } + + return 0; +} + module.exports = { chunkFilenames, + compareVersions, findMatchingFiles, getCurrentBranch, getElectronExec, diff --git a/script/lint-roller-chromium-changes.mjs b/script/lint-roller-chromium-changes.mjs new file mode 100644 index 0000000000..93f1273147 --- /dev/null +++ b/script/lint-roller-chromium-changes.mjs @@ -0,0 +1,287 @@ +#!/usr/bin/env node + +import { execSync } from 'node:child_process'; +import * as fs from 'node:fs/promises'; +import { dirname, resolve } from 'node:path'; +import { fileURLToPath } from 'node:url'; + +import { compareVersions } from './lib/utils.js'; + +const __dirname = dirname(fileURLToPath(import.meta.url)); +const ELECTRON_DIR = resolve(__dirname, '..'); + +const DEPS_REGEX = /chromium_version':\n +'(.+?)',/m; +const CL_REGEX = /https:\/\/chromium-review\.googlesource\.com\/c\/(?:chromium\/src|v8\/v8)\/\+\/(\d+)(#\S+)?/g; +const ROLLER_BRANCH_PATTERN = /^roller\/chromium\/(.+)$/; + +function getCommitsSinceMergeBase (mergeBase) { + try { + const output = execSync(`git log --format=%H%n%B%n---COMMIT_END--- ${mergeBase}..HEAD`, { + cwd: ELECTRON_DIR, + encoding: 'utf8' + }); + + const commits = []; + const parts = output.split('---COMMIT_END---'); + + for (const part of parts) { + const trimmed = part.trim(); + if (!trimmed) continue; + + const lines = trimmed.split('\n'); + const sha = lines[0]; + const message = lines.slice(1).join('\n'); + + if (sha && message) { + commits.push({ sha, message }); + } + } + + return commits; + } catch { + return []; + } +} + +async function fetchChromiumDashCommit (commitSha, repo) { + const url = `https://chromiumdash.appspot.com/fetch_commit?commit=${commitSha}&repo=${repo}`; + const response = await fetch(url); + + if (!response.ok) { + return null; + } + + try { + return await response.json(); + } catch { + return null; + } +} + +async function getGerritPatchDetails (clUrl) { + const parsedUrl = new URL(clUrl); + const match = /^\/c\/(.+?)\/\+\/(\d+)/.exec(parsedUrl.pathname); + if (!match) { + return null; + } + + const [, repo, number] = match; + const changeId = `${repo}~${number}`; + const url = `https://chromium-review.googlesource.com/changes/${encodeURIComponent(changeId)}?o=CURRENT_REVISION`; + const response = await fetch(url); + + if (!response.ok) { + return null; + } + + try { + // Gerrit returns JSON with a magic prefix + const text = await response.text(); + const jsonText = text.startsWith(")]}'") ? text.substring(4) : text; + const data = JSON.parse(jsonText); + + // Get the current revision's commit SHA + const currentRevision = data.current_revision; + return { + commitSha: currentRevision, + project: data.project + }; + } catch { + return null; + } +} + +async function main () { + let currentBranch; + + try { + currentBranch = execSync('git rev-parse --abbrev-ref HEAD', { + cwd: ELECTRON_DIR, + encoding: 'utf8' + }).trim(); + } catch { + console.error('Could not determine current git branch'); + process.exit(1); + } + + // Check if we're on a roller/chromium/* branch + const branchMatch = ROLLER_BRANCH_PATTERN.exec(currentBranch); + if (!branchMatch) { + console.log(`Not on a roller/chromium/* branch (current: ${currentBranch}), skipping lint.`); + process.exit(0); + } + + const targetBranch = branchMatch[1]; + console.log(`Linting roller branch: ${currentBranch} (target: ${targetBranch})`); + + // Get the merge base with the target branch + let mergeBase; + try { + mergeBase = execSync(`git merge-base ${currentBranch} origin/${targetBranch}`, { + cwd: ELECTRON_DIR, + encoding: 'utf8' + }).trim(); + } catch { + console.error(`Could not determine merge base with origin/${targetBranch}`); + process.exit(1); + } + + // Get version range + let baseVersion = null; + try { + const baseDepsContent = execSync(`git show ${mergeBase}:DEPS`, { + cwd: ELECTRON_DIR, + encoding: 'utf8' + }); + baseVersion = DEPS_REGEX.exec(baseDepsContent)?.[1] ?? null; + } catch { + // baseVersion remains null + } + const depsContent = await fs.readFile(resolve(ELECTRON_DIR, 'DEPS'), 'utf8'); + const newVersion = DEPS_REGEX.exec(depsContent)?.[1] ?? null; + + if (!baseVersion || !newVersion) { + console.error('Could not determine Chromium version range'); + console.error(` Base version: ${baseVersion ?? 'unknown'}`); + console.error(` New version: ${newVersion ?? 'unknown'}`); + process.exit(1); + } + + console.log(`Chromium version range: ${baseVersion} -> ${newVersion}`); + + // Get all commits since merge base + const commits = getCommitsSinceMergeBase(mergeBase); + console.log(`Found ${commits.length} commits to check`); + + let hasErrors = false; + const checkedCLs = new Map(); // Cache CL check results + + for (const commit of commits) { + const shortSha = commit.sha.substring(0, 7); + const firstLine = commit.message.split('\n')[0]; + + // Check for Skip-Lint trailer + const skipLintMatch = /^Skip-Lint:\s*(.+)$/m.exec(commit.message); + if (skipLintMatch) { + console.log(`\nSkipping commit ${shortSha}: ${firstLine}`); + console.log(` ⏭️ Reason: ${skipLintMatch[1]}`); + continue; + } + + const cls = [...commit.message.matchAll(CL_REGEX)].map((match) => ({ + url: match[0], + clNumber: match[1], + fragment: match[2] ?? null + })); + + if (cls.length === 0) { + // No CLs in this commit, skip + continue; + } + + console.log(`\nChecking commit ${shortSha}: ${firstLine}`); + + for (const cl of cls) { + // Skip CLs annotated with #nolint + if (cl.fragment === '#nolint') { + console.log(` ⏭️ CL ${cl.clNumber}: skipped (#nolint)`); + continue; + } + + // Check cache first (use URL as key to distinguish same CL number across repos) + if (checkedCLs.has(cl.url)) { + const cached = checkedCLs.get(cl.url); + if (cached.valid) { + console.log(` ✅ CL ${cl.clNumber}: (already validated)`); + } else { + console.error(` ❌ CL ${cl.clNumber}: ${cached.error}`); + hasErrors = true; + } + continue; + } + + // Determine repo from URL + const isV8 = cl.url.startsWith('https://chromium-review.googlesource.com/c/v8/v8/'); + const repo = isV8 ? 'v8' : 'chromium'; + + // Fetch Gerrit details to get commit SHA + const gerritDetails = await getGerritPatchDetails(cl.url); + + if (!gerritDetails) { + const error = 'Could not fetch CL details from Gerrit'; + checkedCLs.set(cl.url, { valid: false, error }); + console.error(` ❌ CL ${cl.clNumber}: ${error}`); + hasErrors = true; + continue; + } + + // Fetch from Chromium Dash to get the earliest version + const dashDetails = await fetchChromiumDashCommit(gerritDetails.commitSha, repo); + + if (!dashDetails) { + const error = 'Could not fetch commit details from Chromium Dash'; + checkedCLs.set(cl.url, { valid: false, error }); + console.error(` ❌ CL ${cl.clNumber}: ${error}`); + hasErrors = true; + continue; + } + + const clEarliestVersion = dashDetails.earliest; + + if (!clEarliestVersion) { + const error = 'CL has no earliest version (may not be merged yet)'; + checkedCLs.set(cl.url, { valid: false, error }); + console.error(` ❌ CL ${cl.clNumber}: ${error}`); + hasErrors = true; + continue; + } + + // For V8 CLs, we need to find the corresponding Chromium commit + let chromiumVersion = clEarliestVersion; + if (isV8 && dashDetails.relations) { + const chromiumRelation = dashDetails.relations.find( + (rel) => rel.from_commit === gerritDetails.commitSha + ); + if (chromiumRelation) { + const chromiumDash = await fetchChromiumDashCommit(chromiumRelation.to_commit, 'chromium'); + if (chromiumDash?.earliest) { + chromiumVersion = chromiumDash.earliest; + } + } + } + + // CL should have landed after the base version and at or before the new version + const isInRange = + compareVersions(chromiumVersion, baseVersion) > 0 && + compareVersions(chromiumVersion, newVersion) <= 0; + + if (!isInRange) { + const error = `CL earliest version ${chromiumVersion} is outside roller range (${baseVersion} -> ${newVersion})`; + checkedCLs.set(cl.url, { valid: false, error }); + console.error(` ❌ CL ${cl.clNumber}: ${error}`); + hasErrors = true; + continue; + } + + checkedCLs.set(cl.url, { valid: true }); + console.log(` ✅ CL ${cl.clNumber}: version ${chromiumVersion} is within range`); + } + } + + console.log(hasErrors ? '\n❌ Lint failed' : '\n✅ Lint passed'); + if (hasErrors) { + console.log(' NOTE: Add "Skip-Lint: " to a commit message to skip linting that commit.'); + } + process.exit(hasErrors && process.env.CI ? 1 : 0); +} + +if ((await fs.realpath(process.argv[1])) === fileURLToPath(import.meta.url)) { + main() + .then(() => { + process.exit(0); + }) + .catch((err) => { + console.error(`ERROR: ${err.message}`); + process.exit(1); + }); +} diff --git a/script/release/notes/notes.ts b/script/release/notes/notes.ts index 334ac08342..c1b12a26cc 100644 --- a/script/release/notes/notes.ts +++ b/script/release/notes/notes.ts @@ -6,7 +6,7 @@ import { spawnSync } from 'node:child_process'; import { existsSync, readFileSync, writeFileSync, mkdirSync } from 'node:fs'; import { resolve as _resolve } from 'node:path'; -import { ELECTRON_DIR } from '../../lib/utils'; +import { compareVersions, ELECTRON_DIR } from '../../lib/utils'; import { createGitHubTokenStrategy } from '../github-token'; import { ELECTRON_ORG, ELECTRON_REPO } from '../types'; @@ -612,26 +612,6 @@ const getNotes = async (fromRef: string, toRef: string, newVersion: string) => { return notes; }; -const compareVersions = (v1: string, v2: string) => { - const [split1, split2] = [v1.split('.'), v2.split('.')]; - - if (split1.length !== split2.length) { - throw new Error( - `Expected version strings to have same number of sections: ${split1} and ${split2}` - ); - } - for (let i = 0; i < split1.length; i++) { - const p1 = parseInt(split1[i], 10); - const p2 = parseInt(split2[i], 10); - - if (p1 > p2) return 1; - else if (p1 < p2) return -1; - // Continue checking the value if this portion is equal - } - - return 0; -}; - const removeSupercededStackUpdates = (commits: Commit[]) => { const updateRegex = /^Updated ([a-zA-Z.]+) to v?([\d.]+)/; const notupdates = [];