diff --git a/src/main.js b/src/main.js index 0bdfde8..c7e0455 100644 --- a/src/main.js +++ b/src/main.js @@ -29,6 +29,8 @@ const { buildMenuTemplate } = require('./menu'); const { killChildTree } = require('./kill-tree'); const { normalizeEol } = require('./git-update.cjs'); const { ensureAutocrlf, readTrunkInfo, collectDirtyFiles, discardChanges, updateToLatestTrunk } = require('./trunk-update'); +const { applyPatchToDir } = require('./patch-apply'); +const { parsePatchFiles, planApply } = require('./patch-plan.cjs'); const { openExternalUrl, ALLOWED_URL_SCHEMES } = require('./external-url'); const { deleteRegisteredSite } = require('./site-registry'); const { getStore } = require('./settings-store'); @@ -421,6 +423,11 @@ ipcMain.handle('git:worktree-dirty', async (_e, sitePath) => { ipcMain.handle('git:discard-changes', async (_e, sitePath) => { try { await discardChanges(sitePath); + // Clearing the applied-patch record belongs with the reset that removed + // the patch from the tree — not with the trunk update that may follow and + // fail on the network, which would leave a revert banner for a patch that + // is already gone. + await mergeSiteMeta(sitePath, { appliedPatch: null }); return { ok: true }; } catch (e) { return { ok: false, error: String(e) }; @@ -440,14 +447,18 @@ ipcMain.handle('git:update-trunk', async (event, sitePath) => { (async () => { try { const result = await updateToLatestTrunk({ dir: sitePath, url: WORDPRESS_GIT_URL, onLog: sendLog }); + // An update resets the worktree, so any applied patch is gone with + // it either way — clear the record so the "applied" banner does not + // outlive the patch. (This is also where a discard's cleanup lands: + // the dirty-tree modal always discards and then updates.) if (result.upToDate) { - await mergeSiteMeta(sitePath, { trunkOid: result.oldOid, trunkDate: result.trunkDate }); + await mergeSiteMeta(sitePath, { trunkOid: result.oldOid, trunkDate: result.trunkDate, appliedPatch: null }); } else { // HEAD has moved but install/build have not run yet: persist // the incomplete flag now so the state survives a crash or // quit mid-chain; the renderer clears it after a successful // build. - await mergeSiteMeta(sitePath, { trunkOid: result.newOid, trunkDate: result.trunkDate, updateIncomplete: true }); + await mergeSiteMeta(sitePath, { trunkOid: result.newOid, trunkDate: result.trunkDate, updateIncomplete: true, appliedPatch: null }); } sendDone({ ok: true, ...result }); } catch (e) { @@ -467,6 +478,146 @@ ipcMain.handle('git:update-trunk', async (event, sitePath) => { return { updateId }; }); +// --- Applying someone else's patch (#11) --- the diff mechanics live in +// src/patch-apply.js; these handlers add IPC plumbing and electron-store writes. + +// A patch big enough to bloat the settings file is not worth keeping around for +// an undo button. Above this the patch still applies, only "Revert" is not +// offered — said out loud rather than silently dropped. +const REVERTABLE_PATCH_LIMIT = 512 * 1024; + +// Reading a patch without touching the checkout, so the contributor sees which +// files it would change — and which of their own edits it collides with — +// before deciding. +ipcMain.handle('git:preview-patch', async (_e, sitePath, patchText) => { + try { + const parsed = parsePatchFiles(patchText); + if (!parsed.ok) return { ok: false, error: parsed.error }; + let dirtyPaths; + try { + dirtyPaths = await collectDirtyFiles(sitePath); + } catch (e) { + // Failing open here would promise "no collisions" precisely when the + // app could not look — surface the failure instead. + logError('git:preview-patch', String(e && e.stack ? e.stack : e)); + return { ok: false, error: 'Could not check your working tree for conflicts, so the preview was not shown.' }; + } + const plan = planApply({ files: parsed.files, dirtyPaths }); + return { ok: true, ...plan, files: parsed.files.map((f) => ({ kind: f.kind, path: f.path })) }; + } catch (e) { + return { ok: false, error: String(e) }; + } +}); + +ipcMain.handle('dialog:choose-patch-file', async () => { + const result = await dialog.showOpenDialog({ + title: 'Choose a patch file', + properties: ['openFile'], + filters: [ + { name: 'Patch Files', extensions: ['patch', 'diff'] }, + { name: 'All Files', extensions: ['*'] } + ] + }); + if (result.canceled || result.filePaths.length === 0) return null; + const filePath = result.filePaths[0]; + try { + const text = await fs.promises.readFile(filePath, 'utf8'); + return { filePath, name: path.basename(filePath), text }; + } catch (e) { + return { filePath, error: String(e) }; + } +}); + +ipcMain.handle('git:apply-patch', async (event, sitePath, options = {}) => { + const applyId = `${Date.now()}-${Math.random().toString(16).slice(2)}`; + const sender = event.sender; + const sendLog = (data) => { + try { sender.send('git:apply-patch:log', { applyId, data }); } catch {} + }; + const sendDone = (payload) => { + try { sender.send('git:apply-patch:done', { applyId, ...payload }); } catch {} + }; + + (async () => { + try { + const reverse = Boolean(options.reverse); + // Reverting reads the patch the app stored when it applied it, so + // the renderer never has to hold a copy of the text. + let patchText = String(options.patchText || ''); + let label = String(options.label || 'patch'); + const s = await getStore(); + // sitePath crosses IPC untrusted and becomes the root for patch + // writes, so it has to be a site the app actually manages — the same + // gate sites:set-ticket applies before it touches metadata. + if (!(s.get('sites') || []).includes(sitePath)) { + sendDone({ ok: false, error: 'Site is not registered' }); + return; + } + const stored = ((s.get('siteMeta') || {})[sitePath] || {}).appliedPatch; + if (reverse) { + if (!stored || !stored.text) { + sendDone({ ok: false, error: 'There is no stored patch to revert.' }); + return; + } + patchText = stored.text; + label = stored.label || label; + } else if (stored) { + // Only one patch is tracked at a time, so a second apply would + // make the first one silently unrevertable and invisible. + sendDone({ ok: false, error: `${stored.label} is already applied. Revert it before applying another patch.` }); + return; + } + sendLog(`\n${reverse ? 'Reverting' : 'Applying'} ${label}…\n`); + + const result = await applyPatchToDir({ dir: sitePath, patchText, reverse, onLog: sendLog }); + if (!result.ok) { + sendDone({ ok: false, ...result }); + return; + } + + if (reverse) { + await mergeSiteMeta(sitePath, { appliedPatch: null }); + } else { + const revertable = patchText.length <= REVERTABLE_PATCH_LIMIT; + if (!revertable) { + sendLog('This patch is too large to keep for an undo, so Revert will not be offered.\n'); + } + try { + await mergeSiteMeta(sitePath, { + appliedPatch: { + label, + appliedAt: new Date().toISOString(), + files: result.applied, + text: revertable ? patchText : null + } + }); + } catch (persistErr) { + // Persistence is part of the transaction: the patch is on disk + // but its revert record could not be saved, so undo the apply + // rather than leave a patch the app cannot revert. If the undo + // also fails, say so plainly instead of reporting a clean fail. + logError('git:apply-patch', `persist failed, undoing apply: ${String(persistErr && persistErr.stack ? persistErr.stack : persistErr)}`); + const undo = await applyPatchToDir({ dir: sitePath, patchText, reverse: true, onLog: sendLog }); + const why = String(persistErr && persistErr.message ? persistErr.message : persistErr); + if (undo.ok) { + sendDone({ ok: false, error: `The patch applied but its revert record could not be saved, so it was undone. ${why}` }); + } else { + sendDone({ ok: false, appliedButUntracked: true, files: result.applied, error: `The patch applied but its revert record could not be saved and it could not be undone — the checkout has the patch and the app cannot revert it. ${why}` }); + } + return; + } + } + sendDone({ ok: true, ...result, reverse }); + } catch (e) { + logError('git:apply-patch', String(e && e.stack ? e.stack : e)); + sendLog(`\nApplying the patch failed: ${String(e && e.message ? e.message : e)}\n`); + sendDone({ ok: false, error: String(e) }); + } + })(); + + return { applyId }; +}); + ipcMain.handle('sites:mark-update-complete', async (_e, sitePath) => { await mergeSiteMeta(sitePath, { updateIncomplete: false }); return true; @@ -546,9 +697,20 @@ ipcMain.handle('site:status', async (_e, sitePath) => { } } catch {} - return { hasNodeModules, hasBuilt, skipInitWizard: Boolean(m.skipInitWizard), initialized: Boolean(m.initialized), installFailed: Boolean(m.installFailed), trunkOid, trunkDate, updateIncomplete: Boolean(m.updateIncomplete), tracTicket: m.tracTicket || null }; + // Summarised rather than passed through: the stored patch text is only + // needed by the main process to reverse it, and this is polled. + const appliedPatch = m.appliedPatch + ? { + label: m.appliedPatch.label, + appliedAt: m.appliedPatch.appliedAt, + files: m.appliedPatch.files || [], + revertable: Boolean(m.appliedPatch.text) + } + : null; + + return { hasNodeModules, hasBuilt, skipInitWizard: Boolean(m.skipInitWizard), initialized: Boolean(m.initialized), installFailed: Boolean(m.installFailed), trunkOid, trunkDate, updateIncomplete: Boolean(m.updateIncomplete), tracTicket: m.tracTicket || null, appliedPatch }; } catch { - return { hasNodeModules: false, hasBuilt: false, skipInitWizard: false, initialized: false, installFailed: false, trunkOid: null, trunkDate: null, updateIncomplete: false, tracTicket: null }; + return { hasNodeModules: false, hasBuilt: false, skipInitWizard: false, initialized: false, installFailed: false, trunkOid: null, trunkDate: null, updateIncomplete: false, tracTicket: null, appliedPatch: null }; } }); diff --git a/src/patch-apply.js b/src/patch-apply.js new file mode 100644 index 0000000..814dddc --- /dev/null +++ b/src/patch-apply.js @@ -0,0 +1,322 @@ +'use strict'; + +/** + * Applying someone else's patch to a checkout (issue #11). + * + * There is no `git apply` available: the app never shells out to a git binary, + * and isomorphic-git has no apply primitive. So hunks are matched and written + * here, using the `diff` package the app already bundles for the generating + * side. + * + * The rule that shapes everything below is **all or nothing**. A patch that + * half-applies is worse than one that does not apply at all: the contributor + * would build it, test it, and draw conclusions from a tree that matches + * neither trunk nor the patch. Every file is resolved in memory first — and + * because a write can still fail on the way out (a directory that is really a + * file, a read-only attribute, Windows holding a file open, a full disk), the + * previous contents are captured during resolution and restored if any write + * throws. + */ + +const fs = require('fs'); +const path = require('path'); +const JsDiff = require('diff'); +const { ensureAutocrlf } = require('./trunk-update'); +const { normalizeEol } = require('./git-update.cjs'); +const { parsePatchFiles } = require('./patch-plan.cjs'); + +/** + * Resolves a patch's repo-relative path inside the site directory, refusing + * anything that climbs out of it. A patch is untrusted input downloaded from a + * ticket, and `../../` in a header would otherwise write anywhere on disk. + * + * `path.resolve` normalises `..` but not symlinks, and a checkout contains + * plenty of those, so the deepest ancestor that exists is realpath-ed before + * the comparison. Otherwise a path leading through a symlinked directory + * passes a purely lexical check and lands outside the site folder. + * + * @param {string} dir + * @param {string} relPath + * @return {string|null} Absolute path, or null if it escapes the directory. + */ +function resolveInside(dir, relPath) { + let root; + try { root = fs.realpathSync(path.resolve(dir)); } catch { root = path.resolve(dir); } + + const lexical = path.resolve(root, relPath); + // Walk up to the nearest existing entry, resolve that for real, then put the + // not-yet-existing remainder back on. lstat, not existsSync: existsSync + // follows links and reports a dangling symlink as absent, so the walk would + // step past a symlink pointing outside the checkout and hand back its lexical + // path — which the writer would then follow out of the tree. + let existing = lexical; + const trailing = []; + const entryExists = (p) => { try { fs.lstatSync(p); return true; } catch { return false; } }; + while (!entryExists(existing)) { + const parent = path.dirname(existing); + if (parent === existing) break; + trailing.unshift(path.basename(existing)); + existing = parent; + } + // The deepest existing entry is realpath-ed for real. A symlink here that + // dangles (realpath throws) or resolves outside root is an escape, not a + // path to write through — fail closed rather than falling back to lexical. + let realExisting; + try { realExisting = fs.realpathSync(existing); } catch { return null; } + const abs = path.join(realExisting, ...trailing); + + if (abs !== root && !abs.startsWith(root + path.sep)) return null; + return abs; +} + +/** + * The line ending a file already uses, so applying a patch does not silently + * rewrite a genuinely-CRLF file to LF. wordpress-develop carries fixtures whose + * line endings are the thing under test. + * + * @param {string} text + * @return {string} + */ +function dominantEol(text) { + const crlf = (text.match(/\r\n/g) || []).length; + if (!crlf) return '\n'; + const lf = (text.match(/\n/g) || []).length; + return crlf * 2 >= lf ? '\r\n' : '\n'; +} + +/** + * Inverts one parsed file so the patch can be undone. + * + * Reversing through `formatPatch` would be simpler but wrong: it emits the + * headers swapped (`--- b/…`, `+++ a/…`), which no longer look like a git + * patch to the prefix-stripping in patch-plan.cjs, and the paths come back + * carrying a literal `b/`. Reversing the hunks while keeping the paths already + * resolved on the way in avoids the round trip entirely. + * + * @param {Object} file + * @return {Object} + */ +function reverseFile(file) { + const reversed = JsDiff.reversePatch(file.patch); + const INVERSE = { add: 'delete', delete: 'add' }; + const kind = INVERSE[file.kind] || file.kind; + const oldPath = file.newPath; + const newPath = file.oldPath; + return { + ...file, + kind, + oldPath, + newPath, + path: kind === 'delete' ? oldPath : newPath, + hunks: reversed.hunks || [], + patch: reversed + }; +} + +/** + * Works out what one file's new content should be, without touching disk. + * Also captures what is there now, so a failed write can be rolled back. + * + * @param {string} dir + * @param {Object} file + * @return {Object} + */ +function resolveFile(dir, file) { + const target = resolveInside(dir, file.path); + if (!target) return { error: `${file.path} points outside the site folder` }; + + if (file.kind === 'delete') { + if (!fs.existsSync(target)) return { error: `${file.path} is already gone, so the patch cannot remove it` }; + const previous = fs.readFileSync(target); + // Validate the file still matches what the patch expects to remove, so an + // edit made after the preview fails all-or-nothing rather than being + // silently deleted with the contributor's changes in it. + if (file.hunks && file.hunks.length + && JsDiff.applyPatch(normalizeEol(previous.toString('utf8')), file.patch) === false) { + return { error: `${file.path} has moved on since the patch was written, so it no longer applies` }; + } + return { op: 'delete', abs: target, path: file.path, previous }; + } + + if (file.kind === 'add') { + if (fs.existsSync(target)) return { error: `${file.path} already exists, so the patch cannot add it` }; + const content = JsDiff.applyPatch('', file.patch); + if (content === false) return { error: `${file.path} could not be created from the patch` }; + return { op: 'write', abs: target, path: file.path, content, previous: null }; + } + + if (file.kind === 'rename') { + const source = resolveInside(dir, file.oldPath); + if (!source) return { error: `${file.oldPath} points outside the site folder` }; + if (!fs.existsSync(source)) return { error: `${file.oldPath} is not in this checkout, so the patch cannot move it` }; + if (fs.existsSync(target)) return { error: `${file.newPath} already exists, so the patch cannot move ${file.oldPath} onto it` }; + const originalBuf = fs.readFileSync(source); + // A 100%-similarity rename has no hunks: the bytes move unchanged, so they + // are carried as a Buffer. Git emits binary renames with no binary marker, + // so this path is reachable for them — decoding through utf8 would corrupt + // the file. Decode to text only when hunks actually need applying. + let content = originalBuf; + if (file.hunks.length) { + const original = originalBuf.toString('utf8'); + const applied = JsDiff.applyPatch(normalizeEol(original), file.patch); + if (applied === false) { + return { error: `${file.oldPath} has moved on since the patch was written, so it no longer applies` }; + } + content = applied.replace(/\n/g, dominantEol(original) === '\r\n' ? '\r\n' : '\n'); + } + return { + op: 'rename', abs: target, from: source, path: file.path, content, + previous: null, previousFrom: originalBuf + }; + } + + if (!fs.existsSync(target)) return { error: `${file.path} is not in this checkout, so the patch does not fit it` }; + + // Matching happens on LF, the way the generating side normalises, so a CRLF + // checkout does not make every context line miss — but the file is written + // back with the endings it already had. + const raw = fs.readFileSync(target, 'utf8'); + const applied = JsDiff.applyPatch(normalizeEol(raw), file.patch); + if (applied === false) { + return { error: `${file.path} has moved on since the patch was written, so it no longer applies` }; + } + const content = dominantEol(raw) === '\r\n' ? applied.replace(/\n/g, '\r\n') : applied; + return { op: 'write', abs: target, path: file.path, content, previous: Buffer.from(raw, 'utf8') }; +} + +/** + * Puts back everything a failed run had already written. + * + * Returns the paths it could not restore. The same full-disk, lock, or + * permission condition that broke a write can also break its undo, and the + * caller must not claim a clean restore when the tree is actually unknown. + * + * @param {Array} done + * @return {Array} paths whose rollback failed (empty when fully restored) + */ +function rollback(done) { + const errors = []; + // Removing something that was never created — already gone, or its parent is + // not even a directory — is the desired end state, not a failure. Actions are + // registered before they mutate (so a half-done one is still undoable), which + // means rollback can see ones that never ran; only a content restoration that + // cannot be written back is a real, unrecoverable loss. + const removeQuietly = (target) => { + try { fs.rmSync(target, { force: true }); } + catch (e) { if (!e || (e.code !== 'ENOTDIR' && e.code !== 'ENOENT')) throw e; } + }; + for (const action of done.reverse()) { + try { + if (action.op === 'rename' && action.previousFrom !== null) { + fs.mkdirSync(path.dirname(action.from), { recursive: true }); + fs.writeFileSync(action.from, action.previousFrom); + removeQuietly(action.abs); + continue; + } + if (action.previous === null) { + removeQuietly(action.abs); + continue; + } + fs.mkdirSync(path.dirname(action.abs), { recursive: true }); + fs.writeFileSync(action.abs, action.previous); + } catch (e) { + errors.push(`${action.path}: ${String(e && e.message ? e.message : e)}`); + } + } + return errors; +} + +/** + * Applies (or reverses) a patch across a checkout. + * + * Binary files are skipped and named rather than failing the whole patch: a + * text diff cannot carry their content, and refusing an otherwise-good pull + * request over an image would help nobody. Everything else is all or nothing. + * + * @param {Object} root0 + * @param {string} root0.dir + * @param {string} root0.patchText + * @param {boolean} [root0.reverse] + * @param {Function} [root0.onLog] + * @return {Promise} + */ +async function applyPatchToDir({ dir, patchText, reverse = false, onLog = () => {} }) { + const parsed = parsePatchFiles(patchText); + if (!parsed.ok) return { ok: false, error: parsed.error }; + + await ensureAutocrlf(dir); + + const files = reverse ? parsed.files.map(reverseFile) : parsed.files; + const actions = []; + const skipped = []; + const failures = []; + + for (const file of files) { + if (file.kind === 'binary') { + skipped.push(file.path); + continue; + } + const resolved = resolveFile(dir, file); + if (resolved.error) { + failures.push(resolved.error); + continue; + } + actions.push(resolved); + } + + if (failures.length) { + onLog(`\nThe patch was not applied — the checkout is unchanged.\n${failures.map((f) => ` • ${f}\n`).join('')}`); + return { ok: false, error: failures[0], failures, applied: [], skipped }; + } + + if (!actions.length && !skipped.length) { + return { ok: false, error: 'The patch does not change any files.', applied: [], skipped }; + } + + // Every file resolved cleanly, so the writes below are the first thing to + // touch the working tree — and the only place a partial result could still + // appear, which is what the rollback is for. + const done = []; + try { + for (const action of actions) { + // Registered before its mutations, not after: a rename that writes its + // destination and then throws removing the source would otherwise be + // invisible to rollback and leave a partial patch behind. Undoing an + // action whose mutations had not started yet is harmless. + done.push(action); + if (action.op === 'delete') { + fs.rmSync(action.abs, { force: true }); + } else if (action.op === 'rename') { + fs.mkdirSync(path.dirname(action.abs), { recursive: true }); + fs.writeFileSync(action.abs, action.content); + fs.rmSync(action.from, { force: true }); + } else { + fs.mkdirSync(path.dirname(action.abs), { recursive: true }); + fs.writeFileSync(action.abs, action.content); + } + } + } catch (e) { + const recovery = rollback(done); + const message = `writing ${String(e && e.message ? e.message : e)}`; + if (recovery.length) { + onLog(`\nThe patch could not be written, and the checkout could not be fully put back — it is in an unknown state. Could not undo: ${recovery.join('; ')}\n`); + return { ok: false, error: message, applied: [], skipped, rolledBack: false, recovery }; + } + onLog(`\nThe patch could not be written, so the checkout was put back as it was: ${message}\n`); + return { ok: false, error: message, applied: [], skipped, rolledBack: true }; + } + + // New files are deliberately left unstaged. Staging them is what leaves the + // residue that updateToLatestTrunk has to clear before a force checkout + // (see staleStagedPaths in git-update.cjs), and an unstaged new file still + // shows up in the patch the contributor generates afterwards. + const applied = actions.map((a) => a.path); + onLog(`\n${reverse ? 'Reverted' : 'Applied'} ${applied.length} file${applied.length === 1 ? '' : 's'}.\n`); + if (skipped.length) { + onLog(`Skipped ${skipped.length} binary file${skipped.length === 1 ? '' : 's'} the app cannot apply: ${skipped.join(', ')}\n`); + } + + return { ok: true, applied, skipped }; +} + +module.exports = { applyPatchToDir, resolveInside, reverseFile, dominantEol, rollback }; diff --git a/src/patch-plan.cjs b/src/patch-plan.cjs new file mode 100644 index 0000000..cf7d47e --- /dev/null +++ b/src/patch-plan.cjs @@ -0,0 +1,247 @@ +'use strict'; + +/** + * Reading a patch before applying it (issue #11): what files it touches, where + * those files live in today's checkout, and what applying it would disturb. + * + * Patches reach the app from three places that do not agree on a format: + * attachments on a Trac ticket (Subversion style, no `a/` `b/` prefixes, paths + * sometimes against the pre-`src/` layout), `.diff` files from a + * wordpress-develop pull request (git style, `a/` `b/` prefixes), and the app's + * own generated patches (`createTwoFilesPatch` output, `a/` `b/` prefixes but + * no `diff --git` line). Everything downstream works in repo-relative paths, so + * normalising happens here, once. + * + * Lives at `src/` rather than `src/renderer/` because both the main process and + * the applier consume it — same placement as git-update.cjs — and it keeps the + * `diff` package out of the renderer bundle. The renderer-facing half of this + * feature (the step chain) is in renderer/update-plan.cjs instead. + */ + +const JsDiff = require('diff'); +const { normalizeEol } = require('./git-update.cjs'); + +// Files that stayed at the repo root when core moved everything else under +// src/. A patch naming one of these is already correct for today's layout. +const ROOT_FILES = [ + '.editorconfig', + '.gitignore', + '.jshintrc', + '.travis.yml', + 'Gruntfile.js', + 'package.json', + 'phpunit.xml.dist', + 'wp-cli.yml', + 'wp-config-sample.php', + 'wp-tests-config-sample.php' +]; + +// Files that did move under src/ despite not being wp-* prefixed, so the +// wp-* rule below would miss them. +const SRC_FILES = ['index.php', 'license.txt', 'readme.html', 'xmlrpc.php']; + +// Directories that only ever existed in the modern layout. +const MODERN_DIRS = ['src/', 'tests/', 'tools/']; + +/** + * Strips the leading `a/` and `b/` that git puts on diff headers. + * + * Deliberately conditional: Subversion-style patches from Trac carry no prefix + * at all, so stripping unconditionally would turn `wp-admin/admin.php` into + * `admin.php` and write to the wrong place. Both sides have to look prefixed + * before either is trusted — `/dev/null` counts as agreement, since an added or + * deleted file only has one real side. + * + * @param {string} oldName + * @param {string} newName + * @return {{oldPath: string, newPath: string}} + */ +function stripPathPrefix(oldName, newName) { + const isNull = (name) => name === '/dev/null'; + const looksPrefixed = (name, letter) => isNull(name) || new RegExp(`^${letter}/`).test(name); + const bothPrefixed = looksPrefixed(oldName, 'a') && looksPrefixed(newName, 'b'); + const drop = (name) => (isNull(name) || !bothPrefixed ? name : name.slice(2)); + // `Index: trunk/wp-…` is what an older Subversion checkout produced; the + // branch name is not part of the repo-relative path either way. + const dropTrunk = (name) => (isNull(name) ? name : name.replace(/^trunk\//, '')); + return { oldPath: dropTrunk(drop(oldName)), newPath: dropTrunk(drop(newName)) }; +} + +/** + * Rewrites a path written against the pre-src/ layout to where that file lives + * today. A patch attached to a ticket years ago still names `wp-admin/…`. + * + * @param {string} filePath + * @return {string} + */ +function mapToSrcLayout(filePath) { + if (!filePath || filePath === '/dev/null') return filePath; + if (MODERN_DIRS.some((dir) => filePath.startsWith(dir))) return filePath; + if (ROOT_FILES.includes(filePath)) return filePath; + if (SRC_FILES.includes(filePath)) return `src/${filePath}`; + if (filePath.startsWith('wp-')) return `src/${filePath}`; + // Unrecognised: leave it alone rather than guess a move that would write + // outside the tree the contributor expects. + return filePath; +} + +/** + * Walks the raw patch for its per-file section headers. + * + * Needed because jsdiff returns the byte-identical shape `[{hunks: []}]` for a + * binary file, a 100%-similarity rename, and text that is not a patch at all — + * it keeps neither the filenames nor the marker that tells them apart. Without + * this, prose pasted in would be reported as a binary file and a pure rename + * would be rejected as garbage. + * + * @param {string} raw + * @return {Array<{path: string, isBinary: boolean, renameFrom: string, renameTo: string}>} + */ +function scanSections(raw) { + const sections = []; + const last = () => sections[sections.length - 1]; + for (const line of raw.split('\n')) { + const git = /^diff --git (?:"?a\/)?(.+?)"? (?:"?b\/)?(.+?)"?$/.exec(line); + if (git) { + sections.push({ path: git[2] || git[1], isBinary: false, renameFrom: '', renameTo: '' }); + continue; + } + const svn = /^Index: (.+)$/.exec(line); + if (svn) { + sections.push({ path: svn[1].trim(), isBinary: false, renameFrom: '', renameTo: '' }); + continue; + } + if (!sections.length) continue; + if (/^Binary files .* differ$/.test(line) || /^GIT binary patch$/.test(line)) { + last().isBinary = true; + continue; + } + const from = /^rename from (.+)$/.exec(line); + if (from) { last().renameFrom = from[1].trim(); continue; } + const to = /^rename to (.+)$/.exec(line); + if (to) last().renameTo = to[1].trim(); + } + return sections; +} + +/** + * @param {Object} file A jsdiff parsePatch entry. + * @param {string} oldPath + * @param {string} newPath + * @return {string} + */ +function classify(file, oldPath, newPath) { + if (oldPath === '/dev/null') return 'add'; + if (newPath === '/dev/null') return 'delete'; + if (oldPath !== newPath) return 'rename'; + return 'modify'; +} + +/** + * Parses a patch into the files it touches, with paths normalised to + * repo-relative form for today's layout. + * + * @param {string} text + * @return {{ok: true, files: Array}|{ok: false, error: string}} + */ +function parsePatchFiles(text) { + const raw = typeof text === 'string' ? text : ''; + if (!raw.trim()) return { ok: false, error: 'The patch is empty.' }; + + let parsed; + try { + // Normalise line endings on the way in so hunk context matches what the + // applier reads off disk, which is normalised the same way. + parsed = JsDiff.parsePatch(normalizeEol(raw)); + } catch (e) { + return { ok: false, error: `Could not read the patch: ${String(e && e.message ? e.message : e)}` }; + } + + if (!parsed || parsed.length === 0) { + return { ok: false, error: 'No file changes found in the patch.' }; + } + + const sections = scanSections(normalizeEol(raw)); + const files = []; + + for (let i = 0; i < parsed.length; i++) { + const file = parsed[i]; + + if (!file.hunks || file.hunks.length === 0) { + // jsdiff kept nothing, so the raw section is the only evidence of + // what this was. + const section = sections[i]; + if (section && section.renameFrom && section.renameTo) { + const oldPath = mapToSrcLayout(section.renameFrom); + const newPath = mapToSrcLayout(section.renameTo); + files.push({ kind: 'rename', oldPath, newPath, path: newPath, hunks: [], patch: file }); + continue; + } + if (section && section.isBinary) { + const binaryPath = mapToSrcLayout(stripPathPrefix(section.path, section.path).newPath); + files.push({ kind: 'binary', oldPath: binaryPath, newPath: binaryPath, path: binaryPath, hunks: [], patch: file }); + continue; + } + return { ok: false, error: 'That does not look like a patch — no file changes found.' }; + } + + const oldName = file.oldFileName || file.index || ''; + const newName = file.newFileName || file.index || ''; + const { oldPath, newPath } = stripPathPrefix(oldName, newName); + const kind = classify(file, oldPath, newPath); + const target = kind === 'delete' ? oldPath : newPath; + files.push({ + kind, + oldPath: mapToSrcLayout(oldPath), + newPath: mapToSrcLayout(newPath), + path: mapToSrcLayout(target), + hunks: file.hunks, + patch: file + }); + } + + return { ok: true, files }; +} + +/** + * What applying these files would mean for a given checkout. + * + * `conflicts` is the honest version of the dirty-tree question: applying a + * patch is not destructive the way a hard reset is, so the only changes worth + * mentioning are the ones on files the contributor has already edited. + * + * @param {Object} root0 + * @param {Array} root0.files + * @param {string[]} [root0.dirtyPaths] + * @return {{paths: string[], conflicts: string[], unsupported: string[], needsInstall: boolean}} + */ +function planApply({ files, dirtyPaths = [] } = {}) { + const list = Array.isArray(files) ? files : []; + const paths = list.map((f) => f.path).filter(Boolean); + // A rename disturbs the file it moves away from as well as the one it + // creates, so both sides count when looking for collisions. + const touched = new Set(paths); + for (const f of list) { + if (f.kind === 'rename' && f.oldPath) touched.add(f.oldPath); + } + const dirty = new Set(dirtyPaths); + return { + paths, + conflicts: [...touched].filter((p) => dirty.has(p)), + // Binary hunks cannot be applied from a text diff. Naming them is the + // difference between "this patch is partly unapplied" and a silent gap. + unsupported: list.filter((f) => f.kind === 'binary').map((f) => f.path || '(unnamed binary file)'), + // Same rule the trunk update uses (#94): the lockfile moving is what + // makes an install necessary rather than merely possible. + needsInstall: touched.has('package-lock.json') + }; +} + +module.exports = { + ROOT_FILES, + SRC_FILES, + stripPathPrefix, + mapToSrcLayout, + parsePatchFiles, + planApply +}; diff --git a/src/preload.js b/src/preload.js index 261b7d9..2c23dc7 100644 --- a/src/preload.js +++ b/src/preload.js @@ -77,6 +77,27 @@ contextBridge.exposeInMainWorld('api', { discardChanges: (sitePath) => ipcRenderer.invoke('git:discard-changes', sitePath) , markUpdateComplete: (sitePath) => ipcRenderer.invoke('sites:mark-update-complete', sitePath) +, + choosePatchFile: () => ipcRenderer.invoke('dialog:choose-patch-file') +, + previewPatch: (sitePath, patchText) => ipcRenderer.invoke('git:preview-patch', sitePath, patchText) +, + applyPatch: async (sitePath, options, onLog, onDone) => { + const { applyId } = await ipcRenderer.invoke('git:apply-patch', sitePath, options); + const logHandler = (_e, payload) => { + if (payload.applyId === applyId && onLog) onLog(payload); + }; + const doneHandler = (_e, payload) => { + if (payload.applyId === applyId) { + ipcRenderer.removeListener('git:apply-patch:log', logHandler); + ipcRenderer.removeListener('git:apply-patch:done', doneHandler); + if (onDone) onDone(payload); + } + }; + ipcRenderer.on('git:apply-patch:log', logHandler); + ipcRenderer.on('git:apply-patch:done', doneHandler); + return { applyId }; + } , updateTrunk: async (sitePath, onLog, onDone) => { const { updateId } = await ipcRenderer.invoke('git:update-trunk', sitePath); diff --git a/src/renderer/index.jsx b/src/renderer/index.jsx index 5c4d766..f701219 100644 --- a/src/renderer/index.jsx +++ b/src/renderer/index.jsx @@ -19,7 +19,7 @@ import 'xterm/css/xterm.css'; import { computeSetupStepState } from './setup-steps.cjs'; import { planDevServerStart, formatElapsed } from './dev-server-command.cjs'; import { pathBasename } from './path-basename.cjs'; -import { trunkAgeInfo, planUpdateSteps, updateStepStatuses, SKIP_INSTALL_MESSAGE } from './update-plan.cjs'; +import { trunkAgeInfo, planUpdateSteps, updateStepStatuses, SKIP_INSTALL_MESSAGE, planApplySteps, APPLY_STATE_TO_STEP } from './update-plan.cjs'; import { parseTicketRef, ticketUrl } from './trac-ticket.cjs'; const TERMINAL_ALLOWED_SCRIPTS = ['build', 'build:dev', 'dev', 'test', 'watch', 'grunt']; @@ -811,6 +811,14 @@ function SiteRow({ sitePath, initialized, createdAt, label, onInitialized, onSit const [trunkDate, setTrunkDate] = useState(null); const [updateIncomplete, setUpdateIncomplete] = useState(false); const [updateState, setUpdateState] = useState('idle'); // idle | fetching | installing | building + // Applying someone else's patch (#11) + const [applyState, setApplyState] = useState('idle'); // idle | applying | installing | building + const [applyPreview, setApplyPreview] = useState(null); + // Held separately from applyPreview: the preview is cleared the moment the + // chain starts, and the step list still has to know whether install runs. + const [applyNeedsInstall, setApplyNeedsInstall] = useState(false); + const [applyError, setApplyError] = useState(''); + const [appliedPatch, setAppliedPatch] = useState(null); const [dirtyModalOpen, setDirtyModalOpen] = useState(false); const [dirtySaving, setDirtySaving] = useState(false); const [dirtyFiles, setDirtyFiles] = useState([]); @@ -938,6 +946,7 @@ function SiteRow({ sitePath, initialized, createdAt, label, onInitialized, onSit setTrunkDate(s?.trunkDate || null); setUpdateIncomplete(Boolean(s?.updateIncomplete)); setTracTicket(s?.tracTicket || null); + setAppliedPatch(s?.appliedPatch || null); if (metaPatchRef.current) { // A null trunkDate here means the git read failed (e.g. clone still // running) — keep whatever the sidebar already shows in that case. @@ -1539,6 +1548,113 @@ function SiteRow({ sitePath, initialized, createdAt, label, onInitialized, onSit } }; + // --- Applying someone else's patch (#11) --- + // Same three-stage shape as the update chain, and the same npm wrappers, so + // exit codes and terminal streaming behave identically. + const isApplying = applyState !== 'idle'; + const applySteps = planApplySteps({ needsInstall: applyNeedsInstall }); + const applyStepStates = updateStepStatuses(applySteps, applyState, APPLY_STATE_TO_STEP); + + const finishApply = (message) => { + terminalStateRef.current.running = false; + terminalKillRef.current = null; + setApplyState('idle'); + if (message) writeToTerminal(message); + loadStatus().catch(() => {}); + }; + + const runApplyInstallAndBuild = (needsInstall, verb) => { + const runBuildStep = () => { + setApplyState('building'); + writeToTerminal('\nRunning npm run build…\n'); + runScript('build', { + onLog: (chunk) => writeToTerminal(chunk), + onDone: ({ code }) => { + finishApply(code === 0 + ? `\n${verb} — open the site to try it out.\n` + : `\nThe patch is ${verb.toLowerCase()} but the build failed, so the site still runs the old assets.\n`); + } + }); + }; + if (needsInstall) { + setApplyState('installing'); + writeToTerminal('\nThe patch changes package-lock.json — running npm install…\n'); + runInstall({ + onLog: (chunk) => writeToTerminal(chunk), + onDone: ({ code }) => { + if (code !== 0) { + finishApply('\nnpm install failed, so the build was skipped. The patch is applied but dependencies are stale.\n'); + return; + } + runBuildStep(); + } + }); + } else { + writeToTerminal(`\n${SKIP_INSTALL_MESSAGE}\n`); + runBuildStep(); + } + }; + + // Reads a patch file and works out what it would do, without touching the + // checkout — the contributor decides after seeing the file list. + const choosePatchFile = async () => { + setApplyError(''); + try { + const chosen = await window.api.choosePatchFile(); + if (!chosen) return; + if (chosen.error) { + setApplyError(`Could not read that file: ${chosen.error}`); + return; + } + const preview = await window.api.previewPatch(sitePath, chosen.text); + if (!preview || !preview.ok) { + setApplyError(preview?.error || 'Could not read that patch.'); + return; + } + setApplyPreview({ ...preview, label: chosen.name, text: chosen.text }); + } catch (e) { + setApplyError(String(e)); + } + }; + + const runApply = ({ reverse = false } = {}) => { + const state = terminalStateRef.current; + if (state.running) { + writeToTerminal('A command is already running. Press Ctrl+C to stop it.\n'); + return; + } + const preview = applyPreview; + const needsInstall = reverse + ? Boolean(appliedPatch?.files?.includes('package-lock.json')) + : Boolean(preview.needsInstall); + setApplyError(''); + setApplyNeedsInstall(needsInstall); + setApplyState('applying'); + state.running = true; + // Same contract as the other chains: while `running` is set, Ctrl+C in the + // terminal has to reach the child process the chain is about to spawn. + terminalKillRef.current = () => { killCurrent().catch(() => {}); }; + window.api.applyPatch( + sitePath, + reverse ? { reverse: true } : { patchText: preview.text, label: preview.label }, + ({ data }) => writeToTerminal(data), + (res) => { + if (!res || !res.ok) { + setApplyError(res?.error || 'The patch could not be applied.'); + finishApply(); + return; + } + setApplyPreview(null); + runApplyInstallAndBuild(needsInstall, reverse ? 'Reverted' : 'Applied'); + } + ).catch((e) => { + // A rejected invoke never reaches onDone, so without this the terminal + // stays wedged with `running` set and no way back short of a reload. + setApplyError(String(e)); + finishApply(); + }); + }; + // Step 1: fetch + reset in the main process, then hand over to the npm // steps. Assumes the tree is clean (startTrunkUpdate handles dirty trees). const beginTrunkUpdate = () => { @@ -2123,6 +2239,88 @@ function SiteRow({ sitePath, initialized, createdAt, label, onInitialized, onSit )} + {skipInit ? ( +
+
Try someone else's patch
+
+ Apply a .diff or .patch file to this checkout and rebuild, so you can test the work before adding your own. Your own changes are left alone. +
+ + {appliedPatch && !isApplying ? ( +
+
+ {appliedPatch.label} is applied — {appliedPatch.files.length} file{appliedPatch.files.length === 1 ? '' : 's'} + {appliedPatch.appliedAt ? `, ${new Date(appliedPatch.appliedAt).toLocaleString()}` : ''}. +
+
+ {appliedPatch.revertable ? ( + + ) : ( + Too large to undo automatically — use Update to latest trunk to reset. + )} +
+
+ ) : null} + + {applyPreview && !isApplying ? ( +
+
+ {applyPreview.label} changes {applyPreview.paths.length} file{applyPreview.paths.length === 1 ? '' : 's'}: +
+
+ {applyPreview.paths.map((p) =>
{p}
)} +
+ {applyPreview.conflicts.length ? ( +
+ You have your own edits to {applyPreview.conflicts.join(', ')}. The patch is applied on top of them: it succeeds if the changes do not overlap, and fails without touching anything if they do. Save a patch of your work first if you want a copy. +
+ ) : null} + {applyPreview.unsupported.length ? ( +
+ {applyPreview.unsupported.join(', ')} {applyPreview.unsupported.length === 1 ? 'is a binary file and will be skipped' : 'are binary files and will be skipped'}. +
+ ) : null} + {applyPreview.needsInstall ? ( +
It changes package-lock.json, so dependencies will be installed before the rebuild.
+ ) : null} +
+ + +
+
+ ) : null} + + {isApplying ? ( +
+ {applyStepStates.map((state, i) => { + const step = applySteps[i]; + const mark = UPDATE_STEP_MARKS[state.status]; + const stepLabel = state.status === 'skipped' ? step.skipMessage : step.label; + return ( +
+ + {stepLabel} +
+ ); + })} +
+ ) : null} + + {applyError ? ( +
+ {applyError} The checkout was not changed. +
+ ) : null} + + {!applyPreview && !isApplying ? ( +
+ +
+ ) : null} +
+ ) : null}
Terminal
diff --git a/src/renderer/update-plan.cjs b/src/renderer/update-plan.cjs index 186a8ac..e2e4567 100644 --- a/src/renderer/update-plan.cjs +++ b/src/renderer/update-plan.cjs @@ -66,16 +66,43 @@ function planUpdateSteps({ lockfileChanged } = {}) { // Which chain step each renderer updateState is executing. const STATE_TO_STEP = { fetching: 'fetch', installing: 'install', building: 'build' }; +// Applying someone else's patch (#11) is the same three-stage chain with a +// different first step, so it shares updateStepStatuses below. It lives here +// rather than beside the patch parsing because this module is the renderer's +// half and carries no dependencies — importing the parser into the bundle +// would drag the `diff` package in for two constants. +const APPLY_STATE_TO_STEP = { applying: 'apply', installing: 'install', building: 'build' }; + +/** + * The chain applying a patch runs. Like the update chain, the install step is + * named even when skipped so "apply" always means the same thing. + * + * @param {Object} root0 + * @param {boolean} [root0.needsInstall] + * @return {Array} + */ +function planApplySteps({ needsInstall } = {}) { + return [ + { key: 'apply', label: 'Apply the patch', skipped: false }, + { key: 'install', label: 'Install dependencies', skipped: !needsInstall, skipMessage: SKIP_INSTALL_MESSAGE }, + { key: 'build', label: 'Rebuild', skipped: false } + ]; +} + /** * Maps the chain steps to checklist visual states for a given renderer * updateState. Steps before the current one are complete, the current one is * current, later ones pending; skipped steps stay 'skipped' once passed. * + * The state→step map is a parameter so a different chain can reuse this: the + * applying chain (#11) has the same three-stage shape with its own state names. + * * @param {Array} steps * @param {string} updateState + * @param {Object} [stateToStep] */ -function updateStepStatuses(steps, updateState) { - const activeKey = STATE_TO_STEP[updateState] || null; +function updateStepStatuses(steps, updateState, stateToStep = STATE_TO_STEP) { + const activeKey = stateToStep[updateState] || null; const order = steps.map((s) => s.key); let activeIndex = -1; if (activeKey) { @@ -116,6 +143,9 @@ function updateOutcome({ fetchOk, upToDate, moved, installNeeded, installCode, b module.exports = { STALE_THRESHOLD_DAYS, SKIP_INSTALL_MESSAGE, + STATE_TO_STEP, + APPLY_STATE_TO_STEP, + planApplySteps, trunkAgeInfo, planUpdateSteps, updateStepStatuses, diff --git a/test/ipc-wiring.test.cjs b/test/ipc-wiring.test.cjs index bbc0e2c..79edea9 100644 --- a/test/ipc-wiring.test.cjs +++ b/test/ipc-wiring.test.cjs @@ -444,12 +444,16 @@ test('git:worktree-dirty reports what trunk-update found, not its own guess', as }); }); -test('git:discard-changes goes through trunk-update', async () => { +test('git:discard-changes resets through trunk-update and clears the applied-patch record', async () => { const discardChanges = spy(async () => {}); - const main = loadMain({ stubs: { ...silentLogging(), './trunk-update': { discardChanges } } }); + const settings = fakeSettingsStore({ sites: ['/sites/wp'], siteMeta: { '/sites/wp': { appliedPatch: { label: 'x' } } } }); + const main = loadMain({ stubs: { ...silentLogging(), ...settings.stubs, './trunk-update': { discardChanges } } }); assert.deepEqual(await main.invoke('git:discard-changes', '/sites/wp'), { ok: true }); assert.deepEqual(discardChanges.calls, [['/sites/wp']]); + // The record is cleared with the reset, not left for a later trunk update to + // clear — otherwise a failed update leaves a revert banner for a gone patch. + assert.equal(settings.values.siteMeta['/sites/wp'].appliedPatch, null); }); test('git:update-trunk hands the update to trunk-update and streams its log back', async () => { @@ -952,6 +956,152 @@ test('sites:set-ticket refuses unregistered site paths before writing metadata', assert.deepEqual(settings.values.siteMeta, {}); }); +// --- apply handlers (#11) ------------------------------------------------ + +test('git:preview-patch reads the patch through patch-plan', async () => { + const parsePatchFiles = spy(() => ({ ok: false, error: 'unreadable' })); + const main = loadMain({ stubs: { ...silentLogging(), './patch-plan.cjs': { parsePatchFiles, planApply: () => ({}) } } }); + + const result = await main.invoke('git:preview-patch', '/sites/wp', 'PATCH TEXT'); + + assert.deepEqual(parsePatchFiles.calls, [['PATCH TEXT']]); + assert.deepEqual(result, { ok: false, error: 'unreadable' }); +}); + +// git:apply-patch reads the store for its guard before delegating, which is the +// seam fakeSettingsStore stands in for — so it is a wired handler, not a hole. +// It streams, so its result comes back on the :done channel, not the return. +async function applyDone(event, applyId, cap = 50) { + for (let i = 0; i < cap; i++) { + const hit = event.sent.find((m) => m.channel === 'git:apply-patch:done' && m.payload.applyId === applyId); + if (hit) return hit.payload; + await new Promise((r) => setImmediate(r)); + } + throw new Error('git:apply-patch never reported done'); +} + +test('git:apply-patch refuses an unregistered site path before touching patch-apply', async () => { + const applyPatchToDir = spy(async () => ({ ok: true, applied: [], skipped: [] })); + const settings = fakeSettingsStore({ sites: ['/sites/wp'] }); + const main = loadMain({ stubs: { ...silentLogging(), ...settings.stubs, './patch-apply': { applyPatchToDir } } }); + + const event = createIpcEvent(); + const { applyId } = await main.invokeWith('git:apply-patch', event, '/sites/unknown', { patchText: 'P' }); + + assert.deepEqual(await applyDone(event, applyId), { applyId, ok: false, error: 'Site is not registered' }); + assert.deepEqual(applyPatchToDir.calls, []); +}); + +test('git:apply-patch delegates a forward apply to patch-apply and records it', async () => { + const applyPatchToDir = spy(async () => ({ ok: true, applied: ['src/a.php'], skipped: [] })); + const settings = fakeSettingsStore({ sites: ['/sites/wp'] }); + const main = loadMain({ stubs: { ...silentLogging(), ...settings.stubs, './patch-apply': { applyPatchToDir } } }); + + const event = createIpcEvent(); + const { applyId } = await main.invokeWith('git:apply-patch', event, '/sites/wp', { patchText: 'PATCH', label: 'PR 11705' }); + const done = await applyDone(event, applyId); + + assert.equal(done.ok, true); + assert.equal(applyPatchToDir.calls.length, 1); + const [args] = applyPatchToDir.calls[0]; + assert.equal(args.dir, '/sites/wp'); + assert.equal(args.patchText, 'PATCH'); + assert.equal(args.reverse, false); + // The revert record is what makes Revert possible; without it the patch is + // applied but silently unrevertable. + const stored = settings.values.siteMeta['/sites/wp'].appliedPatch; + assert.equal(stored.label, 'PR 11705'); + assert.equal(stored.text, 'PATCH'); + assert.deepEqual(stored.files, ['src/a.php']); +}); + +test('git:apply-patch refuses a second patch while one is already applied', async () => { + const applyPatchToDir = spy(async () => ({ ok: true, applied: [], skipped: [] })); + const settings = fakeSettingsStore({ + sites: ['/sites/wp'], + siteMeta: { '/sites/wp': { appliedPatch: { label: 'first', text: 'X' } } } + }); + const main = loadMain({ stubs: { ...silentLogging(), ...settings.stubs, './patch-apply': { applyPatchToDir } } }); + + const event = createIpcEvent(); + const { applyId } = await main.invokeWith('git:apply-patch', event, '/sites/wp', { patchText: 'SECOND' }); + const done = await applyDone(event, applyId); + + assert.equal(done.ok, false); + assert.match(done.error, /already applied/); + assert.deepEqual(applyPatchToDir.calls, []); +}); + +test('git:apply-patch reverts using the stored patch text and clears the record', async () => { + const applyPatchToDir = spy(async () => ({ ok: true, applied: ['src/a.php'], skipped: [] })); + const settings = fakeSettingsStore({ + sites: ['/sites/wp'], + siteMeta: { '/sites/wp': { appliedPatch: { label: 'L', text: 'STORED' } } } + }); + const main = loadMain({ stubs: { ...silentLogging(), ...settings.stubs, './patch-apply': { applyPatchToDir } } }); + + const event = createIpcEvent(); + const { applyId } = await main.invokeWith('git:apply-patch', event, '/sites/wp', { reverse: true }); + const done = await applyDone(event, applyId); + + assert.equal(done.ok, true); + const [args] = applyPatchToDir.calls[0]; + assert.equal(args.reverse, true); + assert.equal(args.patchText, 'STORED', 'a revert applies the patch the app stored, not the renderer'); + assert.equal(settings.values.siteMeta['/sites/wp'].appliedPatch, null); +}); + +// A store whose siteMeta write throws: the patch lands on disk but its revert +// record cannot be saved. Persistence is part of the transaction. +function storeThatFailsToPersist() { + return { + get: (key) => (key === 'sites' ? ['/sites/wp'] : {}), + set: (key) => { if (key === 'siteMeta') throw new Error('disk full'); } + }; +} + +test('git:apply-patch undoes the apply when its revert record cannot be saved', async () => { + const applyPatchToDir = spy(async ({ reverse }) => ({ ok: true, applied: reverse ? [] : ['src/a.php'], skipped: [] })); + const main = loadMain({ + stubs: { + ...silentLogging(), + './settings-store': { getStore: async () => storeThatFailsToPersist() }, + './patch-apply': { applyPatchToDir } + } + }); + + const event = createIpcEvent(); + const { applyId } = await main.invokeWith('git:apply-patch', event, '/sites/wp', { patchText: 'PATCH', label: 'L' }); + const done = await applyDone(event, applyId); + + assert.equal(done.ok, false); + assert.match(done.error, /could not be saved/); + // The forward apply, then the undo — the tree is put back to match what the + // renderer is told rather than left with an unrevertable patch. + assert.deepEqual(applyPatchToDir.calls.map((c) => c[0].reverse), [false, true]); +}); + +test('git:apply-patch reports applied-but-untracked when the undo also fails', async () => { + const applyPatchToDir = spy(async ({ reverse }) => + reverse ? { ok: false, error: 'cannot undo' } : { ok: true, applied: ['src/a.php'], skipped: [] }); + const main = loadMain({ + stubs: { + ...silentLogging(), + './settings-store': { getStore: async () => storeThatFailsToPersist() }, + './patch-apply': { applyPatchToDir } + } + }); + + const event = createIpcEvent(); + const { applyId } = await main.invokeWith('git:apply-patch', event, '/sites/wp', { patchText: 'PATCH', label: 'L' }); + const done = await applyDone(event, applyId); + + assert.equal(done.ok, false); + assert.equal(done.appliedButUntracked, true); + assert.deepEqual(done.files, ['src/a.php']); + assert.match(done.error, /could not be undone/); +}); + // --- the harness's own guard --------------------------------------------- // Requiring the real `electron` package is not a harmless fallback: its @@ -1000,7 +1150,9 @@ const WIRED = new Set([ 'playground:stop', 'playground-web:start', 'playground-web:stop', - 'sites:set-ticket' + 'sites:set-ticket', + 'git:preview-patch', + 'git:apply-patch' ]); // Channels with no module to reach: they read or write electron-store, drive a @@ -1016,6 +1168,7 @@ const NO_DELEGATION = new Map([ ['sites:forget', 'electron-store write'], ['sites:set-label', 'electron-store write'], ['dialog:choose-dir', 'opens the directory dialog'], + ['dialog:choose-patch-file', 'opens the file-open dialog and reads the chosen file'], ['playground-web:available', 'checks a path on disk'], ['smtp:get', 'electron-store read'], ['smtp:clear', 'electron-store write'], diff --git a/test/patch-apply.integration.test.cjs b/test/patch-apply.integration.test.cjs new file mode 100644 index 0000000..6ec5799 --- /dev/null +++ b/test/patch-apply.integration.test.cjs @@ -0,0 +1,439 @@ +'use strict'; + +const test = require('node:test'); +const assert = require('node:assert'); +const fs = require('fs'); +const os = require('os'); +const path = require('path'); +const git = require('isomorphic-git'); +const { applyPatchToDir, resolveInside, dominantEol, rollback } = require('../src/patch-apply'); + +// A real on-disk repo, like trunk-update.integration.test.cjs: applyPatchToDir +// calls ensureAutocrlf, which reads and writes git config, so a bare temp +// directory would not exercise the same path. +async function makeRepo(t, files) { + const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'patch-apply-test-')); + t.after(() => fs.rmSync(dir, { recursive: true, force: true })); + await git.init({ fs, dir, defaultBranch: 'trunk' }); + for (const [relPath, content] of Object.entries(files)) { + const abs = path.join(dir, relPath); + fs.mkdirSync(path.dirname(abs), { recursive: true }); + fs.writeFileSync(abs, content); + await git.add({ fs, dir, filepath: relPath }); + } + await git.commit({ + fs, dir, message: 'base', + author: { name: 'Test', email: 'test@example.com' } + }); + return dir; +} + +// Snapshot every file so "nothing was written" can be asserted byte for byte +// rather than just on the one file we happen to think about. +function snapshot(dir) { + const out = {}; + const walk = (rel) => { + for (const entry of fs.readdirSync(path.join(dir, rel), { withFileTypes: true })) { + if (entry.name === '.git') continue; + const next = path.join(rel, entry.name); + if (entry.isDirectory()) walk(next); + else out[next] = fs.readFileSync(path.join(dir, next), 'utf8'); + } + }; + walk('.'); + return out; +} + +const FOO = 'src/wp-includes/foo.php'; +const BAR = 'src/wp-includes/bar.php'; +const FOO_BODY = 'one\ntwo\nthree\n'; +const BAR_BODY = 'alpha\nbeta\ngamma\n'; + +const FOO_PATCH = `diff --git a/${FOO} b/${FOO} +--- a/${FOO} ++++ b/${FOO} +@@ -1,3 +1,3 @@ + one +-two ++TWO + three +`; + +// Second file's context does not match what is on disk, so this hunk fails. +const BAR_PATCH_THAT_FAILS = `diff --git a/${BAR} b/${BAR} +--- a/${BAR} ++++ b/${BAR} +@@ -1,3 +1,3 @@ + nothing +-like ++LIKE + reality +`; + +test('applyPatchToDir: a single-file patch applies (issue #11)', async (t) => { + const dir = await makeRepo(t, { [FOO]: FOO_BODY }); + const res = await applyPatchToDir({ dir, patchText: FOO_PATCH }); + assert.strictEqual(res.ok, true); + assert.deepStrictEqual(res.applied, [FOO]); + assert.strictEqual(fs.readFileSync(path.join(dir, FOO), 'utf8'), 'one\nTWO\nthree\n'); +}); + +// The rule the whole module is built around. A patch where the second file +// fails must not leave the first one rewritten. +test('applyPatchToDir: one failing file leaves the whole tree untouched (issue #11)', async (t) => { + const dir = await makeRepo(t, { [FOO]: FOO_BODY, [BAR]: BAR_BODY }); + const before = snapshot(dir); + + const res = await applyPatchToDir({ dir, patchText: FOO_PATCH + BAR_PATCH_THAT_FAILS }); + + assert.strictEqual(res.ok, false); + assert.match(res.error, /bar\.php/); + assert.deepStrictEqual(res.applied, []); + assert.deepStrictEqual(snapshot(dir), before, 'no file may change when any file fails'); +}); + +test('applyPatchToDir: reverting restores the original content (issue #11)', async (t) => { + const dir = await makeRepo(t, { [FOO]: FOO_BODY }); + await applyPatchToDir({ dir, patchText: FOO_PATCH }); + const res = await applyPatchToDir({ dir, patchText: FOO_PATCH, reverse: true }); + assert.strictEqual(res.ok, true); + assert.strictEqual(fs.readFileSync(path.join(dir, FOO), 'utf8'), FOO_BODY); +}); + +// Reverting must undo the patch, not reset the checkout: work the contributor +// did on other files has to survive. +test('applyPatchToDir: reverting keeps unrelated local work (issue #11)', async (t) => { + const dir = await makeRepo(t, { [FOO]: FOO_BODY, [BAR]: BAR_BODY }); + await applyPatchToDir({ dir, patchText: FOO_PATCH }); + fs.writeFileSync(path.join(dir, BAR), 'my own work\n'); + + await applyPatchToDir({ dir, patchText: FOO_PATCH, reverse: true }); + + assert.strictEqual(fs.readFileSync(path.join(dir, FOO), 'utf8'), FOO_BODY); + assert.strictEqual(fs.readFileSync(path.join(dir, BAR), 'utf8'), 'my own work\n'); +}); + +test('applyPatchToDir: a patch creates and removes files (issue #11)', async (t) => { + const dir = await makeRepo(t, { [FOO]: FOO_BODY }); + const addPatch = `diff --git a/src/new.php b/src/new.php +new file mode 100644 +--- /dev/null ++++ b/src/new.php +@@ -0,0 +1,2 @@ ++hello ++world +`; + const addRes = await applyPatchToDir({ dir, patchText: addPatch }); + assert.strictEqual(addRes.ok, true); + assert.strictEqual(fs.readFileSync(path.join(dir, 'src/new.php'), 'utf8'), 'hello\nworld\n'); + + // Reversing an addition is a deletion. + await applyPatchToDir({ dir, patchText: addPatch, reverse: true }); + assert.strictEqual(fs.existsSync(path.join(dir, 'src/new.php')), false); +}); + +// A patch is untrusted input downloaded from a ticket. +test('applyPatchToDir: a path escaping the site folder is refused (issue #11)', async (t) => { + const dir = await makeRepo(t, { [FOO]: FOO_BODY }); + const outside = path.join(dir, '..', 'escaped.txt'); + const evil = `diff --git a/../escaped.txt b/../escaped.txt +new file mode 100644 +--- /dev/null ++++ b/../escaped.txt +@@ -0,0 +1 @@ ++pwned +`; + const res = await applyPatchToDir({ dir, patchText: evil }); + assert.strictEqual(res.ok, false); + assert.match(res.error, /outside the site folder/); + assert.strictEqual(fs.existsSync(outside), false); +}); + +test('applyPatchToDir: a missing target file fails without writing (issue #11)', async (t) => { + const dir = await makeRepo(t, { [FOO]: FOO_BODY }); + const before = snapshot(dir); + const res = await applyPatchToDir({ dir, patchText: BAR_PATCH_THAT_FAILS }); + assert.strictEqual(res.ok, false); + assert.match(res.error, /not in this checkout/); + assert.deepStrictEqual(snapshot(dir), before); +}); + +test('applyPatchToDir: binary files are skipped and named, not silently dropped (issue #11)', async (t) => { + const dir = await makeRepo(t, { [FOO]: FOO_BODY }); + const withBinary = FOO_PATCH + `diff --git a/src/x.png b/src/x.png +index 111..222 100644 +Binary files a/src/x.png and b/src/x.png differ +`; + const res = await applyPatchToDir({ dir, patchText: withBinary }); + assert.strictEqual(res.ok, true); + assert.deepStrictEqual(res.applied, [FOO]); + assert.deepStrictEqual(res.skipped, ['src/x.png']); +}); + +test('applyPatchToDir: an unreadable patch reports why and changes nothing (issue #11)', async (t) => { + const dir = await makeRepo(t, { [FOO]: FOO_BODY }); + const before = snapshot(dir); + const res = await applyPatchToDir({ dir, patchText: 'this is not a patch\n' }); + assert.strictEqual(res.ok, false); + assert.deepStrictEqual(snapshot(dir), before); +}); + +// Finding from self-review: the pre-validation rejection above is the path that +// cannot write by construction. This is the one that can — a write that throws +// partway through, after earlier files are already on disk. +test('applyPatchToDir: a write failing partway through is rolled back (issue #11)', async (t) => { + // src/blocker is a regular file, so creating src/blocker/new.php fails with + // ENOTDIR — deterministically, on every platform — after foo.php has + // already been written. + const dir = await makeRepo(t, { [FOO]: FOO_BODY, 'src/blocker': 'not a directory\n' }); + const before = snapshot(dir); + + const blockedAdd = `diff --git a/src/blocker/new.php b/src/blocker/new.php +new file mode 100644 +--- /dev/null ++++ b/src/blocker/new.php +@@ -0,0 +1 @@ ++hello +`; + + const res = await applyPatchToDir({ dir, patchText: FOO_PATCH + blockedAdd }); + + assert.strictEqual(res.ok, false); + assert.strictEqual(res.rolledBack, true); + assert.deepStrictEqual(snapshot(dir), before, 'a failed write must leave nothing behind'); +}); + +test('applyPatchToDir: a rename moves the file and its content (issue #11)', async (t) => { + const dir = await makeRepo(t, { 'src/old.php': 'one\ntwo\n' }); + const renamePatch = `diff --git a/src/old.php b/src/new.php +similarity index 90% +rename from src/old.php +rename to src/new.php +--- a/src/old.php ++++ b/src/new.php +@@ -1,2 +1,2 @@ + one +-two ++TWO +`; + const res = await applyPatchToDir({ dir, patchText: renamePatch }); + assert.strictEqual(res.ok, true, res.error); + assert.strictEqual(fs.existsSync(path.join(dir, 'src/old.php')), false); + assert.strictEqual(fs.readFileSync(path.join(dir, 'src/new.php'), 'utf8'), 'one\nTWO\n'); + + await applyPatchToDir({ dir, patchText: renamePatch, reverse: true }); + assert.strictEqual(fs.readFileSync(path.join(dir, 'src/old.php'), 'utf8'), 'one\ntwo\n'); + assert.strictEqual(fs.existsSync(path.join(dir, 'src/new.php')), false); +}); + +// A 100%-similarity rename has no hunks at all, which used to be rejected as +// "not a patch" — killing the whole apply for any PR that moved a file. +test('applyPatchToDir: a pure rename with no hunks applies (issue #11)', async (t) => { + const dir = await makeRepo(t, { 'src/old.php': 'unchanged\n' }); + const purePatch = `diff --git a/src/old.php b/src/new.php +similarity index 100% +rename from src/old.php +rename to src/new.php +`; + const res = await applyPatchToDir({ dir, patchText: purePatch }); + assert.strictEqual(res.ok, true, res.error); + assert.strictEqual(fs.readFileSync(path.join(dir, 'src/new.php'), 'utf8'), 'unchanged\n'); + assert.strictEqual(fs.existsSync(path.join(dir, 'src/old.php')), false); +}); + +test('applyPatchToDir: reverting a deletion puts the file back (issue #11)', async (t) => { + const dir = await makeRepo(t, { 'src/old.php': 'one\ntwo\n' }); + const deletePatch = `diff --git a/src/old.php b/src/old.php +deleted file mode 100644 +--- a/src/old.php ++++ /dev/null +@@ -1,2 +0,0 @@ +-one +-two +`; + assert.strictEqual((await applyPatchToDir({ dir, patchText: deletePatch })).ok, true); + assert.strictEqual(fs.existsSync(path.join(dir, 'src/old.php')), false); + + const back = await applyPatchToDir({ dir, patchText: deletePatch, reverse: true }); + assert.strictEqual(back.ok, true, back.error); + assert.strictEqual(fs.readFileSync(path.join(dir, 'src/old.php'), 'utf8'), 'one\ntwo\n'); +}); + +// wordpress-develop carries fixtures whose line endings are the thing under +// test; rewriting them to LF because a patch touched the file would corrupt +// exactly those. +test('applyPatchToDir: a CRLF file keeps CRLF after patching (issue #11)', async (t) => { + const dir = await makeRepo(t, { [FOO]: FOO_BODY.replace(/\n/g, '\r\n') }); + const res = await applyPatchToDir({ dir, patchText: FOO_PATCH }); + assert.strictEqual(res.ok, true, res.error); + assert.strictEqual(fs.readFileSync(path.join(dir, FOO), 'utf8'), 'one\r\nTWO\r\nthree\r\n'); +}); + +test('dominantEol: reports the ending a file actually uses (issue #11)', () => { + assert.strictEqual(dominantEol('a\nb\n'), '\n'); + assert.strictEqual(dominantEol('a\r\nb\r\n'), '\r\n'); + assert.strictEqual(dominantEol(''), '\n'); + // A mostly-LF file with one stray CRLF stays LF. + assert.strictEqual(dominantEol('a\nb\nc\r\nd\ne\n'), '\n'); +}); + +// resolveInside is exported so both the lexical and the symlink case can be +// exercised from one machine, the way win-spawn-patch.test.cjs does. +test('resolveInside: refuses paths that climb out, allows ones that stay in (issue #11)', async (t) => { + const dir = await makeRepo(t, { [FOO]: FOO_BODY }); + assert.notStrictEqual(resolveInside(dir, 'src/wp-includes/foo.php'), null); + assert.notStrictEqual(resolveInside(dir, 'src/does/not/exist/yet.php'), null); + assert.strictEqual(resolveInside(dir, '../escaped.txt'), null); + assert.strictEqual(resolveInside(dir, 'src/../../escaped.txt'), null); + assert.strictEqual(resolveInside(dir, path.resolve(os.tmpdir(), 'absolute.txt')), null); +}); + +// path.resolve normalises ".." but not symlinks, so a lexical-only check lets a +// patch write through a symlinked directory to anywhere on disk. +test('resolveInside: refuses a path leading through a symlink out of the tree (issue #11)', async (t) => { + const dir = await makeRepo(t, { [FOO]: FOO_BODY }); + const outside = fs.mkdtempSync(path.join(os.tmpdir(), 'patch-apply-outside-')); + t.after(() => fs.rmSync(outside, { recursive: true, force: true })); + fs.symlinkSync(outside, path.join(dir, 'escape-hatch'), 'dir'); + + assert.strictEqual(resolveInside(dir, 'escape-hatch/evil.txt'), null); +}); + +test('applyPatchToDir: a patch through a symlinked directory is refused (issue #11)', async (t) => { + const dir = await makeRepo(t, { [FOO]: FOO_BODY }); + const outside = fs.mkdtempSync(path.join(os.tmpdir(), 'patch-apply-outside-')); + t.after(() => fs.rmSync(outside, { recursive: true, force: true })); + fs.symlinkSync(outside, path.join(dir, 'escape-hatch'), 'dir'); + + const evil = `diff --git a/escape-hatch/evil.txt b/escape-hatch/evil.txt +new file mode 100644 +--- /dev/null ++++ b/escape-hatch/evil.txt +@@ -0,0 +1 @@ ++pwned +`; + const res = await applyPatchToDir({ dir, patchText: evil }); + assert.strictEqual(res.ok, false); + assert.match(res.error, /outside the site folder/); + assert.strictEqual(fs.existsSync(path.join(outside, 'evil.txt')), false); +}); + +// existsSync follows a symlink and is false for a dangling one, so a naive +// walk-up steps past a link pointing outside the checkout and hands back its +// lexical path — which writeFileSync would then follow out of the tree. lstat +// closes that hole. (Copilot #1.) +test('applyPatchToDir: an add through a dangling symlink out of the tree is refused (issue #11)', async (t) => { + const dir = await makeRepo(t, { [FOO]: FOO_BODY }); + const outsideDir = fs.mkdtempSync(path.join(os.tmpdir(), 'patch-apply-dangling-')); + t.after(() => fs.rmSync(outsideDir, { recursive: true, force: true })); + const outside = path.join(outsideDir, 'target.txt'); // never created → dangling + fs.symlinkSync(outside, path.join(dir, 'sneaky.txt')); + + const evil = `diff --git a/sneaky.txt b/sneaky.txt +new file mode 100644 +--- /dev/null ++++ b/sneaky.txt +@@ -0,0 +1 @@ ++pwned +`; + const res = await applyPatchToDir({ dir, patchText: evil }); + assert.strictEqual(res.ok, false); + assert.match(res.error, /outside the site folder/); + assert.strictEqual(fs.existsSync(outside), false, 'nothing may be written through the link'); +}); + +// A pure (100%-similarity) rename carries the bytes unchanged. Git emits binary +// renames with no binary marker, so reading the source as utf8 and writing the +// string back would corrupt it — the bytes must survive intact. (Copilot #5.) +test('applyPatchToDir: a pure rename preserves non-utf8 (binary) bytes (issue #11)', async (t) => { + const bytes = Buffer.from([0xff, 0xfe, 0x00, 0x01, 0x80, 0x0a]); + const dir = await makeRepo(t, { 'src/logo.bin': bytes }); + const purePatch = `diff --git a/src/logo.bin b/src/moved.bin +similarity index 100% +rename from src/logo.bin +rename to src/moved.bin +`; + const res = await applyPatchToDir({ dir, patchText: purePatch }); + assert.strictEqual(res.ok, true, res.error); + assert.ok(fs.readFileSync(path.join(dir, 'src/moved.bin')).equals(bytes), 'bytes must survive the rename'); + assert.strictEqual(fs.existsSync(path.join(dir, 'src/logo.bin')), false); +}); + +// A deletion with a pre-image must match what is on disk. If the contributor +// edited the file after previewing, deleting it anyway silently discards their +// work; all-or-nothing means failing instead. (Copilot #2.) +test('applyPatchToDir: deleting a file edited since the patch fails all-or-nothing (issue #11)', async (t) => { + const dir = await makeRepo(t, { 'src/old.php': 'one\ntwo\n' }); + const deletePatch = `diff --git a/src/old.php b/src/old.php +deleted file mode 100644 +--- a/src/old.php ++++ /dev/null +@@ -1,2 +0,0 @@ +-one +-two +`; + fs.writeFileSync(path.join(dir, 'src/old.php'), 'my own work\n'); + const before = snapshot(dir); + + const res = await applyPatchToDir({ dir, patchText: deletePatch }); + + assert.strictEqual(res.ok, false); + assert.match(res.error, /moved on since the patch was written/); + assert.deepStrictEqual(snapshot(dir), before, 'the edited file must not be deleted'); +}); + +// A rename that completes and is then undone by a later failure must restore the +// source and remove the destination — registering each action before its +// mutations is what lets rollback see a half-done one. (Copilot #3.) +test('applyPatchToDir: a later failure rolls a completed rename fully back (issue #11)', async (t) => { + const dir = await makeRepo(t, { 'src/old.php': 'one\ntwo\n', 'src/blocker': 'not a directory\n' }); + const before = snapshot(dir); + const renameThenBlocked = `diff --git a/src/old.php b/src/new.php +similarity index 100% +rename from src/old.php +rename to src/new.php +diff --git a/src/blocker/child.php b/src/blocker/child.php +new file mode 100644 +--- /dev/null ++++ b/src/blocker/child.php +@@ -0,0 +1 @@ ++hello +`; + const res = await applyPatchToDir({ dir, patchText: renameThenBlocked }); + + assert.strictEqual(res.ok, false); + assert.strictEqual(res.rolledBack, true); + assert.deepStrictEqual(snapshot(dir), before, 'the rename must be fully undone'); +}); + +// A rollback can hit the same fault that broke the write. rollback must report +// what it could not restore so the caller stops claiming a clean tree. Driven +// directly with an un-restorable action (its parent is a file → ENOTDIR), which +// fails the same way whether or not the tests run as root. (Copilot #4.) +test('rollback: reports the paths it could not restore instead of swallowing them (issue #11)', async (t) => { + const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'patch-apply-rollback-')); + t.after(() => fs.rmSync(dir, { recursive: true, force: true })); + fs.writeFileSync(path.join(dir, 'afile'), 'i am a file\n'); + + // Restoring this action means writing under `afile`, which is a file, not a + // directory — mkdirSync/writeFileSync throw ENOTDIR. + const recovery = rollback([ + { op: 'write', abs: path.join(dir, 'afile', 'child'), path: 'afile/child', previous: Buffer.from('x') } + ]); + + assert.ok(Array.isArray(recovery) && recovery.length === 1); + assert.match(recovery[0], /afile\/child/); +}); + +// The clean path still returns no recovery errors, so the caller reports a real +// rollback as one. +test('rollback: returns an empty list when it restores everything (issue #11)', async (t) => { + const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'patch-apply-rollback-ok-')); + t.after(() => fs.rmSync(dir, { recursive: true, force: true })); + fs.writeFileSync(path.join(dir, 'added'), 'new\n'); + + const recovery = rollback([{ op: 'write', abs: path.join(dir, 'added'), path: 'added', previous: null }]); + + assert.deepStrictEqual(recovery, []); + assert.strictEqual(fs.existsSync(path.join(dir, 'added')), false, 'an added file is removed on rollback'); +}); diff --git a/test/patch-plan.test.cjs b/test/patch-plan.test.cjs new file mode 100644 index 0000000..2cc01fe --- /dev/null +++ b/test/patch-plan.test.cjs @@ -0,0 +1,270 @@ +'use strict'; + +const test = require('node:test'); +const assert = require('node:assert'); +const { + stripPathPrefix, + mapToSrcLayout, + parsePatchFiles, + planApply +} = require('../src/patch-plan.cjs'); +const { updateStepStatuses, SKIP_INSTALL_MESSAGE, planApplySteps, APPLY_STATE_TO_STEP } = require('../src/renderer/update-plan.cjs'); + +// The four header shapes that actually reach the app. Kept verbatim rather than +// generated: the whole point of these tests is that real-world formatting — +// tabs before "(revision N)", missing a/ b/ prefixes, /dev/null sides — is +// handled, and a generator would only produce the shape we already expect. +const GITHUB_DIFF = `diff --git a/src/wp-includes/foo.php b/src/wp-includes/foo.php +index 384bf9e..dbfa038 100644 +--- a/src/wp-includes/foo.php ++++ b/src/wp-includes/foo.php +@@ -1,3 +1,3 @@ + a +-b ++B + c +`; + +const TRAC_SVN_DIFF = `Index: src/wp-includes/foo.php +=================================================================== +--- src/wp-includes/foo.php\t(revision 59234) ++++ src/wp-includes/foo.php\t(working copy) +@@ -1,3 +1,3 @@ + a +-b ++B + c +`; + +const OLD_LAYOUT_DIFF = `Index: wp-admin/admin.php +=================================================================== +--- wp-admin/admin.php\t(revision 1) ++++ wp-admin/admin.php\t(working copy) +@@ -1,2 +1,2 @@ + x +-y ++Y +`; + +const ADD_DIFF = `diff --git a/src/new.php b/src/new.php +new file mode 100644 +index 0000000..e69de29 +--- /dev/null ++++ b/src/new.php +@@ -0,0 +1,2 @@ ++one ++two +`; + +const DELETE_DIFF = `diff --git a/src/old.php b/src/old.php +deleted file mode 100644 +--- a/src/old.php ++++ /dev/null +@@ -1,2 +0,0 @@ +-one +-two +`; + +const BINARY_DIFF = `diff --git a/src/x.png b/src/x.png +index 111..222 100644 +Binary files a/src/x.png and b/src/x.png differ +`; + +test('parsePatchFiles: a GitHub pull request diff resolves to a repo-relative path (issue #11)', () => { + const res = parsePatchFiles(GITHUB_DIFF); + assert.strictEqual(res.ok, true); + assert.strictEqual(res.files.length, 1); + assert.strictEqual(res.files[0].path, 'src/wp-includes/foo.php'); + assert.strictEqual(res.files[0].kind, 'modify'); +}); + +// The bug this guards: Subversion patches carry no a/ b/ prefix, so stripping +// two characters unconditionally would write to "c/wp-includes/foo.php". +test('parsePatchFiles: a Subversion-style Trac attachment keeps its full path (issue #11)', () => { + const res = parsePatchFiles(TRAC_SVN_DIFF); + assert.strictEqual(res.ok, true); + assert.strictEqual(res.files[0].path, 'src/wp-includes/foo.php'); +}); + +test('parsePatchFiles: both formats agree on the same target path (issue #11)', () => { + assert.strictEqual( + parsePatchFiles(GITHUB_DIFF).files[0].path, + parsePatchFiles(TRAC_SVN_DIFF).files[0].path + ); +}); + +test('parsePatchFiles: a patch against the pre-src layout is remapped (issue #11)', () => { + const res = parsePatchFiles(OLD_LAYOUT_DIFF); + assert.strictEqual(res.files[0].path, 'src/wp-admin/admin.php'); +}); + +test('parsePatchFiles: added and deleted files are classified, not treated as renames (issue #11)', () => { + const added = parsePatchFiles(ADD_DIFF).files[0]; + assert.strictEqual(added.kind, 'add'); + assert.strictEqual(added.path, 'src/new.php'); + + const deleted = parsePatchFiles(DELETE_DIFF).files[0]; + assert.strictEqual(deleted.kind, 'delete'); + // A deletion's target is the file that exists today, not /dev/null. + assert.strictEqual(deleted.path, 'src/old.php'); +}); + +// jsdiff represents a binary file as an entry with no hunks. Left unchecked +// that reads as "a file with no changes", so applying would report success +// while silently skipping it. +test('parsePatchFiles: a binary file is reported as binary rather than an empty change (issue #11)', () => { + const res = parsePatchFiles(BINARY_DIFF); + assert.strictEqual(res.ok, true); + assert.strictEqual(res.files[0].kind, 'binary'); +}); + +test('parsePatchFiles: empty and unreadable input is rejected with a reason (issue #11)', () => { + for (const empty of ['', ' ', null, undefined]) { + const res = parsePatchFiles(empty); + assert.strictEqual(res.ok, false); + assert.strictEqual(res.error, 'The patch is empty.'); + } + const notAPatch = parsePatchFiles('this is just prose, not a diff at all\n'); + assert.strictEqual(notAPatch.ok, false); +}); + +test('parsePatchFiles: CRLF line endings parse the same as LF (issue #11)', () => { + const crlf = GITHUB_DIFF.replace(/\n/g, '\r\n'); + const res = parsePatchFiles(crlf); + assert.strictEqual(res.ok, true); + assert.strictEqual(res.files[0].path, 'src/wp-includes/foo.php'); +}); + +test('parsePatchFiles: a multi-file patch yields one entry per file (issue #11)', () => { + const res = parsePatchFiles(GITHUB_DIFF + ADD_DIFF + DELETE_DIFF); + assert.strictEqual(res.ok, true); + assert.deepStrictEqual(res.files.map((f) => f.kind), ['modify', 'add', 'delete']); +}); + +test('stripPathPrefix: only strips when both sides are prefixed (issue #11)', () => { + assert.deepStrictEqual( + stripPathPrefix('a/src/foo.php', 'b/src/foo.php'), + { oldPath: 'src/foo.php', newPath: 'src/foo.php' } + ); + assert.deepStrictEqual( + stripPathPrefix('src/foo.php', 'src/foo.php'), + { oldPath: 'src/foo.php', newPath: 'src/foo.php' } + ); + // /dev/null counts as agreement — an added file has only one real side. + assert.deepStrictEqual( + stripPathPrefix('/dev/null', 'b/src/new.php'), + { oldPath: '/dev/null', newPath: 'src/new.php' } + ); +}); + +test('stripPathPrefix: an old Subversion trunk/ prefix is dropped (issue #11)', () => { + assert.deepStrictEqual( + stripPathPrefix('trunk/wp-admin/admin.php', 'trunk/wp-admin/admin.php'), + { oldPath: 'wp-admin/admin.php', newPath: 'wp-admin/admin.php' } + ); +}); + +test('mapToSrcLayout: modern paths are left alone (issue #11)', () => { + for (const p of ['src/wp-includes/foo.php', 'tests/phpunit/bar.php', 'tools/baz.js']) { + assert.strictEqual(mapToSrcLayout(p), p); + } +}); + +test('mapToSrcLayout: root-level build files stay at the root (issue #11)', () => { + for (const p of ['package.json', 'Gruntfile.js', '.editorconfig', 'wp-cli.yml', 'wp-tests-config-sample.php']) { + assert.strictEqual(mapToSrcLayout(p), p); + } +}); + +// wp-cli.yml and wp-config-sample.php both start with "wp-" but did not move. +// This is the case the bare wp-* rule gets wrong without the exception list. +test('mapToSrcLayout: wp-prefixed files move to src, except the ones that did not (issue #11)', () => { + assert.strictEqual(mapToSrcLayout('wp-admin/admin.php'), 'src/wp-admin/admin.php'); + assert.strictEqual(mapToSrcLayout('wp-includes/post.php'), 'src/wp-includes/post.php'); + assert.strictEqual(mapToSrcLayout('wp-cli.yml'), 'wp-cli.yml'); + assert.strictEqual(mapToSrcLayout('wp-config-sample.php'), 'wp-config-sample.php'); +}); + +test('mapToSrcLayout: the loose root files that did move are remapped (issue #11)', () => { + assert.strictEqual(mapToSrcLayout('index.php'), 'src/index.php'); + assert.strictEqual(mapToSrcLayout('xmlrpc.php'), 'src/xmlrpc.php'); + assert.strictEqual(mapToSrcLayout('license.txt'), 'src/license.txt'); +}); + +test('mapToSrcLayout: an unrecognised path is left alone rather than guessed (issue #11)', () => { + assert.strictEqual(mapToSrcLayout('some/other/thing.php'), 'some/other/thing.php'); +}); + +test('planApply: only files the contributor already edited count as conflicts (issue #11)', () => { + const { files } = parsePatchFiles(GITHUB_DIFF + ADD_DIFF); + const plan = planApply({ files, dirtyPaths: ['src/wp-includes/foo.php', 'src/unrelated.php'] }); + assert.deepStrictEqual(plan.paths, ['src/wp-includes/foo.php', 'src/new.php']); + assert.deepStrictEqual(plan.conflicts, ['src/wp-includes/foo.php']); +}); + +test('planApply: a clean tree has no conflicts (issue #11)', () => { + const { files } = parsePatchFiles(GITHUB_DIFF); + assert.deepStrictEqual(planApply({ files, dirtyPaths: [] }).conflicts, []); +}); + +test('planApply: binary files are listed as unsupported (issue #11)', () => { + const { files } = parsePatchFiles(GITHUB_DIFF + BINARY_DIFF); + assert.deepStrictEqual(planApply({ files }).unsupported, ['src/x.png']); +}); + +test('planApply: an install is needed only when the lockfile is touched (issue #11)', () => { + assert.strictEqual(planApply({ files: parsePatchFiles(GITHUB_DIFF).files }).needsInstall, false); + + const lockDiff = `diff --git a/package-lock.json b/package-lock.json +--- a/package-lock.json ++++ b/package-lock.json +@@ -1,3 +1,3 @@ + { +- "x": 1 ++ "x": 2 + } +`; + assert.strictEqual(planApply({ files: parsePatchFiles(lockDiff).files }).needsInstall, true); +}); + +// Renaming the lockfile away removes it just as much as editing it does, so a +// rebuild is still needed — the rename's source side has to count, not only its +// destination. (Copilot #12.) +test('planApply: renaming the lockfile away still needs an install (issue #11)', () => { + const renameAway = `diff --git a/package-lock.json b/package-lock.json.bak +similarity index 100% +rename from package-lock.json +rename to package-lock.json.bak +`; + assert.strictEqual(planApply({ files: parsePatchFiles(renameAway).files }).needsInstall, true); +}); + +test('planApplySteps: the install step is named even when skipped (issue #11)', () => { + const steps = planApplySteps({ needsInstall: false }); + assert.deepStrictEqual(steps.map((s) => s.key), ['apply', 'install', 'build']); + assert.strictEqual(steps[1].skipped, true); + assert.strictEqual(steps[1].skipMessage, SKIP_INSTALL_MESSAGE); + assert.strictEqual(planApplySteps({ needsInstall: true })[1].skipped, false); +}); + +// The apply chain reuses the update chain's renderer helper by passing its own +// state map; this is what proves the third parameter actually drives it. +test('planApplySteps: updateStepStatuses drives the apply chain too (issue #11)', () => { + const steps = planApplySteps({ needsInstall: false }); + const at = (state) => updateStepStatuses(steps, state, APPLY_STATE_TO_STEP).map((s) => s.status); + + assert.deepStrictEqual(at('applying'), ['current', 'pending', 'pending']); + assert.deepStrictEqual(at('building'), ['complete', 'skipped', 'current']); + assert.deepStrictEqual(at('done'), ['complete', 'skipped', 'complete']); + // An unknown state must not mark anything complete. + assert.deepStrictEqual(at('idle'), ['pending', 'pending', 'pending']); +}); + +test('planApplySteps: the update chain is unaffected by the new state map (issue #11)', () => { + const steps = planApplySteps({ needsInstall: true }); + // 'fetching' belongs to the update chain, not this one. + assert.deepStrictEqual( + updateStepStatuses(steps, 'fetching', APPLY_STATE_TO_STEP).map((s) => s.status), + ['pending', 'pending', 'pending'] + ); +});