zig icon indicating copy to clipboard operation
zig copied to clipboard

Dots in file names are incorrectly interpreted as (back)slashes when `@import`ed on Windows

Open leap0x7b opened this issue 2 years ago • 7 comments

Zig Version

0.12.0-dev.1819+5c1428ea9

Steps to Reproduce and Observed Behavior

  1. Clone https://github.com/kora-org/ydin
  2. Build it on Windows using the native Zig build for Windows (not WSL)
  3. You should encounter something like this:
src\arch\x86_64\main.zig:3:22: error: unable to load 'src\arch\x86_64\zig': FileNotFound
const arch = @import("../x86_64.zig");
                     ^~~~~~~~~~~~~~~

Expected Behavior

Should build just fine. AFAIK it only occurs on Windows and it builds just fine on Linux/WSL.

leap0x7b avatar Dec 23 '23 12:12 leap0x7b

Minimal reproduction:

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

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

Note, though, that I believe the expected result will still be a compile error due to the import being outside the main module path.

squeek502 avatar Dec 23 '23 23:12 squeek502

Somehow has something to do with the file being named the same as the subdirectory. Changing the above example to:

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

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

Gives the expected error:

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

squeek502 avatar Dec 24 '23 00:12 squeek502

With --debug-log module:

> zig build-exe foo/bar.zig --debug-log module
...
debug(module): new importFile. resolved_root_path=foo, resolved_path=foo.zig, sub_file_path=zig, import_string=../foo.zig
...

squeek502 avatar Dec 24 '23 00:12 squeek502

The culprit is this code here:

https://github.com/ziglang/zig/blob/bb0f7d55e8c50e379fa9bdcb8758d89d08e0cc1f/src/Module.zig#L3978-L3990

The startsWith is the problem. Not sure exactly what the fix should be yet, but looks like a similar problem to what https://github.com/ziglang/zig/pull/16886 fixed.

squeek502 avatar Dec 24 '23 01:12 squeek502

There is at least a related problem on Linux:

composite_register.zig:2:19: error: unable to load '/zig_library/library.zig': FileNotFound
const l = @import("../zig_library/library.zig");

Note the dots missing in the error message, as opposed to the line from the source code just below. The relative pathname is correct and the file is there.

I am using the nightly build from yesterday, so perhaps yesterday's commit will fix this. I will test again tmw.

donaldcallen avatar Dec 25 '23 19:12 donaldcallen

@donaldcallen the PR with the fix hasn't been merged yet so it won't be in any nightly builds. Could you provide a more complete example to reproduce the /zig_library/library.zig error? It likely will be fixed by https://github.com/ziglang/zig/pull/18363 but I would like to make sure.

@leap0x7b This bug is not Windows specific for me, and a similar error occurs on Linux when building ydin:

src/arch/x86_64/main.zig:3:22: error: unable to load 'src/arch/x86_64/zig': FileNotFound
const arch = @import("../x86_64.zig");
                     ^~~~~~~~~~~~~~~

Are you sure it was building successfully for you on Linux/WSL?

squeek502 avatar Dec 26 '23 10:12 squeek502

Okay I might have been on an earlier build when I last compiled it on Linux so I didn't know it's also occurs on Linux

leap0x7b avatar Dec 26 '23 15:12 leap0x7b

I've attached a tar file containing two directories -- issue_error and issue_workaround cd to issue_error/main and try

zig build-exe main.zig

You will get the error.

Do the same in issue_workaround and the build will work. Here I added a symlink to ../zig_library/lib.zig in the main directory and changed the import to reference the symlink.

issue.tgz

donaldcallen avatar Dec 27 '23 15:12 donaldcallen

Perfect, thanks @donaldcallen. It is indeed the same problem, but a pretty silly manifestation of it:

Here's the relevant code again:

https://github.com/ziglang/zig/blob/bb0f7d55e8c50e379fa9bdcb8758d89d08e0cc1f/src/Module.zig#L3978-L3990

The resolved_root_path is . and the resolved_path is ../zig_library/lib.zig. The mem.startsWith returns true since ../zig_library/lib.zig starts with ., and then it assumes the part after the match is a path separator (without verifying that) and therefore returns the subpath as the part after the common root, which in this case is /zig_library/lib.zig.

With the fix in https://github.com/ziglang/zig/pull/18363, it will handle things correctly:

$ zig4 build-exe main.zig
main.zig:2:19: error: import of file outside module path: '../zig_library/lib.zig'
const l = @import("../zig_library/lib.zig");
                  ^~~~~~~~~~~~~~~~~~~~~~~~

By the way, the intended way to handle this sort of thing would be to use a module to import from zig_library (either via build.zig or --mod).

- const l = @import("../zig_library/lib.zig");
+ const l = @import("zig_library");
// issue_error/build.zig
const std = @import("std");

pub fn build(b: *std.Build) void {
    const target = b.standardTargetOptions(.{});
    const optimize = b.standardOptimizeOption(.{});

    const zig_library_mod = b.createModule(.{ .source_file = .{ .path = "zig_library/lib.zig" } });

    const exe = b.addExecutable(.{
        .name = "issue_error",
        .root_source_file = .{ .path = "main/main.zig" },
        .target = target,
        .optimize = optimize,
    });
    exe.addModule("zig_library", zig_library_mod);

    b.installArtifact(exe);
}
zig build

or without build.zig:

cd issue_error/main
zig build-exe main.zig --mod zig_library::../zig_library/lib.zig --deps zig_library

Or, if you want a quick workaround, then you can set --main-mod-path to a parent directory that includes both main and zig_library.

cd issue_error/main
zig build-exe main.zig --main-mod-path ..

squeek502 avatar Dec 28 '23 01:12 squeek502