| author |  Andrew Kelley <andrew@ziglang.org> 2022-10-17 16:54:52-07:00 |
| committer | Andrew Kelley <andrew@ziglang.org> 2022-10-18 16:52:43-07:00 |
| log | cb635e084b0c36e3c7a6bf63f49f7e7e9918532d |
| tree | 92d53f835566b630fd39f44a5e8c8ef9b3339add |
| parent | 9ee4530b9b38805f88a1e3df4b4b74db240f05df |
stage2: better handling of CacheMode.whole on Windows
Windows gives AccessDenied if you delete a directory which contains open
file handles. This could be triggered when using CacheMode.whole when
cross compiling macho test binaries.2 files changed, 20 insertions(+), 10 deletions(-)
src/Compilation.zig+14-2
| ... | @@ -2390,9 +2390,21 @@ pub fn update(comp: *Compilation) !void { | ... | @@ -2390,9 +2390,21 @@ pub fn update(comp: *Compilation) !void { |
| 2390 | const o_sub_path = try std.fs.path.join(comp.gpa, &[_][]const u8{ "o", &digest }); | 2390 | const o_sub_path = try std.fs.path.join(comp.gpa, &[_][]const u8{ "o", &digest }); |
| 2391 | defer comp.gpa.free(o_sub_path); | 2391 | defer comp.gpa.free(o_sub_path); |
| 2392 | 2392 | ||
| 2393 | // Work around windows `AccessDenied` if any files within this directory are open | ||
| 2394 | // by doing the makeExecutable/makeWritable dance. | ||
| 2395 | const need_writable_dance = builtin.os.tag == .windows and comp.bin_file.file != null; | ||
| 2396 | if (need_writable_dance) { | ||
| 2397 | try comp.bin_file.makeExecutable(); | ||
| 2398 | } | ||
| 2399 | |||
| 2393 | try comp.bin_file.renameTmpIntoCache(comp.local_cache_directory, tmp_dir_sub_path, o_sub_path); | 2400 | try comp.bin_file.renameTmpIntoCache(comp.local_cache_directory, tmp_dir_sub_path, o_sub_path); |
| 2394 | comp.wholeCacheModeSetBinFilePath(&digest); | 2401 | comp.wholeCacheModeSetBinFilePath(&digest); |
| 2395 | 2402 | ||
| 2403 | // Has to be after the `wholeCacheModeSetBinFilePath` above. | ||
| 2404 | if (need_writable_dance) { | ||
| 2405 | try comp.bin_file.makeWritable(); | ||
| 2406 | } | ||
| 2407 | |||
| 2396 | // This is intentionally sandwiched between renameTmpIntoCache() and writeManifest(). | 2408 | // This is intentionally sandwiched between renameTmpIntoCache() and writeManifest(). |
| 2397 | if (comp.bin_file.options.module) |module| { | 2409 | if (comp.bin_file.options.module) |module| { |
| 2398 | // We need to set the zig_cache_artifact_directory for -femit-asm, -femit-llvm-ir, | 2410 | // We need to set the zig_cache_artifact_directory for -femit-asm, -femit-llvm-ir, |
| ... | @@ -3207,8 +3219,8 @@ fn processOneJob(comp: *Compilation, job: Job) !void { | ... | @@ -3207,8 +3219,8 @@ fn processOneJob(comp: *Compilation, job: Job) !void { |
| 3207 | // TODO Surface more error details. | 3219 | // TODO Surface more error details. |
| 3208 | comp.lockAndSetMiscFailure( | 3220 | comp.lockAndSetMiscFailure( |
| 3209 | .mingw_crt_file, | 3221 | .mingw_crt_file, |
| 3210 | "unable to build mingw-w64 CRT file: {s}", | 3222 | "unable to build mingw-w64 CRT file {s}: {s}", |
| 3211 | .{@errorName(err)}, | 3223 | .{ @tagName(crt_file), @errorName(err) }, |
| 3212 | ); | 3224 | ); |
| 3213 | }; | 3225 | }; |
| 3214 | }, | 3226 | }, |
src/link.zig+6-8
| ... | @@ -403,10 +403,8 @@ pub const File = struct { | ... | @@ -403,10 +403,8 @@ pub const File = struct { |
| 403 | try emit.directory.handle.copyFile(emit.sub_path, emit.directory.handle, emit.sub_path, .{}); | 403 | try emit.directory.handle.copyFile(emit.sub_path, emit.directory.handle, emit.sub_path, .{}); |
| 404 | } | 404 | } |
| 405 | } | 405 | } |
| 406 | if (base.intermediary_basename == null) { | 406 | f.close(); |
| 407 | f.close(); | 407 | base.file = null; |
| 408 | base.file = null; | ||
| 409 | } | ||
| 410 | }, | 408 | }, |
| 411 | .coff, .elf, .plan9, .wasm => if (base.file) |f| { | 409 | .coff, .elf, .plan9, .wasm => if (base.file) |f| { |
| 412 | if (base.intermediary_basename != null) { | 410 | if (base.intermediary_basename != null) { |
| ... | @@ -777,7 +775,7 @@ pub const File = struct { | ... | @@ -777,7 +775,7 @@ pub const File = struct { |
| 777 | _ = base; | 775 | _ = base; |
| 778 | while (true) { | 776 | while (true) { |
| 779 | if (builtin.os.tag == .windows) { | 777 | if (builtin.os.tag == .windows) { |
| 780 | // workaround windows `renameW` can't fail with `PathAlreadyExists` | 778 | // Work around windows `renameW` can't fail with `PathAlreadyExists` |
| 781 | // See https://github.com/ziglang/zig/issues/8362 | 779 | // See https://github.com/ziglang/zig/issues/8362 |
| 782 | if (cache_directory.handle.access(o_sub_path, .{})) |_| { | 780 | if (cache_directory.handle.access(o_sub_path, .{})) |_| { |
| 783 | try cache_directory.handle.deleteTree(o_sub_path); | 781 | try cache_directory.handle.deleteTree(o_sub_path); |
| ... | @@ -791,9 +789,9 @@ pub const File = struct { | ... | @@ -791,9 +789,9 @@ pub const File = struct { |
| 791 | tmp_dir_sub_path, | 789 | tmp_dir_sub_path, |
| 792 | cache_directory.handle, | 790 | cache_directory.handle, |
| 793 | o_sub_path, | 791 | o_sub_path, |
| 794 | ) catch |err| switch (err) { | 792 | ) catch |err| { |
| 795 | error.AccessDenied => unreachable, // We are most likely trying to move a dir with open handles to its resources | 793 | log.err("unable to rename cache dir {s} to {s}: {s}", .{ tmp_dir_sub_path, o_sub_path, @errorName(err) }); |
| 796 | else => |e| return e, | 794 | return err; |
| 797 | }; | 795 | }; |
| 798 | break; | 796 | break; |
| 799 | } else { | 797 | } else { |