From 7172013a72534e770dd211d4fc0bbc29e9d55091 Mon Sep 17 00:00:00 2001 From: Dylan Conway <35280289+dylan-conway@users.noreply.github.com> Date: Fri, 29 Mar 2024 18:13:39 -0700 Subject: [PATCH] fix(windows): use extended max path prefix for hardlinks during install (#9721) * uncomment code * use GetFinalPathNameByHandleW * add packages with large names * delete * test large package name --- src/install/install.zig | 95 ++++++++++--------- .../registry/bun-install-registry.test.ts | 46 +++++++++ 2 files changed, 95 insertions(+), 46 deletions(-) diff --git a/src/install/install.zig b/src/install/install.zig index f4724f31138708..2f84fd331aa95f 100644 --- a/src/install/install.zig +++ b/src/install/install.zig @@ -1115,6 +1115,15 @@ pub const PackageInstall = struct { } }, + pub fn fail(err: anyerror, step: Step) Result { + return .{ + .fail = .{ + .err = err, + .step = step, + }, + }; + } + pub const Tag = enum { success, fail, @@ -1360,23 +1369,17 @@ pub const PackageInstall = struct { fn installWithHardlink(this: *PackageInstall) !Result { const Walker = @import("../walker_skippable.zig"); - var cached_package_dir = bun.openDir(this.cache_dir, this.cache_dir_subpath) catch |err| return Result{ - .fail = .{ .err = err, .step = .opening_cache_dir }, - }; + var cached_package_dir = bun.openDir(this.cache_dir, this.cache_dir_subpath) catch |err| return Result.fail(err, .opening_cache_dir); defer cached_package_dir.close(); var walker_ = Walker.walk( cached_package_dir, this.allocator, &[_]bun.OSPathSlice{}, &[_]bun.OSPathSlice{}, - ) catch |err| return Result{ - .fail = .{ .err = err, .step = .opening_cache_dir }, - }; + ) catch |err| return Result.fail(err, .opening_cache_dir); defer walker_.deinit(); - var subdir = this.destination_dir.makeOpenPath(bun.span(this.destination_dir_subpath), .{}) catch |err| return Result{ - .fail = .{ .err = err, .step = .opening_cache_dir }, - }; + var subdir = this.destination_dir.makeOpenPath(bun.span(this.destination_dir_subpath), .{}) catch |err| return Result.fail(err, .opening_cache_dir); defer subdir.close(); @@ -1385,36 +1388,41 @@ pub const PackageInstall = struct { var to_copy_buf: []u16 = undefined; var to_copy_buf2: []u16 = undefined; if (comptime Environment.isWindows) { - var fd_path_buf: bun.PathBuffer = undefined; - - // buf[0] = '\\'; - // buf[1] = '\\'; - // buf[2] = '?'; - // buf[3] = '\\'; - // const dest_path = try bun.getFdPath(subdir.fd, &fd_path_buf); - // strings.copyU8IntoU16(buf[4..], dest_path); - // buf[dest_path.len + 4] = '\\'; - // to_copy_buf = buf[dest_path.len + 5 ..]; - - // buf2[0] = '\\'; - // buf2[1] = '\\'; - // buf2[2] = '?'; - // buf2[3] = '\\'; - // const cache_path = try bun.getFdPath(cached_package_dir.fd, &fd_path_buf); - // strings.copyU8IntoU16(buf2[4..], cache_path); - // buf2[cache_path.len + 4] = '\\'; - // to_copy_buf2 = buf2[cache_path.len + 5 ..]; - - // TODO(dylan-conway): find out why //?/ isn't working - const dest_path = try bun.getFdPath(subdir.fd, &fd_path_buf); - strings.copyU8IntoU16(&buf, dest_path); - buf[dest_path.len] = '\\'; - to_copy_buf = buf[dest_path.len + 1 ..]; - - const cache_path = try bun.getFdPath(cached_package_dir.fd, &fd_path_buf); - strings.copyU8IntoU16(&buf2, cache_path); - buf2[cache_path.len] = '\\'; - to_copy_buf2 = buf2[cache_path.len + 1 ..]; + const dest_path_length = bun.windows.kernel32.GetFinalPathNameByHandleW(subdir.fd, &buf, buf.len, 0); + if (dest_path_length == 0) { + const e = bun.windows.Win32Error.get(); + const err = if (e.toSystemErrno()) |sys_err| bun.errnoToZigErr(sys_err) else brk: { + // If this code path is reached, it should have a toSystemErrno mapping + Output.warn("Failed to get destination path for package \"{s}\" during installation: {s}", .{ this.package_name, @tagName(e) }); + break :brk error.Unexpected; + }; + return Result.fail(err, .copying_files); + } + const dest_path = buf[0..dest_path_length]; + if (buf[dest_path.len - 1] != '\\') { + buf[dest_path.len] = '\\'; + to_copy_buf = buf[dest_path.len + 1 ..]; + } else { + to_copy_buf = buf[dest_path.len..]; + } + + const cache_path_length = bun.windows.kernel32.GetFinalPathNameByHandleW(cached_package_dir.fd, &buf2, buf2.len, 0); + if (cache_path_length == 0) { + const e = bun.windows.Win32Error.get(); + const err = if (e.toSystemErrno()) |sys_err| bun.errnoToZigErr(sys_err) else brk: { + // If this code path is reached, it should have a toSystemErrno mapping + Output.warn("Failed to get cache path for package \"{s}\" during installation: {s}", .{ this.package_name, @tagName(e) }); + break :brk error.Unexpected; + }; + return Result.fail(err, .copying_files); + } + const cache_path = buf2[0..cache_path_length]; + if (buf2[cache_path.len - 1] != '\\') { + buf2[cache_path.len] = '\\'; + to_copy_buf2 = buf2[cache_path.len + 1 ..]; + } else { + to_copy_buf2 = buf2[cache_path.len..]; + } } const FileCopier = struct { @@ -1490,17 +1498,12 @@ pub const PackageInstall = struct { ) catch |err| { if (comptime Environment.isWindows) { if (err == error.FailedToCopyFile) { - return Result{ - .fail = .{ .err = err, .step = .copying_files }, - }; + return Result.fail(err, .copying_files); } } else if (err == error.NotSameFileSystem or err == error.ENXIO) { return err; } - - return Result{ - .fail = .{ .err = err, .step = .copying_files }, - }; + return Result.fail(err, .copying_files); }; return Result{ diff --git a/test/cli/install/registry/bun-install-registry.test.ts b/test/cli/install/registry/bun-install-registry.test.ts index 10e7144f345824..f7019fba669db0 100644 --- a/test/cli/install/registry/bun-install-registry.test.ts +++ b/test/cli/install/registry/bun-install-registry.test.ts @@ -243,6 +243,52 @@ describe.each(["--production", "without --production"])("%s", flag => { }); }); +test("hardlinks on windows dont fail with long paths", async () => { + await mkdir(join(packageDir, "a-package")); + await writeFile( + join(packageDir, "a-package", "package.json"), + JSON.stringify({ + name: "a-package", + version: "1.0.0", + }), + ); + + await writeFile( + join(packageDir, "package.json"), + JSON.stringify({ + name: "foo", + version: "1.2.3", + dependencies: { + // 255 characters + "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa": + "file:./a-package", + }, + }), + ); + + const { stdout, stderr, exited } = spawn({ + cmd: [bunExe(), "install"], + cwd: packageDir, + stdout: "pipe", + stderr: "pipe", + env, + }); + + const err = await Bun.readableStreamToText(stderr); + const out = await Bun.readableStreamToText(stdout); + expect(err).toContain("Saved lockfile"); + expect(err).not.toContain("not found"); + expect(err).not.toContain("error:"); + expect(err).not.toContain("panic:"); + expect(out.replace(/\s*\[[0-9\.]+m?s\]\s*$/, "").split(/\r?\n/)).toEqual([ + "", + " + aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa@a-package", + "", + " 1 package installed", + ]); + expect(await exited).toBe(0); +}); + test("basic 1", async () => { await writeFile( join(packageDir, "package.json"),