Skip to content

Commit a92427b

Browse files
committed
Build.Cache.Path: fix resolvePosix empty sub_path
This function is sometimes used to assume a canonical representation of a path. However, when the `Path` referred to `root_dir` itself, this function previously resolved `sub_path` to ".", which is incorrect; per doc comments, it should set `sub_path` to "". This fix ultimately didn't solve what I was trying to solve, though I'm still PRing it, because it's still *correct*. The background to this commit is quite interesting and worth briefly discussing. I originally worked on this to try and fix a bug in the build system, where if the root package (i.e. the one you `zig build`) depends on package X which itself depends back on the root package (through a `.path` dependency), invalid dependency modules are generated. I hit this case working on ziglang/translate-c, which wants to depend on "examples" (similar to the Zig compiler's "standalone" test cases) which themselves depend back on the translate-c package. However, after this patch just turned that error into another, I realised that this case simply cannot work, because `std.Build` needs to eagerly execute build scripts at `dependency` calls to learn which artifacts, modules, etc, exist. ...at least, that's how the build system is currently designed. One can imagine a world where `dependency` sort of "queues" the call, `artifact` and `module` etc just pretend that the thing exists, and all configure functions are called non-recursively by the runner. The downside is that it becomes impossible to query state set by a dependency's configure script. For instance, if a dependency exposes an artifact, it would become impossible to get that artifact's resolved target in the configure phase. However, as well as allowing recursive package imports (which are certainly kinda nifty), it would also make lazy dependencies far more useful! Right now, lazy dependencies only really work if you use options (`std.Build.option`) to block their usage, since any call to `lazyDependency` causes the dependency to be fetched. However, if we made this change, lazy dependencies could be made far more versatile by only fetching them *if the final step plan requires them*. I'm not 100% sure if this is a good idea or not, but I might open an issue for it soon.
1 parent 561fdd0 commit a92427b

File tree

1 file changed

+3
-1
lines changed

1 file changed

+3
-1
lines changed

lib/std/Build/Cache/Path.zig

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,9 +30,11 @@ pub fn join(p: Path, arena: Allocator, sub_path: []const u8) Allocator.Error!Pat
3030

3131
pub fn resolvePosix(p: Path, arena: Allocator, sub_path: []const u8) Allocator.Error!Path {
3232
if (sub_path.len == 0) return p;
33+
const new_sub_path = try fs.path.resolvePosix(arena, &.{ p.sub_path, sub_path });
3334
return .{
3435
.root_dir = p.root_dir,
35-
.sub_path = try fs.path.resolvePosix(arena, &.{ p.sub_path, sub_path }),
36+
// Use "" instead of "." to represent `root_dir` itself.
37+
.sub_path = if (std.mem.eql(u8, new_sub_path, ".")) "" else new_sub_path,
3638
};
3739
}
3840

0 commit comments

Comments
 (0)