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
.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/scripts/windows-native-tools.tsmodified+16-4
@@ -1,5 +1,5 @@
11import assert from 'node:assert/strict'
2import { mkdir, mkdtemp, lstat, rename, rm } from 'node:fs/promises'
2import { mkdir, mkdtemp, lstat, readFile, rm } from 'node:fs/promises'
33import { tmpdir } from 'node:os'
44import { join } from 'node:path'
55import { installPackage } from '../../packages/ts-pantry/src/installer'
@@ -9,8 +9,8 @@ const artifacts = join(import.meta.dir, '../../artifacts/windows-native-tools')
99await mkdir(artifacts, { recursive: true })
1010const root = await mkdtemp(join(tmpdir(), 'pantry native Windows '))
1111const proof: Record<string, unknown> = {}
12async function run(name: string, command: string[]): Promise<string> {
13 const child = Bun.spawn(command, { cwd: root, env: { ...process.env, NO_COLOR: '1' }, stdout: 'pipe', stderr: 'pipe' })
12async function run(name: string, command: string[], environment: Record<string, string> = {}): Promise<string> {
13 const child = Bun.spawn(command, { cwd: root, env: { ...process.env, NO_COLOR: '1', ...environment }, stdout: 'pipe', stderr: 'pipe' })
1414 const timeout = setTimeout(() => child.kill(), 180_000)
1515 try {
1616 const [code, stdout, stderr] = await Promise.all([child.exited, new Response(child.stdout).text(), new Response(child.stderr).text()])
@@ -28,18 +28,30 @@ try {
2828 proof.cliVersion = version
2929 let tree = join(root, 'pantry')
3030 await installPackage('ziglang.org', '0.17.0', { installDir: tree })
31 const compilerSha256 = Bun.SHA256.hash(await readFile(join(tree, '.bin/zig.exe')), 'hex')
32 proof.compilerSha256 = compilerSha256
3133 const source = join(root, 'fixture.zig')
3234 await Bun.write(source, 'const std = ; pub fn main() void { std.debug.print("NATIVE_ZIG_STDLIB_OK\\n", .{}); }\n')
3335 for (const phase of ['installed', 'relocated']) {
3436 const alias = join(tree, '.bin/zig.exe')
3537 assert((await lstat(alias)).isSymbolicLink())
38 assert.equal(Bun.SHA256.hash(await readFile(alias), 'hex'), compilerSha256)
3639 assert.equal((await run(`${phase}-version`, [alias, 'version'])).trim(), '0.17.0')
3740 const output = join(root, `${phase}.exe`)
3841 await run(`${phase}-compile`, [alias, 'build-exe', source, '-OReleaseSafe', `-femit-bin=${output}`])
3942 assert.equal((await run(`${phase}-execute`, [output])).trim(), 'NATIVE_ZIG_STDLIB_OK')
4043 if (phase === 'installed') {
4144 const moved = join(root, 'moved pantry')
42 await rename(tree, moved)
45 // Exercise the Windows user operation itself. Bun 1.4.3's rename rejects
46 // this installed tree with EPERM while the native Windows move succeeds.
47 // One native move must complete; source absence and the next phase's
48 // symlink, byte identity, compile and execution assertions stay required.
49 await run('relocate-prefix', ['powershell', '-NoProfile', '-NonInteractive', '-Command', 'Move-Item -LiteralPath $env:PANTRY_RELOCATION_SOURCE -Destination $env:PANTRY_RELOCATION_DESTINATION -ErrorAction Stop'], {
50 PANTRY_RELOCATION_SOURCE: tree,
51 PANTRY_RELOCATION_DESTINATION: moved,
52 })
53 await assert.rejects(lstat(tree), { code: 'ENOENT' })
54 proof.prefixRelocatedByNativeWindows = true
4355 tree = moved
4456 }
4557 }
.github/workflows/ci.ymlmodified+7-1
@@ -153,7 +153,10 @@ jobs:
153153 run: bun .github/scripts/linux-native-tools.ts
154154
155155 - name: Cross-compile the source CLI for native Windows qualification
156 run: cd packages/zig && zig build -Dtarget=x86_64-windows-gnu -Doptimize=Debug -j2 --prefix ../../artifacts/windows-source-cli
156 run: |
157 cd packages/zig
158 zig build -Dtarget=x86_64-windows-gnu -Doptimize=Debug -j2 --prefix ../../artifacts/windows-source-cli
159 zig test src/io_helper.zig -lc -target x86_64-windows-gnu --test-no-exec --test-filter deleteTree -femit-bin=../../artifacts/windows-source-cli/bin/file-tree-tests.exe
157160
158161 - name: Retain the source Windows CLI and Action
159162 uses: actions/upload-artifact@v4
@@ -161,6 +164,7 @@ jobs:
161164 name: windows-source-runtime
162165 path: |
163166 artifacts/windows-source-cli/bin/pantry.exe
167 artifacts/windows-source-cli/bin/file-tree-tests.exe
164168 packages/action/dist/index.js
165169 packages/action/dist/post.js
166170 if-no-files-found: error
@@ -482,6 +486,8 @@ jobs:
482486 with:
483487 name: windows-source-runtime
484488 path: artifacts/windows-source-runtime
489 - name: Test source preservation through directory and nested links
490 run: '& ./artifacts/windows-source-runtime/artifacts/windows-source-cli/bin/file-tree-tests.exe'
485491 - name: Verify the actual source installer and required workspace failures
486492 env:
487493 PANTRY_TEST_CLI: artifacts/windows-source-runtime/artifacts/windows-source-cli/bin/pantry.exe
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) {