CLI: more careful resolution of paths

In general, we prefer compiler code to use relative paths based on open
directory handles because this is the most portable. However, sometimes
absolute paths are used, and sometimes relative paths are used that go
up a directory.

The recent improvements in 81d2135ca6ebd71b8c121a19957c8fbf7f87125b
regressed the use case when an absolute path is used for the zig lib
directory mixed with a relative path used for the root source file. This
could happen when, for example, running the standard library tests, like
this:

stage3/bin/zig test ../lib/std/std.zig

This happened because the zig lib dir was inferred to be an absolute
directory based on the zig executable directory, while the root source
file was detected as a relative path. There was no common prefix and so
it was not determined that the std.zig file was inside the lib
directory.

This commit adds a function for resolving paths that preserves relative
path names while allowing absolute paths, and converting relative
upwards paths (e.g. "../foo") to absolute paths. This restores the
previous functionality while remaining compatible with systems such as
WASI that cannot deal with absolute paths.

4 files changed, 104 insertions, 32 deletions

diff --git a/src/Module.zig b/src/Module.zig
index b7930615c8..8c45fc233f 100644
--- a/src/Module.zig
+++ b/src/Module.zig
@@ -30,6 +30,7 @@ const Sema = @import("Sema.zig");
 const target_util = @import("target.zig");
 const build_options = @import("build_options");
 const Liveness = @import("Liveness.zig");
+const isUpDir = @import("introspect.zig").isUpDir;
 
 /// General-purpose allocator. Used for both temporary and long-term storage.
 gpa: Allocator,
@@ -4957,15 +4958,19 @@ pub fn importFile(
     const resolved_root_path = try std.fs.path.resolve(gpa, &[_][]const u8{cur_pkg_dir_path});
     defer gpa.free(resolved_root_path);
 
-    if (!mem.startsWith(u8, resolved_path, resolved_root_path) or
-        // This prevents this check from triggering when the name of the
-        // imported file starts with the root path's directory name.
-        !std.fs.path.isSep(resolved_path[resolved_root_path.len]))
-    {
+    const sub_file_path = p: {
+        if (mem.startsWith(u8, resolved_path, resolved_root_path)) {
+            // +1 for the directory separator here.
+            break :p try gpa.dupe(u8, resolved_path[resolved_root_path.len + 1 ..]);
+        }
+        if (mem.eql(u8, resolved_root_path, ".") and
+            !isUpDir(resolved_path) and
+            !std.fs.path.isAbsolute(resolved_path))
+        {
+            break :p try gpa.dupe(u8, resolved_path);
+        }
         return error.ImportOutsidePkgPath;
-    }
-    // +1 for the directory separator here.
-    const sub_file_path = try gpa.dupe(u8, resolved_path[resolved_root_path.len + 1 ..]);
+    };
     errdefer gpa.free(sub_file_path);
 
     log.debug("new importFile. resolved_root_path={s}, resolved_path={s}, sub_file_path={s}, import_string={s}", .{
@@ -5015,11 +5020,19 @@ pub fn embedFile(mod: *Module, cur_file: *File, rel_file_path: []const u8) !*Emb
     const resolved_root_path = try std.fs.path.resolve(gpa, &[_][]const u8{cur_pkg_dir_path});
     defer gpa.free(resolved_root_path);
 
-    if (!mem.startsWith(u8, resolved_path, resolved_root_path)) {
+    const sub_file_path = p: {
+        if (mem.startsWith(u8, resolved_path, resolved_root_path)) {
+            // +1 for the directory separator here.
+            break :p try gpa.dupe(u8, resolved_path[resolved_root_path.len + 1 ..]);
+        }
+        if (mem.eql(u8, resolved_root_path, ".") and
+            !isUpDir(resolved_path) and
+            !std.fs.path.isAbsolute(resolved_path))
+        {
+            break :p try gpa.dupe(u8, resolved_path);
+        }
         return error.ImportOutsidePkgPath;
-    }
-    // +1 for the directory separator here.
-    const sub_file_path = try gpa.dupe(u8, resolved_path[resolved_root_path.len + 1 ..]);
+    };
     errdefer gpa.free(sub_file_path);
 
     var file = try cur_file.pkg.root_src_directory.handle.openFile(sub_file_path, .{});
diff --git a/src/introspect.zig b/src/introspect.zig
index 74f0d45c80..27925ab667 100644
--- a/src/introspect.zig
+++ b/src/introspect.zig
@@ -82,7 +82,12 @@ pub fn findZigLibDir(gpa: mem.Allocator) !Compilation.Directory {
 pub fn findZigLibDirFromSelfExe(
     allocator: mem.Allocator,
     self_exe_path: []const u8,
-) error{ OutOfMemory, FileNotFound }!Compilation.Directory {
+) error{
+    OutOfMemory,
+    FileNotFound,
+    CurrentWorkingDirectoryUnlinked,
+    Unexpected,
+}!Compilation.Directory {
     const cwd = fs.cwd();
     var cur_path: []const u8 = self_exe_path;
     while (fs.path.dirname(cur_path)) |dirname| : (cur_path = dirname) {
@@ -90,9 +95,11 @@ pub fn findZigLibDirFromSelfExe(
         defer base_dir.close();
 
         const sub_directory = testZigInstallPrefix(base_dir) orelse continue;
+        const p = try fs.path.join(allocator, &[_][]const u8{ dirname, sub_directory.path.? });
+        defer allocator.free(p);
         return Compilation.Directory{
             .handle = sub_directory.handle,
-            .path = try fs.path.join(allocator, &[_][]const u8{ dirname, sub_directory.path.? }),
+            .path = try resolvePath(allocator, p),
         };
     }
     return error.FileNotFound;
@@ -130,3 +137,46 @@ pub fn resolveGlobalCacheDir(allocator: mem.Allocator) ![]u8 {
         return fs.getAppDataDir(allocator, appname);
     }
 }
+
+/// Similar to std.fs.path.resolve, with a few important differences:
+/// * If the input is an absolute path, check it against the cwd and try to
+///   convert it to a relative path.
+/// * If the resulting path would start with a relative up-dir ("../"), instead
+///   return an absolute path based on the cwd.
+/// * When targeting WASI, fail with an error message if an absolute path is
+///   used.
+pub fn resolvePath(
+    ally: mem.Allocator,
+    p: []const u8,
+) error{
+    OutOfMemory,
+    CurrentWorkingDirectoryUnlinked,
+    Unexpected,
+}![]u8 {
+    if (fs.path.isAbsolute(p)) {
+        const cwd_path = try std.process.getCwdAlloc(ally);
+        defer ally.free(cwd_path);
+        const relative = try fs.path.relative(ally, cwd_path, p);
+        if (isUpDir(relative)) {
+            ally.free(relative);
+            return ally.dupe(u8, p);
+        } else {
+            return relative;
+        }
+    } else {
+        const resolved = try fs.path.resolve(ally, &.{p});
+        if (isUpDir(resolved)) {
+            ally.free(resolved);
+            const cwd_path = try std.process.getCwdAlloc(ally);
+            defer ally.free(cwd_path);
+            return fs.path.resolve(ally, &.{ cwd_path, p });
+        } else {
+            return resolved;
+        }
+    }
+}
+
+/// TODO move this to std.fs.path
+pub fn isUpDir(p: []const u8) bool {
+    return mem.startsWith(u8, p, "..") and (p.len == 2 or p[2] == fs.path.sep);
+}
diff --git a/src/main.zig b/src/main.zig
index f5855da6b9..c372462365 100644
--- a/src/main.zig
+++ b/src/main.zig
@@ -885,24 +885,28 @@ fn buildOutputType(
                             fatal("unexpected end-of-parameter mark: --", .{});
                         }
                     } else if (mem.eql(u8, arg, "--pkg-begin")) {
-                        const pkg_name = args_iter.next();
-                        const pkg_path = args_iter.next();
-                        if (pkg_name == null or pkg_path == null) fatal("Expected 2 arguments after {s}", .{arg});
+                        const opt_pkg_name = args_iter.next();
+                        const opt_pkg_path = args_iter.next();
+                        if (opt_pkg_name == null or opt_pkg_path == null)
+                            fatal("Expected 2 arguments after {s}", .{arg});
+
+                        const pkg_name = opt_pkg_name.?;
+                        const pkg_path = try introspect.resolvePath(arena, opt_pkg_path.?);
 
                         const new_cur_pkg = Package.create(
                             gpa,
-                            fs.path.dirname(pkg_path.?),
-                            fs.path.basename(pkg_path.?),
+                            fs.path.dirname(pkg_path),
+                            fs.path.basename(pkg_path),
                         ) catch |err| {
-                            fatal("Failed to add package at path {s}: {s}", .{ pkg_path.?, @errorName(err) });
+                            fatal("Failed to add package at path {s}: {s}", .{ pkg_path, @errorName(err) });
                         };
 
-                        if (mem.eql(u8, pkg_name.?, "std") or mem.eql(u8, pkg_name.?, "root") or mem.eql(u8, pkg_name.?, "builtin")) {
-                            fatal("unable to add package '{s}' -> '{s}': conflicts with builtin package", .{ pkg_name.?, pkg_path.? });
-                        } else if (cur_pkg.table.get(pkg_name.?)) |prev| {
-                            fatal("unable to add package '{s}' -> '{s}': already exists as '{s}", .{ pkg_name.?, pkg_path.?, prev.root_src_path });
+                        if (mem.eql(u8, pkg_name, "std") or mem.eql(u8, pkg_name, "root") or mem.eql(u8, pkg_name, "builtin")) {
+                            fatal("unable to add package '{s}' -> '{s}': conflicts with builtin package", .{ pkg_name, pkg_path });
+                        } else if (cur_pkg.table.get(pkg_name)) |prev| {
+                            fatal("unable to add package '{s}' -> '{s}': already exists as '{s}", .{ pkg_name, pkg_path, prev.root_src_path });
                         }
-                        try cur_pkg.addAndAdopt(gpa, pkg_name.?, new_cur_pkg);
+                        try cur_pkg.addAndAdopt(gpa, pkg_name, new_cur_pkg);
                         cur_pkg = new_cur_pkg;
                     } else if (mem.eql(u8, arg, "--pkg-end")) {
                         cur_pkg = cur_pkg.parent orelse
@@ -2705,11 +2709,16 @@ fn buildOutputType(
     };
     defer emit_implib_resolved.deinit();
 
-    const main_pkg: ?*Package = if (root_src_file) |src_path| blk: {
-        if (main_pkg_path) |p| {
-            const rel_src_path = try fs.path.relative(gpa, p, src_path);
-            defer gpa.free(rel_src_path);
-            break :blk try Package.create(gpa, p, rel_src_path);
+    const main_pkg: ?*Package = if (root_src_file) |unresolved_src_path| blk: {
+        const src_path = try introspect.resolvePath(arena, unresolved_src_path);
+        if (main_pkg_path) |unresolved_main_pkg_path| {
+            const p = try introspect.resolvePath(arena, unresolved_main_pkg_path);
+            if (p.len == 0) {
+                break :blk try Package.create(gpa, null, src_path);
+            } else {
+                const rel_src_path = try fs.path.relative(arena, p, src_path);
+                break :blk try Package.create(gpa, p, rel_src_path);
+            }
         } else {
             const root_src_dir_path = fs.path.dirname(src_path);
             break :blk Package.create(gpa, root_src_dir_path, fs.path.basename(src_path)) catch |err| {
@@ -2745,7 +2754,7 @@ fn buildOutputType(
 
     const self_exe_path = try introspect.findZigExePath(arena);
     var zig_lib_directory: Compilation.Directory = if (override_lib_dir) |unresolved_lib_dir| l: {
-        const lib_dir = try fs.path.resolve(arena, &.{unresolved_lib_dir});
+        const lib_dir = try introspect.resolvePath(arena, unresolved_lib_dir);
         break :l .{
             .path = lib_dir,
             .handle = fs.cwd().openDir(lib_dir, .{}) catch |err| {
diff --git a/src/test.zig b/src/test.zig
index 0e96be00e8..940db1b4bb 100644
--- a/src/test.zig
+++ b/src/test.zig
@@ -60,7 +60,7 @@ test {
         ctx.addTestCasesFromDir(dir);
     }
 
-    try @import("test_cases").addCases(&ctx);
+    try @import("../test/cases.zig").addCases(&ctx);
 
     try ctx.run();
 }