feat: use a single .bak instead of generating .tmp directories ev…

…ery 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)*.
winglang · Jan 12, 2024 · 85802bf · 85802bf
1 parent 3ea81d9
commit 85802bf
Show file tree

Hide file tree

Showing 6 changed files with 134 additions and 108 deletions.
diff --git 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
@@ -118,7 +118,12 @@ export async function pack(options: PackageOptions = {}): Promise<string> {
     // add default globs to "files" so that Wing files are included in the tarball
     const pkgJsonFiles: Set<string> = 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
@@ -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<T>(message: string, fn: () => Promise<T>): 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<string> {
  * 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
@@ -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<string, string | undefined> = {
-      ...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<string, string | undefined> = {
+        ...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<string, string> = {
       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
@@ -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
@@ -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")) {