More reliable `bun --hot` on Linux

4 files changed, 139 insertions, 31 deletions

diff --git a/src/bun.js/javascript.zig b/src/bun.js/javascript.zig
index ba8a9b276..f51429e3a 100644
--- a/src/bun.js/javascript.zig
+++ b/src/bun.js/javascript.zig
@@ -2633,6 +2633,7 @@ pub const HotReloader = struct {
 
     onAccept: std.ArrayHashMapUnmanaged(Watcher.HashType, bun.BabyList(OnAcceptCallback), bun.ArrayIdentityContext, false) = .{},
     vm: *JSC.VirtualMachine,
+    verbose: bool = false,
 
     pub const HotReloadTask = struct {
         reloader: *HotReloader,
@@ -2696,6 +2697,7 @@ pub const HotReloader = struct {
         var reloader = bun.default_allocator.create(HotReloader) catch @panic("OOM");
         reloader.* = .{
             .vm = this,
+            .verbose = this.log.level.atLeast(.info),
         };
         this.bun_watcher = JSC.Watcher.init(
             reloader,
@@ -2756,7 +2758,7 @@ pub const HotReloader = struct {
             const id = hashes[event.index];
 
             if (comptime Environment.isDebug) {
-                Output.prettyErrorln("[watcher] {s}: -- {}", .{ @tagName(kind), event.op });
+                Output.prettyErrorln("[watch] {s} ({s}, {})", .{ file_path, @tagName(kind), event.op });
             }
 
             switch (kind) {
@@ -2770,9 +2772,8 @@ pub const HotReloader = struct {
                         );
                     }
 
-                    if (comptime bun.FeatureFlags.verbose_watcher) {
+                    if (this.verbose)
                         Output.prettyErrorln("<r><d>File changed: {s}<r>", .{fs.relativeTo(file_path)});
-                    }
 
                     if (event.op.write) {
                         current_task.append(id);
@@ -2794,6 +2795,7 @@ pub const HotReloader = struct {
                             if (changed_name.len == 0 or changed_name[0] == '~' or changed_name[0] == '.') continue;
 
                             const loader = (bundler.options.loaders.get(Fs.PathName.init(changed_name).ext) orelse .file);
+                            var prev_entry_id: usize = std.math.maxInt(usize);
                             if (loader.isJavaScriptLikeOrJSON() or loader == .css) {
                                 var path_string: bun.PathString = undefined;
                                 var file_hash: Watcher.HashType = last_file_hash;
@@ -2806,7 +2808,20 @@ pub const HotReloader = struct {
                                         file_hash = Watcher.getHash(path_string.slice());
                                         for (hashes) |hash, entry_id| {
                                             if (hash == file_hash) {
-                                                file_descriptors[entry_id] = 0;
+                                                if (file_descriptors[entry_id] != 0) {
+                                                    if (prev_entry_id != entry_id) {
+                                                        current_task.append(@truncate(u32, entry_id));
+                                                        ctx.removeAtIndex(
+                                                            @truncate(u16, entry_id),
+                                                            0,
+                                                            &.{},
+                                                            .file,
+                                                        );
+                                                    }
+                                                    file_descriptors[entry_id] = 0;
+                                                }
+
+                                                prev_entry_id = entry_id;
                                                 break;
                                             }
                                         }
@@ -2828,17 +2843,20 @@ pub const HotReloader = struct {
                                 if (last_file_hash == file_hash) continue;
                                 last_file_hash = file_hash;
 
-                                Output.prettyErrorln("<r>   <d>File change: {s}<r>", .{fs.relativeTo(abs_path)});
+                                if (this.verbose)
+                                    Output.prettyErrorln("<r>   <d>File change: {s}<r>", .{fs.relativeTo(abs_path)});
                             }
                         }
                     }
 
-                    // if (event.op.delete or event.op.rename)
-                    //     ctx.watcher.removeAtIndex(event.index, hashes[event.index], parent_hashes, .directory);
-                    if (comptime false) {
-                        Output.prettyErrorln("<r>📁  <d>Dir change: {s}<r>", .{fs.relativeTo(file_path)});
-                    } else {
-                        Output.prettyErrorln("<r>    <d>Dir change: {s}<r>", .{fs.relativeTo(file_path)});
+                    if (this.verbose) {
+                        // if (event.op.delete or event.op.rename)
+                        //     ctx.watcher.removeAtIndex(event.index, hashes[event.index], parent_hashes, .directory);
+                        if (comptime false) {
+                            Output.prettyErrorln("<r>📁  <d>Dir change: {s}<r>", .{fs.relativeTo(file_path)});
+                        } else {
+                            Output.prettyErrorln("<r>    <d>Dir change: {s}<r>", .{fs.relativeTo(file_path)});
+                        }
                     }
                 },
             }
diff --git a/src/watcher.zig b/src/watcher.zig
index 2a20c0db3..7d610088d 100644
--- a/src/watcher.zig
+++ b/src/watcher.zig
@@ -68,24 +68,25 @@ pub const INotify = struct {
 
         pub fn name(this: *const INotifyEvent) [:0]u8 {
             if (comptime Environment.allow_assert) std.debug.assert(this.name_len > 0);
+
             // the name_len field is wrong
             // it includes alignment / padding
             // but it is a sentineled value
             // so we can just trim it to the first null byte
-            return std.mem.sliceTo(@intToPtr([*]u8, @ptrToInt(this) + @sizeOf(INotifyEvent))[0..this.name_len :0], 0);
+            return bun.sliceTo(@intToPtr([*:0]u8, @ptrToInt(&this.name_len) + @sizeOf(u32)), 0)[0.. :0];
         }
     };
     pub var inotify_fd: EventListIndex = 0;
     pub var loaded_inotify = false;
 
-    const EventListBuffer = [@sizeOf([128]INotifyEvent) + (128 * bun.MAX_PATH_BYTES)]u8;
+    const EventListBuffer = [@sizeOf([128]INotifyEvent) + (128 * bun.MAX_PATH_BYTES + (128 * @alignOf(INotifyEvent)))]u8;
     var eventlist: EventListBuffer = undefined;
     var eventlist_ptrs: [128]*const INotifyEvent = undefined;
 
     var watch_count: std.atomic.Atomic(u32) = std.atomic.Atomic(u32).init(0);
 
-    const watch_file_mask = IN_EXCL_UNLINK | IN_MOVE_SELF | IN_DELETE_SELF | IN_CLOSE_WRITE | IN_MOVED_TO;
-    const watch_dir_mask = IN_EXCL_UNLINK | IN_DELETE | IN_DELETE_SELF | IN_CREATE | IN_MOVE_SELF | IN_ONLYDIR | IN_MOVED_TO;
+    const watch_file_mask = std.os.linux.IN.EXCL_UNLINK | std.os.linux.IN.MOVE_SELF | std.os.linux.IN.DELETE_SELF | std.os.linux.IN.MOVED_TO | std.os.linux.IN.MODIFY;
+    const watch_dir_mask = std.os.linux.IN.EXCL_UNLINK | std.os.linux.IN.DELETE | std.os.linux.IN.DELETE_SELF | std.os.linux.IN.CREATE | std.os.linux.IN.MOVE_SELF | std.os.linux.IN.ONLYDIR | std.os.linux.IN.MOVED_TO;
 
     pub fn watchPath(pathname: [:0]const u8) !EventListIndex {
         std.debug.assert(loaded_inotify);
@@ -127,14 +128,46 @@ pub const INotify = struct {
 
             switch (std.os.errno(rc)) {
                 .SUCCESS => {
-                    const len = @intCast(usize, rc);
+                    var len = @intCast(usize, rc);
 
                     if (len == 0) return &[_]*INotifyEvent{};
 
+                    // IN_MODIFY is very noisy
+                    // we do a 0.1ms sleep to try to coalesce events better
+                    if (len < (@sizeOf(EventListBuffer) / 2)) {
+                        var fds = [_]std.os.pollfd{.{
+                            .fd = inotify_fd,
+                            .events = std.os.POLL.IN | std.os.POLL.ERR,
+                            .revents = 0,
+                        }};
+                        var timespec = std.os.timespec{ .tv_sec = 0, .tv_nsec = 100_000 };
+                        if ((std.os.ppoll(&fds, &timespec, null) catch 0) > 0) {
+                            while (true) {
+                                const new_rc = std.os.system.read(
+                                    inotify_fd,
+                                    @ptrCast([*]u8, @alignCast(@alignOf([*]u8), &eventlist)) + len,
+                                    @sizeOf(EventListBuffer) - len,
+                                );
+                                switch (std.os.errno(new_rc)) {
+                                    .SUCCESS => {
+                                        len += @intCast(usize, new_rc);
+                                    },
+                                    .AGAIN => continue,
+                                    .INTR => continue,
+                                    .INVAL => return error.ShortRead,
+                                    .BADF => return error.INotifyFailedToStart,
+                                    else => unreachable,
+                                }
+                                break;
+                            }
+                        }
+                    }
+
                     var count: u32 = 0;
                     var i: u32 = 0;
                     while (i < len) : (i += @sizeOf(INotifyEvent)) {
-                        const event = @ptrCast(*const INotifyEvent, @alignCast(@alignOf(*const INotifyEvent), eventlist[i..][0..@sizeOf(INotifyEvent)]));
+                        @setRuntimeSafety(false);
+                        var event = @ptrCast(*INotifyEvent, @alignCast(@alignOf(*INotifyEvent), eventlist[i..][0..@sizeOf(INotifyEvent)]));
                         i += event.name_len;
 
                         eventlist_ptrs[count] = event;
@@ -225,6 +258,12 @@ pub const WatchEvent = struct {
     name_off: u8 = 0,
     name_len: u8 = 0,
 
+    pub fn ignoreINotifyEvent(event: INotify.INotifyEvent) bool {
+        var stack: WatchEvent = undefined;
+        stack.fromINotify(event, 0);
+        return @bitCast(std.meta.Int(.unsigned, @bitSizeOf(Op)), stack.op) == 0;
+    }
+
     pub fn names(this: WatchEvent, buf: []?[:0]u8) []?[:0]u8 {
         if (this.name_len == 0) return &[_]?[:0]u8{};
         return buf[this.name_off..][0..this.name_len];
@@ -275,8 +314,6 @@ pub const WatchEvent = struct {
     }
 
     pub const Op = packed struct {
-        padding: u3 = 0,
-
         delete: bool = false,
         metadata: bool = false,
         rename: bool = false,
diff --git a/test/bun.js/hot-runner.js b/test/bun.js/hot-runner.js
index fc34f6c64..eda65f0b9 100644
--- a/test/bun.js/hot-runner.js
+++ b/test/bun.js/hot-runner.js
@@ -3,5 +3,4 @@ import "./hot-runner-imported";
 globalThis.counter ??= 0;
 
 console.log(`[${Date.now()}] [#!root] Reloaded: ${++globalThis.counter}`);
-
-setTimeout(() => {}, 9999999999);
+!setTimeout(() => {}, 9999999);
diff --git a/test/bun.js/hot.test.ts b/test/bun.js/hot.test.ts
index bd75d59ee..d95b6c7a9 100644
--- a/test/bun.js/hot.test.ts
+++ b/test/bun.js/hot.test.ts
@@ -2,7 +2,13 @@ import { spawn } from "bun";
 import { expect, it } from "bun:test";
 import { bunEnv } from "bunEnv";
 import { bunExe } from "bunExe";
-import { readFileSync, unlinkSync, writeFileSync } from "fs";
+import {
+  readFileSync,
+  renameSync,
+  rmSync,
+  unlinkSync,
+  writeFileSync,
+} from "fs";
 
 it("should hot reload when file is overwritten", async () => {
   const root = import.meta.dir + "/hot-runner.js";
@@ -22,8 +28,9 @@ it("should hot reload when file is overwritten", async () => {
 
   for await (const line of runner.stdout!) {
     var str = new TextDecoder().decode(line);
-
-    if (str.includes("[#!root]")) {
+    var any = false;
+    for (let line of str.split("\n")) {
+      if (!line.includes("[#!root]")) continue;
       reloadCounter++;
 
       if (reloadCounter === 3) {
@@ -33,16 +40,16 @@ it("should hot reload when file is overwritten", async () => {
       }
 
       expect(str).toContain(`[#!root] Reloaded: ${reloadCounter}`);
-
-      await onReload();
+      any = true;
     }
+
+    if (any) await onReload();
   }
 
   expect(reloadCounter).toBe(3);
 });
 
-// This test fails
-it.skip("should hot reload when a file is deleted and rewritten", async () => {
+it("should hot reload when a file is deleted and rewritten", async () => {
   const root = import.meta.dir + "/hot-runner.js";
   const runner = spawn({
     cmd: [bunExe(), "--hot", "run", root],
@@ -62,8 +69,9 @@ it.skip("should hot reload when a file is deleted and rewritten", async () => {
 
   for await (const line of runner.stdout!) {
     var str = new TextDecoder().decode(line);
-
-    if (str.includes("[#!root]")) {
+    var any = false;
+    for (let line of str.split("\n")) {
+      if (!line.includes("[#!root]")) continue;
       reloadCounter++;
 
       if (reloadCounter === 3) {
@@ -73,9 +81,55 @@ it.skip("should hot reload when a file is deleted and rewritten", async () => {
       }
 
       expect(str).toContain(`[#!root] Reloaded: ${reloadCounter}`);
+      any = true;
+    }
+
+    if (any) await onReload();
+  }
+
+  expect(reloadCounter).toBe(3);
+});
+
+it("should hot reload when a file is renamed() into place", async () => {
+  const root = import.meta.dir + "/hot-runner.js";
+  const runner = spawn({
+    cmd: [bunExe(), "--hot", "run", root],
+    env: bunEnv,
+    stdout: "pipe",
+    stderr: "inherit",
+    stdin: "ignore",
+  });
 
-      await onReload();
+  var reloadCounter = 0;
+
+  async function onReload() {
+    const contents = readFileSync(root, "utf-8");
+    rmSync(root + ".tmpfile", { force: true });
+    await 1;
+    writeFileSync(root + ".tmpfile", contents);
+    await 1;
+    renameSync(root + ".tmpfile", root);
+    await 1;
+  }
+
+  for await (const line of runner.stdout!) {
+    var str = new TextDecoder().decode(line);
+    var any = false;
+    for (let line of str.split("\n")) {
+      if (!line.includes("[#!root]")) continue;
+      reloadCounter++;
+
+      if (reloadCounter === 3) {
+        runner.unref();
+        runner.kill();
+        break;
+      }
+
+      expect(str).toContain(`[#!root] Reloaded: ${reloadCounter}`);
+      any = true;
     }
+
+    if (any) await onReload();
   }
 
   expect(reloadCounter).toBe(3);