From 85802bfce9e59a9aca4917332df759f0f10fad2c Mon Sep 17 00:00:00 2001 From: Mark McCulloh Date: Fri, 12 Jan 2024 18:13:56 -0500 Subject: [PATCH] feat: use a single `.bak` instead of generating `.tmp` directories every time (#5434) Fixes #4988 When starting compilation, if there is already an existing synth directory, it will be renamed to "{dirname}.bak". Files that are not part of the generated output (e.g. terraform state files) will be copied from that backup before running the rest of the process. To facilitate this, a new `.manifest` json file is created after compilation that lists all the files generated. This PR looks like a bigger diff than it really is because I added a `try {} finally {}` block around most of the compilation process. Couple thoughts: - The current method will always replace a previous backup with a new one, regardless of whether the compilation failed or not. I went back and forth on whether this is important or not - The main synth dir will no longer always be valid. I think it would be interesting to move (or just delete) failed compilations, but this causes issues with sourcemaps. - I think there could be more use cases for other data in the .manifest, which is why it's named a little bit generically. It's definitely still not part of the public contract though, which is why it's a dotfile *By submitting this pull request, I confirm that my contribution is made under the terms of the [Wing Cloud Contribution License](https://github.com/winglang/wing/blob/main/CONTRIBUTION_LICENSE.md)*. --- apps/wing/src/commands/compile.test.ts | 37 +++++- apps/wing/src/commands/pack.ts | 7 +- apps/wing/src/util.ts | 16 +-- libs/wingcompiler/src/compile.ts | 159 ++++++++++++++----------- libs/wingcompiler/src/util.ts | 15 --- libs/wingsdk/src/shared/bundling.ts | 8 +- 6 files changed, 134 insertions(+), 108 deletions(-) diff --git a/apps/wing/src/commands/compile.test.ts b/apps/wing/src/commands/compile.test.ts index 644f16c13d7..58d0b7967f8 100644 --- a/apps/wing/src/commands/compile.test.ts +++ b/apps/wing/src/commands/compile.test.ts @@ -1,4 +1,4 @@ -import { readdir, stat, writeFile, mkdtemp } from "fs/promises"; +import { readdir, stat, writeFile, mkdtemp, readFile } from "fs/promises"; import { tmpdir } from "os"; import { join, resolve } from "path"; import { BuiltinPlatform } from "@winglang/compiler"; @@ -26,7 +26,15 @@ describe( expect(stats.isDirectory()).toBeTruthy(); const files = (await readdir(outDir)).sort(); expect(files.length).toBeGreaterThan(0); - expect(files).toEqual([".wing", "connections.json", "simulator.json", "tree.json"]); + expect(files).toMatchInlineSnapshot(` + [ + ".manifest", + ".wing", + "connections.json", + "simulator.json", + "tree.json", + ] + `); }); test("should be able to compile the SDK capture test to tf-aws", async () => { @@ -53,7 +61,15 @@ describe( expect(stats.isDirectory()).toBeTruthy(); const files = (await readdir(outDir)).sort(); expect(files.length).toBeGreaterThan(0); - expect(files).toEqual([".wing", "connections.json", "simulator.json", "tree.json"]); + expect(files).toMatchInlineSnapshot(` + [ + ".manifest", + ".wing", + "connections.json", + "simulator.json", + "tree.json", + ] + `); }); test("should be able to compile the only entrypoint file in current directory", async () => { @@ -118,7 +134,15 @@ describe( expect(stats.isDirectory()).toBeTruthy(); const files = (await readdir(outDir)).sort(); expect(files.length).toBeGreaterThan(0); - expect(files).toEqual([".wing", "connections.json", "simulator.json", "tree.json"]); + expect(files).toMatchInlineSnapshot(` + [ + ".manifest", + ".wing", + "connections.json", + "simulator.json", + "tree.json", + ] + `); } finally { process.chdir(oldCwd); } @@ -150,6 +174,11 @@ describe( const files2 = await readdir(artifactDir2); expect(files2.length).toBeGreaterThan(0); expectedFiles.forEach((file) => expect(files2).toContain(file)); + + // check the manifest file to make sure it does not contain "terraform.tfstate" + const manifestFile = join(artifactDir2, ".manifest"); + const manifest = JSON.parse(await readFile(manifestFile, "utf-8")); + expect(manifest.generatedFiles).not.toContain("terraform.tfstate"); }); }, { timeout: 1000 * 60 * 5 } diff --git a/apps/wing/src/commands/pack.ts b/apps/wing/src/commands/pack.ts index a627407cdbd..481c778e8c6 100644 --- a/apps/wing/src/commands/pack.ts +++ b/apps/wing/src/commands/pack.ts @@ -118,7 +118,12 @@ export async function pack(options: PackageOptions = {}): Promise { // add default globs to "files" so that Wing files are included in the tarball const pkgJsonFiles: Set = new Set(pkgJson.files ?? []); - const expectedGlobs = [compilerOutputFolder, ...defaultGlobs]; + const expectedGlobs = [ + compilerOutputFolder, + // exclude the unnecessary .manifest file + "!" + path.join(compilerOutputFolder, ".manifest"), + ...defaultGlobs, + ]; for (const pat of expectedGlobs) { if (!pkgJsonFiles.has(pat)) { pkgJsonFiles.add(pat); diff --git a/apps/wing/src/util.ts b/apps/wing/src/util.ts index 0633c90a007..1e1a2b70960 100644 --- a/apps/wing/src/util.ts +++ b/apps/wing/src/util.ts @@ -1,5 +1,5 @@ import * as cp from "child_process"; -import { copyFileSync, promises as fsPromise } from "fs"; +import * as fs from "fs/promises"; import { tmpdir } from "os"; import { join } from "path"; import { promisify } from "util"; @@ -41,18 +41,6 @@ export async function withSpinner(message: string, fn: () => Promise): Pro } } -export async function copyDir(src: string, dest: string) { - await fsPromise.mkdir(dest, { recursive: true }); - let entries = await fsPromise.readdir(src, { withFileTypes: true }); - - for (let entry of entries) { - let srcPath = join(src, entry.name); - let destPath = join(dest, entry.name); - - entry.isDirectory() ? await copyDir(srcPath, destPath) : copyFileSync(srcPath, destPath); - } -} - /** * Execute a command and return its stdout. */ @@ -65,7 +53,7 @@ export async function exec(command: string): Promise { * Creates a clean environment for each test by copying the example file to a temporary directory. */ export async function generateTmpDir() { - return fsPromise.mkdtemp(join(tmpdir(), "-wing-compile-test")); + return fs.mkdtemp(join(tmpdir(), "-wing-compile-test")); } /** diff --git a/libs/wingcompiler/src/compile.ts b/libs/wingcompiler/src/compile.ts index 2922bd36884..3e89f72c3b6 100644 --- a/libs/wingcompiler/src/compile.ts +++ b/libs/wingcompiler/src/compile.ts @@ -3,7 +3,7 @@ import { basename, dirname, join, resolve } from "path"; import * as os from "os"; import * as wingCompiler from "./wingc"; -import { copyDir, normalPath } from "./util"; +import { normalPath } from "./util"; import { existsSync } from "fs"; import { BuiltinPlatform } from "./constants"; import { CompileError, PreflightError } from "./errors"; @@ -17,6 +17,9 @@ Error.stackTraceLimit = 50; // const log = debug("wing:compile"); const WINGC_COMPILE = "wingc_compile"; const WINGC_PREFLIGHT = "preflight.js"; +const MANIFEST_FILE = ".manifest"; +const BACKUP_SUFFIX = ".bak"; +const DOT_WING = ".wing"; const BUILTIN_PLATFORMS = [ BuiltinPlatform.SIM, @@ -69,13 +72,7 @@ export interface CompileOptions { * within the output directory where the SDK app will synthesize its artifacts * for the given target. */ -function resolveSynthDir( - outDir: string, - entrypoint: string, - target: string, - testing: boolean = false, - tmp: boolean = false -) { +function resolveSynthDir(outDir: string, entrypoint: string, target: string, testing: boolean) { const targetDirSuffix = defaultSynthDir(target); let entrypointName; @@ -95,9 +92,8 @@ function resolveSynthDir( throw new Error("Source file cannot be found"); } const randomPart = - tmp || (testing && target !== BuiltinPlatform.SIM) ? `.${Date.now().toString().slice(-6)}` : ""; - const tmpSuffix = tmp ? ".tmp" : ""; - const lastPart = `${entrypointName}.${targetDirSuffix}${randomPart}${tmpSuffix}`; + testing && target !== BuiltinPlatform.SIM ? `.${Date.now().toString().slice(-6)}` : ""; + const lastPart = `${entrypointName}.${targetDirSuffix}${randomPart}`; if (testing) { return join(outDir, "test", lastPart); } else { @@ -145,11 +141,10 @@ export async function compile(entrypoint: string, options: CompileOptions): Prom const testing = options.testing ?? false; log?.("testing: %s", testing); const target = determineTargetFromPlatforms(options.platform); - const tmpSynthDir = resolveSynthDir(targetdir, entrypointFile, target, testing, true); - log?.("temp synth dir: %s", tmpSynthDir); const synthDir = resolveSynthDir(targetdir, entrypointFile, target, testing); log?.("synth dir: %s", synthDir); - const workDir = resolve(tmpSynthDir, ".wing"); + const backupSynthDir = synthDir + BACKUP_SUFFIX; + const workDir = resolve(synthDir, DOT_WING); log?.("work dir: %s", workDir); const nearestNodeModules = (dir: string): string => { @@ -169,66 +164,92 @@ export async function compile(entrypoint: string, options: CompileOptions): Prom let wingNodeModules = nearestNodeModules(wingDir); - await Promise.all([ - fs.mkdir(workDir, { recursive: true }), - fs.mkdir(tmpSynthDir, { recursive: true }), - ]); - - let preflightEntrypoint = await compileForPreflight({ - entrypointFile, - workDir, - wingDir, - tmpSynthDir, - color: options.color, - log, - }); - - if (isEntrypointFile(entrypoint)) { - let preflightEnv: Record = { - ...process.env, - WING_TARGET: target, - WING_PLATFORMS: resolvePlatformPaths(options.platform), - WING_SYNTH_DIR: tmpSynthDir, - WING_SOURCE_DIR: wingDir, - WING_IS_TEST: testing.toString(), - WING_VALUES: options.value?.length == 0 ? undefined : options.value, - WING_VALUES_FILE: options.values, - WING_NODE_MODULES: wingNodeModules, - }; - - if (options.rootId) { - preflightEnv.WING_ROOT_ID = options.rootId; + let existingFiles: string[] = []; + + if (existsSync(synthDir)) { + await fs.rm(backupSynthDir, { force: true, recursive: true }); + await fs.rename(synthDir, backupSynthDir); + + await fs.mkdir(workDir, { recursive: true }); + + // use previous manifest to copy non-generated files + const lastManifestPath = join(backupSynthDir, MANIFEST_FILE); + if (existsSync(lastManifestPath)) { + const lastManifest = JSON.parse(await fs.readFile(lastManifestPath, "utf-8")); + const generatedFiles = lastManifest.generatedFiles; + for (const backupDirFile of await fs.readdir(backupSynthDir)) { + if (!generatedFiles.includes(backupDirFile)) { + await fs.cp(join(backupSynthDir, backupDirFile), join(synthDir, backupDirFile), { + recursive: true, + force: true, + }); + existingFiles.push(backupDirFile); + } + } + } else if (existsSync(backupSynthDir)) { + // HACK: Copy commonly known files that are not generated + const stuffToCopy = ["terraform.tfstate", ".terraform.lock.hcl", ".terraform"]; + await Promise.all( + stuffToCopy.map(async (f) => + fs.cp(join(backupSynthDir, f), join(synthDir, f), { recursive: true }).catch(() => {}) + ) + ); } + } else { + await fs.mkdir(workDir, { recursive: true }); + } + + try { + let preflightEntrypoint = await compileForPreflight({ + entrypointFile, + workDir, + wingDir, + synthDir, + color: options.color, + log, + }); + + if (isEntrypointFile(entrypoint)) { + let preflightEnv: Record = { + ...process.env, + WING_TARGET: target, + WING_PLATFORMS: resolvePlatformPaths(options.platform), + WING_SYNTH_DIR: synthDir, + WING_SOURCE_DIR: wingDir, + WING_IS_TEST: testing.toString(), + WING_VALUES: options.value?.length == 0 ? undefined : options.value, + WING_VALUES_FILE: options.values, + WING_NODE_MODULES: wingNodeModules, + }; + + if (options.rootId) { + preflightEnv.WING_ROOT_ID = options.rootId; + } - if (os.platform() === "win32") { - // In worker threads on Windows, environment variables are case-sensitive. - // Most people probably already assume this is the case everywhere, so - // it is sufficient for now to just to normalize common automatic env vars. + if (os.platform() === "win32") { + // In worker threads on Windows, environment variables are case-sensitive. + // Most people probably already assume this is the case everywhere, so + // it is sufficient for now to just to normalize common automatic env vars. - if ("Path" in preflightEnv) { - preflightEnv.PATH = preflightEnv.Path; - delete preflightEnv.Path; + if ("Path" in preflightEnv) { + preflightEnv.PATH = preflightEnv.Path; + delete preflightEnv.Path; + } } - } - await runPreflightCodeInWorkerThread(preflightEntrypoint, preflightEnv); - } + await runPreflightCodeInWorkerThread(preflightEntrypoint, preflightEnv); + } - if (os.platform() === "win32") { - // Windows doesn't really support fully atomic moves. - // So we just copy the directory instead. - // Also only using sync methods to avoid possible async fs issues. - await fs.rm(synthDir, { recursive: true, force: true }); - await fs.mkdir(synthDir, { recursive: true }); - await copyDir(tmpSynthDir, synthDir); - await fs.rm(tmpSynthDir, { recursive: true, force: true }); - } else { - // Move the temporary directory to the final target location in an atomic operation - await copyDir(tmpSynthDir, synthDir); - await fs.rm(tmpSynthDir, { recursive: true, force: true }); + return synthDir; + } finally { + // generate manifest file + const newManifestPath = join(synthDir, MANIFEST_FILE); + const newManifest = { + generatedFiles: (await fs.readdir(synthDir)).filter((f) => !existingFiles.includes(f)), + }; + newManifest.generatedFiles.push(MANIFEST_FILE); + await fs.writeFile(newManifestPath, JSON.stringify(newManifest)); } - - return synthDir; } function isEntrypointFile(path: string) { @@ -246,7 +267,7 @@ async function compileForPreflight(props: { entrypointFile: string; workDir: string; wingDir: string; - tmpSynthDir: string; + synthDir: string; color?: boolean; log?: (...args: any[]) => void; }) { @@ -269,7 +290,7 @@ npm i ts4w } else { let env: Record = { RUST_BACKTRACE: "full", - WING_SYNTH_DIR: normalPath(props.tmpSynthDir), + WING_SYNTH_DIR: normalPath(props.synthDir), }; if (props.color !== undefined) { env.CLICOLOR = props.color ? "1" : "0"; diff --git a/libs/wingcompiler/src/util.ts b/libs/wingcompiler/src/util.ts index 6a8d1c0e483..cca3ba73c80 100644 --- a/libs/wingcompiler/src/util.ts +++ b/libs/wingcompiler/src/util.ts @@ -1,6 +1,3 @@ -import { copyFileSync, promises as fsPromise } from "fs"; -import path from "path"; - /** * Normalize windows paths to be posix-like. */ @@ -15,15 +12,3 @@ export function normalPath(path: string) { return path; } } - -export async function copyDir(src: string, dest: string) { - await fsPromise.mkdir(dest, { recursive: true }); - let entries = await fsPromise.readdir(src, { withFileTypes: true }); - - for (let entry of entries) { - let srcPath = path.join(src, entry.name); - let destPath = path.join(dest, entry.name); - - entry.isDirectory() ? await copyDir(srcPath, destPath) : copyFileSync(srcPath, destPath); - } -} diff --git a/libs/wingsdk/src/shared/bundling.ts b/libs/wingsdk/src/shared/bundling.ts index c2bbcdeafba..145de7fec00 100644 --- a/libs/wingsdk/src/shared/bundling.ts +++ b/libs/wingsdk/src/shared/bundling.ts @@ -71,11 +71,9 @@ export function createBundle( const sourcemapData = JSON.parse( new TextDecoder().decode(esbuild.outputFiles[0].contents) ); - // unrandomize the sourceRoot - sourcemapData.sourceRoot = normalPath(sourcemapData.sourceRoot).replace( - /\.\d+\.tmp\/\.wing\//g, - "/.wing/" - ); + + // ensure sourceRoot has posix path separators + sourcemapData.sourceRoot = normalPath(sourcemapData.sourceRoot); for (const [idx, source] of Object.entries(sourcemapData.sources)) { if ((source as any).endsWith(".w")) {