ReviewOS
Reload

also looking at this

pantry-pm/pantry

fix: preserve sources when removing linked workspace directories

#247
Merged chris wants to merge codex/preserve-linked-workspace-sources into main
4 files +98 -46

Review threads live on the whole diff, not on one commit, so none are shown here - a thread's line means something in the branch's final form, and painting it into an intermediate step would put it on code it is not about.

.github/scripts/windows-native-runtime.tsmodified+27-1
@@ -1,5 +1,5 @@
11import assert from 'node:assert/strict'
2import { cp, mkdir, mkdtemp, readFile, realpath, rm } from 'node:fs/promises'
2import { cp, mkdir, mkdtemp, readFile, realpath, rm, symlink } from 'node:fs/promises'
33import { tmpdir } from 'node:os'
44import { join, resolve } from 'node:path'
55
@@ -48,6 +48,32 @@ try {
4848 proof.sdkRuntimeSha256 = originalHash
4949 proof.nativeRuntimeReusedAndActivated = true
5050
51 // Cached workspace links point back into the source checkout. Reconciliation
52 // must replace the directory link itself, never walk and empty its target.
53 const workspace = join(owned, 'linked-workspaces')
54 const member = join(workspace, 'packages/zig')
55 await mkdir(member, { recursive: true })
56 await mkdir(join(workspace, 'pantry'))
57 await Bun.write(join(workspace, 'pantry.jsonc'), JSON.stringify({ dependencies: {} }))
58 await Bun.write(join(workspace, 'package.json'), JSON.stringify({ name: 'linked-workspace-fixture', private: true, workspaces: ['packages/*'] }))
59 await Bun.write(join(member, 'pantry.json'), JSON.stringify({ name: 'zig', version: '1.0.0' }))
60 const source = join(member, 'build.zig')
61 await Bun.write(source, 'tracked workspace source\n')
62 const sourceHash = Bun.SHA256.hash(await readFile(source), 'hex')
63 for (const type of ['dir', 'junction'] as const) {
64 const link = join(workspace, 'pantry/zig')
65 await rm(link, { recursive: true, force: true })
66 await symlink(member, link, type)
67 for (const attempt of [1, 2]) {
68 const result = await run(`workspace-relink-${type}-${attempt}`, [cli, 'install', '--no-save', '--verbose'], workspace)
69 assert.match(result, /Linked 1 workspace package/)
70 assert.equal(Bun.SHA256.hash(await readFile(source), 'hex'), sourceHash)
71 assert.equal(await realpath(link), await realpath(member))
72 }
73 }
74 proof.workspaceSourcePreservedThroughRepeatedRelinking = true
75 proof.workspaceSourceSha256 = sourceHash
76
5177 // Exercise the bundled Action itself. A missing local dependency must fail
5278 // inside workspace installation, before the Action can say setup completed.
5379 const negative = join(owned, 'required-workspace-failure')
.github/workflows/ci.ymlmodified+18-1
@@ -405,7 +405,7 @@ jobs:
405405 if-no-files-found: error
406406
407407 continuous-release:
408 needs: [lint, typecheck, test, action-redis-service, action-native-services, action-native-tools, native-browser, windows-native-runtime]
408 needs: [lint, typecheck, test, action-redis-service, action-native-services, action-native-tools, native-browser, windows-native-runtime, windows-file-tree]
409409 runs-on: ubuntu-latest
410410 timeout-minutes: 15
411411 # Best-effort commit-preview publishing. Don't fail the whole workflow
@@ -465,6 +465,23 @@ jobs:
465465 ]
466466 webhook_url: ${{ secrets.DISCORD_WEBHOOK_URL }}
467467
468 windows-file-tree:
469 name: Windows linked workspace cleanup
470 runs-on: windows-latest
471 timeout-minutes: 10
472 steps:
473 - uses: actions/checkout@v6
474 - name: Install the pinned compiler and runtime
475 uses: pantry-pm/pantry/packages/action@ae59a7b7fc5f4e6f7b8041a75e6b5772680bebdf # v0.11.101
476 with:
477 version: '0.11.101'
478 install: 'false'
479 packages: bun.com@1.4.3 ziglang.org@0.17.0
480 - name: Test source preservation through directory and nested links
481 # Keep the compiler's own adjacent standard library when invoking a
482 # managed binary through the package's executable link.
483 run: bun -e 'const zig = require("node:fs").realpathSync(Bun.which("zig")); process.exit(Bun.spawnSync([zig, "test", "packages/zig/src/io_helper.zig", "-lc", "--test-filter", "deleteTree"], { stdout: "inherit", stderr: "inherit" }).exitCode)'
484
468485 windows-native-runtime:
469486 needs: test
470487 runs-on: windows-latest
packages/zig/src/io_helper.zigmodified+48-40
@@ -375,48 +375,12 @@ pub fn deleteFile(path: []const u8) !void {
375375 }
376376}
377377
378/// Delete a directory using platform-specific syscalls
379fn deleteDir(path: []const u8) !void {
380 var path_buf: [std.fs.max_path_bytes:0]u8 = undefined;
381 if (path.len >= path_buf.len) return error.NameTooLong;
382 @memcpy(path_buf[0..path.len], path);
383 path_buf[path.len] = 0;
384
385 switch (builtin.os.tag) {
386 .windows => {
387 cwd().deleteDir(io, path) catch return error.DirNotEmpty;
388 },
389 .linux => {
390 const AT_REMOVEDIR = 0x200;
391 const rc = std.os.linux.unlinkat(std.os.linux.AT.FDCWD, &path_buf, AT_REMOVEDIR);
392 if (rc != 0) return error.DirNotEmpty;
393 },
394 else => {
395 // macOS and others - use libc
396 const result = c.rmdir(&path_buf);
397 if (result != 0) return error.DirNotEmpty;
398 },
399 }
400}
401
402378/// Delete a directory tree recursively
403379pub fn deleteTree(path: []const u8) !void {
404 // Try to delete as file first
405 deleteFile(path) catch {
406 // If that fails, try as directory
407 var dir = openDirForIteration(path) catch return;
408
409 var iter = dir.iterate();
410 while (iter.next() catch null) |entry| {
411 var child_path_buf: [std.fs.max_path_bytes]u8 = undefined;
412 const child_path = std.fmt.bufPrint(&child_path_buf, "{s}/{s}", .{ path, entry.name }) catch continue;
413 deleteTree(child_path) catch {};
414 }
415 dir.close();
416
417 // Now remove the empty directory
418 deleteDir(path) catch {};
419 };
380 // The standard implementation removes links themselves, including Windows
381 // directory reparse points. Opening a directory after deleteFile fails can
382 // follow a workspace link and recursively delete its source checkout.
383 try cwd().deleteTree(io, path);
420384}
421385
422386/// Get the current working directory as a path string
@@ -1112,6 +1076,50 @@ test "symLink creates a relative executable alias at an absolute path" {
11121076 try std.testing.expectEqualStrings("pantry", target);
11131077}
11141078
1079test "deleteTree removes a workspace directory link without deleting its sources" {
1080 const allocator = std.testing.allocator;
1081 var tmp = std.testing.tmpDir(.{});
1082 defer tmp.cleanup();
1083 try tmp.dir.createDir(io, "sources", .default_dir);
1084 try tmp.dir.writeFile(io, .{ .sub_path = "sources/build.zig", .data = "tracked workspace source" });
1085 const working_directory = try getCwdAlloc(allocator);
1086 defer allocator.free(working_directory);
1087 const link_path = try std.fs.path.join(allocator, &.{ working_directory, ".zig-cache", "tmp", tmp.sub_path[0..], "workspace" });
1088 defer allocator.free(link_path);
1089 try symLink("sources", link_path);
1090
1091 // Workspace relinking removes an existing link before recreating it.
1092 // Windows directory symlinks cannot be removed with deleteFile; following
1093 // them here empties the tracked source checkout instead of just the link.
1094 try deleteTree(link_path);
1095 const source = try tmp.dir.readFileAlloc(io, "sources/build.zig", allocator, .limited(1024));
1096 defer allocator.free(source);
1097 try std.testing.expectEqualStrings("tracked workspace source", source);
1098 try std.testing.expectError(error.FileNotFound, tmp.dir.statFile(io, "workspace", .{ .follow_symlinks = false }));
1099}
1100
1101test "deleteTree does not follow directory links inside a removed tree" {
1102 const allocator = std.testing.allocator;
1103 var tmp = std.testing.tmpDir(.{});
1104 defer tmp.cleanup();
1105 try tmp.dir.createDir(io, "sources", .default_dir);
1106 try tmp.dir.createDir(io, "cache", .default_dir);
1107 try tmp.dir.writeFile(io, .{ .sub_path = "sources/build.zig", .data = "keep source bytes" });
1108 try tmp.dir.writeFile(io, .{ .sub_path = "cache/owned.txt", .data = "remove cache bytes" });
1109 const working_directory = try getCwdAlloc(allocator);
1110 defer allocator.free(working_directory);
1111 const cache_path = try std.fs.path.join(allocator, &.{ working_directory, ".zig-cache", "tmp", tmp.sub_path[0..], "cache" });
1112 defer allocator.free(cache_path);
1113 const link_path = try std.fs.path.join(allocator, &.{ cache_path, "workspace" });
1114 defer allocator.free(link_path);
1115 try symLink("../sources", link_path);
1116 try deleteTree(cache_path);
1117 const source = try tmp.dir.readFileAlloc(io, "sources/build.zig", allocator, .limited(1024));
1118 defer allocator.free(source);
1119 try std.testing.expectEqualStrings("keep source bytes", source);
1120 try std.testing.expectError(error.FileNotFound, tmp.dir.statFile(io, "cache", .{}));
1121}
1122
11151123/// Read a symbolic link target
11161124pub fn readLink(path: []const u8, buf: []u8) ![]const u8 {
11171125 if (comptime is_windows) {