Skip to content

Commit

Permalink
fix(windows): use extended max path prefix for hardlinks during insta…
Browse files Browse the repository at this point in the history 
…ll (#9721)

* uncomment code

* use GetFinalPathNameByHandleW

* add packages with large names

* delete

* test large package name
  • Loading branch information
@dylan-conway
dylan-conway authored Mar 30, 2024
1 parent 8ff7ee0 commit 7172013
Show file tree
Hide file tree
Showing 2 changed files with 95 additions and 46 deletions.
95 changes: 49 additions & 46 deletions src/install/install.zig
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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();

Expand All @@ -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 {
Expand Down Expand Up @@ -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{
Expand Down
46 changes: 46 additions & 0 deletions test/cli/install/registry/bun-install-registry.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"),
Expand Down

0 comments on commit 7172013

Please sign in to comment.