From 05abf6d5aa0e4326c79a8cfabda721f8c06ec8c5 Mon Sep 17 00:00:00 2001 From: Ruben Fiszel Date: Thu, 20 Aug 2026 15:08:13 +0200 Subject: [PATCH] feat(cli): deduplicate identical script lockfiles (dedupeLockfiles) (#10769) * feat(cli): deduplicate identical script lockfiles into one per language * test: pin shared lockfile path classification * fix(cli): never delete a lockfile the dedup plan also writes * fix(cli): plan lockfile dedup from the whole tree, not the sync scope * fix(cli): keep dedup out of dry runs and stop hiding scripts from its scan * test: pin which files the shared-lock scan counts as readers * fix(cli): validate shared-lock refs and let the majority keep its file * fix(cli): snapshot shared-lock ownership before regeneration moves it * fix(cli): address dedup review nits (dry-run push, json shape, scan scope) * refactor(cli): put shared lockfiles in a top-level locks/ directory * fix(cli): claim only the shared lock names windmill writes, and only when on * fix(cli): read the lock field itself, and count only scripts sync reads * fix(cli): parse metadata by its real format and lint from the sync root * fix(cli): never re-hash a script whose generation failed * fix(cli): share sync's walk exclusions and fail closed on unreadable dirs * fix(cli): keep a lockfile the metadata on disk still references * fix(cli): decide a lock is unread from the metadata field sync reads * refactor(cli): name shared lockfiles after the dependency file they resolve * fix(cli): carry shared lockfiles a narrowed sync cannot speak for * fix(cli): move a shared lockfile when its dependency file moved, not on a head count * fix(cli): let the many correct a shared lockfile a lone variant planted * fix(cli): read why a lock differs from the stamp the worker writes into it * fix(cli): let an agreeing majority speak whatever the stamps say * docs(cli): count the disjuncts the comment introduces * fix(cli): keep a private lock for any script the worker locks differently * fix(cli): match annotations by the worker's own names, not by shape * fix(cli): recognize the py: interpreter pin the macro does not cover * fix(cli): let the map speak for dependency-file deletions * perf(cli): group lock entries without rebuilding the group per insert * fix(cli): defer shared-lock deletions until the metadata has settled * fix(cli): decide shared-lock readers by the lock field, failing closed * fix(cli): read folded lock refs and keep locks read by unparseable metadata * refactor(cli): one shared-lock reader scan, shared by the pull and push paths * fix(cli): keep nested dependency set names out of shared lockfiles * fix(cli): drop a shared-lock scan gate that no real repo took --- .../generate-metadata/generate-metadata.ts | 61 ++ cli/src/commands/init/template.ts | 2 + cli/src/commands/lint/lint.ts | 42 +- cli/src/commands/sync/sync.ts | 446 +++++++++++++- cli/src/core/conf.ts | 1 + cli/src/types.ts | 6 + cli/src/utils/lock_dedup.ts | 556 ++++++++++++++++++ cli/src/utils/metadata.ts | 176 ++---- cli/src/utils/script_common.ts | 216 +++++++ cli/test/lock_dedup_unit.test.ts | 480 +++++++++++++++ cli/test/utils_unit.test.ts | 14 + cli/wmill.schema.json | 14 +- 12 files changed, 1872 insertions(+), 142 deletions(-) create mode 100644 cli/src/utils/lock_dedup.ts create mode 100644 cli/test/lock_dedup_unit.test.ts diff --git a/cli/src/commands/generate-metadata/generate-metadata.ts b/cli/src/commands/generate-metadata/generate-metadata.ts index ffc8fb117e..7310efc483 100644 --- a/cli/src/commands/generate-metadata/generate-metadata.ts +++ b/cli/src/commands/generate-metadata/generate-metadata.ts @@ -19,6 +19,7 @@ import { import { generateFlowLockInternal, FlowLocksResult } from "../flow/flow_metadata.ts"; import { generateAppLocksInternal, AppLocksResult } from "../app/app_metadata.ts"; import { + dedupeLockfilesOnDisk, elementsToMap, FSFSElement, ignoreF, @@ -411,6 +412,51 @@ export async function rehashOnly( return counts; } +/** + * Normalize the tree's shared lockfiles, unless the run asked to stay inside one + * folder: shared lockfiles are workspace-wide, so the pass reads and rewrites + * metadata outside it, which `--strict-folder-boundaries` promises not to do. + */ +async function maybeDedupeLockfiles( + opts: GlobalOptions & SyncOptions & { strictFolderBoundaries?: boolean }, + workspace: Workspace, + codebases: SyncCodebase[], + ignore: (p: string, isD: boolean) => boolean, + rawWorkspaceDependencies: Record, + tree: DoubleLinkedDependencyTree, + folder: string | undefined, + failed: string[] = [], +): Promise { + if (!opts.dedupeLockfiles) return; + if (folder && opts.strictFolderBoundaries) { + log.info( + colors.yellow( + `Skipping lockfile deduplication: it spans the whole workspace, and --strict-folder-boundaries keeps this run inside "${folder}".`, + ), + ); + return; + } + const args = { + opts, + workspace, + codebases, + ignore, + rawWorkspaceDependencies, + tree, + failed, + }; + if (opts.dryRun) { + await dedupeLockfilesOnDisk({ ...args, dryRun: true }); + return; + } + await beginLockfileBatch(); + try { + await dedupeLockfilesOnDisk(args); + } finally { + await flushLockfileBatch(); + } +} + export async function generateMetadata( opts: GlobalOptions & { yes?: boolean; @@ -610,6 +656,10 @@ export async function generateMetadata( // === Show stale items and confirm === if (filteredItems.length === 0) { log.info(colors.green("All metadata up-to-date")); + // Turning `dedupeLockfiles` on in a repo whose metadata is already current + // is exactly the case where nothing is stale, and the conversion still has + // to happen — the sync compares against a deduplicated remote either way. + await maybeDedupeLockfiles(opts, workspace, codebases, ignore, rawWorkspaceDependencies, tree, folder); return; } @@ -637,6 +687,9 @@ export async function generateMetadata( printItems("Apps", apps); if (opts.dryRun) { + // The preview belongs on this path too: the conversion is what a stale tree + // is about to get, and only the up-to-date path reported it. + await maybeDedupeLockfiles(opts, workspace, codebases, ignore, rawWorkspaceDependencies, tree, folder); return; } @@ -775,10 +828,18 @@ export async function generateMetadata( // Persist all stale workspace dep hashes (not just filtered — deps are global, not folder-scoped) const allStaleDeps = staleItems.filter((i) => i.type === "dependencies"); await tree.persistDepsHashes(allStaleDeps.map((d) => d.path)); + } finally { await flushLockfileBatch(); } + // The scripts whose generation failed keep whatever lock they had, and must + // not be re-hashed as though this run had refreshed them. + await maybeDedupeLockfiles( + opts, workspace, codebases, ignore, rawWorkspaceDependencies, tree, folder, + errors.map((e) => e.path), + ); + const succeeded = total - errors.length; log.info(""); if (errors.length > 0) { diff --git a/cli/src/commands/init/template.ts b/cli/src/commands/init/template.ts index ef2bcad0a6..9032cee397 100644 --- a/cli/src/commands/init/template.ts +++ b/cli/src/commands/init/template.ts @@ -117,6 +117,8 @@ export const CONFIG_REFERENCE: ConfigOption[] = [ section: "Sync behavior", commented: true, templateValue: "4" }, { name: "locksRequired", type: "boolean", default: "false", description: "Require lock files for all scripts", commented: true, templateValue: "true" }, + { name: "dedupeLockfiles", type: "boolean", default: "false", description: "Share one lockfile per workspace dependency file (locks/.lock), instead of an identical .script.lock per script", + commented: true, templateValue: "true" }, { name: "lint", type: "boolean", default: "false", description: "Run linting before push", commented: true, templateValue: "true" }, { name: "plainSecrets", type: "boolean", default: "false", description: "Handle secrets as plain text (not recommended)", diff --git a/cli/src/commands/lint/lint.ts b/cli/src/commands/lint/lint.ts index d3db402833..f6a35429ef 100644 --- a/cli/src/commands/lint/lint.ts +++ b/cli/src/commands/lint/lint.ts @@ -26,6 +26,7 @@ import { inferContentTypeFromFilePath, languageNeedsLock, ScriptLanguage, + isSharedLockPath, } from "../../utils/script_common.ts"; import { isFlowInlineScriptPath, @@ -133,6 +134,7 @@ function formatYamlDiagnostics(parsed: { diagnostics?: Array<{ message?: string async function isLockResolved( lockValue: string | string[] | undefined, baseDir: string, + sharedLockBase?: string, ): Promise { if (lockValue === undefined) return false; @@ -141,7 +143,11 @@ async function isLockResolved( const joined = lockValue.join("\n"); if (joined === "") return false; if (joined.startsWith("!inline ")) { - return await checkInlineFile(joined.substring("!inline ".length), baseDir); + return await checkInlineFile( + joined.substring("!inline ".length), + baseDir, + sharedLockBase, + ); } return true; } @@ -150,7 +156,11 @@ async function isLockResolved( // Inline file reference if (lockValue.startsWith("!inline ")) { - return await checkInlineFile(lockValue.substring("!inline ".length), baseDir); + return await checkInlineFile( + lockValue.substring("!inline ".length), + baseDir, + sharedLockBase, + ); } // Embedded lock content @@ -160,8 +170,17 @@ async function isLockResolved( async function checkInlineFile( relativePath: string, baseDir: string, + sharedLockBase?: string, ): Promise { - const fullPath = path.join(baseDir, relativePath.trim()); + const trimmed = relativePath.trim(); + // A shared lockfile (`dedupeLockfiles`) is referenced from the sync root, not + // from the directory being linted. Only a standalone script's metadata passes + // a base for it: a flow or app inline lock is folder-relative even when its + // name happens to look like one. + const fullPath = + sharedLockBase !== undefined && isSharedLockPath(trimmed) + ? path.resolve(sharedLockBase, trimmed) + : path.join(baseDir, trimmed); try { const s = await stat(fullPath); return s.size > 0; @@ -170,6 +189,21 @@ async function checkInlineFile( } } +/** Where a repo-root-relative reference resolves from: the directory holding + * wmill.yaml at or above the linted one, and that directory itself when there + * is none. */ +async function findSyncRoot(dir: string): Promise { + let current = path.resolve(dir); + while (true) { + if (await stat(path.join(current, "wmill.yaml")).then(() => true).catch(() => false)) { + return current; + } + const parent = path.dirname(current); + if (parent === current) return path.resolve(dir); + current = parent; + } +} + /** * Recursively find rawscript modules in a flow's module tree. */ @@ -481,6 +515,7 @@ export async function checkMissingLocks( } // Check standalone scripts + const syncRoot = await findSyncRoot(targetDirectory); for (const yamlPath of scriptYamls) { const basePath = yamlPath.replace(/\.script\.yaml$/, ""); @@ -506,6 +541,7 @@ export async function checkMissingLocks( const lockResolved = await isLockResolved( metadata?.lock, targetDirectory, + syncRoot, ); if (!lockResolved) { issues.push({ diff --git a/cli/src/commands/sync/sync.ts b/cli/src/commands/sync/sync.ts index ebc7b731f5..a407d07516 100644 --- a/cli/src/commands/sync/sync.ts +++ b/cli/src/commands/sync/sync.ts @@ -9,7 +9,7 @@ import { copyFile, mkdir, } from "node:fs/promises"; -import { existsSync } from "node:fs"; +import { existsSync, type Dirent } from "node:fs"; import { colors } from "@cliffy/ansi/colors"; import { Command } from "@cliffy/command"; import { Confirm } from "@cliffy/prompt/confirm"; @@ -113,6 +113,8 @@ import { Workspace } from "../workspace/workspace.ts"; import { removePathPrefix } from "../../types.ts"; import { listSyncCodebases, SyncCodebase } from "../../utils/codebase.ts"; import { + beginLockfileBatch, + flushLockfileBatch, generateScriptMetadataInternal, getRawWorkspaceDependencies, readLockfile, @@ -174,6 +176,136 @@ import { DBT_DESCRIPTOR_NAME, isDbtDescriptorPath, } from "../../utils/resource_folders.ts"; +import { isSharedLockPath, SHARED_LOCK_DIR } from "../../utils/script_common.ts"; +import { + applySharedLockPlanToDisk, + applySharedLockPlanToMap, + metadataLockUnreadable, + sharedLockRefOf, + computeSharedLockPlan, + isEmptySharedLockPlan, + scriptsReferencingSharedLock, + type LockDedupOptions, +} from "../../utils/lock_dedup.ts"; + +/** A lockfile belonging to one script, as opposed to a shared one. */ +function isScriptLockPath(p: string): boolean { + const n = p.replaceAll(SEP, "/"); + return n.endsWith(".script.lock") || n.endsWith("__mod/script.lock"); +} + +/** + * Every shared lockfile the tree still reads, from a single walk. + * + * One pull can retire several at once — `--skip-workspace-dependencies` retires + * all of them, and consolidating k dependency files retires k-1 — so a walk per + * deletion would re-read and re-parse the same metadata each time. Read after + * the pull has applied (or refused) every metadata change, because that is the + * only moment the answer is settled. + */ +export type SharedLockReaders = { + /** Reference (`locks/.lock`) to the metadata files reading it. */ + byRef: Map; + /** Metadata whose `lock` cannot be read, which pins every shared lockfile. */ + unreadable: string[]; +}; + +export async function collectSharedLockReaders( + json: boolean, +): Promise { + const metaExt = json ? ".script.json" : ".script.yaml"; + const modMeta = json ? "__mod/script.json" : "__mod/script.yaml"; + const readers: SharedLockReaders = { byRef: new Map(), unreadable: [] }; + const walk = async (dir: string): Promise => { + let entries: Dirent[]; + try { + entries = await readdir(dir, { withFileTypes: true }); + } catch (e) { + // A directory that is not there holds no reader. Anything else hides + // scripts, and a lockfile deleted out from under one resolves to nothing. + if ((e as { code?: string })?.code === "ENOENT") return; + throw e; + } + for (const entry of entries) { + const full = path.join(dir, entry.name); + if (entry.isDirectory()) { + if (isNeverWalkedDir(entry.name)) continue; + await walk(full); + continue; + } + const rel = full.replaceAll(SEP, "/"); + if (!rel.endsWith(metaExt) && !rel.endsWith(modMeta)) continue; + const content = await readTextFile(full); + // The `lock` field, not the raw text: a folded line, a summary quoting + // the path, or a stale twin of the other format would each answer wrongly. + const ref = sharedLockRefOf(rel, content, json); + if (ref === undefined) { + if (metadataLockUnreadable(rel, content, json)) readers.unreadable.push(rel); + continue; + } + const existing = readers.byRef.get(ref); + if (existing) existing.push(rel); + else readers.byRef.set(ref, [rel]); + } + }; + for (const root of ["f", "u", "g"]) { + await walk(root); + } + return readers; +} + +/** + * Whether the metadata beside a script lockfile still points at it. Read from + * disk, after the pull has applied (or refused) every metadata change, because + * that is the only moment the answer is settled - and from the `lock` field of + * the twin this sync reads, not the raw text: a folded line, a summary quoting + * the path, or a stale twin of the other format would each answer wrongly. + * + * Returns the reason it is kept, or undefined when nothing reads it. + */ +async function lockStillReadBecause( + lockPath: string, + json: boolean, + sharedReaders: SharedLockReaders, +): Promise { + const n = lockPath.replaceAll(SEP, "/"); + // A shared lockfile is read by any number of scripts, so the whole tree + // answers rather than one sibling. + if (isSharedLockPath(n)) { + const readers = sharedReaders.byRef.get(n)?.length ?? 0; + if (readers > 0) return `${readers} script(s) on disk still reference it`; + if (sharedReaders.unreadable.length > 0) { + // Naming the file matters: on a dependency-file deletion this line is the + // only signal, and "still references it" would point away from the fix. + return `${sharedReaders.unreadable[0]} cannot be parsed, so what it reads is unknown`; + } + return undefined; + } + const metaPath = n.endsWith("__mod/script.lock") + ? n.slice(0, -".lock".length) + (json ? ".json" : ".yaml") + : n.slice(0, -".script.lock".length) + + (json ? ".script.json" : ".script.yaml"); + let content: string; + try { + content = await readTextFile(metaPath.replaceAll("/", SEP)); + } catch { + return undefined; // no metadata: nothing reads it + } + try { + const parsed = json + ? JSON.parse(content) + : yamlParseContent(metaPath, content); + return parsed?.["lock"] === "!inline " + n + ? `${metaPath} still references it` + : undefined; + } catch { + // Unparseable metadata is not proof that nothing reads the lock. + return `${metaPath} cannot be parsed, so what it reads is unknown`; + } +} + +/** Sync maps are keyed with the platform separator; `!inline` refs are not. */ +const toMapKeySep = (refPath: string) => refPath.replaceAll("/", SEP); let branchDeprecationWarned = false; @@ -1719,6 +1851,18 @@ function ZipFSElement( return _internal_folder("." + SEP, zip); } +/** + * Directories no walk over a workspace ever descends, whatever the sync scope: + * dependency trees, and the dot-directories that hold tooling state and + * fixtures. Exported because a second walk that disagrees with this one reads + * files sync will never see, and draws conclusions from them. + */ +export function isNeverWalkedDir(dirName: string | undefined): boolean { + return ( + dirName === "node_modules" || (dirName !== undefined && dirName.startsWith(".")) + ); +} + export async function* readDirRecursiveWithIgnore( ignore: (path: string, isDirectory: boolean) => boolean, root: DynFSElement, @@ -1755,15 +1899,8 @@ export async function* readDirRecursiveWithIgnore( const e = stack.pop()!; yield e; for await (const e2 of e.c()) { - if (e2.isDirectory) { - const dirName = e2.path.split(SEP).pop(); - if ( - dirName == "node_modules" || - dirName == ".claude" || - dirName?.startsWith(".") - ) { - continue; - } + if (e2.isDirectory && isNeverWalkedDir(e2.path.split(SEP).pop())) { + continue; } stack.push({ path: e2.path, @@ -1941,7 +2078,13 @@ export async function elementsToMap( try { const fileType = getTypeStrFromPath(path); if (skips.skipVariables && fileType === "variable") continue; - if (skips.skipScripts && fileType === "script") continue; + // A shared lockfile is part of the scripts that reference it. + if ( + skips.skipScripts && + (fileType === "script" || fileType === "shared_lock") + ) { + continue; + } if (skips.skipFlows && fileType === "flow") continue; if (skips.skipApps && fileType === "app") continue; if (skips.skipFolders && fileType === "folder") continue; @@ -2409,7 +2552,7 @@ async function compareDynFSElement( els2: DynFSElement | undefined, ignore: (path: string, isDirectory: boolean) => boolean, json: boolean, - skips: Skips, + skips: Skips & LockDedupOptions, ignoreMetadataDeletion: boolean, codebases: SyncCodebase[], ignoreCodebaseChanges: boolean, @@ -2498,6 +2641,36 @@ async function compareDynFSElement( preservePendingScriptLocks(m1, m2); } + // The remote serializes one lock per script; `dedupeLockfiles` is how the repo + // represents them. Collapsing the remote side (in both directions) is what + // makes the two sides comparable: a pull then writes the shared file instead + // of thousands of copies, and a push sees no diff for the copies it does not + // keep. + if (skips.dedupeLockfiles) { + const remoteMap = isEls1Remote === true ? m1 : m2; + const localMapForLocks = isEls1Remote === true ? m2 : m1; + // The local side supplies what the remote never serializes: the shared + // lockfiles already on disk, so one whose scripts are out of this sync's + // scope is carried forward rather than read as a deletion. + const present: Record = {}; + for (const [key, content] of Object.entries(localMapForLocks)) { + if (isSharedLockPath(key)) present[key.replaceAll(SEP, "/")] = content; + } + applySharedLockPlanToMap( + remoteMap, + computeSharedLockPlan(remoteMap, { + defaultTs: skips.defaultTs, + present, + // Only when the map cannot speak for them: with dependency files in the + // map, its absences are real deletions, and reading disk here would keep + // a lockfile alive one sync past the file it is named after. + depFiles: skips.skipWorkspaceDependencies + ? Object.keys(await getRawWorkspaceDependencies(false)) + : undefined, + }), + ); + } + const changes: Change[] = []; function parseYaml(k: string, v: string) { @@ -2761,6 +2934,7 @@ const isNotWmillFile = (p: string, isDirectory: boolean) => { !p.startsWith("users" + SEP) && !p.startsWith("groups" + SEP) && !p.startsWith("dependencies" + SEP) && + !p.startsWith(SHARED_LOCK_DIR + SEP) && !p.startsWith("migrations" + SEP) ); } @@ -2785,6 +2959,8 @@ const isNotWmillFile = (p: string, isDirectory: boolean) => { typ == "encryption_key" ) { return p.includes(SEP); + } else if (typ == "shared_lock") { + return false; } else { return ( !p.startsWith("u" + SEP) && @@ -2811,6 +2987,7 @@ export const isWhitelisted = (p: string) => { p == "users" || p == "groups" || p == "dependencies" || + p == SHARED_LOCK_DIR || p == "migrations" ); }; @@ -2819,6 +2996,7 @@ export async function ignoreF(wmillconf: { includes?: string[]; excludes?: string[]; extraIncludes?: string[]; + dedupeLockfiles?: boolean; skipResourceTypes?: boolean; skipWorkspaceDependencies?: boolean; skipDatatableMigrations?: boolean; @@ -2859,6 +3037,15 @@ export async function ignoreF(wmillconf: { // new Gitignore.default({ initialRules: ignoreContent.split("\n")}).ignoreContent).compile(); return (p: string, isDirectory: boolean) => { + // Without the option, `locks/` is not Windmill's: a repo that keeps its own + // lockfiles there would otherwise see them pulled into the diff and deleted + // as absent from a remote that never serializes shared locks. + if ( + !wmillconf.dedupeLockfiles && + (p === SHARED_LOCK_DIR || p.startsWith(SHARED_LOCK_DIR + SEP)) + ) { + return true; + } const ext = wmillconf.json ? ".json" : ".yaml"; if (!isDirectory && p.endsWith(".resource-type" + ext)) { return wmillconf.skipResourceTypes ?? false; @@ -2886,6 +3073,12 @@ export async function ignoreF(wmillconf: { ) { return false; // Don't ignore workspace dependencies (they are always included unless explicitly skipped) } + // A shared lockfile lives outside the u/f/g namespaces the include + // patterns are written against, and dropping it from the diff would + // leave every script that references it pointing at nothing. + if (fileType === "shared_lock") { + return false; + } // `migrations/datatable/**` is outside the u/f/g namespaces the path // filters are written against, so the skip flag is its only control. if ( @@ -3527,6 +3720,13 @@ export async function pull( return; } + // Script lockfile deletions, held back until every metadata edit has been + // applied or refused — see the deletion branch below. + const deferredLockDeletions: { + path: string; + target: string; + stateTarget: string; + }[] = []; const conflicts = []; log.info(colors.gray(`Applying changes to files ...`)); @@ -3666,6 +3866,15 @@ export async function pull( await copyFile(target, stateTarget); } } else if (change.name === "deleted") { + // A script's lockfile goes last, once the metadata around it has + // settled: `dedupeLockfiles` deletes the per-script locks it collapses, + // and a conflict resolved as "preserve local" keeps metadata that still + // reads one. Deleted here, that reference would dangle and the script + // would deploy with an empty lock. + if (isScriptLockPath(change.path) || isSharedLockPath(change.path)) { + deferredLockDeletions.push({ path: change.path, target, stateTarget }); + continue; + } log.info(`Deleting ${changeTypeLabel(change.path)}${change.path}`); // `force` on both: the goal is that neither copy exists, and a file // already absent — a dbt project's optional descriptor is never written @@ -3678,6 +3887,28 @@ export async function pull( } } } + + const sharedReaders = deferredLockDeletions.some((d) => + isSharedLockPath(d.path), + ) + ? await collectSharedLockReaders(opts.json ?? false) + : { byRef: new Map(), unreadable: [] }; + for (const deferred of deferredLockDeletions) { + const keptBecause = await lockStillReadBecause( + deferred.path, + opts.json ?? false, + sharedReaders, + ); + if (keptBecause !== undefined) { + log.info(colors.yellow(`Keeping ${deferred.path}: ${keptBecause}.`)); + continue; + } + log.info(`Deleting ${changeTypeLabel(deferred.path)}${deferred.path}`); + await rm(deferred.target, { force: true }); + if (opts.stateful) { + await rm(deferred.stateTarget, { force: true }); + } + } if (opts.failConflicts) { if (conflicts.length > 0) { console.error(colors.red(`Conflicts were found`)); @@ -3868,6 +4099,92 @@ export async function pull( // stays exported for callers that want the same commit/push behavior. } +/** + * Fold the lockfile that the scripts of a language share back into the one + * shared file (`dedupeLockfiles`), and give the scripts that ended up with a + * lock of their own theirs back. + * + * A lock is regenerated one script at a time, so only a pass over the whole + * tree can tell a dependency bump every script took (the shared file moves) + * from one script drifting away from the rest (it gets a lock of its own). + * + * Rewriting a script's metadata invalidates the hash the generation above just + * recorded, so every rewritten script is re-hashed from disk — the same + * lock-untouching pass `sync pull` runs, no dependency job involved. + */ +export async function dedupeLockfilesOnDisk(args: { + opts: GlobalOptions & SyncOptions; + workspace: Workspace; + codebases: SyncCodebase[]; + ignore: (p: string, isD: boolean) => boolean; + rawWorkspaceDependencies: Record; + tree: DoubleLinkedDependencyTree; + /** Content paths whose generation failed this run. Their metadata may be + * rewritten, but never re-hashed: recording a hash for a script whose lock + * never regenerated marks it up-to-date, and it is never retried. */ + failed?: string[]; + dryRun?: boolean; +}): Promise { + const { + opts, + workspace, + codebases, + ignore, + rawWorkspaceDependencies, + tree, + failed = [], + dryRun, + } = args; + const map = await elementsToMap( + await FSFSElement(process.cwd(), codebases, false), + ignore, + opts.json ?? false, + opts, + ); + const plan = computeSharedLockPlan(map, { + defaultTs: opts.defaultTs, + depFiles: opts.skipWorkspaceDependencies + ? Object.keys(await getRawWorkspaceDependencies(false)) + : undefined, + }); + if (isEmptySharedLockPlan(plan)) return; + + const summary = `${Object.keys(plan.writes).length} file(s) written, ${plan.deletes.length} removed`; + if (dryRun) { + log.info(`Would deduplicate lockfiles: ${summary}`); + return; + } + + await applySharedLockPlanToDisk(plan); + log.info(`Deduplicated lockfiles: ${summary}`); + + for (const rewritten of Object.keys(plan.writes)) { + // Metadata only — the lockfiles the plan also writes are not hashed. + if (!rewritten.endsWith(".yaml") && !rewritten.endsWith(".json")) continue; + let contentPath: string | undefined; + try { + contentPath = await findContentFile(rewritten); + } catch { + continue; + } + if (!contentPath || failed.includes(contentPath)) continue; + await generateScriptMetadataInternal( + contentPath, + workspace, + opts, + false, // dryRun + true, // noStaleMessage + rawWorkspaceDependencies, + codebases, + true, // justUpdateMetadataLock: re-hash from disk, no lock generation + // The same tree the generation above ran with: it is what decides whether + // the workspace dependencies are part of the hash, and a hash written the + // other way would read as stale on every later run. + tree, + ); + } +} + // Internal git-sync deployment-callback entrypoint. Invoked only by the // git-sync hub script (not user-facing — see the hidden `git-deploy` // subcommand). Runs inside an existing clone of the repo: switches to the @@ -4360,6 +4677,86 @@ export async function push( const tracker: ChangeTracker = await buildTracker(changes); + // A shared lockfile (`dedupeLockfiles`) has no object of its own on the + // remote: it IS the lock of every script that references it, and those + // scripts are what carries its new content over. Nothing else queues them — + // their own metadata is byte-identical on both sides. + // + // After the tracker on purpose: these scripts need no metadata regeneration + // (their lock is on disk already, in the shared file), and `--auto-metadata` + // would otherwise run one dependency job per script sharing the lock. + const changedPaths = new Set(changes.map((c) => c.path)); + let unconvertedTree = false; + // The whole tree, not `localMap`: `includes`/`excludes` have already filtered + // that, and a shared lockfile's readers are exactly what the filter hides. + // One metadata pass, on the first shared-lock change and never otherwise, so + // it is a dependency bump that pays for it and not an ordinary push. + let treeReaders: SharedLockReaders | undefined; + for (let i = changes.length - 1; i >= 0; i--) { + const change = changes[i]; + if (!isSharedLockPath(change.path)) continue; + if (change.name === "deleted") { + // The remote view is deduplicated whether or not the tree is: a shared + // lockfile missing from the tree reads as a deletion to push, and there is + // no such object to delete. + unconvertedTree = true; + changes.splice(i, 1); + continue; + } + const referrers = scriptsReferencingSharedLock(localMap, change.path); + if (treeReaders === undefined) { + try { + treeReaders = await collectSharedLockReaders(opts.json ?? false); + } catch (e) { + // The walk is fail-loud because a deletion hangs on it. Nothing hangs + // on an advisory, so an unreadable directory costs the advisory, not + // the push. + log.debug(`Could not scan for shared-lock readers: ${e}`); + treeReaders = { byRef: new Map(), unreadable: [] }; + } + } + const outOfScope = + (treeReaders?.byRef.get(change.path.replaceAll(SEP, "/"))?.length ?? 0) - + referrers.length; + // Out of `changes` either way: a shared lockfile has no object on the + // remote, so the apply loop skips it. Left in, the preview and the "N + // changes" count would report something no push ever applies as such. + changes.splice(i, 1); + // A scoped push deploys what it was scoped to, so the readers the filter + // excluded keep the previous lock on the remote. Silence there is the + // trap: the changed file has no object of its own, so nothing else in the + // output would account for it. + if (outOfScope > 0) { + log.warn( + colors.yellow( + `${change.path} changed, but ${outOfScope} of the script(s) sharing it are outside this push's scope and keep the previous lock on the remote. Widen --includes/--excludes to deploy the new lock to all of them.`, + ), + ); + } + if (referrers.length === 0) continue; + log.info( + colors.gray( + `${change.path} changed: re-pushing the ${referrers.length} script(s) sharing it`, + ), + ); + for (const metaPath of referrers) { + if (changedPaths.has(metaPath)) continue; + changes.push({ + name: "edited", + path: metaPath, + before: localMap[metaPath], + after: localMap[metaPath], + }); + } + } + if (unconvertedTree) { + log.warn( + colors.yellow( + `dedupeLockfiles is on but this checkout still holds one lockfile per script. Run 'wmill generate-metadata' (or pull) to convert it — until then every script reads as changed.`, + ), + ); + } + const autoRegenerate = !!(opts as any).autoMetadata; const staleScripts: string[] = []; const staleFlows: string[] = []; @@ -4517,6 +4914,26 @@ export async function push( staleApps.push(generated as string); } } + + if (opts.dedupeLockfiles) { + // Batched: the pass re-hashes every metadata file it rewrites, and one + // wmill-lock.yaml write per script is what a workspace-wide conversion + // would otherwise cost. + await beginLockfileBatch(); + try { + await dedupeLockfilesOnDisk({ + opts, + workspace, + codebases, + ignore: await ignoreF(opts), + rawWorkspaceDependencies, + tree, + dryRun: opts.dryRun, + }); + } finally { + await flushLockfileBatch(); + } + } } if (staleScripts.length > 0) { @@ -5085,6 +5502,11 @@ export async function push( } for await (const change of changes) { + // A shared lockfile is a repo-side artifact: the scripts queued + // above are what deploys its content. + if (isSharedLockPath(change.path)) { + continue; + } // A datatable migration is one record across two files; upsert/delete // it from disk once (deduped), regardless of which file changed. if (isDatatableMigrationPath(change.path)) { diff --git a/cli/src/core/conf.ts b/cli/src/core/conf.ts index ebad7bc51c..1fcce7a43f 100644 --- a/cli/src/core/conf.ts +++ b/cli/src/core/conf.ts @@ -109,6 +109,7 @@ export interface SyncOptions { promotion?: string; lint?: boolean; locksRequired?: boolean; + dedupeLockfiles?: boolean; syncBehavior?: string; } diff --git a/cli/src/types.ts b/cli/src/types.ts index 9684b78c5f..c9244a8f4c 100644 --- a/cli/src/types.ts +++ b/cli/src/types.ts @@ -35,6 +35,7 @@ import { buildFolderPath, isScriptModulePath, } from "./utils/resource_folders.ts"; +import { isSharedLockPath } from "./utils/script_common.ts"; export interface DifferenceCreate { type: "CREATE"; @@ -366,6 +367,7 @@ export function getTypeStrFromPath( | "group" | "settings" | "encryption_key" + | "shared_lock" | "workspace_dependencies" { if (isDatatableMigrationPath(p)) { return "datatable_migration"; @@ -382,6 +384,10 @@ export function getTypeStrFromPath( if (isRawAppPath(p)) { return "raw_app"; } + // A repo-side artifact of `dedupeLockfiles`: it has no object on the server. + if (isSharedLockPath(p)) { + return "shared_lock"; + } if (p.startsWith("dependencies" + SEP)) { return "workspace_dependencies"; } diff --git a/cli/src/utils/lock_dedup.ts b/cli/src/utils/lock_dedup.ts new file mode 100644 index 0000000000..88778bc569 --- /dev/null +++ b/cli/src/utils/lock_dedup.ts @@ -0,0 +1,556 @@ +import { stringify as yamlStringify } from "yaml"; +import { mkdir, rm, writeFile } from "node:fs/promises"; +import { existsSync } from "node:fs"; +import * as path from "node:path"; +import { yamlOptions } from "../commands/sync/sync.ts"; +import { yamlParseContent } from "./yaml.ts"; +import { + depFileOfSharedLock, + extractWorkspaceDepsAnnotation, + hasLockAffectingAnnotation, + inferContentTypeFromFilePath, + isSharedLockPath, + languageNeedsLock, + sharedLockPathFor, + workspaceDependenciesPathToLanguageAndFilename, + type ScriptLanguage, +} from "./script_common.ts"; + +/** + * Lockfile deduplication (`dedupeLockfiles` in wmill.yaml). + * + * A workspace whose dependencies come from `dependencies/` resolves to the + * very same lock for every script of that language, so the repo ends up holding + * thousands of byte-identical `.script.lock` files: one dependency bump rewrites + * all of them, and every open branch conflicts on all of them. + * + * With dedup on, the scripts that resolve against a workspace dependency file + * reference ONE lockfile named after it — `dependencies/requirements.in` -> + * `locks/requirements.in.lock` — through the `!inline` indirection their + * metadata already uses. A dependency bump is a one-file diff. + * + * Identity comes from the dependency file, never from the content or from which + * scripts happen to be in view. That is what makes the pass stateless: a sync + * narrowed to a single script (the git-sync deploy callback) computes the same + * NAME as a full one. + * + * The CONTENT follows, because a group only ever holds scripts whose lock IS + * that file's lock: one carrying an annotation the worker acts on — a pinned + * interpreter, `npm`, `nobundling` — never joins, so there is no variant inside + * a group to tell apart from a bump. What is left is a script whose committed + * lock is simply behind, and the many outvote it. + * + * Two cases are not shared at all and keep a `.script.lock` of their own: a + * script whose annotation is `extra_` or carries inline dependencies (its lock + * folds in its own imports), and one naming several dependency files at once + * (its lock is no single file's). + */ + +const INLINE_PREFIX = "!inline "; + +/** Sync maps are keyed with the platform separator, while an `!inline` + * reference is always forward-slash. */ +const toMapKey = (refPath: string) => refPath.replaceAll("/", path.sep); +const toRefPath = (mapKey: string) => mapKey.replaceAll("\\", "/"); + +/** What the sync layer needs to know to dedup: whether to, and how to read a + * `.ts` script's language. Both ride on the same `wmill.yaml` options object. */ +export type LockDedupOptions = { + dedupeLockfiles?: boolean | undefined; + defaultTs?: "bun" | "deno" | undefined; +}; + +/** The files a dedup pass writes and removes, as paths relative to the sync + * root — applicable to an in-memory sync map or to the working tree. */ +export type SharedLockPlan = { + writes: Record; + deletes: string[]; +}; + +export function isEmptySharedLockPlan(plan: SharedLockPlan): boolean { + return Object.keys(plan.writes).length === 0 && plan.deletes.length === 0; +} + +type ScriptEntry = { + metaKey: string; + isJson: boolean; + compactJson: boolean; + parsed: Record; + /** The lockfile the metadata references today, as a map key. */ + lockKey: string; + /** The lockfile this script owns when it is not sharing one. */ + ownLockKey: string; + lock: string; + /** The workspace dependency file whose lock this is, when it is one. */ + depFile: string | undefined; +}; + +/** `f/foo.script.yaml` -> base `f/foo`, lock `f/foo.script.lock`; + * `f/foo__mod/script.yaml` -> base `f/foo__mod/script`, lock `…/script.lock`. + * The base is what the script's content file is named after. All returned + * forward-slashed, whatever the map's separator. */ +function scriptMetaBase( + key: string, +): { base: string; ownLockKey: string; isJson: boolean } | undefined { + const ref = toRefPath(key); + for (const [suffix, isJson] of [ + [".script.yaml", false], + [".script.json", true], + ["/script.yaml", false], + ["/script.json", true], + ] as const) { + if (!ref.endsWith(suffix)) continue; + const stripped = ref.slice(0, ref.length - suffix.length); + if (suffix.startsWith("/")) { + // `/script.yaml` is only script metadata inside a module folder; anywhere + // else it is an ordinary file that happens to be called `script.yaml`. + if (!stripped.endsWith("__mod")) return undefined; + return { + base: stripped + "/script", + ownLockKey: stripped + "/script.lock", + isJson, + }; + } + return { base: stripped, ownLockKey: stripped + ".script.lock", isJson }; + } + return undefined; +} + +/** Content-file extensions of the languages that carry a lock, longest first. + * A language absent here is simply never deduplicated. + * for related places search: ADD_NEW_LANG */ +const LOCKABLE_EXTS = [ + ".fetch.ts", + ".deno.ts", + ".bun.ts", + ".playbook.yml", + ".ts", + ".py", + ".go", + ".php", + ".rs", +]; + +/** Map keys grouped by directory, so a script's content file is looked up among + * its own directory's entries: splitting a name at its first dot would put + * `f/a.b.py` under `f/a` and leave every dotted script path undeduplicated. */ +function indexByDirectory( + map: Record, +): Map> { + const index = new Map>(); + for (const key of Object.keys(map)) { + const ref = toRefPath(key); + const dir = ref.slice(0, ref.lastIndexOf("/") + 1); + const bucket = index.get(dir); + if (bucket) { + bucket.add(ref); + } else { + index.set(dir, new Set([ref])); + } + } + return index; +} + +/** A script's content file and its language: `` and nothing looser, + * since `f/a.b.py` is the content file of `f/a.b`, not of `f/a`. */ +function contentOfScript( + map: Record, + base: string, + byDirectory: Map>, + defaultTs: "bun" | "deno" | undefined, +): { content: string; language: ScriptLanguage } | undefined { + const siblings = byDirectory.get(base.slice(0, base.lastIndexOf("/") + 1)); + if (!siblings) return undefined; + for (const ext of LOCKABLE_EXTS) { + const candidate = base + ext; + if (!siblings.has(candidate)) continue; + const content = map[toMapKey(candidate)] ?? map[candidate]; + if (content === undefined) continue; + try { + return { + content, + language: inferContentTypeFromFilePath(candidate, defaultTs), + }; + } catch { + // not a language this CLI knows + } + } + return undefined; +} + +/** A map entry addressed by a forward-slashed path, whatever separator the map + * was keyed with. */ +function lookup( + map: Record, + refPath: string, +): { key: string; content: string } | undefined { + for (const key of [toMapKey(refPath), refPath]) { + const content = map[key]; + if (content !== undefined) return { key, content }; + } + return undefined; +} + +/** + * Both parsers, declared format first. YAML is a superset of JSON, and + * flow-style YAML (`{summary: x, lock: '!inline …'}`) starts with `{` while + * failing `JSON.parse` — deciding by the first character loses the reference. + */ +function parseMetadata( + metaPath: string, + metaContent: string, + isJson: boolean, +): Record | undefined { + for (const asJson of isJson ? [true, false] : [false, true]) { + try { + const parsed = asJson + ? JSON.parse(metaContent) + : yamlParseContent(metaPath, metaContent); + if (typeof parsed === "object" && parsed !== null) return parsed; + } catch { + // try the other one + } + } + return undefined; +} + +/** + * The shared lockfile a metadata file's `lock` field names, if any. The raw text + * is only a prefilter: a summary or a comment can carry the same words, and + * `!inline` decides where a lock is written, so it is read from the parsed field + * and nowhere else. + */ +export function sharedLockRefOf( + metaPath: string, + metaContent: string, + isJson: boolean, +): string | undefined { + // Without the trailing space: the YAML serializer folds a long `lock:` line + // at a space, so `!inline locks/…` can reach disk as `!inline\n locks/…` + // and a prefilter looking for the space would call it a non-reader. + if (!metaContent.includes(INLINE_PREFIX.trimEnd())) return undefined; + const lock = parseMetadata(metaPath, metaContent, isJson)?.["lock"]; + if (typeof lock !== "string" || !lock.startsWith(INLINE_PREFIX)) { + return undefined; + } + const ref = lock.slice(INLINE_PREFIX.length); + return isSharedLockPath(ref) ? ref : undefined; +} + +/** + * Whether a metadata file may reference a shared lockfile but cannot say which. + * + * A `.script.yaml` carrying git conflict markers is a file whose `lock` cannot + * be read, not one that reads nothing, and deleting a lockfile it may point at + * is the unrecoverable half of that guess. + */ +export function metadataLockUnreadable( + metaPath: string, + metaContent: string, + isJson: boolean, +): boolean { + if (!metaContent.includes(INLINE_PREFIX.trimEnd())) return false; + return parseMetadata(metaPath, metaContent, isJson) === undefined; +} + +/** + * The shared lockfile a metadata FILE reads, when it reads one that is there. + * `parseMetadataFile` resolves `lock` to the lockfile's content, so the + * reference itself survives only in the raw text. + */ +export function sharedLockRefIn( + metadataContent: string, + isJson: boolean, + root: string = ".", +): string | undefined { + const ref = sharedLockRefOf("metadata", metadataContent, isJson); + return ref && existsSync(path.resolve(root, ref)) ? ref : undefined; +} + +/** The key a dependency file answers to, i.e. what a script names it by. */ +function depKeyOf(depFilePath: string): string | undefined { + const info = workspaceDependenciesPathToLanguageAndFilename(depFilePath); + return info && languageNeedsLock(info.language) + ? `${info.language} ${info.name ?? "default"}` + : undefined; +} + +/** Workspace dependency files keyed by the language and name a script names. */ +function depFilesByKey(paths: Iterable): Map { + const byKey = new Map(); + for (const key of paths) { + const ref = toRefPath(key); + if (!ref.startsWith("dependencies/")) continue; + // A set named `team/python` exports as `dependencies/team/python.`, + // which has no distinct name under `locks/`: flattened it collides with the + // top-level file, and the sweep would then retire a lockfile whose scripts + // still read it. Such a set shares nothing and its scripts keep own locks. + if (ref.slice("dependencies/".length).includes("/")) continue; + const depKey = depKeyOf(ref); + if (depKey) byKey.set(depKey, ref); + } + return byKey; +} + +/** + * The workspace dependency file whose lock a script's lock IS — undefined when + * the script's lock is its own (see the header for the two cases). + */ +function shareableDepFile( + scriptContent: string, + language: ScriptLanguage, + depFiles: Map, +): string | undefined { + // A script the worker locks differently for reasons of its own — a pinned + // interpreter, `//npm`, `//nobundling` — cannot stand for its dependency + // file's lock, so it never joins a group and its lock stays its own. + if (hasLockAffectingAnnotation(scriptContent, language)) return undefined; + const annotation = extractWorkspaceDepsAnnotation(scriptContent, language); + if (annotation && (annotation.mode === "extra" || annotation.inline)) { + return undefined; + } + const names = annotation ? annotation.external : ["default"]; + if (names.length !== 1) return undefined; + return depFiles.get(`${language} ${names[0]}`); +} + +/** + * The shared lockfile a script belongs in, given the workspace dependency files + * available — the one place that decides it, for both the sync planner and the + * per-script regeneration in `updateScriptLock`. + */ +export function sharedLockTargetFor( + scriptContent: string, + language: ScriptLanguage, + depPaths: Iterable, +): string | undefined { + const depFile = shareableDepFile( + scriptContent, + language, + depFilesByKey(depPaths), + ); + return depFile === undefined ? undefined : sharedLockPathFor(depFile); +} + +function collectScripts( + map: Record, + defaultTs: "bun" | "deno" | undefined, + depFiles: Map, +): ScriptEntry[] { + const byDirectory = indexByDirectory(map); + const entries: ScriptEntry[] = []; + for (const [metaKey, metaContent] of Object.entries(map)) { + const meta = scriptMetaBase(metaKey); + if (!meta) continue; + + const parsed = parseMetadata(metaKey, metaContent, meta.isJson); + if (parsed === undefined) continue; + + const lockRef = parsed["lock"]; + if (typeof lockRef !== "string" || !lockRef.startsWith(INLINE_PREFIX)) { + continue; + } + const lockFile = lookup(map, lockRef.slice(INLINE_PREFIX.length)); + // An absent or empty lock is not a lock to share: a script with no + // dependencies carries `lock: ''` and no file at all. + if (lockFile === undefined || lockFile.content === "") continue; + + const script = contentOfScript(map, meta.base, byDirectory, defaultTs); + if (script === undefined || !languageNeedsLock(script.language)) continue; + + entries.push({ + metaKey, + isJson: meta.isJson, + compactJson: !metaContent.includes("\n"), + parsed, + lockKey: lockFile.key, + ownLockKey: toMapKey(meta.ownLockKey), + lock: lockFile.content, + depFile: shareableDepFile(script.content, script.language, depFiles), + }); + } + return entries; +} + +function serializeMetadata(entry: ScriptEntry): string { + if (!entry.isJson) return yamlStringify(entry.parsed, yamlOptions); + // Indented or compact as it was found: `sync` writes JSON metadata indented + // and `generate-metadata` writes it compact, so imposing either one here + // reformats files this feature exists to keep quiet. + return entry.compactJson + ? JSON.stringify(entry.parsed) + : JSON.stringify(entry.parsed, null, 2); +} + +/** + * What a sync map (path -> content) has to change for the scripts of a workspace + * dependency file to share one lockfile. Pure: the map is not touched. + */ +export type SharedLockPlanContext = { + defaultTs?: "bun" | "deno" | undefined; + /** + * Workspace dependency files to consider beyond the ones in `map`, for the + * one caller whose map cannot hold them: `--skip-workspace-dependencies`. + * Pass nothing otherwise — with dependency files in the map, an absence there + * is a deletion, and adding disk's copy would keep a lockfile alive one sync + * past the file it is named after. + */ + depFiles?: Iterable; + /** + * Shared lockfiles the working tree already holds. The remote never + * serializes one, so without this a sync that has no script for a dependency + * file reads its lockfile as deleted — and every script still pointing at it + * is left with an `!inline` that resolves to nothing. + */ + present?: Record; +}; + +export function computeSharedLockPlan( + map: Record, + ctx: SharedLockPlanContext = {}, +): SharedLockPlan { + const plan: SharedLockPlan = { writes: {}, deletes: [] }; + const depFiles = depFilesByKey([...Object.keys(map), ...(ctx.depFiles ?? [])]); + // Every shared lockfile this sync can see, from either side. + const present: Record = { ...ctx.present }; + for (const [key, content] of Object.entries(map)) { + if (isSharedLockPath(toRefPath(key))) present[toRefPath(key)] = content; + } + + const byDepFile = new Map(); + const ownLock: ScriptEntry[] = []; + for (const entry of collectScripts(map, ctx.defaultTs, depFiles)) { + if (entry.depFile === undefined) { + ownLock.push(entry); + continue; + } + // Push into the existing array rather than rebuild it: a workspace where + // every script shares one dependency file is the case this exists for, and + // copying the group per insert makes that quadratic. + const group = byDepFile.get(entry.depFile); + if (group) group.push(entry); + else byDepFile.set(entry.depFile, [entry]); + } + + const point = (entry: ScriptEntry, targetKey: string) => { + if (entry.lockKey === targetKey) return; + // A shared lockfile is dropped by the sweep below, which knows whether its + // dependency file is still there; only a private one goes with its script. + if (!isSharedLockPath(entry.lockKey)) plan.deletes.push(entry.lockKey); + entry.parsed["lock"] = INLINE_PREFIX + toRefPath(targetKey); + plan.writes[entry.metaKey] = serializeMetadata(entry); + }; + + const takeOwnLock = (entry: ScriptEntry) => { + if (map[entry.ownLockKey] !== entry.lock) { + plan.writes[entry.ownLockKey] = entry.lock; + } + point(entry, entry.ownLockKey); + }; + + for (const [depFile, group] of byDepFile) { + // Every script here resolves against the same file and carries nothing the + // worker locks separately, so their locks agree — unless one's committed + // lock is simply behind. The many outvote the one; ties break on the content + // itself so the outcome never depends on map ordering. + const byContent = new Map(); + for (const entry of group) { + const sameLock = byContent.get(entry.lock); + if (sameLock) sameLock.push(entry); + else byContent.set(entry.lock, [entry]); + } + let content = ""; + let count = 0; + for (const [lock, members] of byContent) { + if ( + members.length > count || + (members.length === count && lock < content) + ) { + content = lock; + count = members.length; + } + } + + const sharedKey = toMapKey(sharedLockPathFor(depFile)); + if (map[sharedKey] !== content) plan.writes[sharedKey] = content; + for (const entry of group) { + if (entry.lock === content) point(entry, sharedKey); + else takeOwnLock(entry); + } + } + + // A script that stopped resolving against a dependency file takes its lock + // back with it. + for (const entry of ownLock) { + if (entry.lockKey !== entry.ownLockKey) takeOwnLock(entry); + } + + // A shared lockfile lives exactly as long as the dependency file it is named + // after. Asking that, rather than "does any script still read it", is what + // lets a sync narrowed to one item leave the rest of the workspace alone: a + // lockfile with no script in view is carried forward, not deleted. + for (const [sharedRef, content] of Object.entries(present)) { + const depFile = depFileOfSharedLock(sharedRef); + if (depFile === undefined) continue; + const key = toMapKey(sharedRef); + if (plan.writes[key] !== undefined) continue; + if (depFiles.has(depKeyOf(depFile) ?? "")) { + if (map[key] === undefined) plan.writes[key] = content; + } else { + plan.deletes.push(key); + } + } + + // Deletes are applied after writes, so a path some script still writes must + // not also be dropped — two metadata files pointing at one lock file would + // otherwise cancel each other out and leave the survivor without a lock. + plan.deletes = plan.deletes.filter((key) => plan.writes[key] === undefined); + + return plan; +} + +export function applySharedLockPlanToMap( + map: Record, + plan: SharedLockPlan, +): void { + for (const [key, content] of Object.entries(plan.writes)) { + map[key] = content; + } + for (const key of plan.deletes) { + delete map[key]; + } +} + +export async function applySharedLockPlanToDisk( + plan: SharedLockPlan, +): Promise { + for (const [key, content] of Object.entries(plan.writes)) { + // Per write, so `locks/` comes into existence only when there is a shared + // lockfile to put in it. + await mkdir(path.dirname(key), { recursive: true }); + await writeFile(key, content, "utf-8"); + } + for (const key of plan.deletes) { + await rm(key, { force: true }); + } +} + +/** + * The script metadata files that read a given shared lockfile. A change to that + * file is a change to their lock, and they are what carries it to the remote. + */ +export function scriptsReferencingSharedLock( + map: Record, + sharedKey: string, +): string[] { + const reference = toRefPath(sharedKey); + const referrers: string[] = []; + for (const [metaKey, metaContent] of Object.entries(map)) { + const meta = scriptMetaBase(metaKey); + if (!meta) continue; + if (sharedLockRefOf(metaKey, metaContent, meta.isJson) === reference) { + referrers.push(metaKey); + } + } + return referrers; +} diff --git a/cli/src/utils/metadata.ts b/cli/src/utils/metadata.ts index 1905121c3a..1f2ef86769 100644 --- a/cli/src/utils/metadata.ts +++ b/cli/src/utils/metadata.ts @@ -4,7 +4,7 @@ import { colors } from "@cliffy/ansi/colors"; import * as log from "../core/log.ts"; import { stringify as yamlStringify } from "yaml"; import { yamlParseFile } from "./yaml.ts"; -import { writeFile, stat, rm, readdir } from "node:fs/promises"; +import { writeFile, stat, rm, readdir, mkdir } from "node:fs/promises"; import { readFileSync, existsSync, readdirSync, statSync, mkdirSync, writeFileSync } from "node:fs"; import * as path from "node:path"; import { createRequire } from "node:module"; @@ -17,8 +17,20 @@ import { ScriptLanguage, workspaceDependenciesLanguages, languageNeedsLock, + LANG_COMMENT_LIT, } from "./script_common.ts"; import { inferContentTypeFromFilePath } from "./script_common.ts"; +// Workspace-dependency vocabulary lives with the languages it describes; these +// re-exports keep the CLI's existing import sites working. +export { + workspaceDependenciesPathToLanguageAndFilename, + extractWorkspaceDepsAnnotation, + type WorkspaceDepsAnnotation, +} from "./script_common.ts"; +import { + workspaceDependenciesPathToLanguageAndFilename, + extractWorkspaceDepsAnnotation, +} from "./script_common.ts"; import { dbtGeneratedDirs, isUnderGeneratedDir, isBundledModuleFile, getModuleFolderSuffix, isModuleEntryPoint, scriptPathToRemotePath } from "./resource_folders.ts"; import { findCodebase, yamlOptions } from "../commands/sync/sync.ts"; import { generateHash, readInlinePathSync, getHeaders, readTextFile, readTextFileSync } from "./utils.ts"; @@ -31,6 +43,7 @@ import { getIsWin } from "./utils.ts"; import { extractRelativeImports } from "./relative_imports.ts"; import { DoubleLinkedDependencyTree } from "./dependency_tree.ts"; import { pollJobWithQueueLogging } from "./job_polling.ts"; +import { sharedLockRefIn, sharedLockTargetFor } from "./lock_dedup.ts"; const _require = createRequire(import.meta.url); const _parserCache = new Map>(); @@ -106,17 +119,6 @@ export async function getRawWorkspaceDependencies(legacyBehaviour: boolean): Pro return rawWorkspaceDeps; } -export function workspaceDependenciesPathToLanguageAndFilename(path: string): { name: string | undefined, language: ScriptLanguage } | undefined { - const relativePath = path.replace("dependencies/", ""); - for (const { filename, language } of workspaceDependenciesLanguages) { - if (relativePath.endsWith(filename)) { - return { - name: relativePath === filename ? undefined : relativePath.replace("." + filename, ""), - language - }; - } - } -} /** * Filters raw workspace dependencies to only include those that: @@ -203,6 +205,7 @@ export async function generateScriptMetadataInternal( schemaOnly?: boolean | undefined; defaultTs?: "bun" | "deno"; rehashOnly?: boolean | undefined; + dedupeLockfiles?: boolean | undefined; }, dryRun: boolean, noStaleMessage: boolean, @@ -375,6 +378,15 @@ export async function generateScriptMetadataInternal( filteredRawWorkspaceDependencies, tempScriptRefs, lockPathOverride, + // The lockfile of the workspace dependency file this script resolves + // against, if any: joining it is a matter of resolving to its content. + opts.dedupeLockfiles + ? sharedLockTargetFor( + scriptContent, + language, + Object.keys(rawWorkspaceDependencies), + ) + : undefined, ); } else { metadataParsedContent.lock = ""; @@ -406,7 +418,14 @@ export async function generateScriptMetadataInternal( ); } } else { - if (metadataInFolder) { + // `parseMetadataFile` resolved `lock` to the lockfile's CONTENT, so the + // reference has to be restored from the raw text — including a shared one + // (`dedupeLockfiles`), which `--schema-only` would otherwise replace with a + // per-script path whose file deduplication removed. + const sharedRef = sharedLockRefIn(metadataContent, metadataWithType.isJson); + if (sharedRef) { + metadataParsedContent.lock = "!inline " + sharedRef; + } else if (metadataInFolder) { metadataParsedContent.lock = "!inline " + remotePath.replaceAll(SEP, "/") + getModuleFolderSuffix() + "/script.lock"; } else { @@ -493,119 +512,6 @@ export async function updateScriptSchema( delete metadataContent.no_main_func; } -// --------------------------------------------------------------------------- -// Annotation parser — mirrors backend's WorkspaceDependenciesAnnotatedRefs::parse -// (windmill-common/src/workspace_dependencies.rs) so the cache key captures -// exactly the parts of scriptContent that affect lockfile generation. -// --------------------------------------------------------------------------- - -type AnnotationMode = "manual" | "extra"; - -interface WorkspaceDepsAnnotation { - mode: AnnotationMode; - external: string[]; - inline: string | null; -} - -const LANG_ANNOTATION_CONFIG: Partial< - Record -> = { - python3: { comment: "#", keyword: "requirements", validityRe: /^#\s?(\S+)\s*$/ }, - bun: { comment: "//", keyword: "package_json" }, - nativets: { comment: "//", keyword: "package_json" }, - go: { comment: "//", keyword: "go_mod" }, - php: { comment: "//", keyword: "composer_json" }, - powershell: { comment: "#", keyword: "modules_json" }, -}; - -export function extractWorkspaceDepsAnnotation( - scriptContent: string, - language: ScriptLanguage, -): WorkspaceDepsAnnotation | null { - const config = LANG_ANNOTATION_CONFIG[language]; - if (!config) return null; - - const { comment, keyword, validityRe } = config; - const extraMarkerUnderscore = `extra_${keyword}:`; - const extraMarkerHyphen = `extra-${keyword}:`; - const manualMarker = `${keyword}:`; - - const stripComment = (l: string): string | null => { - if (!l.startsWith(comment)) return null; - return l.substring(comment.length).trimStart(); - }; - const isExtra = (l: string): boolean => { - const s = stripComment(l); - return s !== null && (s.startsWith(extraMarkerUnderscore) || s.startsWith(extraMarkerHyphen)); - }; - const isManual = (l: string): boolean => { - const s = stripComment(l); - return s !== null && s.startsWith(manualMarker); - }; - - const lines = scriptContent.split("\n"); - - // Find first annotation line (mirrors Rust find_position) - let pos = -1; - for (let i = 0; i < lines.length; i++) { - if (isExtra(lines[i]) || isManual(lines[i])) { - pos = i; - break; - } - } - if (pos === -1) return null; - - const annotationLine = lines[pos]; - const mode: AnnotationMode = isExtra(annotationLine) ? "extra" : "manual"; - - // Parse external references from the annotation line - const marker = mode === "extra" - ? (annotationLine.includes(extraMarkerUnderscore) ? extraMarkerUnderscore : extraMarkerHyphen) - : manualMarker; - const unparsed = annotationLine.replaceAll(marker, "").replaceAll(comment, ""); - const external = unparsed - .split(",") - .map((s) => s.trim()) - .filter((s) => s.length > 0); - - // Parse inline deps from subsequent lines - const inlineParts: string[] = []; - for (let i = pos + 1; i < lines.length; i++) { - const l = lines[i]; - if (validityRe) { - const match = validityRe.exec(l); - if (match && match[1]) { - inlineParts.push(match[1]); - } else { - break; - } - } else { - if (!l.startsWith(comment)) { - break; - } - inlineParts.push(l.substring(comment.length)); - } - } - - const inlineStr = inlineParts.join("\n"); - const inline = inlineStr.trim().length > 0 ? inlineStr : null; - - return { mode, external, inline }; -} - -// Mirrors backend ScriptLang::as_comment_lit (windmill-types/src/scripts.rs) -// for the languages that can reach the lock cache. -const LANG_COMMENT_LIT: Partial> = { - python3: "#", - ansible: "#", - powershell: "#", - bun: "//", - nativets: "//", - deno: "//", - go: "//", - php: "//", - rust: "//!", -}; /** * Returns the leading comment/blank-line block of the script, verbatim. @@ -783,6 +689,7 @@ async function updateScriptLock( rawWorkspaceDependencies: Record, tempScriptRefs?: Record, lockPathOverride?: string, + sharedLockRef?: string, ): Promise { if (!languageNeedsLock(language)) { // A dbt lock is written by the dependency job on a worker, from a real @@ -816,6 +723,23 @@ async function updateScriptLock( const lockPath = lockPathOverride ?? remotePath + ".script.lock"; if (lock != "") { + // Joins an agreeing shared lockfile, and never creates or moves one: this + // runs per script and in parallel, so two scripts that resolve differently + // — a pinned Python version, an `//npm` annotation — would both find the + // file absent, both write it, and one would lose its lock with no copy left + // anywhere. Deciding a shared lockfile's content is the whole-tree pass's + // job, which sees every script at once. + if ( + sharedLockRef && + existsSync(sharedLockRef) && + readTextFileSync(sharedLockRef) === lock + ) { + if (existsSync(lockPath)) { + await rm(lockPath); + } + metadataContent.lock = "!inline " + sharedLockRef; + return; + } await writeFile(lockPath, lock, "utf-8"); metadataContent.lock = "!inline " + lockPath.replaceAll(SEP, "/"); } else { diff --git a/cli/src/utils/script_common.ts b/cli/src/utils/script_common.ts index 318f84b7c9..d804318491 100644 --- a/cli/src/utils/script_common.ts +++ b/cli/src/utils/script_common.ts @@ -44,6 +44,222 @@ export const workspaceDependenciesLanguages: WorkspaceDependenciesLanguage[] = [ { language: "powershell", filename: "modules.json" }, ] as const; +export function workspaceDependenciesPathToLanguageAndFilename(path: string): { name: string | undefined, language: ScriptLanguage } | undefined { + const relativePath = path.replace("dependencies/", ""); + for (const { filename, language } of workspaceDependenciesLanguages) { + if (relativePath.endsWith(filename)) { + return { + name: relativePath === filename ? undefined : relativePath.replace("." + filename, ""), + language + }; + } + } +} + +// --------------------------------------------------------------------------- +// Annotation parser — mirrors backend's WorkspaceDependenciesAnnotatedRefs::parse +// (windmill-common/src/workspace_dependencies.rs), so the CLI can tell which +// workspace dependency file a script resolves against without asking a worker. +// --------------------------------------------------------------------------- + +export type AnnotationMode = "manual" | "extra"; + +export interface WorkspaceDepsAnnotation { + mode: AnnotationMode; + external: string[]; + inline: string | null; +} + +const LANG_ANNOTATION_CONFIG: Partial< + Record +> = { + python3: { comment: "#", keyword: "requirements", validityRe: /^#\s?(\S+)\s*$/ }, + bun: { comment: "//", keyword: "package_json" }, + nativets: { comment: "//", keyword: "package_json" }, + go: { comment: "//", keyword: "go_mod" }, + php: { comment: "//", keyword: "composer_json" }, + powershell: { comment: "#", keyword: "modules_json" }, +}; + +export function extractWorkspaceDepsAnnotation( + scriptContent: string, + language: ScriptLanguage, +): WorkspaceDepsAnnotation | null { + const config = LANG_ANNOTATION_CONFIG[language]; + if (!config) return null; + + const { comment, keyword, validityRe } = config; + const extraMarkerUnderscore = `extra_${keyword}:`; + const extraMarkerHyphen = `extra-${keyword}:`; + const manualMarker = `${keyword}:`; + + const stripComment = (l: string): string | null => { + if (!l.startsWith(comment)) return null; + return l.substring(comment.length).trimStart(); + }; + const isExtra = (l: string): boolean => { + const s = stripComment(l); + return s !== null && (s.startsWith(extraMarkerUnderscore) || s.startsWith(extraMarkerHyphen)); + }; + const isManual = (l: string): boolean => { + const s = stripComment(l); + return s !== null && s.startsWith(manualMarker); + }; + + const lines = scriptContent.split("\n"); + + // Find first annotation line (mirrors Rust find_position) + let pos = -1; + for (let i = 0; i < lines.length; i++) { + if (isExtra(lines[i]) || isManual(lines[i])) { + pos = i; + break; + } + } + if (pos === -1) return null; + + const annotationLine = lines[pos]; + const mode: AnnotationMode = isExtra(annotationLine) ? "extra" : "manual"; + + // Parse external references from the annotation line + const marker = mode === "extra" + ? (annotationLine.includes(extraMarkerUnderscore) ? extraMarkerUnderscore : extraMarkerHyphen) + : manualMarker; + const unparsed = annotationLine.replaceAll(marker, "").replaceAll(comment, ""); + const external = unparsed + .split(",") + .map((s) => s.trim()) + .filter((s) => s.length > 0); + + // Parse inline deps from subsequent lines + const inlineParts: string[] = []; + for (let i = pos + 1; i < lines.length; i++) { + const l = lines[i]; + if (validityRe) { + const match = validityRe.exec(l); + if (match && match[1]) { + inlineParts.push(match[1]); + } else { + break; + } + } else { + if (!l.startsWith(comment)) { + break; + } + inlineParts.push(l.substring(comment.length)); + } + } + + const inlineStr = inlineParts.join("\n"); + const inline = inlineStr.trim().length > 0 ? inlineStr : null; + + return { mode, external, inline }; +} + +/** The comment marker each language's annotations are written behind. */ +export const LANG_COMMENT_LIT: Partial> = { + python3: "#", + ansible: "#", + powershell: "#", + bun: "//", + nativets: "//", + deno: "//", + go: "//", + php: "//", + rust: "//!", +}; + +/** + * The annotations each language recognises, by the exact names the worker + * matches (`#[annotations(..)]` structs in windmill-common/src/worker.rs). + * Several change what it locks — a pinned interpreter, `npm`, `nobundling` — + * and the rest are cheap to treat the same way, since the only cost is that + * such a script keeps a lockfile of its own. + * for related places search: ADD_NEW_LANG + */ +const LANG_ANNOTATIONS: Partial> = { + python3: [ + "no_cache", + "no_postinstall", + "py_select_latest", + "skip_result_postprocessing", + "py310", + "py311", + "py312", + "py313", + "sandbox", + ], + bun: ["npm", "nodejs", "native", "nobundling", "sandbox"], + nativets: ["npm", "nodejs", "native", "nobundling", "sandbox"], + deno: ["npm", "nodejs", "native", "nobundling", "sandbox"], + go: ["go1_22_compat"], +}; + +/** + * Whether a script's leading comment block carries an annotation the worker + * acts on, which means its lock may not be its dependency file's. + * + * Matched the way the worker matches: the key is the line, or what precedes the + * first `=`, and it has to BE one of the names above. Unknown keys are ignored + * there and so here — which is what keeps `# TODO:` or `# type: ignore` from + * quietly dropping an ordinary documented script out of deduplication. + * + * `# py: ` is the exception the macro does not cover: the python + * import parser reads it directly (`windmill-parser-py-imports`, alongside the + * `py310`..`py313` flags) to pick the interpreter, which changes what resolves. + */ +export function hasLockAffectingAnnotation( + scriptContent: string, + language: ScriptLanguage, +): boolean { + const comment = LANG_COMMENT_LIT[language]; + const names = LANG_ANNOTATIONS[language]; + if (!comment || !names) return false; + for (const line of scriptContent.split("\n")) { + const trimmed = line.trim(); + if (trimmed === "") continue; + if (!trimmed.startsWith(comment)) break; // past the header block + // Matched on the raw line: the parser tests `# py:`/`#py:` before trimming. + if (language === "python3" && /^#\s?py:/.test(line)) return true; + const body = trimmed.slice(comment.length).trim(); + const key = body.split("=")[0].trim(); + if (names.includes(key)) return true; + } + return false; +} + +/** Where the lockfiles shared by several scripts live when `dedupeLockfiles` + * is on — see `utils/lock_dedup.ts`. A top-level directory of its own: what a + * group shares is a resolved lock, which needs no workspace dependency file + * behind it, and inline-script locks would belong here too. */ +export const SHARED_LOCK_DIR = "locks"; + +/** The lockfile shared by the scripts that resolve against a workspace + * dependency file: its own name, plus `.lock`. Appending rather than replacing + * the extension keeps the correspondence exact and reversible — + * `dependencies/team_a.requirements.in` <-> `locks/team_a.requirements.in.lock`. */ +export function sharedLockPathFor(depFilePath: string): string { + const name = depFilePath.replaceAll("\\", "/").split("/").pop()!; + return `${SHARED_LOCK_DIR}/${name}.lock`; +} + +/** The workspace dependency file a shared lockfile belongs to, if it is one. */ +export function depFileOfSharedLock(p: string): string | undefined { + const normalized = p.replaceAll("\\", "/"); + if (!normalized.startsWith(SHARED_LOCK_DIR + "/")) return undefined; + const name = normalized.slice(SHARED_LOCK_DIR.length + 1); + if (name.includes("/") || !name.endsWith(".lock")) return undefined; + const depFile = "dependencies/" + name.slice(0, -".lock".length); + const info = workspaceDependenciesPathToLanguageAndFilename(depFile); + // `locks/vendor.lock` names no dependency file, so it is not Windmill's: a + // repo that already keeps lockfiles here keeps them. + return info && languageNeedsLock(info.language) ? depFile : undefined; +} + +export function isSharedLockPath(p: string): boolean { + return depFileOfSharedLock(p) !== undefined; +} + /** * Returns true if a script in the given language requires a lock file. * Matches the condition in updateScriptLock (metadata.ts). diff --git a/cli/test/lock_dedup_unit.test.ts b/cli/test/lock_dedup_unit.test.ts new file mode 100644 index 0000000000..62e17f8ae1 --- /dev/null +++ b/cli/test/lock_dedup_unit.test.ts @@ -0,0 +1,480 @@ +/** + * Lockfile deduplication (`dedupeLockfiles`) — WIN-1756. + * + * A workspace with one `dependencies/requirements.in` resolves the same lock for + * every Python script, so the repo carries thousands of identical + * `.script.lock` files: a dependency bump rewrites all of them and every open + * branch conflicts on all of them. Dedup keeps one lockfile per dependency file, + * named after it, and points the metadata at it. + * + * What these pin, per the invariants in `lock_dedup.ts`: + * - the name comes from the dependency file, so a bump is a one-file diff and a + * sync narrowed to one script reaches the same answer as a full one + * - a script whose lock is its own (an `extra_`/inline annotation, or several + * dependency files at once) keeps a `.script.lock` + * - the pass is idempotent, which is what keeps `sync pull`/`push` from seeing + * a diff on every run + */ + +import { expect, test, describe } from "bun:test"; +import { stringify as yamlStringify } from "yaml"; +import * as path from "node:path"; +import { mkdir, mkdtemp, rm, writeFile } from "node:fs/promises"; +import os from "node:os"; +import { + applySharedLockPlanToMap, + computeSharedLockPlan, + isEmptySharedLockPlan, + metadataLockUnreadable, + scriptsReferencingSharedLock, + sharedLockRefOf, + sharedLockRefIn, +} from "../src/utils/lock_dedup.ts"; +import { isSharedLockPath } from "../src/utils/script_common.ts"; +import { yamlOptions } from "../src/commands/sync/sync.ts"; + +const PY_LOCK = "requests==2.32.0\nurllib3==2.2.1\n"; +const PY_LOCK_BUMPED = "requests==2.32.3\nurllib3==2.2.1\n"; +const OTHER_LOCK = "requests==2.32.0\nurllib3==2.2.1\npandas==2.2.0\n"; +const PY_DEPS = "dependencies/requirements.in"; +const TEAM_DEPS = "dependencies/team_a.requirements.in"; +const BUN_DEPS = "dependencies/package.json"; +const SHARED_PY = "locks/requirements.in.lock"; +const SHARED_TEAM = "locks/team_a.requirements.in.lock"; +const SHARED_BUN = "locks/package.json.lock"; + +function meta(lockRef: string, summary = ""): string { + return yamlStringify({ summary, lock: lockRef }, yamlOptions); +} + +/** A workspace holding the given dependency files. */ +function workspace(...depFiles: string[]): Record { + const map: Record = {}; + for (const dep of depFiles) map[dep] = "requests\n"; + return map; +} + +/** A script with its own lockfile, as `sync pull` writes it without dedup. */ +function ownLock( + map: Record, + base: string, + ext: string, + lock: string, + body = "def main(): ...", +) { + map[`${base}${ext}`] = body; + map[`${base}.script.yaml`] = meta(`!inline ${base}.script.lock`); + map[`${base}.script.lock`] = lock; +} + +/** A script already reading a shared lockfile. */ +function sharedLock( + map: Record, + base: string, + ext: string, + shared: string, + body = "def main(): ...", +) { + map[`${base}${ext}`] = body; + map[`${base}.script.yaml`] = meta(`!inline ${shared}`); +} + +function lockRefOf(metaContent: string): string { + return metaContent.match(/lock: '(.*)'/)![1]; +} + +const plan = (map: Record) => + computeSharedLockPlan(map, { defaultTs: "bun" }); + +describe("computeSharedLockPlan", () => { + test("collapses the scripts of a dependency file into its lockfile", () => { + const map = workspace(PY_DEPS, BUN_DEPS); + ownLock(map, "f/a", ".py", PY_LOCK); + ownLock(map, "f/b", ".py", PY_LOCK); + ownLock(map, "f/c", ".py", PY_LOCK); + ownLock(map, "f/ts", ".ts", "bun-lock", "export async function main() {}"); + ownLock(map, "f/ts2", ".ts", "bun-lock", "export async function main() {}"); + + applySharedLockPlanToMap(map, plan(map)); + + expect(map[SHARED_PY]).toEqual(PY_LOCK); + expect(map[SHARED_BUN]).toEqual("bun-lock"); + for (const base of ["f/a", "f/b", "f/c"]) { + expect(map[`${base}.script.lock`]).toBeUndefined(); + expect(lockRefOf(map[`${base}.script.yaml`])).toEqual( + `!inline ${SHARED_PY}`, + ); + } + expect(lockRefOf(map["f/ts.script.yaml"])).toEqual(`!inline ${SHARED_BUN}`); + }); + + test("is idempotent — a deduplicated tree yields no further changes", () => { + const map = workspace(PY_DEPS); + ownLock(map, "f/a", ".py", PY_LOCK); + ownLock(map, "f/b", ".py", PY_LOCK); + applySharedLockPlanToMap(map, plan(map)); + + expect(isEmptySharedLockPlan(plan(map))).toBe(true); + }); + + test("a dependency bump moves only the shared file", () => { + const committed = workspace(PY_DEPS); + sharedLock(committed, "f/a", ".py", SHARED_PY); + sharedLock(committed, "f/b", ".py", SHARED_PY); + committed[SHARED_PY] = PY_LOCK; + + // What the remote sends after the bump: one lock per script, all bumped. + const remote = workspace(PY_DEPS); + ownLock(remote, "f/a", ".py", PY_LOCK_BUMPED); + ownLock(remote, "f/b", ".py", PY_LOCK_BUMPED); + applySharedLockPlanToMap(remote, plan(remote)); + + expect( + Object.keys(remote).filter((k) => remote[k] !== committed[k]), + ).toEqual([SHARED_PY]); + }); + + test("each named dependency file gets a lockfile of its own", () => { + const map = workspace(PY_DEPS, TEAM_DEPS); + ownLock(map, "f/a", ".py", PY_LOCK); + ownLock(map, "f/b", ".py", PY_LOCK); + const team = "# requirements: team_a\ndef main(): ..."; + ownLock(map, "f/t1", ".py", OTHER_LOCK, team); + ownLock(map, "f/t2", ".py", OTHER_LOCK, team); + + applySharedLockPlanToMap(map, plan(map)); + + expect(map[SHARED_PY]).toEqual(PY_LOCK); + expect(map[SHARED_TEAM]).toEqual(OTHER_LOCK); + expect(lockRefOf(map["f/t1.script.yaml"])).toEqual(`!inline ${SHARED_TEAM}`); + }); + + // The git-sync deploy callback narrows the sync to a single item, so this is + // the normal case rather than an edge one. + test("one script in view reaches the same answer as the whole workspace", () => { + const full = workspace(PY_DEPS); + for (const base of ["f/a", "f/b", "f/c"]) { + ownLock(full, base, ".py", PY_LOCK); + } + applySharedLockPlanToMap(full, plan(full)); + + const narrow = workspace(PY_DEPS); + ownLock(narrow, "f/a", ".py", PY_LOCK); + applySharedLockPlanToMap(narrow, plan(narrow)); + + expect(narrow[SHARED_PY]).toEqual(full[SHARED_PY]); + expect(lockRefOf(narrow["f/a.script.yaml"])).toEqual( + lockRefOf(full["f/a.script.yaml"]), + ); + }); + + test("a script the worker locks differently keeps a private lockfile", () => { + // The worker reads these from the leading comment block and several of them + // change what it locks, so such a script cannot stand for the file's lock. + const map = workspace(PY_DEPS, BUN_DEPS); + ownLock(map, "f/pinned", ".py", OTHER_LOCK, "# py311\nimport requests"); + + ownLock(map, "f/plain", ".py", PY_LOCK); + ownLock(map, "f/plain2", ".py", PY_LOCK); + // Only the names the worker matches count, so a documented script — or one + // with a `# TODO:` or a `# type: ignore` — still deduplicates. + ownLock(map, "f/doc", ".py", PY_LOCK, "# TODO: clean up\n# type: ignore\nimport requests"); + // Both annotation forms the worker accepts: a bare name and `name=value`. + ownLock(map, "f/npm", ".ts", "npm-lock", "//npm\nexport async function main() {}"); + ownLock(map, "f/nb", ".ts", "nb-lock", "//nobundling=true\nexport async function main() {}"); + ownLock(map, "f/ts", ".ts", "bun-lock", "export async function main() {}"); + ownLock(map, "f/ts2", ".ts", "bun-lock", "export async function main() {}"); + + applySharedLockPlanToMap(map, plan(map)); + + expect(map[SHARED_PY]).toEqual(PY_LOCK); + expect(map["f/pinned.script.lock"]).toEqual(OTHER_LOCK); + expect(lockRefOf(map["f/doc.script.yaml"])).toEqual(`!inline ${SHARED_PY}`); + expect(map[SHARED_BUN]).toEqual("bun-lock"); + expect(map["f/npm.script.lock"]).toEqual("npm-lock"); + expect(map["f/nb.script.lock"]).toEqual("nb-lock"); + }); + + test("an annotated script alone in its group creates no shared lockfile", () => { + // Alone, so nothing outvotes it: without the gate its variant would BECOME + // the dependency file's lock. `# py: ` is the interpreter pin the + // python import parser reads, which the annotations macro does not cover. + for (const header of ["# py: 3.11", "#py:3.11.4", "# py311"]) { + const map = workspace(PY_DEPS); + ownLock(map, "f/only", ".py", OTHER_LOCK, `${header}\nimport requests`); + + applySharedLockPlanToMap(map, plan(map)); + + expect(map[SHARED_PY]).toBeUndefined(); + expect(map["f/only.script.lock"]).toEqual(OTHER_LOCK); + expect(lockRefOf(map["f/only.script.yaml"])).toEqual( + "!inline f/only.script.lock", + ); + } + }); + + test("a script whose lock is its own keeps a private lockfile", () => { + const map = workspace(PY_DEPS, TEAM_DEPS); + // `extra_` folds the script's own imports into the lock… + const extra = "# extra_requirements: default\ndef main(): ..."; + ownLock(map, "f/extra", ".py", OTHER_LOCK, extra); + // …and naming two files makes it no single file's lock. + const both = "# requirements: default, team_a\ndef main(): ..."; + ownLock(map, "f/both", ".py", OTHER_LOCK, both); + ownLock(map, "f/plain", ".py", PY_LOCK); + ownLock(map, "f/plain2", ".py", PY_LOCK); + + applySharedLockPlanToMap(map, plan(map)); + + expect(map["f/extra.script.lock"]).toEqual(OTHER_LOCK); + expect(map["f/both.script.lock"]).toEqual(OTHER_LOCK); + expect(lockRefOf(map["f/extra.script.yaml"])).toEqual( + "!inline f/extra.script.lock", + ); + expect(map[SHARED_PY]).toEqual(PY_LOCK); + }); + + test("a script with no dependency file behind it is left alone", () => { + const map: Record = {}; + ownLock(map, "f/a", ".py", PY_LOCK); + ownLock(map, "f/b", ".py", PY_LOCK); + + expect(isEmptySharedLockPlan(plan(map))).toBe(true); + }); + + test("a stale committed lock is outvoted, and keeps its own", () => { + const map = workspace(PY_DEPS); + ownLock(map, "f/a", ".py", PY_LOCK_BUMPED); + ownLock(map, "f/b", ".py", PY_LOCK_BUMPED); + ownLock(map, "f/stale", ".py", PY_LOCK); + + applySharedLockPlanToMap(map, plan(map)); + + expect(map[SHARED_PY]).toEqual(PY_LOCK_BUMPED); + expect(map["f/stale.script.lock"]).toEqual(PY_LOCK); + }); + + test("a shared lockfile outlives its scripts but not its dependency file", () => { + // No script in view reads it — a narrowed sync must still leave it be. + const withDep = workspace(PY_DEPS); + withDep[SHARED_PY] = PY_LOCK; + expect(plan(withDep).deletes).not.toContain(SHARED_PY); + + const withoutDep: Record = { [SHARED_PY]: PY_LOCK }; + expect(plan(withoutDep).deletes).toContain(SHARED_PY); + }); + + // The git-sync deploy callback narrows to one item, so a dependency file with + // no script in view is routine — and its lockfile is read by scripts this sync + // cannot see. + test("a dependency file with no script in view keeps its lockfile", () => { + const map = workspace(PY_DEPS, BUN_DEPS); + ownLock(map, "f/a", ".py", PY_LOCK); + ownLock(map, "f/b", ".py", PY_LOCK); + const present = { [SHARED_BUN]: "bun-lock" }; + + const result = computeSharedLockPlan(map, { defaultTs: "bun", present }); + applySharedLockPlanToMap(map, result); + + expect(result.deletes).not.toContain(SHARED_BUN); + // Carried into the map, or the diff reads it as a local-only deletion. + expect(map[SHARED_BUN]).toEqual("bun-lock"); + }); + + test("dependency files absent from the map are not gone", () => { + // `--skip-workspace-dependencies` keeps them out of both maps; taking that + // as "deleted" would un-deduplicate the tree and sweep the lockfiles. + const map: Record = {}; + ownLock(map, "f/a", ".py", PY_LOCK); + ownLock(map, "f/b", ".py", PY_LOCK); + + const result = computeSharedLockPlan(map, { + defaultTs: "bun", + depFiles: [PY_DEPS], + present: { [SHARED_PY]: PY_LOCK }, + }); + applySharedLockPlanToMap(map, result); + + expect(result.deletes).not.toContain(SHARED_PY); + expect(lockRefOf(map["f/a.script.yaml"])).toEqual(`!inline ${SHARED_PY}`); + }); + + + + + + + test("a language that needs no lock is never deduplicated", () => { + const map = workspace("dependencies/modules.json"); + ownLock(map, "f/a", ".ps1", "some-lock", "echo hi"); + ownLock(map, "f/b", ".ps1", "some-lock", "echo hi"); + + expect(isEmptySharedLockPlan(plan(map))).toBe(true); + }); + + test("module-layout scripts share too, from their folder", () => { + const map = workspace(PY_DEPS); + for (const base of ["f/a__mod", "f/b__mod"]) { + map[`${base}/script.py`] = "def main(): ..."; + map[`${base}/script.yaml`] = meta(`!inline ${base}/script.lock`); + map[`${base}/script.lock`] = PY_LOCK; + } + + applySharedLockPlanToMap(map, plan(map)); + + expect(map[SHARED_PY]).toEqual(PY_LOCK); + expect(map["f/a__mod/script.lock"]).toBeUndefined(); + }); + + test("a script path containing dots reads its own content file", () => { + const map = workspace(PY_DEPS, BUN_DEPS); + ownLock(map, "f/a.b", ".py", PY_LOCK); + ownLock(map, "f/c.d", ".py", PY_LOCK); + // `f/a` is a bun script whose name is a prefix of `f/a.b`: its language and + // its annotation must come from its own file, not its neighbour's. + ownLock(map, "f/a", ".ts", "bun-lock", "export async function main() {}"); + ownLock(map, "f/e", ".ts", "bun-lock", "export async function main() {}"); + + applySharedLockPlanToMap(map, plan(map)); + + expect(lockRefOf(map["f/a.b.script.yaml"])).toEqual(`!inline ${SHARED_PY}`); + expect(lockRefOf(map["f/a.script.yaml"])).toEqual(`!inline ${SHARED_BUN}`); + }); + + test("only the lock line of the metadata changes", () => { + const map = workspace(PY_DEPS); + ownLock(map, "f/a", ".py", PY_LOCK); + ownLock(map, "f/b", ".py", PY_LOCK); + map["f/a.script.yaml"] = meta("!inline f/a.script.lock", "does a thing"); + const before = map["f/a.script.yaml"]; + + applySharedLockPlanToMap(map, plan(map)); + + expect(map["f/a.script.yaml"]).toEqual( + before.replace("f/a.script.lock", SHARED_PY), + ); + }); + + test("a dependency set named with a slash shares nothing", () => { + // `dependencies/team/python.requirements.in` has no distinct name under + // `locks/`: flattened to `locks/python.requirements.in.lock` it names a + // different (top-level) dependency file, and every later sweep would then + // read the lockfile as orphaned and retire it. + const map = workspace("dependencies/team/python.requirements.in"); + const body = "# requirements: team/python\ndef main(): ..."; + ownLock(map, "f/a", ".py", PY_LOCK, body); + ownLock(map, "f/b", ".py", PY_LOCK, body); + + const p = plan(map); + expect(p.writes).toEqual({}); + expect(p.deletes).toEqual([]); + }); +}); + +describe("isSharedLockPath", () => { + test("claims only the names a dependency file gives", () => { + for (const p of [SHARED_PY, SHARED_TEAM, SHARED_BUN]) { + expect(isSharedLockPath(p)).toBe(true); + } + // Not a dependency file's name, so a repo that already keeps lockfiles here + // keeps them; `modules.json` is powershell, which takes no lock. + for (const p of [ + "locks/vendor.lock", + "locks/Cargo.lock", + "locks/modules.json.lock", + "locks/sub/requirements.in.lock", + "locks/../../escape.lock", + ]) { + expect(isSharedLockPath(p)).toBe(false); + } + }); +}); + +describe("scriptsReferencingSharedLock", () => { + test("finds every script on the shared lock and nothing else", () => { + const map = workspace(PY_DEPS); + ownLock(map, "f/a", ".py", PY_LOCK); + ownLock(map, "f/b", ".py", PY_LOCK); + const extra = "# extra_requirements: default\ndef main(): ..."; + ownLock(map, "f/extra", ".py", OTHER_LOCK, extra); + applySharedLockPlanToMap(map, plan(map)); + + expect(scriptsReferencingSharedLock(map, SHARED_PY).sort()).toEqual([ + "f/a.script.yaml", + "f/b.script.yaml", + ]); + }); + + test("prose naming the file is not a reference", () => { + const map = { + "f/a.py": "def main(): ...", + "f/a.script.yaml": yamlStringify( + { summary: `see !inline ${SHARED_PY}`, lock: "!inline f/a.script.lock" }, + yamlOptions, + ), + }; + expect(scriptsReferencingSharedLock(map, SHARED_PY)).toEqual([]); + }); +}); + +describe("sharedLockRefIn", () => { + test("returns a reference only when it is one, and it exists", async () => { + const root = await mkdtemp(path.join(os.tmpdir(), "wmill-dedup-ref-")); + try { + await mkdir(path.join(root, "locks"), { recursive: true }); + await writeFile(path.join(root, SHARED_PY), PY_LOCK, "utf-8"); + + expect(sharedLockRefIn(meta(`!inline ${SHARED_PY}`), false, root)).toEqual( + SHARED_PY, + ); + // Flow-style YAML starts with `{` and is not JSON: deciding the format by + // the first character would drop the reference and repoint the script. + expect( + sharedLockRefIn( + `{summary: x, lock: '!inline ${SHARED_PY}'}`, + false, + root, + ), + ).toEqual(SHARED_PY); + // A regenerated script falls back to its own lock rather than point at a + // shared file that is not there. + expect( + sharedLockRefIn(meta(`!inline ${SHARED_BUN}`), false, root), + ).toBeUndefined(); + expect( + sharedLockRefIn(meta("!inline f/a.script.lock"), false, root), + ).toBeUndefined(); + } finally { + await rm(root, { recursive: true, force: true }); + } + }); +}); + +describe("reading the lock field off disk", () => { + test("a folded reference is still a reference", () => { + // A long enough dependency-set name pushes `lock:` past the serializer's + // 80-column default, which breaks the line at the space inside the value. + const ref = + "locks/" + "very_long_set_name_".repeat(4) + "requirements.in.lock"; + const folded = yamlStringify( + { lock: `!inline ${ref}`, summary: "" }, + yamlOptions, + ); + expect(folded).not.toContain("!inline " + ref); + + expect(sharedLockRefOf("f/a.script.yaml", folded, false)).toEqual(ref); + expect(metadataLockUnreadable("f/a.script.yaml", folded, false)).toBe(false); + }); + + test("metadata that cannot be parsed is flagged rather than read as empty", () => { + const conflicted = `summary: ''\n<<<<<<< HEAD\nlock: '!inline ${SHARED_PY}'\n=======\nlock: '!inline ${SHARED_BUN}'\n>>>>>>> other\n`; + expect(sharedLockRefOf("f/a.script.yaml", conflicted, false)).toBeUndefined(); + expect(metadataLockUnreadable("f/a.script.yaml", conflicted, false)).toBe( + true, + ); + // Nothing to read: no reference of any kind in the file. + expect(metadataLockUnreadable("f/a.script.yaml", "summary: ''\n", false)).toBe( + false, + ); + }); +}); diff --git a/cli/test/utils_unit.test.ts b/cli/test/utils_unit.test.ts index 705f41ddf6..e38f9d2013 100644 --- a/cli/test/utils_unit.test.ts +++ b/cli/test/utils_unit.test.ts @@ -272,6 +272,20 @@ describe("getTypeStrFromPath", () => { expect(getTypeStrFromPath("f/test/my_script.rs")).toBe("script"); }); + test("a shared lockfile is its own type, not a workspace dependency", () => { + // A repo-side artifact with no object on the server: classified as a + // workspace dependency, `sync push` would try to deploy it as one. + expect(getTypeStrFromPath("locks/requirements.in.lock")).toBe( + "shared_lock", + ); + expect(getTypeStrFromPath("dependencies/requirements.in")).toBe( + "workspace_dependencies", + ); + // `locks/` is an ordinary word: only the names Windmill writes are claimed, + // so a repo that already keeps its own lockfiles there keeps them. + expect(() => getTypeStrFromPath("locks/vendor.lock")).toThrow(); + }); + test("detects metadata types by name suffix", () => { expect(getTypeStrFromPath("f/test/my_var.variable.yaml")).toBe("variable"); expect(getTypeStrFromPath("f/test/my_res.resource.yaml")).toBe("resource"); diff --git a/cli/wmill.schema.json b/cli/wmill.schema.json index 6acbd4794b..b4b10087a8 100644 --- a/cli/wmill.schema.json +++ b/cli/wmill.schema.json @@ -24,7 +24,7 @@ "items": { "type": "string" }, - "description": "Additional glob patterns merged with includes (useful in branch overrides)" + "description": "Additional glob patterns merged with includes (useful in workspace overrides)" }, "excludes": { "type": "array", @@ -69,6 +69,10 @@ "type": "boolean", "description": "Skip syncing workspace dependencies" }, + "skipDatatableMigrations": { + "type": "boolean", + "description": "Skip syncing data table SQL migrations" + }, "includeSchedules": { "type": "boolean", "description": "Include schedules in sync" @@ -101,6 +105,10 @@ "type": "boolean", "description": "Require lock files for all scripts" }, + "dedupeLockfiles": { + "type": "boolean", + "description": "Share one lockfile per workspace dependency file (locks/.lock), instead of an identical .script.lock per script" + }, "lint": { "type": "boolean", "description": "Run linting before push" @@ -125,6 +133,10 @@ "type": "boolean", "description": "Use __flow/__app/__raw_app suffixes instead of .flow/.app/.raw_app" }, + "syncBehavior": { + "type": "string", + "description": "Sync behavior version — controls ownership handling during push/pull (v1: preserve permissioned_as on update, strip on_behalf_of_email on pull)" + }, "codebases": { "type": "array", "description": "Codebase bundling configurations for shared libraries",