From 5ede46f0039512c584d1bd0b53559721fd55f1dc Mon Sep 17 00:00:00 2001 From: Lee Cannon Date: Thu, 23 Feb 2023 12:18:52 -0800 Subject: [PATCH] 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 | 9 +++- src/DocumentStore.zig | 42 +++++++++++++-- src/Server.zig | 54 +++++++++---------- src/ZigVersionWrapper.zig | 13 +++++ src/configuration.zig | 37 ++++++++----- src/main.zig | 6 +-- src/zls.zig | 2 + tests/context.zig | 8 +-- .../comptime_interpreter.zig | 13 ++++- 9 files changed, 133 insertions(+), 51 deletions(-) create mode 100644 src/ZigVersionWrapper.zig diff --git a/.github/workflows/build_runner.yml b/.github/workflows/build_runner.yml index aced4f9..80e4ef4 100644 --- a/.github/workflows/build_runner.yml +++ b/.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 diff --git a/src/DocumentStore.zig b/src/DocumentStore.zig index 8b24280..8896051 100644 --- a/src/DocumentStore.zig +++ b/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 }); diff --git a/src/Server.zig b/src/Server.zig index 020584e..97ed2c6 100644 --- a/src/Server.zig +++ b/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); } diff --git a/src/ZigVersionWrapper.zig b/src/ZigVersionWrapper.zig new file mode 100644 index 0000000..7c98506 --- /dev/null +++ b/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); +} diff --git a/src/configuration.zig b/src/configuration.zig index 399e276..ce5013a 100644 --- a/src/configuration.zig +++ b/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", .{}); } diff --git a/src/main.zig b/src/main.zig index 34ece28..f0fd38f 100644 --- a/src/main.zig +++ b/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); diff --git a/src/zls.zig b/src/zls.zig index 77ca108..a92799b 100644 --- a/src/zls.zig +++ b/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()); diff --git a/tests/context.zig b/tests/context.zig index 696ff3b..4d7da4d 100644 --- a/tests/context.zig +++ b/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(); } diff --git a/tests/language_features/comptime_interpreter.zig b/tests/language_features/comptime_interpreter.zig index 24debcd..378bf17 100644 --- a/tests/language_features/comptime_interpreter.zig +++ b/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 {