diff --git a/README.md b/README.md index 4fa1844..142daa4 100644 --- a/README.md +++ b/README.md @@ -23,6 +23,7 @@ Focuses on Git history hygiene, developer metadata constraints, and layout stand * **Identity Integrity:** Validates author and committer name formats and strictly blocks generic GitHub `noreply.github.com` email addresses. * **Linked GitHub Account:** Verifies that the commit author email address is registered and verified on a GitHub account, linking the commit to a valid GitHub username. Can be downgraded to a non-blocking warning or disabled entirely, see `require_linked_github_account` below. * **Autosquash Compliance:** Automatically bypasses style constraints for development-phase `fixup!` and `squash!` syntax blocks. +* **Revert Compliance (`allow_revert`):** Accepts the subject line `git revert` generates — `Revert ""`, its nested `Revert "Revert """` form, and the prefixed `: Revert "..."` variant. The quoted text is copied verbatim from the reverted commit, so the prefix and lowercase rules are not applied to it, and the `Revert "..."` wrapper is excluded from the subject length limits. A revert also puts `PKG_VERSION` and `PKG_RELEASE` back to the values that preceded the reverted commit, so those restored values are accepted instead of a bump (see **PKG_RELEASE Validation** below). * **Subject String Hygiene:** Enforces `: ` prefix headers, checks lowercase starting strings post-prefix, and rejects trailing periods. * **Length Constraints:** Implements dual-layered (soft and hard) line width boundaries for both subject lines and description body text blocks. * **Signed-off-by Check:** Ensures a consistent, properly structured `Signed-off-by:` declaration is present and matches the original author metadata. @@ -42,7 +43,7 @@ Inspects file modification trees targeting OpenWrt build recipes: * **Conffiles Tracker:** Mandates the definition of the `Package/.../conffiles` tracking macro whenever configuration file installations (`INSTALL_CONF`) are triggered. * **Line Ending Sanitization:** Inspects modifications for Windows-style Carriage Returns (CRLF) to guarantee exclusive UNIX (LF) formatting compliance. * **Trailing Newline Check:** Verifies that newly created or modified files end with a trailing newline character, catching the common `\ No newline at end of file` issue in diffs (customizable level: warning/error/disabled). -* **PKG_RELEASE Validation:** Enforces correct release values on package changes: new packages must initialize `PKG_RELEASE` to `1`, version updates must reset `PKG_RELEASE` to `1`, and modifications to package files must be accompanied by a version/release change (customizable level: warning/error/disabled). +* **PKG_RELEASE Validation:** Enforces correct release values on package changes: new packages must initialize `PKG_RELEASE` to `1`, version updates must reset `PKG_RELEASE` to `1`, and modifications to package files must be accompanied by a version/release change (customizable level: warning/error/disabled). Packages that only revert commits touched are exempt from the reset/initialize rules, because a revert restores the version and release of an already released state. They must still carry a release bump if the revert changes package content without touching the version or release, otherwise users would never receive it. * **UCI Config Validation:** Ensures that any configuration files destined to be installed into `/etc/config/` conform to the standard OpenWrt UCI format (consisting of only `package`, `config`, `option`, `list` statements, comments, and empty lines). * **PKG_NAME Reuse Prevention:** Ensures `PKG_NAME` is not reused inside `call`, `define`, and `eval` Makefile lines, requiring the literal package name instead to keep recipes readable and searchable (default true). @@ -122,6 +123,7 @@ Some configuration keys offer advanced options: * `check_pkg_name_reuse`: Set to `true` (default) to detect and reject reuse of the `PKG_NAME` variable in `call`, `define`, and `eval` lines, or `false` to disable. * `show_force_push_tip`: Set to `true` (default) to append a helpful tip regarding how to correct validation errors using force-pushing. Set to `false` to disable. * `check_openwrt_spelling`: Set to `true` (default) to validate the correct capitalization of "OpenWrt" in commit subjects and descriptions. Set to `false` to disable. +* `allow_revert`: Set to `true` (default) to accept the subject format produced by `git revert` (`Revert ""`, nested reverts, and the prefixed `: Revert "..."` variant) and the `PKG_VERSION`/`PKG_RELEASE` values a revert restores. Set to `false` to hold revert commits to the regular subject and release bump rules. * `enable_stale_bot`: Set to `true` to enable the stale PR bot cleanup for this repository. Defaults to `false` (opt-in). * `enable_labeler_yml`: Set to `true` to enable dynamic pull request labeling based on matching files in the `.github/labeler.yml` configuration file. Defaults to `false` (opt-in). * `enable_issue_labeller`: Set to `true` to enable automated issue form validation and labelling (replaces the GitHub Actions `issue-labeller.yml` workflow). Defaults to `false` (opt-in). @@ -136,6 +138,7 @@ Here is a comprehensive example containing all available toggle options: "check_signoff": true, "check_signature": true, "allow_autosquash": true, + "allow_revert": true, "enable_comments": true, "show_force_push_tip": true, "max_subject_len_soft": 60, diff --git a/cloudflare-worker/src/config.js b/cloudflare-worker/src/config.js index a8503c8..76a106b 100644 --- a/cloudflare-worker/src/config.js +++ b/cloudflare-worker/src/config.js @@ -6,6 +6,7 @@ export const DEFAULT_CONFIG = { check_signoff: true, check_signature: true, allow_autosquash: true, + allow_revert: true, enable_comments: true, show_force_push_tip: true, max_subject_len_soft: 60, diff --git a/cloudflare-worker/src/validators.js b/cloudflare-worker/src/validators.js index a404f8e..626cf95 100644 --- a/cloudflare-worker/src/validators.js +++ b/cloudflare-worker/src/validators.js @@ -80,6 +80,32 @@ export function isVirtuallyIdentical(subject, body, pkgName) { return !hasMeaningfulWord; } +// `git revert` builds the subject from the reverted commit verbatim as +// `Revert ""`, reverting a revert nests the wrapper another +// level, and OpenWrt also uses a prefixed `: Revert ""` variant. +// None of these can satisfy the regular subject rules - the package prefix sits +// inside the quotes, `Revert` is capitalized, and the wrapper eats into the +// length budget - and the author cannot rewrite the quoted part without losing +// the reference to the commit being reverted. +// Returns { prefix, original, depth } for a revert subject, null otherwise. +export function parseRevertSubject(subject) { + if (typeof subject !== 'string') return null; + + // `(?:: )*` also covers `tools/cmake: ` and chained `toolchain: binutils: ` prefixes. + const outer = subject.trim().match(/^((?:[a-zA-Z0-9_-]+(?:\/[a-zA-Z0-9_-]+)?: )*)[Rr]evert "(.+)"$/); + if (!outer) return null; + + let original = outer[2]; + let depth = 1; + let nested; + while ((nested = original.match(/^[Rr]evert "(.+)"$/))) { + original = nested[1]; + depth++; + } + + return { prefix: outer[1], original, depth }; +} + async function getSshKeyFingerprint(sigText) { try { let cleanSig = sigText.replace(/-----[a-zA-Z0-9\s]+-----/g, ''); @@ -191,42 +217,57 @@ export async function validateFormalities(fullCommit, CONFIG) { subject = subject.replace(/^(fixup!|squash!)\s+/, ''); } + const revert = CONFIG.allow_revert === false ? null : parseRevertSubject(subject); + if (!isAutosquash) { if (/^\s/.test(lines[0])) subjectErrors.push("Commit subject must not start with whitespace"); - - // Special case for tools/* prefix (e.g., tools/cmake: backport bootstrap fix) - // These use a subdirectory naming convention like tools/cmake, tools/bison, etc. - const toolsPrefixMatch = subject.match(/^(tools\/[a-zA-Z0-9_-]+): /); - if (toolsPrefixMatch) { - const afterPrefix = subject.replace(/^(tools\/[a-zA-Z0-9_-]+): \s*/, ''); - if (afterPrefix.length > 0 && afterPrefix[0] === afterPrefix[0].toUpperCase() && /[a-zA-Z]/.test(afterPrefix[0])) { - subjectErrors.push("Commit subject must start with a lower-case word after the prefix"); - } - if (subject.endsWith('.')) { - subjectErrors.push("Commit subject must not end with a period"); - } - } else if (!/^[a-zA-Z0-9_-]+: /.test(subject)) { - subjectErrors.push("Commit subject must start with `: `"); - } else { - const afterPrefix = subject.replace(/^[a-zA-Z0-9_-]+: \s*/, ''); - if (afterPrefix.length > 0 && afterPrefix[0] === afterPrefix[0].toUpperCase() && /[a-zA-Z]/.test(afterPrefix[0])) { - subjectErrors.push("Commit subject must start with a lower-case word after the prefix"); - } - if (subject.endsWith('.')) { - subjectErrors.push("Commit subject must not end with a period"); + + // The quoted part of a revert subject is copied from the reverted commit, + // so the prefix/lower-case/period rules apply to the original subject and + // not to the wrapper `git revert` generated around it. + if (!revert) { + // Special case for tools/* prefix (e.g., tools/cmake: backport bootstrap fix) + // These use a subdirectory naming convention like tools/cmake, tools/bison, etc. + const toolsPrefixMatch = subject.match(/^(tools\/[a-zA-Z0-9_-]+): /); + if (toolsPrefixMatch) { + const afterPrefix = subject.replace(/^(tools\/[a-zA-Z0-9_-]+): \s*/, ''); + if (afterPrefix.length > 0 && afterPrefix[0] === afterPrefix[0].toUpperCase() && /[a-zA-Z]/.test(afterPrefix[0])) { + subjectErrors.push("Commit subject must start with a lower-case word after the prefix"); + } + if (subject.endsWith('.')) { + subjectErrors.push("Commit subject must not end with a period"); + } + } else if (!/^[a-zA-Z0-9_-]+: /.test(subject)) { + subjectErrors.push("Commit subject must start with `: `"); + } else { + const afterPrefix = subject.replace(/^[a-zA-Z0-9_-]+: \s*/, ''); + if (afterPrefix.length > 0 && afterPrefix[0] === afterPrefix[0].toUpperCase() && /[a-zA-Z]/.test(afterPrefix[0])) { + subjectErrors.push("Commit subject must start with a lower-case word after the prefix"); + } + if (subject.endsWith('.')) { + subjectErrors.push("Commit subject must not end with a period"); + } } } } - const subjectLen = lines[0].length; + // Measure a revert against the subject underneath the `Revert "..."` wrapper: + // the wrapper is generated from the reverted commit, so the author cannot + // shorten it without breaking the reference. + const subjectLen = revert ? (revert.prefix + revert.original).length : lines[0].length; + const lenSuffix = revert ? ' chars, excluding the `Revert "..."` wrapper' : ' chars'; if (subjectLen > CONFIG.max_subject_len_hard) { - subjectErrors.push(`Subject line exceeds hard limit (${subjectLen}/${CONFIG.max_subject_len_hard} chars)`); + subjectErrors.push(`Subject line exceeds hard limit (${subjectLen}/${CONFIG.max_subject_len_hard}${lenSuffix})`); } else if (subjectLen > CONFIG.max_subject_len_soft) { - warnings.push(`Subject line exceeds soft limit (${subjectLen}/${CONFIG.max_subject_len_soft} chars)`); + warnings.push(`Subject line exceeds soft limit (${subjectLen}/${CONFIG.max_subject_len_soft}${lenSuffix})`); } if (subjectErrors.length === 0) { - successes.push(`✅ Commit subject layout and length are valid: "${lines[0]}"`); + if (revert) { + successes.push(`✅ Commit subject layout and length are valid (revert of "${revert.original}")`); + } else { + successes.push(`✅ Commit subject layout and length are valid: "${lines[0]}"`); + } } else { subjectErrors.forEach(err => errors.push("- " + err)); } @@ -496,7 +537,14 @@ export function validateMakefileContext(fullCommit, commitPatch, CONFIG, state) successes.push(`✅ PKG_VERSION is dynamically defined: '${newVersion}', skipping subject validation`); } else { const cleanSubject = subject.replace(/^(fixup!|squash!)\s+/, ''); - if (!matchVersionString(cleanSubject, newVersion)) { + const isRevert = CONFIG.allow_revert !== false && parseRevertSubject(cleanSubject) !== null; + if (isRevert) { + // A revert restores the PKG_VERSION that preceded the reverted commit, + // while its subject quotes that commit (and therefore the version being + // undone). Requiring the restored version here would force the author + // away from the `git revert` subject format. + successes.push(`✅ Commit reverts a previous change, skipping subject validation for restored PKG_VERSION '${newVersion}'`); + } else if (!matchVersionString(cleanSubject, newVersion)) { errors.push(`- Makefile introduces PKG_VERSION '${newVersion}', but this version string is missing in the commit subject line. Please mention the new version in the subject, e.g. ': update to ${newVersion}'.`); } else { successes.push(`✅ PKG_VERSION bump matches context information inside subject line (${newVersion})`); @@ -1410,7 +1458,18 @@ export async function validatePkgReleaseBumps(commitDetails, CONFIG, fetchFileCo const fileChanges = {}; // filePath -> { added: [], deleted: [] } const candidateFiles = []; + // A revert puts PKG_VERSION and PKG_RELEASE back to the values that preceded + // the reverted commit, so the version moves backwards and PKG_RELEASE is + // whatever it was before instead of 1. Record which commits touched a file so + // packages changed by reverts alone can skip the bump requirements. The + // PR-wide patch fallback carries no per-commit message and therefore never + // qualifies, keeping the audit strict when the origin of a change is unknown. + const nonRevertedFiles = new Set(); + for (const item of commitDetails) { + const isRevertCommit = CONFIG.allow_revert !== false && + parseRevertSubject((item.fullCommit?.commit?.message || '').split('\n')[0]) !== null; + if (item.commitPatch) { const states = parseDiffFileStates(item.commitPatch); states.addedFiles.forEach(f => addedFiles.add(f)); @@ -1448,6 +1507,7 @@ export async function validatePkgReleaseBumps(commitDetails, CONFIG, fetchFileCo const files = getChangedFilesFromPatch(item.commitPatch); for (const file of files) { modifiedFiles.add(file); + if (!isRevertCommit) nonRevertedFiles.add(file); if (isHiddenOrSpecial(file)) continue; // Ignore test files that serve only within CI/CD (e.g. test.sh, test-version.sh) @@ -1493,6 +1553,11 @@ export async function validatePkgReleaseBumps(commitDetails, CONFIG, fetchFileCo return empty; } + // Only revert commits touched this package, so its PKG_VERSION and + // PKG_RELEASE are back at the values that preceded the reverted commit. + const pkgFiles = [...modifiedFiles].filter(file => file === pkgRoot || file.startsWith(pkgRoot + '/')); + const isRevertOnly = pkgFiles.length > 0 && pkgFiles.every(file => !nonRevertedFiles.has(file)); + // OPTIMIZATION: If the Makefile itself was not modified in the PR, // then the version and release cannot have changed (bumped = false). // We can skip fetching the Makefile contents entirely! @@ -1522,6 +1587,11 @@ export async function validatePkgReleaseBumps(commitDetails, CONFIG, fetchFileCo headRelease = resolveMakefileVar(headContent, 'PKG_RELEASE'); if (isNew) { + if (isRevertOnly) { + // Reverting the removal of a package restores it with the PKG_RELEASE + // it was dropped with; it is not a new package starting from scratch. + return { errors: [], successes: [`✅ Package \`${pkgRoot}\` is restored by a revert with its previous PKG_RELEASE ('${headRelease || 'not defined'}')`] }; + } if (headRelease !== '1') { return { errors: [`New package \`${pkgRoot}\` must start with PKG_RELEASE set to 1 (currently: '${headRelease || 'not defined'}')`], successes: [] }; } @@ -1629,6 +1699,18 @@ export async function validatePkgReleaseBumps(commitDetails, CONFIG, fetchFileCo }; } + // A revert takes the package back to a state that was already released, so + // its version legitimately moves backwards and PKG_RELEASE keeps the value + // it had before the reverted commit. Requiring a reset to 1 here would ask + // for a version bump that a revert must not make. + if (isRevertOnly) { + const restoredVersion = headVersion || headSourceVer || headSourceDate; + const restoredState = restoredVersion + ? `PKG_VERSION '${restoredVersion}' with PKG_RELEASE '${headRelease || 'not defined'}'` + : `PKG_RELEASE '${headRelease || 'not defined'}'`; + return { errors: [], successes: [`✅ Package \`${pkgRoot}\` is reverted to ${restoredState}, matching its state before the reverted commit`] }; + } + if (versionChanged) { if (headRelease !== '1') { return { errors: [`Package \`${pkgRoot}\` version updated from '${baseVersion || baseSourceVer || baseSourceDate}' to '${headVersion || headSourceVer || headSourceDate}', but PKG_RELEASE was not reset to 1 (currently: '${headRelease || 'not defined'}')`], successes: [] }; diff --git a/cloudflare-worker/test/validators.test.js b/cloudflare-worker/test/validators.test.js index 2b901af..8fd3773 100644 --- a/cloudflare-worker/test/validators.test.js +++ b/cloudflare-worker/test/validators.test.js @@ -1,6 +1,6 @@ import { describe, test } from 'node:test'; import assert from 'node:assert'; -import { isValidName, validateFormalities, validateMakefileContext, validateEmbeddedPatches, validatePkgReleaseBumps, findPkgRoot, validateUciConfigs } from '../src/validators.js'; +import { isValidName, parseRevertSubject, validateFormalities, validateMakefileContext, validateEmbeddedPatches, validatePkgReleaseBumps, findPkgRoot, validateUciConfigs } from '../src/validators.js'; // Mock Config Object const CONFIG = { @@ -510,6 +510,94 @@ describe('validateFormalities', () => { }); }); +// ─── Revert Subjects ───────────────────────────────────────────── + +const revertBody = (sha) => `\n\nThis reverts commit ${sha}.\nIt broke the build on several targets.\n\nSigned-off-by: John Doe `; + +const revertCommit = (subject) => ({ + commit: { + message: subject + revertBody('9fceb02d0ae598e95dc970b74767f19372d61af8'), + author: { name: 'John Doe', email: 'john@doe.com' }, + committer: { name: 'John Doe', email: 'john@doe.com' } + } +}); + +describe('parseRevertSubject', () => { + test('parses the plain git revert format', () => { + const res = parseRevertSubject('Revert "generic: permit support of standalone PCS for external kernel module"'); + assert.deepStrictEqual(res, { + prefix: '', + original: 'generic: permit support of standalone PCS for external kernel module', + depth: 1 + }); + }); + + test('parses a prefixed revert (e.g. sing-box: Revert "...")', () => { + const res = parseRevertSubject('sing-box: Revert "sing-box: update to 1.12.3"'); + assert.deepStrictEqual(res, { prefix: 'sing-box: ', original: 'sing-box: update to 1.12.3', depth: 1 }); + }); + + test('parses chained and tools/ style prefixes', () => { + assert.strictEqual(parseRevertSubject('tools/cmake: revert "tools/cmake: update to 4.0"').prefix, 'tools/cmake: '); + assert.strictEqual(parseRevertSubject('toolchain: binutils: Revert "toolchain: binutils: update to 2.45"').prefix, 'toolchain: binutils: '); + }); + + test('unwraps a revert of a revert', () => { + const res = parseRevertSubject('Revert "Revert "ramips: mt7620: fix patching mac address in caldata""'); + assert.strictEqual(res.depth, 2); + assert.strictEqual(res.original, 'ramips: mt7620: fix patching mac address in caldata'); + }); + + test('rejects subjects that only mention a revert', () => { + assert.strictEqual(parseRevertSubject('mypkg: revert the broken change'), null); + assert.strictEqual(parseRevertSubject('Revert the broken change'), null); + assert.strictEqual(parseRevertSubject('Reverted "mypkg: update to 1.2.3"'), null); + assert.strictEqual(parseRevertSubject('toolchain: binutils: partially revert commit 525a1e94b343 "fix update to 2.45.1"'), null); + }); +}); + +describe('validateFormalities revert subjects', () => { + test('accepts the plain git revert format without a package prefix', async () => { + const res = await validateFormalities(revertCommit('Revert "generic: permit support of standalone PCS for external kernel module"'), CONFIG); + assert.strictEqual(res.errors.length, 0, `Unexpected errors: ${res.errors.join(', ')}`); + assert.ok(res.successes.some(s => s.includes('Commit subject layout and length are valid (revert of'))); + }); + + test('accepts a revert of a revert', async () => { + const res = await validateFormalities(revertCommit('Revert "Revert "ramips: mt7620: fix patching mac address in caldata""'), CONFIG); + assert.strictEqual(res.errors.length, 0, `Unexpected errors: ${res.errors.join(', ')}`); + }); + + test('accepts an upper-case Revert after a package prefix', async () => { + const res = await validateFormalities(revertCommit('irqbalance: Revert "irqbalance: update to 1.9.5"'), CONFIG); + assert.strictEqual(res.errors.length, 0, `Unexpected errors: ${res.errors.join(', ')}`); + }); + + test('excludes the Revert wrapper from the subject length limits', async () => { + // 86 chars as written, 77 without the wrapper. + const res = await validateFormalities(revertCommit('Revert "base-files: handle name collision between kernel UBI volume and MTD partition"'), CONFIG); + assert.strictEqual(res.errors.length, 0, `Unexpected errors: ${res.errors.join(', ')}`); + }); + + test('still enforces the hard limit on the reverted subject itself', async () => { + const original = 'base-files: handle a name collision between the kernel UBI volume and the MTD partition'; + assert.ok(original.length > CONFIG.max_subject_len_hard); + const res = await validateFormalities(revertCommit(`Revert "${original}"`), CONFIG); + assert.ok(res.errors.some(e => e.includes('exceeds hard limit') && e.includes('excluding the `Revert "..."` wrapper'))); + }); + + test('still rejects a revert-like subject without the quoted original', async () => { + const res = await validateFormalities(revertCommit('Revert the broken PCS support'), CONFIG); + assert.ok(res.errors.some(e => e.includes('must start with `: `'))); + }); + + test('enforces the regular subject rules when allow_revert is disabled', async () => { + const customConfig = { ...CONFIG, allow_revert: false }; + const res = await validateFormalities(revertCommit('Revert "generic: permit support of standalone PCS for external kernel module"'), customConfig); + assert.ok(res.errors.some(e => e.includes('must start with `: `'))); + }); +}); + // ─── Makefile Context ──────────────────────────────────────────── describe('validateMakefileContext', () => { @@ -561,6 +649,31 @@ describe('validateMakefileContext', () => { assert.ok(res.errors.some(e => e.includes('PKG_VERSION'))); }); + test('skips subject validation for a revert restoring the previous PKG_VERSION', () => { + const commit = { commit: { message: 'sing-box: Revert "sing-box: update to 1.12.3"\n\nThis reverts commit 9fceb02d0ae598e95dc970b74767f19372d61af8.' } }; + const patch = ` +--- a/package/net/sing-box/Makefile ++++ b/package/net/sing-box/Makefile ++PKG_VERSION:=1.12.2 + `; + const state = { isNewPackage: false, isDroppedPackage: false }; + const res = validateMakefileContext(commit, patch, CONFIG, state); + assert.strictEqual(res.errors.length, 0, `Unexpected errors: ${res.errors.join(', ')}`); + assert.ok(res.successes.some(s => s.includes('Commit reverts a previous change'))); + }); + + test('catches version mismatch on a revert when allow_revert is disabled', () => { + const commit = { commit: { message: 'sing-box: Revert "sing-box: update to 1.12.3"' } }; + const patch = ` +--- a/package/net/sing-box/Makefile ++++ b/package/net/sing-box/Makefile ++PKG_VERSION:=1.12.2 + `; + const state = { isNewPackage: false, isDroppedPackage: false }; + const res = validateMakefileContext(commit, patch, { ...CONFIG, allow_revert: false }, state); + assert.ok(res.errors.some(e => e.includes('PKG_VERSION'))); + }); + test('skips subject validation for dynamic/templated PKG_VERSION', () => { const commit = { commit: { message: 'apk: update to 2.14.0' } }; const patch = ` @@ -2576,6 +2689,103 @@ diff --git a/utils/prometheus-node-exporter-ucode/Makefile b/utils/prometheus-no const res = await validatePkgReleaseBumps(commitDetails, defaultConf, headFetch, baseFetch); assert.ok(res.errors.some(e => e.includes('content changed without a PKG_RELEASE or version bump'))); }); + + // ─── Reverts ─────────────────────────────────────────────────── + + const commitWith = (subject) => ({ commit: { message: `${subject}\n\nThis reverts commit 9fceb02d0ae598e95dc970b74767f19372d61af8.` } }); + + // Reverting `mypkg: update to 1.2.3` restores both the older version and the + // PKG_RELEASE that preceded the bump. + const versionRevertPatch = ` +diff --git a/package/utils/mypkg/Makefile b/package/utils/mypkg/Makefile +--- a/package/utils/mypkg/Makefile ++++ b/package/utils/mypkg/Makefile +-PKG_VERSION:=1.2.3 ++PKG_VERSION:=1.2.2 +-PKG_RELEASE:=1 ++PKG_RELEASE:=3 +`; + const versionRevertHead = async (path) => + path === 'package/utils/mypkg/Makefile' ? 'PKG_NAME:=mypkg\nPKG_VERSION:=1.2.2\nPKG_RELEASE:=3\n' : null; + const versionRevertBase = async (path) => + path === 'package/utils/mypkg/Makefile' ? 'PKG_NAME:=mypkg\nPKG_VERSION:=1.2.3\nPKG_RELEASE:=1\n' : null; + + test('accepts a revert restoring an older version and its previous PKG_RELEASE', async () => { + const commitDetails = [{ fullCommit: commitWith('mypkg: Revert "mypkg: update to 1.2.3"'), commitPatch: versionRevertPatch }]; + const res = await validatePkgReleaseBumps(commitDetails, defaultConf, versionRevertHead, versionRevertBase); + assert.strictEqual(res.errors.length, 0, `Unexpected errors: ${res.errors.join(', ')}`); + assert.ok(res.successes.some(s => s.includes('matching its state before the reverted commit'))); + }); + + test('still demands a PKG_RELEASE reset for the same downgrade in a regular commit', async () => { + const commitDetails = [{ fullCommit: commitWith('mypkg: downgrade to 1.2.2'), commitPatch: versionRevertPatch }]; + const res = await validatePkgReleaseBumps(commitDetails, defaultConf, versionRevertHead, versionRevertBase); + assert.ok(res.errors.some(e => e.includes('PKG_RELEASE was not reset to 1'))); + }); + + test('keeps the audit strict when no commit message is available (PR-wide patch fallback)', async () => { + const commitDetails = [{ commitPatch: versionRevertPatch }]; + const res = await validatePkgReleaseBumps(commitDetails, defaultConf, versionRevertHead, versionRevertBase); + assert.ok(res.errors.some(e => e.includes('PKG_RELEASE was not reset to 1'))); + }); + + test('keeps the audit strict when allow_revert is disabled', async () => { + const commitDetails = [{ fullCommit: commitWith('mypkg: Revert "mypkg: update to 1.2.3"'), commitPatch: versionRevertPatch }]; + const res = await validatePkgReleaseBumps(commitDetails, { ...defaultConf, allow_revert: false }, versionRevertHead, versionRevertBase); + assert.ok(res.errors.some(e => e.includes('PKG_RELEASE was not reset to 1'))); + }); + + test('keeps the audit strict when a regular commit touches the same package', async () => { + const commitDetails = [ + { fullCommit: commitWith('mypkg: Revert "mypkg: update to 1.2.3"'), commitPatch: versionRevertPatch }, + { + fullCommit: { commit: { message: 'mypkg: refresh patches' } }, + commitPatch: ` +diff --git a/package/utils/mypkg/patches/001-fix.patch b/package/utils/mypkg/patches/001-fix.patch +--- a/package/utils/mypkg/patches/001-fix.patch ++++ b/package/utils/mypkg/patches/001-fix.patch ++context +` + } + ]; + const res = await validatePkgReleaseBumps(commitDetails, defaultConf, versionRevertHead, versionRevertBase); + assert.ok(res.errors.some(e => e.includes('PKG_RELEASE was not reset to 1'))); + }); + + test('accepts a revert restoring a dropped package with its previous PKG_RELEASE', async () => { + const commitDetails = [{ + fullCommit: commitWith('Revert "oldpkg: remove abandoned package"'), + commitPatch: ` +diff --git a/package/utils/oldpkg/Makefile b/package/utils/oldpkg/Makefile +new file mode 100644 +--- /dev/null ++++ b/package/utils/oldpkg/Makefile +` + }]; + const headFetch = async (path) => + path === 'package/utils/oldpkg/Makefile' ? 'PKG_NAME:=oldpkg\nPKG_VERSION:=2.0\nPKG_RELEASE:=5\n' : null; + + const res = await validatePkgReleaseBumps(commitDetails, defaultConf, headFetch, async () => null); + assert.strictEqual(res.errors.length, 0, `Unexpected errors: ${res.errors.join(', ')}`); + assert.ok(res.successes.some(s => s.includes('restored by a revert with its previous PKG_RELEASE'))); + }); + + test('still requires a bump when a revert changes content without touching version or release', async () => { + const commitDetails = [{ + fullCommit: commitWith('Revert "mypkg: tweak init script"'), + commitPatch: ` +diff --git a/package/utils/mypkg/files/mypkg.init b/package/utils/mypkg/files/mypkg.init +--- a/package/utils/mypkg/files/mypkg.init ++++ b/package/utils/mypkg/files/mypkg.init ++start_service() { +` + }]; + const headFetch = async (path) => + path === 'package/utils/mypkg/Makefile' ? 'PKG_NAME:=mypkg\nPKG_VERSION:=1.2.2\nPKG_RELEASE:=3\n' : null; + + const res = await validatePkgReleaseBumps(commitDetails, defaultConf, headFetch, headFetch); + assert.ok(res.errors.some(e => e.includes('content changed without a PKG_RELEASE or version bump'))); + }); }); describe('findPkgRoot', () => {