wip

instructions.md

@@ -0,0 +1,46 @@
+# Fixing symlinks (reparse point) with the shell `rm` built-in on Windows
+
+On Posix systems for the `rm` shell built-in, if we don't know yet whether a
+file is a regular file or directory, we try to `unlink` it first.
+
+If it is actually a directory, it will return `EISDIR` and we go and delete its
+children before calling `rmdirat`.
+
+Conveniently, this handles _symlinks_ to directories too.
+
+However, this breaks on Windows if we have a symlink to a directory. Why? We
+don't have `unlink` on Windows. Instead we have a `DeleteFileBun(...)` function
+inside of `src/windows.zig` that _tries_ to do the equivalent.
+
+Unfortunately, it does not have the same behavior as posix for deleting
+symlinks. It will return with `EISDIR` (despite the `FILE_OPEN_REPARSE_POINT`
+being set, it seems to always try to delete the referenced directory).
+
+So the solution is that we need to use the Windows API to delete the reparse
+point. We can do it like this:
+
+```C
+HANDLE h = CreateFileW(L"linkDir",
+                       GENERIC_WRITE,                        // must allow write
+                       FILE_SHARE_READ | FILE_SHARE_WRITE | FILE_SHARE_DELETE,
+                       NULL,
+                       OPEN_EXISTING,
+                       FILE_FLAG_OPEN_REPARSE_POINT |        // don’t follow
+                       FILE_FLAG_BACKUP_SEMANTICS,
+                       NULL);
+
+// header only – tag identifies which reparse point to remove
+REPARSE_GUID_DATA_BUFFER hdr = {0};
+hdr.ReparseTag = IO_REPARSE_TAG_SYMLINK;   // or _MOUNT_POINT, _SOCK, etc.
+
+DWORD bytes;
+DeviceIoControl(h,
+                FSCTL_DELETE_REPARSE_POINT,                 // core op
+                &hdr, REPARSE_GUID_DATA_BUFFER_HEADER_SIZE, // in buf / len
+                NULL, 0,                                    // no out
+                &bytes,
+                NULL);
+
+```
+
+I want you to implement this code in Zig.

src/bun.zig

@@ -140,6 +140,7 @@ pub const sys = @import("./sys.zig");
 /// Deprecated: use bun.sys.S
 pub const S = sys.S;
 pub const O = sys.O;
+pub const AT = sys.AT;
 pub const Mode = sys.Mode;
 
 // Platform-specific system APIs. If something can be implemented on multiple

src/shell/builtin/rm.zig

@@ -923,7 +923,7 @@ pub const ShellRmTask = struct {
         }
 
         debug("[removeEntryDir] remove after children {s}", .{path});
-        switch (ShellSyscall.unlinkatWithFlags(this.getcwd(), path, std.posix.AT.REMOVEDIR)) {
+        switch (ShellSyscall.unlinkatWithFlags(this.getcwd(), path, std.posix.AT.REMOVEDIR | bun.AT.SYMLINK_NOFOLLOW)) {
             .result => {
                 switch (this.verboseDeleted(dir_task, path)) {
                     .err => |e| return .{ .err = e },
@@ -1083,7 +1083,7 @@ pub const ShellRmTask = struct {
             }
         };
         const dirfd = this.cwd;
-        switch (ShellSyscall.unlinkatWithFlags(dirfd, path, 0)) {
+        switch (ShellSyscall.unlinkatWithFlags(dirfd, path, bun.AT.SYMLINK_NOFOLLOW)) {
             .result => return this.verboseDeleted(parent_dir_task, path),
             .err => |e| {
                 debug("unlinkatWithFlags({s}) = {s}", .{ path, @tagName(e.getErrno()) });
@@ -1107,7 +1107,7 @@ pub const ShellRmTask = struct {
                                 // If `path` points to a directory, then it is deleted (if empty) or we handle it as a directory
                                 // If it's actually a file, we get an error so we don't need to call `stat` to check that.
                                 if (this.opts.recursive or this.opts.remove_empty_dirs) {
-                                    return switch (ShellSyscall.unlinkatWithFlags(this.getcwd(), path, std.posix.AT.REMOVEDIR)) {
+                                    return switch (ShellSyscall.unlinkatWithFlags(this.getcwd(), path, std.posix.AT.REMOVEDIR | bun.AT.SYMLINK_NOFOLLOW)) {
                                         // it was empty, we saved a syscall
                                         .result => return this.verboseDeleted(parent_dir_task, path),
                                         .err => |e2| {

src/sys.zig

@@ -62,6 +62,16 @@ fn toPackedO(number: anytype) std.posix.O {
 
 pub const Mode = std.posix.mode_t;
 
+pub const AT = switch (Environment.os) {
+    .mac, .wasm, .linux => std.posix.AT,
+    .windows => struct {
+        pub const REMOVEDIR = std.posix.AT.REMOVEDIR;
+        // We're making this one up, doesn't exist in the Zig standard library
+        // or Windows APIs
+        pub const SYMLINK_NOFOLLOW = 0x400;
+    },
+};
+
 pub const O = switch (Environment.os) {
     .mac => struct {
         pub const PATH = 0x0000;
@@ -880,7 +890,7 @@ pub fn lutimes(path: [:0]const u8, atime: JSC.Node.TimeLike, mtime: JSC.Node.Tim
         return sys_uv.lutimes(path, atime, mtime);
     }
 
-    return utimensWithFlags(path, atime, mtime, std.posix.AT.SYMLINK_NOFOLLOW);
+    return utimensWithFlags(path, atime, mtime, bun.AT.SYMLINK_NOFOLLOW);
 }
 
 pub fn mkdiratA(dir_fd: bun.FileDescriptor, file_path: []const u8) Maybe(void) {
@@ -2834,9 +2844,11 @@ pub fn unlinkatWithFlags(dirfd: bun.FileDescriptor, to: anytype, flags: c_uint)
             return unlinkatWithFlags(dirfd, bun.strings.toNTPath(w_buf, bun.span(to)), flags);
         }
 
+        // const follow_symlinks = flags
         return bun.windows.DeleteFileBun(to, .{
             .dir = if (dirfd != bun.invalid_fd) dirfd.cast() else null,
             .remove_dir = flags & std.posix.AT.REMOVEDIR != 0,
+            .no_follow_symlinks = flags & bun.AT.SYMLINK_NOFOLLOW != 0,
         });
     }
 

src/windows.zig

@@ -3483,6 +3483,7 @@ pub extern "kernel32" fn SetConsoleCP(wCodePageID: std.os.windows.UINT) callconv
 pub const DeleteFileOptions = struct {
     dir: ?HANDLE,
     remove_dir: bool = false,
+    no_follow_symlinks: bool = false,
 };
 
 const FILE_DISPOSITION_DELETE: ULONG = 0x00000001;
@@ -3492,11 +3493,12 @@ const FILE_DISPOSITION_ON_CLOSE: ULONG = 0x00000008;
 const FILE_DISPOSITION_IGNORE_READONLY_ATTRIBUTE: ULONG = 0x00000010;
 
 // Copy-paste of the standard library function except without unreachable.
-pub fn DeleteFileBun(sub_path_w: []const u16, options: DeleteFileOptions) bun.JSC.Maybe(void) {
+pub fn DeleteFileBun(sub_path_w: [:0]const u16, options: DeleteFileOptions) bun.JSC.Maybe(void) {
+    const FOLLOW_SYMLINKS: ULONG = if (options.no_follow_symlinks) FILE_OPEN_REPARSE_POINT else 0;
     const create_options_flags: ULONG = if (options.remove_dir)
-        FILE_DIRECTORY_FILE | FILE_OPEN_REPARSE_POINT
+        windows.FILE_DIRECTORY_FILE | FOLLOW_SYMLINKS
     else
-        windows.FILE_NON_DIRECTORY_FILE | FILE_OPEN_REPARSE_POINT; // would we ever want to delete the target instead?
+        windows.FILE_NON_DIRECTORY_FILE | FOLLOW_SYMLINKS; // would we ever want to delete the target instead? Yes, `rm` is expected to delete symlinks, not the actual files
 
     const path_len_bytes = @as(u16, @intCast(sub_path_w.len * 2));
     var nt_name = UNICODE_STRING{
@@ -3536,6 +3538,10 @@ pub fn DeleteFileBun(sub_path_w: []const u16, options: DeleteFileOptions) bun.JS
     );
     bun.sys.syslog("NtCreateFile({}, DELETE) = {}", .{ bun.fmt.fmtPath(u16, sub_path_w, .{}), rc });
     if (bun.JSC.Maybe(void).errnoSys(rc, .open)) |err| {
+        const attributes = bun.sys.getFileAttributes(sub_path_w) orelse @panic("FUCK");
+        if (attributes.is_reparse_point) {
+            std.debug.print("IS REPARSE mofo!!\n", .{});
+        }
         return err;
     }
     defer _ = bun.windows.CloseHandle(tmp_handle);

test/js/bun/shell/commands/rm.test.ts

@@ -144,6 +144,47 @@ foo/
       expect(await fileExists(`${tempdir}/sub_dir_files`)).toBeTrue();
     }
   });
+
+  test("removes symlinks, not the files referenced by the links", async () => {
+    const tempdir = tempDirWithFiles("rm-symlinks", {
+      "target.txt": "original content",
+      "dir/file.txt": "directory file",
+    });
+
+    // Create symlinks
+    await $`ln -s ${tempdir}/target.txt ${tempdir}/link.txt`.cwd(tempdir);
+    await $`ln -s ${tempdir}/dir ${tempdir}/dirlink`.cwd(tempdir);
+
+    // Verify symlinks exist and point to correct targets
+    expect(await fileExists(`${tempdir}/link.txt`)).toBeTrue();
+    expect(await fileExists(`${tempdir}/dirlink`)).toBeTrue();
+    expect(await Bun.file(`${tempdir}/link.txt`).text()).toBe("original content");
+    expect(await Bun.file(`${tempdir}/dirlink/file.txt`).text()).toBe("directory file");
+
+    // Remove the symlinks
+    // {
+    //   const { stdout, exitCode } = await $`rm -v ${tempdir}/link.txt`;
+    //   expect(stdout.toString()).toEqual(`${tempdir}/link.txt\n`);
+    //   expect(exitCode).toBe(0);
+    // }
+
+    {
+      const { stdout, exitCode } = await $`rm -rv ${tempdir}/dirlink`;
+      expect(stdout.toString()).toEqual(`${tempdir}/dirlink\n`);
+      expect(exitCode).toBe(0);
+    }
+
+    // Verify symlinks are gone but targets remain
+    // expect(await fileExists(`${tempdir}/link.txt`)).toBeFalse();
+    // expect(await fileExists(`${tempdir}/dirlink`)).toBeFalse();
+    // expect(await fileExists(`${tempdir}/target.txt`)).toBeTrue();
+    // expect(await fileExists(`${tempdir}/dir`)).toBeTrue();
+    // expect(await fileExists(`${tempdir}/dir/file.txt`)).toBeTrue();
+
+    // // Verify target files still have their content
+    // expect(await Bun.file(`${tempdir}/target.txt`).text()).toBe("original content");
+    // expect(await Bun.file(`${tempdir}/dir/file.txt`).text()).toBe("directory file");
+  });
 });
 
 function packagejson() {