diff options
| author | Andrew Kelley <andrew@ziglang.org> | 2024-12-10 17:43:42 -0800 |
|---|---|---|
| committer | Andrew Kelley <andrew@ziglang.org> | 2024-12-10 18:11:12 -0800 |
| commit | d37ee79535188263bd6a907eb26a48364c4c12f2 (patch) | |
| tree | 9cff3c562afce6f3ddedfa236a7a819d92a9728a /src | |
| parent | c172877b81f4eff50cf214eb553c9df108fbd9eb (diff) | |
| download | zig-d37ee79535188263bd6a907eb26a48364c4c12f2.tar.gz zig-d37ee79535188263bd6a907eb26a48364c4c12f2.zip | |
std.Build.Cache.hit: more discipline in error handling
Previous commits
2b0929929d67e222ca6a9523a3a594ed456c4a51
4ea2f441df36cec61e1017f4d795d4037326c98c
had this text:
> There are no dir components, so you would think that this was
> unreachable, however we have observed on macOS two processes racing to
> do openat() with O_CREAT manifest in ENOENT.
This appears to have been a misunderstanding based on the issue
report #12138 and corresponding PR #12139 in which the steps to
reproduce removed the cache directory in a loop which also executed
detached Zig compiler processes.
There is no evidence for the macOS kernel bug however the ENOENT is
easily explained by the removal of the cache directory.
This commit reverts those commits, ultimately reporting the ENOENT as an
error rather than repeating the create file operation. However this
commit also adds an explicit error set to `std.Build.Cache.hit` as well
as changing the `failed_file_index` to a proper diagnostic field that
fully communicates what failed, leading to more informative error
messages on failure to check the cache.
The equivalent failure when occuring for AstGen performs a fatal process
kill, reasoning being that the compiler has an invariant of the cache
directory not being yanked out from underneath it while executing. This
could be made a more granular error in the future but I suspect such
thing is not valuable to pursue.
Related to #18340 but does not solve it.
Diffstat (limited to 'src')
| -rw-r--r-- | src/Compilation.zig | 31 | ||||
| -rw-r--r-- | src/Zcu/PerThread.zig | 12 | ||||
| -rw-r--r-- | src/link.zig | 2 |
3 files changed, 32 insertions, 13 deletions
diff --git a/src/Compilation.zig b/src/Compilation.zig index c198b5af82..12e8a87831 100644 --- a/src/Compilation.zig +++ b/src/Compilation.zig @@ -2048,15 +2048,30 @@ pub fn update(comp: *Compilation, main_progress_node: std.Progress.Node) !void { whole.cache_manifest = &man; try addNonIncrementalStuffToCacheManifest(comp, arena, &man); - const is_hit = man.hit() catch |err| { - const i = man.failed_file_index orelse return err; - const pp = man.files.keys()[i].prefixed_path; - const prefix = man.cache.prefixes()[pp.prefix]; - return comp.setMiscFailure( + const is_hit = man.hit() catch |err| switch (err) { + error.CacheCheckFailed => switch (man.diagnostic) { + .none => unreachable, + .manifest_create, .manifest_read, .manifest_lock => |e| return comp.setMiscFailure( + .check_whole_cache, + "failed to check cache: {s} {s}", + .{ @tagName(man.diagnostic), @errorName(e) }, + ), + .file_open, .file_stat, .file_read, .file_hash => |op| { + const pp = man.files.keys()[op.file_index].prefixed_path; + const prefix = man.cache.prefixes()[pp.prefix]; + return comp.setMiscFailure( + .check_whole_cache, + "failed to check cache: '{}{s}' {s} {s}", + .{ prefix, pp.sub_path, @tagName(man.diagnostic), @errorName(op.err) }, + ); + }, + }, + error.OutOfMemory => return error.OutOfMemory, + error.InvalidFormat => return comp.setMiscFailure( .check_whole_cache, - "unable to check cache: stat file '{}{s}' failed: {s}", - .{ prefix, pp.sub_path, @errorName(err) }, - ); + "failed check cache: invalid manifest file format", + .{}, + ), }; if (is_hit) { // In this case the cache hit contains the full set of file system inputs. Nice! diff --git a/src/Zcu/PerThread.zig b/src/Zcu/PerThread.zig index aeda9ea1b1..360e479f3c 100644 --- a/src/Zcu/PerThread.zig +++ b/src/Zcu/PerThread.zig @@ -135,10 +135,14 @@ pub fn astGenFile( error.PipeBusy => unreachable, // it's not a pipe error.NoDevice => unreachable, // it's not a pipe error.WouldBlock => unreachable, // not asking for non-blocking I/O - // There are no dir components, so you would think that this was - // unreachable, however we have observed on macOS two processes racing - // to do openat() with O_CREAT manifest in ENOENT. - error.FileNotFound => continue, + error.FileNotFound => { + // Since there are no dir components this could only occur if + // `zir_dir` is deleted after the compiler process obtains an + // open directory handle. + std.process.fatal("cache directory '{}' unexpectedly removed during compiler execution", .{ + cache_directory, + }); + }, else => |e| return e, // Retryable errors are handled at callsite. }; diff --git a/src/link.zig b/src/link.zig index 6eab62eced..b71c94d570 100644 --- a/src/link.zig +++ b/src/link.zig @@ -768,7 +768,7 @@ pub const File = struct { /// TODO audit this error set. most of these should be collapsed into one error, /// and Diags.Flags should be updated to convey the meaning to the user. pub const FlushError = error{ - CacheUnavailable, + CacheCheckFailed, CurrentWorkingDirectoryUnlinked, DivisionByZero, DllImportLibraryNotFound, |
