zig icon indicating copy to clipboard operation
zig copied to clipboard

Use `std.fs.path.relative` for `@import` and `@embedFile` sub paths

Open squeek502 opened this issue 2 years ago • 1 comments

Fixes edge cases where the startsWith that was used previously would return a false positive on a resolved path like foo.zig when the resolved root was foo. Before this commit, such a path would be treated as a sub path of 'foo' with a resolved sub file path of 'zig' (and the . would be assumed to be a path separator). After this commit, foo.zig will be correctly treated as outside of the root of foo.

Closes #18355

With this setup:

// foo.zig
pub fn foo() void {}
// foo/bar.zig
const foo = @import("../foo.zig");

pub fn main() void {
    foo.foo();
}

Before:

> zig build-exe foo/bar.zig
foo\bar.zig:1:21: error: unable to load 'foo\zig': FileNotFound
const foo = @import("../foo.zig");
                    ^~~~~~~~~~~~

After:

> zig build-exe foo/bar.zig
foo\bar.zig:1:21: error: import of file outside module path: '../foo.zig'
const foo = @import("../foo.zig");
                    ^~~~~~~~~~~~

squeek502 avatar Dec 24 '23 13:12 squeek502

Some notes about the usage of std.fs.path.relative:

  • The particular manifestation of the problem being solved could probably be solved by doing something simpler (without std.fs.path.relative) like verifying that there is a path separator after the shared portion of the resolved root path and the resolved path
  • The use of std.fs.path.relative handles other edge cases beyond this particular one, but they may not be particularly relevant for @import/@embedFile. In particular, here are some examples from https://github.com/ziglang/zig/pull/16886 which used std.fs.path.relative to fix something very similar:
  • If a prefix contained double path separators after any component, then the startsWith check would erroneously fail. Example: prefix: /foo//bar, file_path: /foo/bar/abc.txt would not see that abc.txt is a sub path of the prefix /foo//bar
  • On Windows, case insensitivity was not respected at all, instead the UTF-8 bytes were compared directly
  • @import/@embedFile allowing case-insensitive root matching on Windows might seem strange/undesirable in some sense, but IMO it's better to successfully find the file and then have a specific error when the import path's casing doesn't match the filesystem casing (this is https://github.com/ziglang/zig/issues/9786) rather than unexpected file outside of module root errors.

squeek502 avatar Dec 25 '23 07:12 squeek502