From 38664617c09743a984dd95fb4e70fae51ba08f92 Mon Sep 17 00:00:00 2001 From: Danyang Chen Date: Sat, 24 Oct 2026 21:41:24 +0800 Subject: [PATCH] Check the patch series without git, at git apply's strictness MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `check` could only run where git exists, so on the machine the Bots work on the two shapes that broke the series — #934's extra blank context line and #966's hunk header running to 662 in a 661-line file — waited for CI's `package` job, a review cycle later. `scripts/ds-bot-patches.mjs` now carries its own applier, used by `build()` and `diffReproduces()` only when git is absent: a hunk body must carry the counts its header declares, every hunk must match at the old-side line its header names (no search for nearby context), no hunk may run past the end of the file, and `\ No newline at end of file` is honoured on both sides. Tracked files come from a walk with the root .gitignore rules, and the pin prints `skipped pin (no git)` instead of passing silently. A machine with git keeps the git path: CI is the reference, and a test builds the series both ways and compares the trees byte for byte. `generate` still needs git, because it asks git for the diff. Fixes #978 --- .github/scripts/ds-bot-patches.test.mjs | 218 ++++++++++++++++++++---- docs/subsystems/ds-bot-core.md | 1 + patches/ds-bot/README.md | 20 +++ scripts/ds-bot-patches.mjs | 201 +++++++++++++++++++++- 4 files changed, 407 insertions(+), 33 deletions(-) diff --git a/.github/scripts/ds-bot-patches.test.mjs b/.github/scripts/ds-bot-patches.test.mjs index 5b3a3fa2a..b896a6795 100644 --- a/.github/scripts/ds-bot-patches.test.mjs +++ b/.github/scripts/ds-bot-patches.test.mjs @@ -2,10 +2,12 @@ import { test } from 'node:test' import assert from 'node:assert/strict' import { execFileSync } from 'node:child_process' -import { mkdtempSync, mkdirSync, writeFileSync, readFileSync, rmSync } from 'node:fs' +import { cpSync, mkdtempSync, mkdirSync, readdirSync, readFileSync, rmSync, statSync, writeFileSync } from 'node:fs' import { tmpdir } from 'node:os' -import { join } from 'node:path' -import { normalizeDiff } from '../../scripts/ds-bot-patches.mjs' +import { dirname, join, relative, sep } from 'node:path' +import { applyPatchText, normalizeDiff } from '../../scripts/ds-bot-patches.mjs' + +const gitAvailable = (() => { try { execFileSync('git', ['--version'], { stdio: 'ignore' }); return true } catch { return false } })() const lines = (n, edits = {}) => Array.from({ length: n }, (_, i) => edits[i + 1] ?? `line ${i + 1}`).join('\n') + '\n' @@ -37,37 +39,193 @@ const base = lines(200) const top = { 3: 'line 3\nadded at the top\nand a second line' } const bottom = { 190: 'line 190\nadded at the bottom' } -test('no index line, no ds-bot/ or core/ prefix, no per-file counts of other edits', () => { - const patch = diffOf(base, lines(200, top)) - assert.doesNotMatch(patch, /^index /m) - assert.match(patch, /^diff --git a\/x\.js b\/x\.js$/m) - assert.match(patch, /^--- a\/x\.js$/m) - assert.match(patch, /^\+\+\+ b\/x\.js$/m) +// The format cases ask git for the diff, so they run where git exists (CI); the applier cases below +// are git-free on purpose, because that is the machine #978 is for. +if (gitAvailable) { + test('no index line, no ds-bot/ or core/ prefix, no per-file counts of other edits', () => { + const patch = diffOf(base, lines(200, top)) + assert.doesNotMatch(patch, /^index /m) + assert.match(patch, /^diff --git a\/x\.js b\/x\.js$/m) + assert.match(patch, /^--- a\/x\.js$/m) + assert.match(patch, /^\+\+\+ b\/x\.js$/m) + }) + + test('a hunk header does not depend on edits in other hunks', () => { + const both = { ...top, ...bottom } + const hunks = text => text.split('\n').filter(l => l.startsWith('@@')) + const alone = hunks(diffOf(base, lines(200, bottom))) + const withTop = hunks(diffOf(base, lines(200, both))) + assert.equal(alone.length, 1) + assert.ok(withTop.includes(alone[0]), `${alone[0]} not in ${withTop}`) + // the exact form shifts the new-side start, which is what made two branches conflict + const exact = diffOf(base, lines(200, both), { exact: true }).split('\n').filter(l => l.startsWith('@@')) + assert.ok(!exact.includes(alone[0])) + }) + + test('the stored form still applies, and gives the new file', () => { + const both = { ...top, ...bottom } + assert.equal(applies(base, diffOf(base, lines(200, both))), lines(200, both)) + assert.equal(applies(base, diffOf(base, lines(200, both), { exact: true })), lines(200, both)) + }) + + test('hunk bodies are never rewritten, even when they look like headers', () => { + const tricky = ['--- a/ds-bot/x.js', '+++ b/core/x.js', 'index abc..def 100644', '@@ -1,2 +9,9 @@', 'ctx'] + const before = tricky.join('\n') + '\n' + const after = ['ctx', ...tricky.slice(0, 4)].join('\n') + '\n' + const patch = diffOf(before, after) + for (const l of tricky.slice(0, 4)) assert.ok(patch.includes(l), `lost: ${l}`) + assert.equal(applies(before, patch), after) + }) +} + +// The git-free applier (#978). Two review cycles were lost to shapes only CI's git apply caught, so +// both are fixtures: a hunk body with more lines than its header declares (#934) and a hunk header +// that runs past the last line of the file (#966). +function applyTo(files, patch) { + const dir = mkdtempSync(join(tmpdir(), 'patch-apply-')) + try { + for (const [path, content] of Object.entries(files)) { + mkdirSync(dirname(join(dir, path)), { recursive: true }) + writeFileSync(join(dir, path), content) + } + applyPatchText(patch, dir) + const out = {} + const walk = abs => { + for (const entry of readdirSync(abs, { withFileTypes: true })) { + const path = join(abs, entry.name) + if (entry.isDirectory()) { walk(path); continue } + out[relative(dir, path).split(sep).join('/')] = { text: readFileSync(path, 'utf8'), mode: statSync(path).mode & 0o777 } + } + } + walk(dir) + return out + } finally { rmSync(dir, { recursive: true, force: true }) } +} + +test('#978: a hunk that runs past the end of the file fails, naming the hunk and both numbers', () => { + const before = lines(661) + const patch = [ + 'diff --git a/src/client/sidebar.js b/src/client/sidebar.js', + '--- a/src/client/sidebar.js', + '+++ b/src/client/sidebar.js', + '@@ -658,5 +658,5 @@', + ...['line 658', 'line 659', 'line 660', 'line 661', ''].map(line => ` ${line}`), + '', + ].join('\n') + assert.throws(() => applyTo({ 'src/client/sidebar.js': before }, patch), /sidebar\.js hunk 1 \(@@ -658,5 \+658,5 @@\): runs past the end of the file \(661 lines\)/) }) -test('a hunk header does not depend on edits in other hunks', () => { - const both = { ...top, ...bottom } - const hunks = text => text.split('\n').filter(l => l.startsWith('@@')) - const alone = hunks(diffOf(base, lines(200, bottom))) - const withTop = hunks(diffOf(base, lines(200, both))) - assert.equal(alone.length, 1) - assert.ok(withTop.includes(alone[0]), `${alone[0]} not in ${withTop}`) - // the exact form shifts the new-side start, which is what made two branches conflict - const exact = diffOf(base, lines(200, both), { exact: true }).split('\n').filter(l => l.startsWith('@@')) - assert.ok(!exact.includes(alone[0])) +test('#978: a hunk body with one line too many fails instead of landing the extra line', () => { + const patch = [ + 'diff --git a/x.js b/x.js', + '--- a/x.js', + '+++ b/x.js', + '@@ -2,2 +2,3 @@', + ' line 2', + ' line 3', + ' ', + '', + ].join('\n') + assert.throws(() => applyTo({ 'x.js': lines(10) }, patch), /hunk 1 \(@@ -2,2 \+2,3 @@\): the body has 3 old and 3 new line\(s\), the header declares 2 and 3/) }) -test('the stored form still applies, and gives the new file', () => { - const both = { ...top, ...bottom } - assert.equal(applies(base, diffOf(base, lines(200, both))), lines(200, both)) - assert.equal(applies(base, diffOf(base, lines(200, both), { exact: true })), lines(200, both)) +test('#978: a hunk that only matches somewhere else is refused, not moved to fit', () => { + // The context sits on line 7; a searching applier finds it there and writes the file 4 lines off, + // which is how #966's phantom line got through. The header says 3, so it fails at 3. + const before = ['pad', 'pad', 'pad', 'pad', 'pad', 'pad', 'alpha', 'beta', 'pad'].join('\n') + '\n' + const patch = [ + 'diff --git a/x.txt b/x.txt', + '--- a/x.txt', + '+++ b/x.txt', + '@@ -3,2 +3,2 @@', + ' alpha', + '-beta', + '+gamma', + '', + ].join('\n') + assert.throws(() => applyTo({ 'x.txt': before }, patch), /hunk 1 \(@@ -3,2 \+3,2 @@\): context at 3 is "pad", the hunk has "alpha"/) }) -test('hunk bodies are never rewritten, even when they look like headers', () => { - const tricky = ['--- a/ds-bot/x.js', '+++ b/core/x.js', 'index abc..def 100644', '@@ -1,2 +9,9 @@', 'ctx'] - const before = tricky.join('\n') + '\n' - const after = ['ctx', ...tricky.slice(0, 4)].join('\n') + '\n' - const patch = diffOf(before, after) - for (const l of tricky.slice(0, 4)) assert.ok(patch.includes(l), `lost: ${l}`) - assert.equal(applies(before, patch), after) +test('#978: a missing final newline is kept on both sides, and a false marker is refused', () => { + const patch = [ + 'diff --git a/x.txt b/x.txt', + '--- a/x.txt', + '+++ b/x.txt', + '@@ -1,2 +1,2 @@', + ' one', + '-two', + '\\ No newline at end of file', + '+two!', + '\\ No newline at end of file', + '', + ].join('\n') + assert.equal(applyTo({ 'x.txt': 'one\ntwo' }, patch)['x.txt'].text, 'one\ntwo!') + assert.throws(() => applyTo({ 'x.txt': 'one\ntwo\n' }, patch), /says "No newline at end of file" after a line that has one/) }) + +test('#978: -p1 strips the path, a whole file is created with its mode, and /dev/null deletes one', () => { + const created = [ + 'diff --git a/lib/new.js b/lib/new.js', + 'new file mode 100755', + '--- /dev/null', + '+++ b/lib/new.js', + '@@ -0,0 +1,2 @@', + '+#!/usr/bin/env node', + '+console.log(1)', + '', + ].join('\n') + const out = applyTo({}, created) + assert.equal(out['lib/new.js'].text, '#!/usr/bin/env node\nconsole.log(1)\n') + assert.equal(out['lib/new.js'].mode, 0o755) + const deleted = [ + 'diff --git a/lib/old.js b/lib/old.js', + '--- a/lib/old.js', + '+++ /dev/null', + '@@ -1 +0,0 @@', + '-gone', + '', + ].join('\n') + assert.deepEqual(Object.keys(applyTo({ 'lib/old.js': 'gone\n', 'keep.js': 'kept\n' }, deleted)), ['keep.js']) +}) + +if (gitAvailable) { + test('#978: the git-free applier and git apply produce the same tree on every stored patch', () => { + const root = join(import.meta.dirname, '..', '..') + const patchesDir = join(root, 'patches', 'ds-bot') + const stored = readdirSync(patchesDir).filter(name => /^\d{4}-.*\.patch$/.test(name)).sort() + assert.equal(stored.length, 10) + const viaGit = mkdtempSync(join(tmpdir(), 'patch-git-')) + const viaOurs = mkdtempSync(join(tmpdir(), 'patch-ours-')) + try { + for (const dir of [viaGit, viaOurs]) { + cpSync(join(root, 'ds-bot'), dir, { recursive: true }) + cpSync(join(patchesDir, 'files'), dir, { recursive: true }) + } + for (const name of stored) { + execFileSync('git', ['apply', '--whitespace=nowarn', '-p1', join(patchesDir, name)], { cwd: viaGit, stdio: 'pipe' }) + applyPatchText(readFileSync(join(patchesDir, name), 'utf8'), viaOurs) + } + const list = dir => { + const out = [] + const walk = abs => { + for (const entry of readdirSync(abs, { withFileTypes: true })) { + const path = join(abs, entry.name) + if (entry.isDirectory()) { walk(path); continue } + out.push(relative(dir, path).split(sep).join('/')) + } + } + walk(dir) + return out.sort() + } + const files = list(viaGit) + assert.deepEqual(list(viaOurs), files, 'both appliers produced the same file list') + for (const file of files) { + assert.equal(readFileSync(join(viaOurs, file)).equals(readFileSync(join(viaGit, file))), true, file) + assert.equal(statSync(join(viaOurs, file)).mode & 0o777, statSync(join(viaGit, file)).mode & 0o777, `${file} mode`) + } + } finally { + rmSync(viaGit, { recursive: true, force: true }) + rmSync(viaOurs, { recursive: true, force: true }) + } + }) +} diff --git a/docs/subsystems/ds-bot-core.md b/docs/subsystems/ds-bot-core.md index 91e5e6c43..f41b2e73f 100644 --- a/docs/subsystems/ds-bot-core.md +++ b/docs/subsystems/ds-bot-core.md @@ -55,6 +55,7 @@ holds. Task bookkeeping does not imply runtime execution or tool permission. 1. `ds-bot/` stays pristine; CI fails if it differs from the pin. 2. `core/` == `ds-bot/` + `patches/ds-bot/files/` + `patches/ds-bot/*.patch`; regenerate after every `core/` change. A new file under `core/` (for example `test/new-chat.test.js`) is stored whole in `patches/ds-bot/files/` by `generate` and needs no `topics.json` entry; an upstream file edited for the first time must be listed in `patches/ds-bot/topics.json`. Otherwise `scripts/ds-bot-patches.mjs check` fails with “committed but not produced by the patches”. The format has no `index` lines or cross-hunk line numbers so parallel PRs merge cleanly (see `patches/ds-bot/README.md`). + `check` also runs on a machine without `git` (#978), through the script's own applier at `git apply`'s strictness: a hunk body must carry the counts its header declares, must match at the old-side line that header names (no search for nearby context — the loose applier that did let `#934`'s extra blank line and `#966`'s 662nd line through was only caught by CI), may not run past the end of the file, and honours `\ No newline at end of file`. There the pin prints `skipped pin (no git): the tree hash cannot be computed here` instead of passing, and `generate` still needs git because it asks git for the diff. CI keeps the git path as the reference, and a test builds the series with both appliers and compares the trees. 3. `client.js` matches `core/src/client/`; `npm run build` + `--check`. ### FlyBot branding topic diff --git a/patches/ds-bot/README.md b/patches/ds-bot/README.md index 7bb6644ab..699c5f407 100644 --- a/patches/ds-bot/README.md +++ b/patches/ds-bot/README.md @@ -32,6 +32,26 @@ running `generate` (see the conflict recipe in [AGENTS.md](../../AGENTS.md)). `c patch `0008-client-bundle` are generated from `core/src/client/` and still conflict whenever two branches change the client; that is a known follow-up (build the bundle instead of patching it). +### What `check` holds the series to (#978) + +`scripts/ds-bot-patches.mjs check` applies the series with `git apply` where git exists, and with its own +applier where it does not (the machine the Bots work on has no `git`, and `github.com:443` does not +resolve there). Both hold the same rules, and a test runs both over every patch and compares the trees +byte for byte: + +- a hunk body carries exactly the line counts its header declares on each side — that is `#934`'s extra + blank context line; +- every hunk matches at the old-side line its header names. The applier never searches for the context + nearby, so a header that is off by a line fails instead of writing the file one line off (`#966`); +- no hunk runs past the end of the file on either side; +- `\ No newline at end of file` is honoured on both sides, and a missing final newline survives; +- `-p1` strip, whole-file create/delete and `new file mode` work as with `git apply`. + +The old-side start is exact because the pristine upstream fixes it; the new-side start is only a position +hint (see above) and is not used to place a hunk. On a machine without git the pin is skipped out loud +(`skipped pin (no git): the tree hash cannot be computed here`) instead of passing quietly, and `generate` +still refuses to run because it asks git for the diff. `apply` and `check` work either way. + | Patch | Topic | |---|---| | `0001-core-fixes` | Generic core fixes (diagnostics, memory, models, prompt, time context, turns, text, i18n, model menu); upstream candidates | diff --git a/scripts/ds-bot-patches.mjs b/scripts/ds-bot-patches.mjs index e7de5a2d1..4faa4d37b 100644 --- a/scripts/ds-bot-patches.mjs +++ b/scripts/ds-bot-patches.mjs @@ -17,8 +17,16 @@ // Upgrading the pin: replace ds-bot/ with upstream's ds-bot/ at the new commit, update // ds-bot.pin.json (commit, tree = `git rev-parse :ds-bot` upstream, version), run // `apply`; fix each patch that fails (edit core/ by hand), then `generate`, `check`, test. +// +// Without git (#978): `check` and `apply` still run, through a strict applier of our own that holds +// the same rules git apply does — a hunk body must carry the counts its header declares, must match +// the file AT the old-side line the header names (no "find this context nearby"), may not run past the +// end of the file, and honours `\ No newline at end of file`. Tracked files come from a walk with the +// root .gitignore rules, and the pin is skipped out loud instead of passing silently. A machine with +// git keeps the git path: CI is the reference, and the test compares both appliers on every patch. +// `generate` needs git either way, because it asks git for the diff. import { execFileSync } from 'node:child_process' -import { cpSync, existsSync, mkdtempSync, mkdirSync, readFileSync, readdirSync, rmSync, statSync, writeFileSync } from 'node:fs' +import { chmodSync, cpSync, existsSync, mkdtempSync, mkdirSync, readFileSync, readdirSync, rmSync, statSync, writeFileSync } from 'node:fs' import { tmpdir } from 'node:os' import { dirname, join, relative } from 'node:path' import { fileURLToPath } from 'node:url' @@ -30,10 +38,184 @@ const CORE = 'core' const PATCHES = join(ROOT, 'patches', 'ds-bot') const FILES = join(PATCHES, 'files') // whole files FlyBot adds (or replaces, when binary): copied over ds-bot/ before the patches apply const git = (args, opts = {}) => execFileSync('git', args, { cwd: ROOT, encoding: 'utf8', maxBuffer: 1 << 28, ...opts }) -const tracked = dir => git(['ls-files', '-z', '--', dir]).split('\0').filter(Boolean).map(f => relative(dir, f)) +const hasGit = (() => { try { execFileSync('git', ['--version'], { stdio: 'ignore' }); return true } catch { return false } })() const series = () => readdirSync(PATCHES).filter(f => /^\d{4}-.*\.patch$/.test(f)).sort() const fail = (message) => { console.error(`ds-bot-patches: ${message}`); process.exit(1) } +// The tracked files of a directory. git answers on a machine that has it; without git the walk below +// answers, and the root .gitignore decides what is not in the repository. The walk is a list of what +// is on disk, which is what `ls-files` reports too before a commit, apart from ignored files. +const tracked = dir => (hasGit ? git(['ls-files', '-z', '--', dir]).split('\0').filter(Boolean).map(f => relative(dir, f)) : walkTracked(dir)) + +const globToRegExp = glob => new RegExp(`^${glob.split('*').map(part => part.replace(/[.*+?^${}()|[\]\\]/g, '\\$&')).join('[^/]*')}$`) + +// The root .gitignore, read the way a walk needs it: no `/` in a rule matches a path segment (so +// `node_modules/` prunes it at any depth), a `/` anchors the rule to the root, a trailing `/` means +// directories only, and `!` re-includes. The repository has no nested ignore files. +function ignoreRules() { + const file = join(ROOT, '.gitignore') + if (!existsSync(file)) return [] + return readFileSync(file, 'utf8').split('\n').map(line => line.trim()).filter(line => line && !line.startsWith('#')) + .map(line => { + const negate = line.startsWith('!') + let pattern = negate ? line.slice(1) : line + const dirOnly = pattern.endsWith('/') + if (dirOnly) pattern = pattern.slice(0, -1) + const anchored = pattern.includes('/') + if (anchored) pattern = pattern.replace(/^\/+/, '') + return { negate, dirOnly, anchored, re: globToRegExp(pattern) } + }) +} + +function isIgnored(rules, rel, isDir) { + const segments = rel.split('/') + const ancestors = [] + let prefix = '' + for (const segment of segments.slice(0, -1)) { prefix = prefix ? `${prefix}/${segment}` : segment; ancestors.push(prefix) } + let ignored = false + for (const rule of rules) { + const hit = rule.anchored + ? rule.re.test(rel) || (rule.dirOnly && ancestors.some(p => rule.re.test(p))) + : (rule.dirOnly ? (isDir ? segments : segments.slice(0, -1)) : segments).some(segment => rule.re.test(segment)) + if (hit) ignored = !rule.negate + } + return ignored +} + +function walkTracked(dir) { + const rules = ignoreRules() + const out = [] + const walk = (abs, rel) => { + for (const entry of readdirSync(abs, { withFileTypes: true })) { + if (entry.name === '.git') continue + const child = `${rel}/${entry.name}` + // A symlink to a directory is a file to git, but a directory to an ignore rule (`node_modules/`). + const directory = entry.isDirectory() || (entry.isSymbolicLink() && statSync(join(abs, entry.name), { throwIfNoEntry: false })?.isDirectory() === true) + if (isIgnored(rules, child, directory)) continue + if (entry.isDirectory()) walk(join(abs, entry.name), child) + else out.push(child.slice(dir.length + 1)) + } + } + walk(join(ROOT, dir), dir) + return out +} + +// A file's text as lines that remember whether the last one ends with a newline, so `\ No newline at +// end of file` has somewhere to land. +const toLines = text => { + if (text === '') return [] + const ends = text.endsWith('\n') + const lines = (ends ? text.slice(0, -1) : text).split('\n').map(lineText => ({ text: lineText, noEol: false })) + if (!ends) lines[lines.length - 1].noEol = true + return lines +} + +// git-free applier for the stored series (#978), at `git apply`'s strictness: `-p1` strip, whole-file +// create/delete, and one rule per hunk that is checked rather than searched for. Used only when git is +// absent; the test compares it against git apply on every patch so the two cannot drift. +export function applyPatchText(text, dest) { + const rows = text.split('\n') + const sections = [] + let section = null + for (const row of rows) { + if (row.startsWith('diff --git ')) { section = [row]; sections.push(section); continue } + if (section) section.push(row) + } + if (!sections.length) throw new Error('the patch has no `diff --git` section') + const strip = line => { + const raw = line.slice(4).split('\t')[0].trim() + if (raw === '/dev/null') return null + const parts = raw.split('/') + if (parts.length < 2 || (parts[0] !== 'a' && parts[0] !== 'b')) throw new Error(`cannot strip -p1 from ${line}`) + return parts.slice(1).join('/') + } + for (const sectionRows of sections) { + const oldHeader = sectionRows.find(row => row.startsWith('--- ')) + const newHeader = sectionRows.find(row => row.startsWith('+++ ')) + if (!oldHeader || !newHeader) throw new Error(`a file header is missing in ${sectionRows[0]}`) + const oldPath = strip(oldHeader) + const newPath = strip(newHeader) + const file = newPath ?? oldPath + const target = join(dest, file) + const modeLine = sectionRows.find(row => /^new file mode \d+$/.test(row)) + const before = oldPath && existsSync(target) ? toLines(readFileSync(target, 'utf8')) : [] + const after = [] + let cursor = 0 + let hunkNumber = 0 + for (let h = 0; h < sectionRows.length; h++) { + const head = /^@@ -(\d+)(?:,(\d+))? \+(\d+)(?:,(\d+))? @@/.exec(sectionRows[h]) + if (!head) continue + hunkNumber++ + const oldStart = Number(head[1]) + const oldCount = head[2] === undefined ? 1 : Number(head[2]) + const newCount = head[4] === undefined ? 1 : Number(head[4]) + const name = `${file} hunk ${hunkNumber} (@@ -${oldStart},${oldCount} +${head[3]},${newCount} @@)` + const body = [] + let j = h + 1 + for (; j < sectionRows.length; j++) { + const row = sectionRows[j] + if (row.startsWith('@@ ') || row.startsWith('diff --git ')) break + if (row === '' && j === sectionRows.length - 1) break // the patch's own final newline + if (row[0] !== ' ' && row[0] !== '+' && row[0] !== '-' && row[0] !== '\\') throw new Error(`${name}: not a hunk line: ${JSON.stringify(row.slice(0, 60))}`) + body.push(row) + } + h = j - 1 + // 1. The body must carry exactly the counts the header declares (#934's shape: one blank line too many). + let oldSeen = 0 + let newSeen = 0 + for (const row of body) { + if (row[0] === ' ') { oldSeen++; newSeen++ } + else if (row[0] === '-') oldSeen++ + else if (row[0] === '+') newSeen++ + } + if (oldSeen !== oldCount || newSeen !== newCount) throw new Error(`${name}: the body has ${oldSeen} old and ${newSeen} new line(s), the header declares ${oldCount} and ${newCount}`) + // 2. The hunk must land where its header says, and 3. it may not run past the end of the file (#966's shape). + const at = oldCount === 0 ? oldStart : oldStart - 1 + if (at < cursor) throw new Error(`${name}: starts before the end of the previous hunk`) + if (at + oldCount > before.length) throw new Error(`${name}: runs past the end of the file (${before.length} lines)`) + for (let k = cursor; k < at; k++) after.push({ ...before[k] }) + cursor = at + for (let b = 0; b < body.length; b++) { + const row = body[b] + if (row[0] === '\\') { + if (row !== '\\ No newline at end of file') throw new Error(`${name}: unknown marker ${JSON.stringify(row)}`) + const previous = body[b - 1] + if (!previous) throw new Error(`${name}: a marker with nothing before it`) + if (previous[0] === '+') { + if (!after.length) throw new Error(`${name}: a marker with nothing before it`) + after[after.length - 1].noEol = true + continue + } + const last = before[cursor - 1] + if (!last || !last.noEol) throw new Error(`${name}: says "No newline at end of file" after a line that has one`) + last.marked = true + if (previous[0] === ' ') after[after.length - 1].noEol = true + continue + } + const lineText = row.slice(1) + if (row[0] === '+') { after.push({ text: lineText, noEol: false }); continue } + const old = before[cursor] + if (!old) throw new Error(`${name}: line ${cursor + 1} does not exist in ${file}`) + if (old.text !== lineText) throw new Error(`${name}: ${row[0] === ' ' ? 'context' : 'removed line'} at ${cursor + 1} is ${JSON.stringify(old.text)}, the hunk has ${JSON.stringify(lineText)}`) + if (row[0] === ' ') after.push({ ...old }) + cursor++ + } + } + for (let k = cursor; k < before.length; k++) after.push({ ...before[k] }) + // What the patch says about the old file's last newline must be what that file is. + if (before.length) { + const last = before[before.length - 1] + if (last.noEol && !last.marked && cursor >= before.length) throw new Error(`${file}: the last line has no newline; the hunk must mark it`) + } + for (let k = 0; k < after.length - 1; k++) if (after[k].noEol) throw new Error(`${file}: only the last line may lack a newline`) + if (!newPath) { rmSync(target, { force: true }); continue } + const content = after.map((line, k) => (k === after.length - 1 ? line.text + (line.noEol ? '' : '\n') : `${line.text}\n`)).join('') + mkdirSync(dirname(target), { recursive: true }) + writeFileSync(target, content) + if (modeLine) chmodSync(target, parseInt(modeLine.split(' ').pop(), 8)) + } +} + // ds-bot/'s tracked files, copied as they are on disk, then files/ over them, then each patch applied in order. function build(out) { rmSync(join(ROOT, out), { recursive: true, force: true }) @@ -46,6 +228,10 @@ function build(out) { cpSync(join(FILES, file), join(ROOT, out, file)) } for (const patch of series()) { + if (!hasGit) { + try { applyPatchText(readFileSync(join(PATCHES, patch), 'utf8'), join(ROOT, out)) } catch (error) { fail(`${patch} does not apply cleanly:\n${error.message}`) } + continue + } try { git(['apply', '--whitespace=nowarn', `--directory=${out}`, '-p1', join(PATCHES, patch)], { stdio: ['ignore', 'pipe', 'pipe'] }) } catch (error) { fail(`${patch} does not apply cleanly:\n${error.stderr || error.message}`) } @@ -53,6 +239,11 @@ function build(out) { } function checkPin() { + if (!hasGit) { + // Not a pass: the tree hash cannot be computed here, so say what was skipped (#978). + console.log(`skipped pin (no git): the tree hash cannot be computed here`) + return + } // The tree of ds-bot/ as staged (equals HEAD in CI), so the check also works before a commit. const tree = git(['ls-tree', git(['write-tree']).trim(), UPSTREAM]).split(/\s+/)[2] if (tree !== PIN.tree) fail(`${UPSTREAM}/ is tree ${tree}, the pin says ${PIN.tree} (${PIN.repo}@${PIN.commit.slice(0, 7)}). Do not edit ${UPSTREAM}/; change patches/ds-bot/.`) @@ -132,12 +323,16 @@ function diffReproduces(file, diff, hadUpstream) { cpSync(join(ROOT, UPSTREAM, file), join(dir, file)) } writeFileSync(join(dir, 'one.patch'), diff) - execFileSync('git', ['apply', '--whitespace=nowarn', '-p1', 'one.patch'], { cwd: dir, stdio: 'ignore' }) + if (hasGit) execFileSync('git', ['apply', '--whitespace=nowarn', '-p1', 'one.patch'], { cwd: dir, stdio: 'ignore' }) + else applyPatchText(diff, dir) return existsSync(join(ROOT, CORE, file)) ? existsSync(join(dir, file)) && readFileSync(join(dir, file)).equals(readFileSync(join(ROOT, CORE, file))) : !existsSync(join(dir, file)) } catch { return false } finally { rmSync(dir, { recursive: true, force: true }) } } function generate() { + // The diff itself comes from git. Without git there is nothing to ask for it: the git-free applier + // (#978) is a validator, not a diff writer. + if (!hasGit) fail('generate needs git: it asks git for the diff of ds-bot/ against core/. Without git, edit core/ and patches/ds-bot/ together and run check.') const topics = JSON.parse(readFileSync(join(PATCHES, 'topics.json'), 'utf8')) const upstream = new Set(tracked(UPSTREAM)) const core = new Set(tracked(CORE))