support new module cli arguments (#1019)

* support new module cli arguments

* capture the runtime zig version and store it on `Server`

* update build_runner action

* Use correct version for selecting arguments

.github/workflows/build_runner.yml

@@ -3,9 +3,11 @@ name: BuildRunner
 on:
   push:
     paths:
+      - ".github/workflows/build_runner.yml"
       - "src/special/build_runner.zig"
   pull_request:
     paths:
+      - ".github/workflows/build_runner.yml"
       - "src/special/build_runner.zig"
   schedule:
     - cron: '0 0 * * *'
@@ -37,5 +39,10 @@ jobs:
           cd $RUNNER_TEMP/TEMP_ZIG_PROJECT
           zig init-exe
 
-      - name: Check build_runner builds
+      - name: Check build_runner builds on master
+        if: ${{ matrix.zig_version == 'master' }}
+        run: zig build-exe $GITHUB_WORKSPACE/src/special/build_runner.zig --mod @build@::$RUNNER_TEMP/TEMP_ZIG_PROJECT/build.zig --deps @build@
+
+      - name: Check build_runner builds on older tagged releases
+        if: ${{ matrix.zig_version != 'master' }}
         run: zig build-exe $GITHUB_WORKSPACE/src/special/build_runner.zig --pkg-begin @build@ $RUNNER_TEMP/TEMP_ZIG_PROJECT/build.zig --pkg-end

src/DocumentStore.zig

@@ -10,6 +10,7 @@ const BuildAssociatedConfig = @import("BuildAssociatedConfig.zig");
 const BuildConfig = @import("special/build_runner.zig").BuildConfig;
 const tracy = @import("tracy.zig");
 const Config = @import("Config.zig");
+const ZigVersionWrapper = @import("ZigVersionWrapper.zig");
 const translate_c = @import("translate_c.zig");
 const ComptimeInterpreter = @import("ComptimeInterpreter.zig");
 
@@ -91,6 +92,7 @@ pub const Handle = struct {
 
 allocator: std.mem.Allocator,
 config: *const Config,
+runtime_zig_version: *const ?ZigVersionWrapper,
 handles: std.StringArrayHashMapUnmanaged(*Handle) = .{},
 build_files: std.StringArrayHashMapUnmanaged(BuildFile) = .{},
 cimports: std.AutoArrayHashMapUnmanaged(Hash, translate_c.Result) = .{},
@@ -244,7 +246,12 @@ pub fn applySave(self: *DocumentStore, handle: *const Handle) !void {
     if (std.process.can_spawn and isBuildFile(handle.uri)) {
         const build_file = self.build_files.getPtr(handle.uri).?;
 
-        const build_config = loadBuildConfiguration(self.allocator, build_file.*, self.config.*) catch |err| {
+        const build_config = loadBuildConfiguration(
+            self.allocator,
+            build_file.*,
+            self.config.*,
+            self.runtime_zig_version.*.?, // if we have the path to zig we should have the zig version
+        ) catch |err| {
             log.err("Failed to load build configuration for {s} (error: {})", .{ build_file.uri, err });
             return;
         };
@@ -406,6 +413,7 @@ fn loadBuildConfiguration(
     allocator: std.mem.Allocator,
     build_file: BuildFile,
     config: Config,
+    runtime_zig_version: ZigVersionWrapper,
 ) !BuildConfig {
     const tracy_zone = tracy.trace(@src());
     defer tracy_zone.end();
@@ -420,7 +428,30 @@ fn loadBuildConfiguration(
     // TODO extract this option from `BuildAssociatedConfig.BuildOption`
     const zig_cache_root: []const u8 = try std.fs.path.join(arena_allocator, &.{ directory_path, "zig-cache" });
 
-    const standard_args = [_][]const u8{
+    // introduction of modified module cli arguments https://github.com/ziglang/zig/pull/14664
+    const module_version = comptime std.SemanticVersion.parse("0.11.0-dev.1718+2737dce84") catch unreachable;
+    const use_new_module_cli = runtime_zig_version.version.order(module_version) != .lt;
+
+    const standard_args = if (use_new_module_cli) blk: {
+        const build_module = try std.fmt.allocPrint(arena_allocator, "@build@::{s}", .{build_file_path});
+
+        break :blk [_][]const u8{
+            config.zig_exe_path.?,
+            "run",
+            config.build_runner_path.?,
+            "--cache-dir",
+            config.global_cache_path.?,
+            "--mod",
+            build_module,
+            "--deps",
+            "@build@",
+            "--",
+            config.zig_exe_path.?,
+            directory_path,
+            zig_cache_root,
+            config.build_runner_global_cache_path.?,
+        };
+    } else [_][]const u8{
         config.zig_exe_path.?,
         "run",
         config.build_runner_path.?,
@@ -561,7 +592,12 @@ fn createBuildFile(self: *const DocumentStore, uri: Uri) error{OutOfMemory}!Buil
 
     // TODO: Do this in a separate thread?
     // It can take quite long.
-    if (loadBuildConfiguration(self.allocator, build_file, self.config.*)) |build_config| {
+    if (loadBuildConfiguration(
+        self.allocator,
+        build_file,
+        self.config.*,
+        self.runtime_zig_version.*.?, // if we have the path to zig we should have the zig version
+    )) |build_config| {
         build_file.config = build_config;
     } else |err| {
         log.err("Failed to load build configuration for {s} (error: {})", .{ build_file.uri, err });

src/Server.zig

@@ -22,6 +22,7 @@ const uri_utils = @import("uri.zig");
 const diff = @import("diff.zig");
 const ComptimeInterpreter = @import("ComptimeInterpreter.zig");
 const analyser = @import("analyser/analyser.zig");
+const ZigVersionWrapper = @import("ZigVersionWrapper.zig");
 
 const data = @import("data/data.zig");
 const snipped_data = @import("data/snippets.zig");
@@ -38,6 +39,7 @@ arena: *std.heap.ArenaAllocator = undefined,
 document_store: DocumentStore = undefined,
 builtin_completions: ?std.ArrayListUnmanaged(types.CompletionItem),
 client_capabilities: ClientCapabilities = .{},
+runtime_zig_version: ?ZigVersionWrapper,
 outgoing_messages: std.ArrayListUnmanaged([]const u8) = .{},
 recording_enabled: bool,
 replay_enabled: bool,
@@ -1847,14 +1849,8 @@ fn initializeHandler(server: *Server, request: types.InitializeParams) Error!typ
 
     server.status = .initializing;
 
-    if (server.config.zig_exe_path) |exe_path| blk: {
-        if (!std.process.can_spawn) break :blk;
-        // TODO avoid having to call getZigEnv twice
-        // once in init and here
-        const env = configuration.getZigEnv(server.allocator, exe_path) orelse break :blk;
-        defer std.json.parseFree(configuration.Env, env, .{ .allocator = server.allocator });
-
-        const zig_version = std.SemanticVersion.parse(env.version) catch break :blk;
+    if (server.runtime_zig_version) |zig_version_wrapper| {
+        const zig_version = zig_version_wrapper.version;
         const zls_version = comptime std.SemanticVersion.parse(build_options.version) catch unreachable;
 
         const zig_version_simple = std.SemanticVersion{
@@ -1881,10 +1877,6 @@ fn initializeHandler(server: *Server, request: types.InitializeParams) Error!typ
                 , .{ zig_version, zls_version });
             },
         }
-    } else {
-        server.showMessage(.Warning,
-            \\ZLS failed to find Zig. Please add Zig to your PATH or set the zig_exe_path config option in your zls.json.
-        , .{});
     }
 
     if (server.recording_enabled) {
@@ -2115,7 +2107,7 @@ fn handleConfiguration(server: *Server, json: std.json.Value) error{OutOfMemory}
     }
     log.debug("{}", .{server.client_capabilities});
 
-    configuration.configChanged(server.config, server.allocator, null) catch |err| {
+    configuration.configChanged(server.config, &server.runtime_zig_version, server.allocator, null) catch |err| {
         log.err("failed to update configuration: {}", .{err});
     };
 }
@@ -2458,7 +2450,7 @@ fn didChangeConfigurationHandler(server: *Server, request: configuration.DidChan
             }
         }
 
-        configuration.configChanged(server.config, server.allocator, null) catch |err| {
+        configuration.configChanged(server.config, &server.runtime_zig_version, server.allocator, null) catch |err| {
             log.err("failed to update config: {}", .{err});
         };
     } else if (server.client_capabilities.supports_configuration) {
@@ -3065,38 +3057,40 @@ fn processMessage(server: *Server, message: Message) Error!void {
     }
 }
 
-pub fn init(
+pub fn create(
     allocator: std.mem.Allocator,
     config: *Config,
     config_path: ?[]const u8,
     recording_enabled: bool,
     replay_enabled: bool,
-) !Server {
+) !*Server {
     // TODO replace global with something like an Analyser struct
     // which contains using_trail & resolve_trail and place it inside Server
     // see: https://github.com/zigtools/zls/issues/536
     analysis.init(allocator);
 
-    try configuration.configChanged(config, allocator, config_path);
-
-    var document_store = DocumentStore{
-        .allocator = allocator,
-        .config = config,
-    };
-    errdefer document_store.deinit();
-
-    return Server{
+    const server = try allocator.create(Server);
+    server.* = Server{
         .config = config,
+        .runtime_zig_version = null,
         .allocator = allocator,
-        .document_store = document_store,
+        .document_store = .{
+            .allocator = allocator,
+            .config = config,
+            .runtime_zig_version = &server.runtime_zig_version,
+        },
         .builtin_completions = null,
         .recording_enabled = recording_enabled,
         .replay_enabled = replay_enabled,
         .status = .uninitialized,
     };
+
+    try configuration.configChanged(config, &server.runtime_zig_version, allocator, config_path);
+
+    return server;
 }
 
-pub fn deinit(server: *Server) void {
+pub fn destroy(server: *Server) void {
     server.document_store.deinit();
     analysis.deinit();
 
@@ -3106,4 +3100,10 @@ pub fn deinit(server: *Server) void {
         server.allocator.free(message);
     }
     server.outgoing_messages.deinit(server.allocator);
+
+    if (server.runtime_zig_version) |zig_version| {
+        zig_version.free();
+    }
+
+    server.allocator.destroy(server);
 }

src/ZigVersionWrapper.zig

@@ -0,0 +1,13 @@
+const std = @import("std");
+const Self = @This();
+
+// This is necessary as `std.SemanticVersion` keeps pointers into the parsed string
+
+version: std.SemanticVersion,
+
+allocator: std.mem.Allocator,
+raw_string: []const u8,
+
+pub fn free(self: Self) void {
+    self.allocator.free(self.raw_string);
+}

src/configuration.zig

@@ -1,6 +1,7 @@
 const std = @import("std");
 const builtin = @import("builtin");
 
+const ZigVersionWrapper = @import("ZigVersionWrapper.zig");
 const tracy = @import("tracy.zig");
 const known_folders = @import("known-folders");
 
@@ -60,7 +61,7 @@ pub fn loadFromFolder(allocator: std.mem.Allocator, folder_path: []const u8) ?Co
 }
 
 /// Invoke this once all config values have been changed.
-pub fn configChanged(config: *Config, allocator: std.mem.Allocator, builtin_creation_dir: ?[]const u8) !void {
+pub fn configChanged(config: *Config, runtime_zig_version: *?ZigVersionWrapper, allocator: std.mem.Allocator, builtin_creation_dir: ?[]const u8) !void {
     if (!std.process.can_spawn) return;
     // Find the zig executable in PATH
     find_zig: {
@@ -78,22 +79,34 @@ pub fn configChanged(config: *Config, allocator: std.mem.Allocator, builtin_crea
     if (config.zig_exe_path) |exe_path| blk: {
         logger.info("Using zig executable {s}", .{exe_path});
 
-        if (config.zig_lib_path != null and config.build_runner_global_cache_path != null) break :blk;
-
         var env = getZigEnv(allocator, exe_path) orelse break :blk;
         defer std.json.parseFree(Env, env, .{ .allocator = allocator });
 
-        if (config.zig_lib_path == null) {
-            // Make sure the path is absolute
-            config.zig_lib_path = try std.fs.realpathAlloc(allocator, env.lib_dir.?);
-            logger.info("Using zig lib path '{s}'", .{config.zig_lib_path.?});
-        }
+        if (config.zig_lib_path) |lib_path| allocator.free(lib_path);
+        // Make sure the path is absolute
+        config.zig_lib_path = try std.fs.realpathAlloc(allocator, env.lib_dir.?);
+        logger.info("Using zig lib path '{s}'", .{config.zig_lib_path.?});
 
-        if (config.build_runner_global_cache_path == null) {
-            config.build_runner_global_cache_path = try allocator.dupe(u8, env.global_cache_dir);
-            logger.info("Using build runner global cache path '{s}'", .{config.build_runner_global_cache_path.?});
-        }
+        if (config.build_runner_global_cache_path) |global_cache_path| allocator.free(global_cache_path);
+        config.build_runner_global_cache_path = try allocator.dupe(u8, env.global_cache_dir);
+        logger.info("Using build runner global cache path '{s}'", .{config.build_runner_global_cache_path.?});
+
+        if (runtime_zig_version.*) |current_version| current_version.free();
+        errdefer runtime_zig_version.* = null;
+
+        const duped_zig_version_string = try allocator.dupe(u8, env.version);
+        errdefer allocator.free(duped_zig_version_string);
+
+        logger.info("Detected runtime zig version: '{s}'", .{duped_zig_version_string});
+
+        runtime_zig_version.* = .{
+            .version = try std.SemanticVersion.parse(duped_zig_version_string),
+            .allocator = allocator,
+            .raw_string = duped_zig_version_string,
+        };
     } else {
+        if (runtime_zig_version.*) |version| version.free();
+        runtime_zig_version.* = null;
         logger.warn("Zig executable path not specified in zls.json and could not be found in PATH", .{});
     }
 

src/main.zig

@@ -392,16 +392,16 @@ pub fn main() !void {
 
     try updateConfig(allocator, &config.config, record_file, replay_file);
 
-    var server = try Server.init(
+    const server = try Server.create(
         allocator,
         &config.config,
         config.config_path,
         record_file != null,
         replay_file != null,
     );
-    defer server.deinit();
+    defer server.destroy();
 
-    try loop(&server, record_file, replay_file);
+    try loop(server, record_file, replay_file);
 
     if (server.status == .exiting_failure) {
         std.process.exit(1);

src/zls.zig

@@ -16,6 +16,8 @@ pub const ComptimeInterpreter = @import("ComptimeInterpreter.zig");
 pub const diff = @import("diff.zig");
 pub const analyser = @import("analyser/analyser.zig");
 
+pub const ZigVersionWrapper = @import("ZigVersionWrapper.zig");
+
 comptime {
     const std = @import("std");
     std.testing.refAllDecls(@This());

tests/context.zig

@@ -29,7 +29,7 @@ const default_config: Config = .{
 const allocator = std.testing.allocator;
 
 pub const Context = struct {
-    server: Server,
+    server: *Server,
     arena: std.heap.ArenaAllocator,
     config: *Config,
     request_id: u32 = 1,
@@ -40,8 +40,8 @@ pub const Context = struct {
 
         config.* = default_config;
 
-        var server = try Server.init(allocator, config, null, false, false);
-        errdefer server.deinit();
+        const server = try Server.create(allocator, config, null, false, false);
+        errdefer server.destroy();
 
         var context: Context = .{
             .server = server,
@@ -63,7 +63,7 @@ pub const Context = struct {
         allocator.destroy(self.config);
 
         self.request("shutdown", "{}", null) catch {};
-        self.server.deinit();
+        self.server.destroy();
         self.arena.deinit();
     }
 

tests/language_features/comptime_interpreter.zig

@@ -3,7 +3,7 @@ const zls = @import("zls");
 const builtin = @import("builtin");
 
 const Ast = std.zig.Ast;
-
+const ZigVersionWrapper = zls.ZigVersionWrapper;
 const ComptimeInterpreter = zls.ComptimeInterpreter;
 const InternPool = zls.analyser.InternPool;
 const Index = InternPool.Index;
@@ -286,7 +286,14 @@ const Context = struct {
     document_store: *zls.DocumentStore,
     interpreter: *ComptimeInterpreter,
 
+    // this is very annoying and ugly
+    boxed_null: *const ?ZigVersionWrapper,
+
     pub fn init(source: []const u8) !Context {
+        var boxed_null = try allocator.create(?ZigVersionWrapper);
+        errdefer allocator.destroy(boxed_null);
+        boxed_null.* = null;
+
         var config = try allocator.create(zls.Config);
         errdefer allocator.destroy(config);
 
@@ -300,6 +307,7 @@ const Context = struct {
         document_store.* = .{
             .allocator = allocator,
             .config = config,
+            .runtime_zig_version = boxed_null,
         };
         errdefer document_store.deinit();
 
@@ -326,6 +334,8 @@ const Context = struct {
             .config = config,
             .document_store = document_store,
             .interpreter = interpreter,
+
+            .boxed_null = boxed_null,
         };
     }
 
@@ -336,6 +346,7 @@ const Context = struct {
         allocator.destroy(self.config);
         allocator.destroy(self.document_store);
         allocator.destroy(self.interpreter);
+        allocator.destroy(self.boxed_null);
     }
 
     pub fn call(self: *Context, func_node: Ast.Node.Index, arguments: []const KV) !KV {