fix: delete old files & folders on each js_run_devserver sync (#1873)

aspect-build · Jul 27, 2024 · e306629 · e306629
1 parent bf23b3b
commit e306629
Show file tree

Hide file tree

Showing 7 changed files with 205 additions and 15 deletions.
diff --git a/e2e/webpack_devserver/BUILD.bazel b/e2e/webpack_devserver/BUILD.bazel
@@ -23,6 +23,7 @@ js_run_devserver(
     ],
     data = [
         "package.json",
+        "src/404.html",
         "src/index.html",
         "src/index.js",
         "webpack.config.js",
@@ -41,6 +42,7 @@ js_run_devserver(
     ],
     data = [
         "package.json",
+        "src/404.html",
         "src/index.html",
         "src/index.js",
         "webpack.config.cjs",

diff --git a/e2e/webpack_devserver/serve_test.sh b/e2e/webpack_devserver/serve_test.sh
@@ -28,6 +28,7 @@ function _exit {
     git checkout src/index.html >/dev/null 2>&1
     git checkout mypkg/index.js >/dev/null 2>&1
     git checkout mylib/index.js >/dev/null 2>&1
+    git checkout BUILD.bazel >/dev/null 2>&1
     rm -f "$ibazel_logs"
 }
 trap _exit EXIT
@@ -93,48 +94,97 @@ if ! curl http://localhost:8080/main.js --fail 2>/dev/null | grep "chalk.cyan(pa
     exit 1
 fi
 
+_sedi 's#"src/404.html",##' BUILD.bazel
+
+echo "Waiting 10 seconds for ibazel rebuild after change to BUILD.bazel..."
+sleep 10
+
+git checkout BUILD.bazel >/dev/null 2>&1
+
+echo "Waiting 10 seconds for ibazel rebuild after change to BUILD.bazel..."
+sleep 10
+
 echo "Checking log file $ibazel_logs"
 
 count=$(grep -c "Syncing symlink node_modules/.aspect_rules_js/@mycorp+mylib@0.0.0/node_modules/@mycorp/mylib (1p)" "$ibazel_logs" || true)
 if [[ "$count" -ne 1 ]]; then
+    echo "==========="
+    cat "$ibazel_logs"
+    echo "==========="
     echo "ERROR: expected to have synced @mycorp/mylib symlink 1 time but found ${count}"
     exit 1
 fi
 
 count=$(grep -c "Syncing file node_modules/.aspect_rules_js/@mycorp+mypkg@0.0.0/node_modules/@mycorp/mypkg/index.js" "$ibazel_logs" || true)
 if [[ "$count" -ne 2 ]]; then
+    echo "==========="
+    cat "$ibazel_logs"
+    echo "==========="
     echo "ERROR: expected to have synced @mycorp/mypkg/index.js 2 times but found ${count}"
     exit 1
 fi
 
 count=$(grep -c "Syncing file mylib/index.js" "$ibazel_logs" || true)
 if [[ "$count" -ne 2 ]]; then
+    echo "==========="
+    cat "$ibazel_logs"
+    echo "==========="
     echo "ERROR: expected to have synced mylib/index.js 2 times but found ${count}"
     exit 1
 fi
 
 count=$(grep -c "Skipping file node_modules/.aspect_rules_js/@mycorp+mypkg@0.0.0/node_modules/@mycorp/mypkg/index.js since its timestamp has not changed" "$ibazel_logs" || true)
-if [[ "$count" -ne 2 ]]; then
-    echo "ERROR: expected to have skipped @mycorp/mypkg/index.js due to timestamp 2 times but found ${count}"
+if [[ "$count" -ne 4 ]]; then
+    echo "==========="
+    cat "$ibazel_logs"
+    echo "==========="
+    echo "ERROR: expected to have skipped @mycorp/mypkg/index.js due to timestamp 4 times but found ${count}"
     exit 1
 fi
 
 count=$(grep -c "Syncing file node_modules/.aspect_rules_js/@mycorp+mypkg@0.0.0/node_modules/@mycorp/mypkg/package.json" "$ibazel_logs" || true)
 if [[ "$count" -ne 1 ]]; then
+    echo "==========="
+    cat "$ibazel_logs"
+    echo "==========="
     echo "ERROR: expected to have synced @mycorp/mypkg/package.json 1 time but found ${count}"
     exit 1
 fi
 
 count=$(grep -c "Skipping file node_modules/.aspect_rules_js/@mycorp+mypkg@0.0.0/node_modules/@mycorp/mypkg/package.json since its timestamp has not changed" "$ibazel_logs" || true)
-if [[ "$count" -ne 2 ]]; then
-    echo "ERROR: expected to have skipped @mycorp/mypkg/package.json due to timestamp 2 times but found ${count}"
+if [[ "$count" -ne 4 ]]; then
+    echo "==========="
+    cat "$ibazel_logs"
+    echo "==========="
+    echo "ERROR: expected to have skipped @mycorp/mypkg/package.json due to timestamp 4 times but found ${count}"
     exit 1
 fi
 
 count=$(grep -c "Skipping file node_modules/.aspect_rules_js/@mycorp+mypkg@0.0.0/node_modules/@mycorp/mypkg/package.json since contents have not changed" "$ibazel_logs" || true)
 if [[ "$count" -ne 1 ]]; then
+    echo "==========="
+    cat "$ibazel_logs"
+    echo "==========="
     echo "ERROR: expected to have skipped @mycorp/mypkg/package.json due to contents 1 times but found ${count}"
     exit 1
 fi
 
+count=$(grep -c "Deleting src/404.html" "$ibazel_logs" || true)
+if [[ "$count" -ne 1 ]]; then
+    echo "==========="
+    cat "$ibazel_logs"
+    echo "==========="
+    echo "ERROR: expected to have deleted src/404.html 1 time but found ${count}"
+    exit 1
+fi
+
+count=$(grep -c "Syncing file src/404.html" "$ibazel_logs" || true)
+if [[ "$count" -ne 2 ]]; then
+    echo "==========="
+    cat "$ibazel_logs"
+    echo "==========="
+    echo "ERROR: expected to have synced src/404.html 2 times but found ${count}"
+    exit 1
+fi
+
 echo "All tests passed"
diff --git a/e2e/webpack_devserver/src/404.html b/e2e/webpack_devserver/src/404.html
@@ -0,0 +1,10 @@
+<!doctype html>
+<html>
+    <head>
+        <meta charset="utf-8" />
+        <title>404</title>
+    </head>
+    <body>
+        404
+    </body>
+</html>
diff --git a/e2e/webpack_devserver_esm/BUILD.bazel b/e2e/webpack_devserver_esm/BUILD.bazel
@@ -23,6 +23,7 @@ js_run_devserver(
     ],
     data = [
         "package.json",
+        "src/404.html",
         "src/index.html",
         "src/index.js",
         "webpack.config.mjs",

diff --git a/e2e/webpack_devserver_esm/serve_test.sh b/e2e/webpack_devserver_esm/serve_test.sh
@@ -26,6 +26,7 @@ function _exit {
     git checkout src/index.html >/dev/null 2>&1
     git checkout mypkg/index.js >/dev/null 2>&1
     git checkout mylib/index.js >/dev/null 2>&1
+    git checkout BUILD.bazel >/dev/null 2>&1
     rm -f "$ibazel_logs"
 }
 trap _exit EXIT
@@ -91,46 +92,97 @@ if ! curl http://localhost:8080/main.js --fail 2>/dev/null | grep "chalk__WEBPAC
     exit 1
 fi
 
+_sedi 's#"src/404.html",##' BUILD.bazel
+
+echo "Waiting 10 seconds for ibazel rebuild after change to BUILD.bazel..."
+sleep 10
+
+git checkout BUILD.bazel >/dev/null 2>&1
+
+echo "Waiting 10 seconds for ibazel rebuild after change to BUILD.bazel..."
+sleep 10
+
+echo "Checking log file $ibazel_logs"
+
 count=$(grep -c "Syncing symlink node_modules/.aspect_rules_js/@mycorp+mylib@0.0.0/node_modules/@mycorp/mylib (1p)" "$ibazel_logs" || true)
 if [[ "$count" -ne 1 ]]; then
+    echo "==========="
+    cat "$ibazel_logs"
+    echo "==========="
     echo "ERROR: expected to have synced @mycorp/mylib symlink 1 time but found ${count}"
     exit 1
 fi
 
 count=$(grep -c "Syncing file node_modules/.aspect_rules_js/@mycorp+mypkg@0.0.0/node_modules/@mycorp/mypkg/index.js" "$ibazel_logs" || true)
 if [[ "$count" -ne 2 ]]; then
+    echo "==========="
+    cat "$ibazel_logs"
+    echo "==========="
     echo "ERROR: expected to have synced @mycorp/mypkg/index.js 2 times but found ${count}"
     exit 1
 fi
 
 count=$(grep -c "Syncing file mylib/index.js" "$ibazel_logs" || true)
 if [[ "$count" -ne 2 ]]; then
+    echo "==========="
+    cat "$ibazel_logs"
+    echo "==========="
     echo "ERROR: expected to have synced mylib/index.js 2 times but found ${count}"
     exit 1
 fi
 
 count=$(grep -c "Skipping file node_modules/.aspect_rules_js/@mycorp+mypkg@0.0.0/node_modules/@mycorp/mypkg/index.js since its timestamp has not changed" "$ibazel_logs" || true)
-if [[ "$count" -ne 2 ]]; then
-    echo "ERROR: expected to have skipped @mycorp/mypkg/index.js due to timestamp 2 times but found ${count}"
+if [[ "$count" -ne 4 ]]; then
+    echo "==========="
+    cat "$ibazel_logs"
+    echo "==========="
+    echo "ERROR: expected to have skipped @mycorp/mypkg/index.js due to timestamp 4 times but found ${count}"
     exit 1
 fi
 
 count=$(grep -c "Syncing file node_modules/.aspect_rules_js/@mycorp+mypkg@0.0.0/node_modules/@mycorp/mypkg/package.json" "$ibazel_logs" || true)
 if [[ "$count" -ne 1 ]]; then
+    echo "==========="
+    cat "$ibazel_logs"
+    echo "==========="
     echo "ERROR: expected to have synced @mycorp/mypkg/package.json 1 time but found ${count}"
     exit 1
 fi
 
 count=$(grep -c "Skipping file node_modules/.aspect_rules_js/@mycorp+mypkg@0.0.0/node_modules/@mycorp/mypkg/package.json since its timestamp has not changed" "$ibazel_logs" || true)
-if [[ "$count" -ne 2 ]]; then
-    echo "ERROR: expected to have skipped @mycorp/mypkg/package.json due to timestamp 2 times but found ${count}"
+if [[ "$count" -ne 4 ]]; then
+    echo "==========="
+    cat "$ibazel_logs"
+    echo "==========="
+    echo "ERROR: expected to have skipped @mycorp/mypkg/package.json due to timestamp 4 times but found ${count}"
     exit 1
 fi
 
 count=$(grep -c "Skipping file node_modules/.aspect_rules_js/@mycorp+mypkg@0.0.0/node_modules/@mycorp/mypkg/package.json since contents have not changed" "$ibazel_logs" || true)
 if [[ "$count" -ne 1 ]]; then
+    echo "==========="
+    cat "$ibazel_logs"
+    echo "==========="
     echo "ERROR: expected to have skipped @mycorp/mypkg/package.json due to contents 1 times but found ${count}"
     exit 1
 fi
 
+count=$(grep -c "Deleting src/404.html" "$ibazel_logs" || true)
+if [[ "$count" -ne 1 ]]; then
+    echo "==========="
+    cat "$ibazel_logs"
+    echo "==========="
+    echo "ERROR: expected to have deleted src/404.html 1 time but found ${count}"
+    exit 1
+fi
+
+count=$(grep -c "Syncing file src/404.html" "$ibazel_logs" || true)
+if [[ "$count" -ne 2 ]]; then
+    echo "==========="
+    cat "$ibazel_logs"
+    echo "==========="
+    echo "ERROR: expected to have synced src/404.html 2 times but found ${count}"
+    exit 1
+fi
+
 echo "All tests passed"
diff --git a/e2e/webpack_devserver_esm/src/404.html b/e2e/webpack_devserver_esm/src/404.html
@@ -0,0 +1,10 @@
+<!doctype html>
+<html>
+    <head>
+        <meta charset="utf-8" />
+        <title>404</title>
+    </head>
+    <body>
+        404
+    </body>
+</html>
diff --git a/js/private/js_run_devserver.mjs b/js/private/js_run_devserver.mjs
@@ -246,8 +246,62 @@ async function syncRecursive(src, dst, sandbox, writePerm) {
     }
 }
 
+// Delete files from sandbox
+async function deleteFiles(previousFiles, updatedFiles, sandbox) {
+    const startTime = perf_hooks.performance.now()
+
+    let totalDeleted = 0
+
+    // Remove files that were previously synced but are no longer in the updated list of files to sync
+    const updatedFilesSet = new Set(updatedFiles)
+    for (const f of previousFiles) {
+        if (updatedFilesSet.has(f)) {
+            continue
+        }
+
+        console.error(`Deleting ${f}`)
+
+        // clear any matching files or files rooted at this folder from the
+        // syncedTime and syncedChecksum maps
+        const srcPath = path.join(RUNFILES_ROOT, f)
+        for (const k of syncedTime.keys()) {
+            if (k == srcPath || k.startsWith(srcPath + '/')) {
+                syncedTime.delete(k)
+            }
+        }
+        for (const k of syncedChecksum.keys()) {
+            if (k == srcPath || k.startsWith(srcPath + '/')) {
+                syncedChecksum.delete(k)
+            }
+        }
+
+        // clear mkdirs if we have deleted any files so we re-populate on next sync
+        mkdirs.clear()
+
+        const rmPath = path.join(sandbox, f)
+        try {
+            fs.rmSync(rmPath, { recursive: true, force: true })
+        } catch (e) {
+            console.error(
+                `An error has occurred while deleting the synced file ${rmPath}. Error: ${e}`
+            )
+        }
+        totalDeleted++
+    }
+
+    var endTime = perf_hooks.performance.now()
+
+    if (totalDeleted > 0) {
+        console.error(
+            `${totalDeleted} file${totalDeleted > 1 ? 's' : ''}/folder${
+                totalDeleted > 1 ? 's' : ''
+            } deleted in ${Math.round(endTime - startTime)} ms`
+        )
+    }
+}
+
 // Sync list of files to the sandbox
-async function sync(files, sandbox, writePerm) {
+async function syncFiles(files, sandbox, writePerm) {
     console.error(`+ Syncing ${files.length} files && folders...`)
     const startTime = perf_hooks.performance.now()
 
@@ -331,7 +385,7 @@ async function main(args, sandbox) {
 
     const config = JSON.parse(await fs.promises.readFile(configPath))
 
-    await sync(
+    await syncFiles(
         config.data_files,
         sandbox,
         config.grant_sandbox_write_permissions
@@ -402,14 +456,25 @@ async function main(args, sandbox) {
                         console.error('IBAZEL_BUILD_COMPLETED SUCCESS')
                     }
                     // Chain promises via syncing.then()
-                    syncing = syncing.then(() =>
-                        sync(
-                            // Re-parse the config file to get the latest list of data files to copy
-                            JSON.parse(fs.readFileSync(configPath)).data_files,
+                    syncing = syncing.then(() => {
+                        // Re-parse the config file to get the latest list of data files to copy
+                        const updatedDataFiles = JSON.parse(
+                            fs.readFileSync(configPath)
+                        ).data_files
+                        // Remove files that were previously synced but are no longer in the updated list of files to sync
+                        deleteFiles(
+                            config.data_files,
+                            updatedDataFiles,
+                            sandbox
+                        )
+                        // Sync changed files
+                        config.data_files = updatedDataFiles
+                        syncFiles(
+                            config.data_files,
                             sandbox,
                             config.grant_sandbox_write_permissions
                         )
-                    )
+                    })
                     // Await promise to catch any exceptions, and wait for the
                     // sync to be complete before writing to stdin of the child
                     // process