some fixs and optimze (#54)

* fix: use `std.fs.accessAbsolute` to replace `std.fs.openDirAbsolute`

This can solve the problem of file descriptors not being freed

* fix: verify zig version failed

When the user has installed a higher zig, zvm verify zig version will failed, because the child process will use system environment variables

* optimize: simplify the logic of `set_zig_version`

* optimize: improve performance by using `comptime`

rename origin `detect` func in `architecture.zig` to `platform_str` and make it become comptime func

* add some comments

* fix compile error

src/alias.zig

@@ -3,46 +3,43 @@ const assert = std.debug.assert;
 const builtin = @import("builtin");
 const tools = @import("tools.zig");
 
+/// try to set zig version
+/// this will use system link on unix-like
+/// for windows, this will use copy dir
 pub fn set_zig_version(version: []const u8) !void {
-    const allocator = tools.get_allocator();
-    var arena = std.heap.ArenaAllocator.init(allocator);
+    var arena = std.heap.ArenaAllocator.init(tools.get_allocator());
     defer arena.deinit();
     const arena_allocator = arena.allocator();
 
     const user_home = tools.get_home();
-    const zig_path = try std.fs.path.join(arena_allocator, &[_][]const u8{ user_home, ".zm", "versions", version });
+    const version_path = try std.fs.path.join(arena_allocator, &[_][]const u8{ user_home, ".zm", "versions", version });
     const symlink_path = try tools.get_zvm_path_segment(arena_allocator, "current");
 
-    try update_symlink(zig_path, symlink_path);
-    try verify_zig_version(allocator, version);
+    try update_current(version_path, symlink_path);
+    try verify_zig_version(version);
 }
 
-fn update_symlink(zig_path: []const u8, symlink_path: []const u8) !void {
+fn update_current(zig_path: []const u8, symlink_path: []const u8) !void {
     assert(zig_path.len > 0);
     assert(symlink_path.len > 0);
 
     if (builtin.os.tag == .windows) {
-        if (std.fs.path.dirname(symlink_path)) |dirname| {
-            var parent_dir = try std.fs.openDirAbsolute(dirname, .{ .iterate = true });
-            defer parent_dir.close();
-            try parent_dir.deleteTree(std.fs.path.basename(symlink_path));
-        } else {
-            @panic("dirname is not available!");
-        }
-        if (does_dir_exist(symlink_path)) try std.fs.deleteDirAbsolute(symlink_path);
+        if (does_dir_exist(symlink_path)) try std.fs.deleteTreeAbsolute(symlink_path);
         try copy_dir(zig_path, symlink_path);
-    } else {
-        if (does_file_exist(symlink_path)) try std.fs.cwd().deleteFile(symlink_path);
-        std.posix.symlink(zig_path, symlink_path) catch |err| switch (err) {
-            error.PathAlreadyExists => {
-                try std.fs.cwd().deleteFile(symlink_path);
-                try std.posix.symlink(zig_path, symlink_path);
-            },
-            else => return err,
-        };
+        return;
     }
+
+    // when platform is not windows, this is execute here
+
+    // when file exist(it is a systemlink), delete it
+    if (does_file_exist(symlink_path)) try std.fs.cwd().deleteFile(symlink_path);
+
+    // system link it
+    try std.posix.symlink(zig_path, symlink_path);
 }
 
+/// Nested copy dir
+/// only copy dir and file, no including link
 fn copy_dir(source_dir: []const u8, dest_dir: []const u8) !void {
     assert(source_dir.len > 0);
     assert(dest_dir.len > 0);
@@ -80,38 +77,40 @@ fn copy_dir(source_dir: []const u8, dest_dir: []const u8) !void {
     }
 }
 
+/// detect the dir whether exist
 fn does_dir_exist(path: []const u8) bool {
     const result = blk: {
-        _ = std.fs.openDirAbsolute(path, .{}) catch |err| {
-            switch (err) {
-                error.FileNotFound => break :blk false,
-                else => break :blk true,
-            }
+        std.fs.accessAbsolute(path, .{}) catch |err| {
+            if (err == error.FileNotFound)
+                break :blk false;
+            break :blk true;
         };
         break :blk true;
     };
     return result;
 }
 
+/// detect the dir whether exist
 fn does_file_exist(path: []const u8) bool {
     const result = blk: {
-        _ = std.fs.cwd().openFile(path, .{}) catch |err| {
-            switch (err) {
-                error.FileNotFound => break :blk false,
-                else => break :blk true,
-            }
+        std.fs.accessAbsolute(path, .{}) catch |err| {
+            if (err == error.FileNotFound)
+                break :blk false;
+            break :blk true;
         };
         break :blk true;
     };
     return result;
 }
 
-fn verify_zig_version(allocator: std.mem.Allocator, expected_version: []const u8) !void {
+/// verify current zig version
+fn verify_zig_version(expected_version: []const u8) !void {
+    const allocator = tools.get_allocator();
+
     const actual_version = try retrieve_zig_version(allocator);
     defer allocator.free(actual_version);
 
     assert(actual_version.len > 0);
-    assert(std.mem.eql(u8, expected_version, actual_version));
 
     if (!std.mem.eql(u8, expected_version, actual_version)) {
         std.debug.print("Expected Zig version {s}, but currently using {s}. Please check.\n", .{ expected_version, actual_version });
@@ -120,11 +119,15 @@ fn verify_zig_version(allocator: std.mem.Allocator, expected_version: []const u8
     }
 }
 
+/// try to get zig version
 fn retrieve_zig_version(allocator: std.mem.Allocator) ![]u8 {
-    const symlink_path = try tools.get_zvm_path_segment(allocator, "current");
-    defer allocator.free(symlink_path);
+    const home_dir = tools.get_home();
+    const current_zig_path = try std.fs.path.join(allocator, &.{ home_dir, ".zm", "current", tools.zig_name });
+    defer allocator.free(current_zig_path);
 
-    var child_process = std.process.Child.init(&[_][]const u8{ "zig", "version" }, allocator);
+    // here we must use the absolute path, we can not just use "zig"
+    // because child process will use environment variable
+    var child_process = std.process.Child.init(&[_][]const u8{ current_zig_path, "version" }, allocator);
 
     child_process.stdin_behavior = .Close;
     child_process.stdout_behavior = .Pipe;

src/architecture.zig

@@ -27,50 +27,36 @@ fn archToString(arch: std.Target.Cpu.Arch) ?[]const u8 {
     };
 }
 
-pub fn detect(allocator: std.mem.Allocator, params: DetectParams) !?[]u8 {
-    const os_str = osToString(params.os) orelse return error.UnsupportedSystem;
-    const arch_str = archToString(params.arch) orelse return error.UnsupportedSystem;
-
-    const len = os_str.len + arch_str.len + 1; // +1 for the '-'
-    const result = try allocator.alloc(u8, len);
-
-    if (params.reverse) {
-        @memcpy(result[0..arch_str.len], arch_str);
-        result[arch_str.len] = '-';
-        @memcpy(result[arch_str.len + 1 ..], os_str);
-    } else {
-        @memcpy(result[0..os_str.len], os_str);
-        result[os_str.len] = '-';
-        @memcpy(result[os_str.len + 1 ..], arch_str);
-    }
-
-    return result;
+/// get platform str
+///
+/// in offical zig json
+/// use "arch-platform" as key
+/// use "platform-arch" as file name
+///
+/// for performance, we treat this function as comptime-func
+pub fn platform_str(comptime params: DetectParams) !?[]const u8 {
+    const os_str = (comptime osToString(params.os)) orelse
+        return error.UnsupportedSystem;
+
+    const arch_str = (comptime archToString(params.arch)) orelse
+        return error.UnsupportedSystem;
+
+    if (params.reverse)
+        return arch_str ++ "-" ++ os_str;
+
+    return os_str ++ "-" ++ arch_str;
 }
 
 test "detect() Test" {
-    const allocator = std.testing.allocator;
-
-    {
-        const result = try detect(allocator, DetectParams{ .os = std.Target.Os.Tag.linux, .arch = std.Target.Cpu.Arch.x86_64 }) orelse unreachable;
-        defer allocator.free(result);
-        try std.testing.expectEqualStrings("linux-x86_64", result);
-    }
+    const result_1 = try platform_str(DetectParams{ .os = std.Target.Os.Tag.linux, .arch = std.Target.Cpu.Arch.x86_64 }) orelse unreachable;
+    try std.testing.expectEqualStrings("linux-x86_64", result_1);
 
-    {
-        const result = try detect(allocator, DetectParams{ .os = std.Target.Os.Tag.linux, .arch = std.Target.Cpu.Arch.aarch64, .reverse = true }) orelse unreachable;
-        defer allocator.free(result);
-        try std.testing.expectEqualStrings("aarch64-linux", result);
-    }
+    const result_2 = try platform_str(DetectParams{ .os = std.Target.Os.Tag.linux, .arch = std.Target.Cpu.Arch.aarch64, .reverse = true }) orelse unreachable;
+    try std.testing.expectEqualStrings("aarch64-linux", result_2);
 
-    {
-        const result = try detect(allocator, DetectParams{ .os = std.Target.Os.Tag.macos, .arch = std.Target.Cpu.Arch.x86_64 }) orelse unreachable;
-        defer allocator.free(result);
-        try std.testing.expectEqualStrings("macos-x86_64", result);
-    }
+    const result_3 = try platform_str(DetectParams{ .os = std.Target.Os.Tag.macos, .arch = std.Target.Cpu.Arch.x86_64 }) orelse unreachable;
+    try std.testing.expectEqualStrings("macos-x86_64", result_3);
 
-    {
-        const result = try detect(allocator, DetectParams{ .os = std.Target.Os.Tag.windows, .arch = std.Target.Cpu.Arch.x86_64 }) orelse unreachable;
-        defer allocator.free(result);
-        try std.testing.expectEqualStrings("windows-x86_64", result);
-    }
+    const result_4 = try platform_str(DetectParams{ .os = std.Target.Os.Tag.windows, .arch = std.Target.Cpu.Arch.x86_64 }) orelse unreachable;
+    try std.testing.expectEqualStrings("windows-x86_64", result_4);
 }

src/download.zig

@@ -107,16 +107,19 @@ fn download_and_extract(
     var zvm_dir = try open_or_create_zvm_dir();
     defer zvm_dir.close();
 
-    const platform = try architecture.detect(allocator, architecture.DetectParams{ .os = builtin.os.tag, .arch = builtin.cpu.arch, .reverse = false }) orelse unreachable;
-    defer allocator.free(platform);
+    const platform_str = try architecture.platform_str(architecture.DetectParams{
+        .os = builtin.os.tag,
+        .arch = builtin.cpu.arch,
+        .reverse = false,
+    }) orelse unreachable;
 
-    const file_name = try std.mem.concat(allocator, u8, &[_][]const u8{ "zig-", platform, "-", version, ".", archive_ext });
+    const file_name = try std.mem.concat(allocator, u8, &[_][]const u8{ "zig-", platform_str, "-", version, ".", archive_ext });
     defer allocator.free(file_name);
 
     const total_size: usize = @intCast(req.response.content_length orelse 0);
     var downloaded_bytes: usize = 0;
 
-    const download_message = try std.fmt.allocPrint(allocator, "Downloading Zig version {s} for platform {s}...", .{ version, platform });
+    const download_message = try std.fmt.allocPrint(allocator, "Downloading Zig version {s} for platform {s}...", .{ version, platform_str });
     defer allocator.free(download_message);
     var download_node = root_node.start(download_message, total_size);
 

src/extract.zig

@@ -2,24 +2,37 @@ const std = @import("std");
 const builtin = @import("builtin");
 const tools = @import("tools.zig");
 
+const xz = std.compress.xz;
+const tar = std.tar;
+
+/// extract tar.xz to dir
 pub fn extract_tarxz_to_dir(allocator: std.mem.Allocator, out_dir: std.fs.Dir, file: std.fs.File) !void {
     var buffered_reader = std.io.bufferedReader(file.reader());
-    var decompressed = try std.compress.xz.decompress(allocator, buffered_reader.reader());
+
+    var decompressed = try xz.decompress(allocator, buffered_reader.reader());
     defer decompressed.deinit();
-    try std.tar.pipeToFileSystem(out_dir, decompressed.reader(), .{ .mode_mode = .executable_bit_only, .strip_components = 1 });
+
+    try tar.pipeToFileSystem(
+        out_dir,
+        decompressed.reader(),
+        .{ .mode_mode = .executable_bit_only, .strip_components = 1 },
+    );
 }
 
+/// extract zip to directory
 pub fn extract_zip_dir(out_dir: std.fs.Dir, file: std.fs.File) !void {
     var arena = std.heap.ArenaAllocator.init(tools.get_allocator());
     defer arena.deinit();
 
     const allocator = arena.allocator();
+    // for decompressing zig, we need to make a temp directory
     const tmp_path = try tools.get_zvm_path_segment(allocator, "tmpdir");
     defer std.fs.deleteDirAbsolute(tmp_path) catch unreachable;
 
     try std.fs.makeDirAbsolute(tmp_path);
     var tmp_dir = try std.fs.openDirAbsolute(tmp_path, .{ .iterate = true });
 
+    // extract zig
     try std.zip.extract(tmp_dir, file.seekableStream(), .{});
 
     var iterate = tmp_dir.iterate();

src/install.zig

@@ -97,17 +97,17 @@ fn fetch_version_data(allocator: Allocator, requested_version: []const u8, sub_k
 }
 
 pub fn from_version(version: []const u8) !void {
-    var allocator = tools.get_allocator();
+    const allocator = tools.get_allocator();
 
-    const platform_str = try architecture.detect(allocator, architecture.DetectParams{
+    const platform_str = try architecture.platform_str(architecture.DetectParams{
         .os = builtin.os.tag,
         .arch = builtin.cpu.arch,
         .reverse = true,
     }) orelse unreachable;
-    defer allocator.free(platform_str);
 
     var arena = std.heap.ArenaAllocator.init(allocator);
     defer arena.deinit();
+
     const version_data = try fetch_version_data(arena.allocator(), version, platform_str);
     if (version_data) |data| {
         std.debug.print("Install {s}\n", .{data.name});

src/tools.zig

@@ -6,6 +6,13 @@ var home_dir: []const u8 = undefined;
 
 pub const log = std.log.scoped(.zvm);
 
+pub const zig_name = switch (builtin.os.tag) {
+    .windows => "zig.exe",
+    .linux => "zig",
+    .macos => "zig",
+    else => @compileError("not support current platform"),
+};
+
 /// Initialize the data.
 pub fn data_init(tmp_allocator: std.mem.Allocator) !void {
     allocator = tmp_allocator;