diff --git a/docs/src/content/docs/release-workflow.md b/docs/src/content/docs/release-workflow.md index 560bd829..d885a039 100644 --- a/docs/src/content/docs/release-workflow.md +++ b/docs/src/content/docs/release-workflow.md @@ -222,6 +222,10 @@ After publishing, Craft merges the release branch into the merge target and deletes the branch. The merge target defaults to the repository's default branch; override it with `--merge-target`. +- If the merge target changed lines next to the version strings the release + branch bumped, Craft resolves those conflicts itself: it keeps the merge + target's lines and applies the version bump to them. Any other conflict + aborts the merge and leaves it to you; the release is still published. - `--no-merge` skips the merge. `--keep-branch` merges but keeps the branch. - Set `noMerge: true` in `.craft.yml` to never merge. Craft turns this on automatically for compiled Node.js GitHub Actions (repositories with a diff --git a/src/commands/__tests__/publish.test.ts b/src/commands/__tests__/publish.test.ts index b4191f50..5c6d105d 100644 --- a/src/commands/__tests__/publish.test.ts +++ b/src/commands/__tests__/publish.test.ts @@ -1,4 +1,4 @@ -import { mkdir, mkdtemp, rm } from 'fs/promises'; +import { mkdir, mkdtemp, readFile, rm, writeFile } from 'fs/promises'; import { tmpdir } from 'os'; import { vi, describe, test, expect, beforeEach, type Mock } from 'vitest'; import { join as pathJoin } from 'path'; @@ -702,6 +702,113 @@ describe('handleReleaseBranch', () => { expect(git.branch).not.toHaveBeenCalled(); }); + describe('with a version bump conflict', () => { + const skill = (version: string, description: string) => + `---\nname: cli-init\nversion: ${version}\ndescription: ${description}\nrequires: sentry\n---\n`; + + /** + * Builds the situation from a real release: the release branch bumps a + * version in a file whose neighbouring line changed on the target branch. + */ + async function createConflictingRepository(releaseSkill: string) { + const directory = await mkdtemp( + pathJoin(tmpdir(), 'craft-publish-conflict-'), + ); + const repository = pathJoin(directory, 'repository'); + const remote = pathJoin(directory, 'remote.git'); + const branch = 'release/0.48.0'; + + await createGitClient(directory).raw('init', '--bare', remote); + await mkdir(repository); + const git = createGitClient(repository); + await git.raw('init', '-b', 'main'); + await git.addConfig('user.name', 'Craft Test'); + await git.addConfig('user.email', 'craft@example.com'); + await git.addConfig('commit.gpgsign', 'false'); + await git.addRemote('origin', remote); + + const skillPath = pathJoin(repository, 'SKILL.md'); + await writeFile(skillPath, skill('0.48.0-dev.0', 'Old description')); + await git.add('SKILL.md'); + await git.commit('initial'); + + await git.checkoutLocalBranch(branch); + await writeFile(skillPath, releaseSkill); + await git.commit('release: 0.48.0', ['--all']); + const releaseRevision = (await git.revparse('HEAD')).trim(); + + await git.checkout('main'); + await writeFile(skillPath, skill('0.48.0-dev.0', 'New description')); + await git.commit('docs: Update description', ['--all']); + await git.raw('push', 'origin', 'main', branch); + + return { directory, git, branch, releaseRevision, skillPath }; + } + + test('resolves and pushes the merge when only the version conflicts', async () => { + const { directory, git, branch, releaseRevision, skillPath } = + await createConflictingRepository(skill('0.48.0', 'Old description')); + + try { + await handleReleaseBranch( + git, + 'origin', + branch, + releaseRevision, + 'main', + false, + branch, + '0.48.0', + ); + + expect(await readFile(skillPath, 'utf-8')).toBe( + skill('0.48.0', 'New description'), + ); + const pushed = (await git.revparse('origin/main')).trim(); + expect(pushed).toBe((await git.revparse('HEAD')).trim()); + expect( + (await git.raw('rev-list', '--parents', '-n', '1', pushed)) + .trim() + .split(' '), + ).toContain(releaseRevision); + expect(await git.raw('ls-remote', '--heads', 'origin', branch)).toBe( + '', + ); + } finally { + await rm(directory, { recursive: true, force: true }); + } + }); + + test('aborts the merge when the release changed more than the version', async () => { + const { directory, git, branch, releaseRevision, skillPath } = + await createConflictingRepository(skill('0.48.0', 'Hotfix')); + + try { + const error = await handleReleaseBranch( + git, + 'origin', + branch, + releaseRevision, + 'main', + false, + branch, + '0.48.0', + ).catch((e: unknown) => e); + + expect(error).toBeInstanceOf(MergeConflictError); + expect((error as MergeConflictError).conflictedFiles).toEqual([ + 'SKILL.md', + ]); + expect(await readFile(skillPath, 'utf-8')).toBe( + skill('0.48.0-dev.0', 'New description'), + ); + expect((await git.status()).conflicted).toEqual([]); + } finally { + await rm(directory, { recursive: true, force: true }); + } + }); + }); + test('resolves default branch when mergeTarget is not provided', async () => { const { getDefaultBranch } = await import('../../utils/git'); vi.mocked(getDefaultBranch).mockResolvedValue('master'); diff --git a/src/commands/publish.ts b/src/commands/publish.ts index 2b4c8f1a..1983751e 100644 --- a/src/commands/publish.ts +++ b/src/commands/publish.ts @@ -59,6 +59,7 @@ import { buildReleaseCommandEnv } from '../utils/releaseCommandEnv'; import { getPublishStatePath } from '../utils/publishState'; import { DEFAULT_CALVER_CONFIG, getNextCalVerPeriod } from '../utils/calver'; import { runPreReleaseCommand } from '../utils/preReleaseCommand'; +import { resolveVersionBumpMergeConflicts } from '../utils/versionConflicts'; /** Default path to post-release script, relative to project root */ const DEFAULT_POST_RELEASE_SCRIPT_PATH = join('scripts', 'post-release.sh'); @@ -482,6 +483,8 @@ export class BranchCleanupError extends Error { * @param [mergeTarget] Branch name to merge the release branch into * @param keepBranch If set to "true", the branch will not be deleted * @param mergeSource Revision to merge, defaults to the release branch + * @param releaseVersion Released version, used to resolve conflicts that + * only come from the release branch's version bump */ export async function handleReleaseBranch( git: SimpleGit, @@ -491,6 +494,7 @@ export async function handleReleaseBranch( mergeTarget?: string, keepBranch = false, mergeSource = branch, + releaseVersion?: string, ): Promise { if (!mergeTarget) { mergeTarget = await getDefaultBranch(git, remoteName); @@ -519,7 +523,16 @@ export async function handleReleaseBranch( // Retry with the resolve strategy which handles criss-cross ambiguities // differently and often succeeds where ort fails on files like CHANGELOG.md try { - await git.merge(['-s', 'resolve', '--no-ff', '--no-edit', mergeSource]); + try { + await git.merge(['-s', 'resolve', '--no-ff', '--no-edit', mergeSource]); + } catch (resolveError) { + // The target branch may have changed lines next to the version + // strings the release branch bumped. Those conflicts have an obvious + // resolution. + if (!(await tryResolveVersionBumpConflicts(git, releaseVersion))) { + throw resolveError; + } + } } catch (resolveError) { // Resolve also failed — capture conflict details before aborting let conflictedFiles: string[] = []; @@ -603,6 +616,40 @@ export async function handleReleaseBranch( } } +/** + * Commits the merge resolution when every conflict of the in-progress merge + * only comes from the release version bump. + * + * @returns Whether the merge was resolved and committed + */ +async function tryResolveVersionBumpConflicts( + git: SimpleGit, + releaseVersion: string | undefined, +): Promise { + if (!releaseVersion) { + return false; + } + try { + const resolvedFiles = await resolveVersionBumpMergeConflicts( + git, + releaseVersion, + ); + if (!resolvedFiles) { + return false; + } + logger.info( + `Resolved conflicts caused only by the ${releaseVersion} version bump in: ${resolvedFiles.join(', ')}`, + ); + return true; + } catch (resolutionError) { + logger.debug( + 'Could not auto-resolve the version bump conflicts:', + resolutionError, + ); + return false; + } +} + /** * Pull with `--rebase`, aborting the rebase if the pull fails so the * repository isn't left mid-rebase. @@ -1004,6 +1051,7 @@ export async function publishMain(argv: PublishOptions): Promise { mergeTarget, argv.keepBranch, rev ? revision : branchName, + newVersion, ); mergedInto = mergeTarget; } catch (mergeError) { diff --git a/src/utils/__tests__/versionConflicts.test.ts b/src/utils/__tests__/versionConflicts.test.ts new file mode 100644 index 00000000..9df87b0e --- /dev/null +++ b/src/utils/__tests__/versionConflicts.test.ts @@ -0,0 +1,157 @@ +import { describe, expect, test } from 'vitest'; + +import { + resolveVersionBumpConflicts, + resolveVersionBumpHunk, +} from '../versionConflicts'; + +const OURS = '<'.repeat(32); +const BASE = '|'.repeat(32); +const SEP = '='.repeat(32); +const THEIRS = '>'.repeat(32); + +function conflict(ours: string, base: string, theirs: string): string { + return `${OURS} ours\n${ours}${BASE} base\n${base}${SEP}\n${theirs}${THEIRS} theirs\n`; +} + +describe('resolveVersionBumpHunk', () => { + test('applies the version bump to the target branch side', () => { + expect( + resolveVersionBumpHunk( + 'version: 0.48.0-dev.0\ndescription: New description\n', + 'version: 0.48.0-dev.0\ndescription: Old description\n', + 'version: 0.48.0\ndescription: Old description\n', + '0.48.0', + ), + ).toBe('version: 0.48.0\ndescription: New description\n'); + }); + + test('bumps every occurrence of the old version in the hunk', () => { + expect( + resolveVersionBumpHunk( + '"version": "1.1.0",\n"dependencies": { "core": "1.1.0", "x": "2" }\n', + '"version": "1.1.0",\n"dependencies": { "core": "1.1.0" }\n', + '"version": "1.2.0",\n"dependencies": { "core": "1.2.0" }\n', + '1.2.0', + ), + ).toBe( + '"version": "1.2.0",\n"dependencies": { "core": "1.2.0", "x": "2" }\n', + ); + }); + + test('handles PEP 440 style development versions', () => { + expect( + resolveVersionBumpHunk( + 'version = "2.1.0.dev0"\nrequires-python = ">=3.10"\n', + 'version = "2.1.0.dev0"\nrequires-python = ">=3.9"\n', + 'version = "2.1.0"\nrequires-python = ">=3.9"\n', + '2.1.0', + ), + ).toBe('version = "2.1.0"\nrequires-python = ">=3.10"\n'); + }); + + test('does not touch versions that only contain the old version', () => { + expect( + resolveVersionBumpHunk( + 'version: 1.0.0\nmin: 11.0.0\nnext: 1.0.0-rc.1\nnew line\n', + 'version: 1.0.0\nmin: 11.0.0\nnext: 1.0.0-rc.1\n', + 'version: 1.1.0\nmin: 11.0.0\nnext: 1.0.0-rc.1\n', + '1.1.0', + ), + ).toBe('version: 1.1.0\nmin: 11.0.0\nnext: 1.0.0-rc.1\nnew line\n'); + }); + + test('refuses when the release side changed more than versions', () => { + expect( + resolveVersionBumpHunk( + 'version: 1.0.0\ndescription: Target\n', + 'version: 1.0.0\ndescription: Base\n', + 'version: 1.1.0\ndescription: Hotfix\n', + '1.1.0', + ), + ).toBeNull(); + }); + + test('refuses when the release side added a line with the version', () => { + expect( + resolveVersionBumpHunk( + '## Unreleased\n\n- Target change\n', + '## Unreleased\n', + '## 1.1.0\n\n- Release change\n', + '1.1.0', + ), + ).toBeNull(); + }); + + test('refuses when the release side bumped to another version', () => { + expect( + resolveVersionBumpHunk( + 'dep: 1.0.0\nother: x\n', + 'dep: 1.0.0\n', + 'dep: 3.0.0\n', + '1.1.0', + ), + ).toBeNull(); + }); + + test('refuses when the target branch changed the version itself', () => { + expect( + resolveVersionBumpHunk( + 'version: 2.0.0-dev.0\ndescription: Target\n', + 'version: 1.1.0-dev.0\ndescription: Base\n', + 'version: 1.1.0\ndescription: Base\n', + '1.1.0', + ), + ).toBeNull(); + }); +}); + +describe('resolveVersionBumpConflicts', () => { + test('resolves every hunk and keeps the merged content around them', () => { + const merged = [ + '---\n', + 'name: init\n', + conflict( + 'version: 0.48.0-dev.0\ndescription: New\n', + 'version: 0.48.0-dev.0\ndescription: Old\n', + 'version: 0.48.0\ndescription: Old\n', + ), + 'requires: sentry\n', + '=======\n', + conflict('v0.48.0-dev.0 (new)\n', 'v0.48.0-dev.0\n', 'v0.48.0\n'), + ].join(''); + + expect(resolveVersionBumpConflicts(merged, '0.48.0')).toBe( + '---\nname: init\nversion: 0.48.0\ndescription: New\nrequires: sentry\n=======\nv0.48.0 (new)\n', + ); + }); + + test('refuses when any hunk is not a version bump', () => { + const merged = [ + conflict( + 'version: 1.0.0\na\n', + 'version: 1.0.0\nb\n', + 'version: 1.1.0\nb\n', + ), + conflict('x\n', 'y\n', 'z\n'), + ].join(''); + + expect(resolveVersionBumpConflicts(merged, '1.1.0')).toBeNull(); + }); + + test('refuses conflicts without a base section', () => { + expect( + resolveVersionBumpConflicts( + `${OURS}\nversion: 1.0.0\n${SEP}\nversion: 1.1.0\n${THEIRS}\n`, + '1.1.0', + ), + ).toBeNull(); + }); + + test('refuses unterminated conflicts and content without conflicts', () => { + expect( + resolveVersionBumpConflicts(`${OURS}\na\n${BASE}\nb\n`, '1.1.0'), + ).toBeNull(); + expect(resolveVersionBumpConflicts('no conflicts\n', '1.1.0')).toBeNull(); + }); +}); diff --git a/src/utils/versionConflicts.ts b/src/utils/versionConflicts.ts new file mode 100644 index 00000000..5c79dbf8 --- /dev/null +++ b/src/utils/versionConflicts.ts @@ -0,0 +1,311 @@ +import { writeFile } from 'fs/promises'; +import path from 'path'; +import type { SimpleGit } from 'simple-git'; + +import { logger } from '../logger'; +import { safeFs } from './dryRun'; +import { withTempDir } from './files'; + +/** + * Conflict marker size passed to `git merge-file`. Large enough that file + * content (e.g. a Markdown setext heading underlined with `=======`) is not + * mistaken for a marker. + */ +const MARKER_SIZE = 32; +const OURS_MARKER = '<'.repeat(MARKER_SIZE); +const BASE_MARKER = '|'.repeat(MARKER_SIZE); +const SEPARATOR_MARKER = '='.repeat(MARKER_SIZE); +const THEIRS_MARKER = '>'.repeat(MARKER_SIZE); + +/** + * A version-like token: at least `MAJOR.MINOR`, optionally followed by + * dot, dash or plus separated segments (`1.2.3`, `1.2.3-dev.0`, + * `1.2.3.dev0`, `24.1.0+build.5`). + */ +const VERSION_PATTERN = '\\d+\\.[0-9A-Za-z]+(?:[-+.][0-9A-Za-z]+)*'; + +function escapeRegExp(text: string): string { + return text.replace(/[.*+?^${}()|[\]\\]/g, '\\$&'); +} + +/** + * Matches `version` only where it is a whole version token, so `1.2.3` does + * not match inside `11.2.3` or `1.2.3-dev.0`. + */ +function wholeVersionRegExp(version: string): RegExp { + return new RegExp( + `(? v !== version)); + if (oldVersions.size === 0) { + return null; + } + + let resolved = ours; + for (const oldVersion of oldVersions) { + const oldVersionRegExp = wholeVersionRegExp(oldVersion); + if (resolved.search(oldVersionRegExp) === -1) { + // The target branch changed the version line itself; let a human + // decide which version it should have. + return null; + } + resolved = resolved.replace(oldVersionRegExp, version); + } + return resolved; +} + +function isMarker(line: string, marker: string): boolean { + if (!line.startsWith(marker)) { + return false; + } + const next = line.charAt(marker.length); + return next === '' || next === ' ' || next === '\n' || next === '\r'; +} + +/** + * Resolves every conflict hunk in `git merge-file --diff3` output that only + * conflicts because of a version bump on the release side. + * + * @param merged Output of `git merge-file -p --diff3` with markers of + * `MARKER_SIZE` characters + * @param version The released version + * @returns The fully resolved content, or null if any hunk cannot be + * resolved this way + */ +export function resolveVersionBumpConflicts( + merged: string, + version: string, +): string | null { + const lines = merged.split(/(?<=\n)/); + const output: string[] = []; + let state: 'outside' | 'ours' | 'base' | 'theirs' = 'outside'; + let ours = ''; + let base = ''; + let theirs = ''; + let resolvedAny = false; + + for (const line of lines) { + switch (state) { + case 'outside': + if (isMarker(line, OURS_MARKER)) { + state = 'ours'; + ours = base = theirs = ''; + } else { + output.push(line); + } + break; + case 'ours': + if (isMarker(line, BASE_MARKER)) { + state = 'base'; + } else if ( + isMarker(line, SEPARATOR_MARKER) || + isMarker(line, OURS_MARKER) || + isMarker(line, THEIRS_MARKER) + ) { + // Without the base section we cannot tell what the release changed. + return null; + } else { + ours += line; + } + break; + case 'base': + if (isMarker(line, SEPARATOR_MARKER)) { + state = 'theirs'; + } else if ( + isMarker(line, OURS_MARKER) || + isMarker(line, BASE_MARKER) || + isMarker(line, THEIRS_MARKER) + ) { + return null; + } else { + base += line; + } + break; + case 'theirs': + if (isMarker(line, THEIRS_MARKER)) { + const resolved = resolveVersionBumpHunk(ours, base, theirs, version); + if (resolved === null) { + return null; + } + output.push(resolved); + resolvedAny = true; + state = 'outside'; + } else if ( + isMarker(line, OURS_MARKER) || + isMarker(line, BASE_MARKER) || + isMarker(line, SEPARATOR_MARKER) + ) { + return null; + } else { + theirs += line; + } + break; + } + } + + if (state !== 'outside' || !resolvedAny) { + return null; + } + return output.join(''); +} + +interface UnmergedEntry { + base?: string; + ours?: string; + theirs?: string; +} + +/** + * Lists unmerged index entries as repository-root-relative paths mapped to + * the blob of each conflict stage. + */ +async function getUnmergedEntries( + git: SimpleGit, +): Promise> { + const output = await git.raw(['ls-files', '-u', '-z', '--full-name']); + const entries = new Map(); + for (const record of output.split('\0')) { + // " \t" + const match = /^\d+ ([0-9a-f]+) ([123])\t(.+)$/s.exec(record); + if (!match) { + continue; + } + const [, object, stage, file] = match; + const entry = entries.get(file) ?? {}; + if (stage === '1') { + entry.base = object; + } else if (stage === '2') { + entry.ours = object; + } else { + entry.theirs = object; + } + entries.set(file, entry); + } + return entries; +} + +/** + * Resolves an in-progress merge whose conflicts are caused only by the + * release version bump, then commits the merge. + * + * The release branch bumps version strings (e.g. `1.2.0-dev.0` to `1.2.0`). + * When the target branch changed lines next to those strings after the + * release branch was cut, Git reports a conflict even though the intent of + * both sides is clear. For each such hunk, this keeps the target branch's + * content and applies the version bump to it. + * + * Nothing is written unless every conflicted file can be resolved this way. + * + * @param git Git client with a conflicted merge in progress + * @param version The released version that the release branch bumped to + * @returns The resolved files, or null if the merge was left untouched + */ +export async function resolveVersionBumpMergeConflicts( + git: SimpleGit, + version: string, +): Promise { + const entries = await getUnmergedEntries(git); + if (entries.size === 0) { + return null; + } + + const repoRoot = (await git.revparse(['--show-toplevel'])).trim(); + const resolutions = new Map(); + + for (const [file, entry] of entries) { + if (!entry.base || !entry.ours || !entry.theirs) { + // Add/add, modify/delete, and rename conflicts are not version bumps. + logger.debug(`Cannot auto-resolve "${file}": not a content conflict.`); + return null; + } + const stages = { + ours: entry.ours, + base: entry.base, + theirs: entry.theirs, + }; + const merged = await withTempDir( + async directory => { + const paths: string[] = []; + for (const [name, object] of Object.entries(stages)) { + const content = await git.raw(['cat-file', 'blob', object]); + // Skip binary and non-UTF-8 files: they would not survive the + // round trip through a JS string. + if (content.includes('\0') || content.includes('\uFFFD')) { + return null; + } + const stagePath = path.join(directory, name); + await writeFile(stagePath, content); + paths.push(stagePath); + } + // Exits with the number of conflicts. simple-git only rejects when + // there is also stderr output, which `-q` suppresses for conflicts. + return git.raw([ + 'merge-file', + '-p', + '-q', + '--diff3', + `--marker-size=${MARKER_SIZE}`, + ...paths, + ]); + }, + true, + 'craft-merge-', + ); + if (merged === null) { + logger.debug(`Cannot auto-resolve "${file}": not a UTF-8 text file.`); + return null; + } + + const resolved = resolveVersionBumpConflicts(merged, version); + if (resolved === null) { + logger.debug( + `Cannot auto-resolve "${file}": the conflict is not only a version bump.`, + ); + return null; + } + resolutions.set(file, resolved); + } + + for (const [file, content] of resolutions) { + const filePath = path.join(repoRoot, file); + await safeFs.writeFile(filePath, content); + await git.add(filePath); + } + await git.raw(['commit', '--no-edit']); + return [...resolutions.keys()]; +}