From 3936b471a8a3e67573d9ad0628f98c8970f56705 Mon Sep 17 00:00:00 2001 From: Krzysztof Wolicki Date: Thu, 19 Oct 2023 09:39:20 +0200 Subject: [PATCH 1/8] Disable iconv on Windows by default (enabled via cli flag). Skip various tests not implemented on windows. --- build.zig | 2 ++ pkg/fontconfig/build.zig | 7 ++++++- src/Command.zig | 4 ++++ src/Pty.zig | 1 + src/os/desktop.zig | 1 + src/os/passwd.zig | 1 + src/os/xdg.zig | 8 +++++++- 7 files changed, 22 insertions(+), 2 deletions(-) diff --git a/build.zig b/build.zig index a6b4f9364..4850027a5 100644 --- a/build.zig +++ b/build.zig @@ -621,9 +621,11 @@ fn addDeps( const js_dep = b.dependency("zig_js", .{ .target = step.target, .optimize = step.optimize }); const libxev_dep = b.dependency("libxev", .{ .target = step.target, .optimize = step.optimize }); const objc_dep = b.dependency("zig_objc", .{ .target = step.target, .optimize = step.optimize }); + const iconv_win_enabled = b.option(bool, "enable-iconv-win", "Build libxml2 for fontconfig with iconv on Windows") orelse false; const fontconfig_dep = b.dependency("fontconfig", .{ .target = step.target, .optimize = step.optimize, + .iconv_win_enabled = iconv_win_enabled, }); const freetype_dep = b.dependency("freetype", .{ .target = step.target, diff --git a/pkg/fontconfig/build.zig b/pkg/fontconfig/build.zig index 01eb46181..c5350235d 100644 --- a/pkg/fontconfig/build.zig +++ b/pkg/fontconfig/build.zig @@ -6,6 +6,7 @@ pub fn build(b: *std.Build) !void { const optimize = b.standardOptimizeOption(.{}); const libxml2_enabled = b.option(bool, "enable-libxml2", "Build libxml2") orelse true; + const iconv_win_enabled = b.option(bool, "enable-iconv-win", "Build libxml2 with iconv on Windows") orelse false; const freetype_enabled = b.option(bool, "enable-freetype", "Build freetype") orelse true; _ = b.addModule("fontconfig", .{ .source_file = .{ .path = "main.zig" } }); @@ -25,7 +26,11 @@ pub fn build(b: *std.Build) !void { lib.linkLibrary(freetype_dep.artifact("freetype")); } if (libxml2_enabled) { - const libxml2_dep = b.dependency("libxml2", .{ .target = target, .optimize = optimize }); + const libxml2_dep = b.dependency("libxml2", .{ + .target = target, + .optimize = optimize, + .iconv = !(target.getOsTag() == .windows) or iconv_win_enabled, + }); lib.linkLibrary(libxml2_dep.artifact("xml2")); } diff --git a/src/Command.zig b/src/Command.zig index b9d282aa7..ce606f770 100644 --- a/src/Command.zig +++ b/src/Command.zig @@ -355,6 +355,7 @@ test "createNullDelimitedEnvMap" { } test "Command: pre exec" { + if (builtin.os.tag == .windows) return error.SkipZigTest; var cmd: Command = .{ .path = "/usr/bin/env", .args = &.{ "/usr/bin/env", "-v" }, @@ -375,6 +376,7 @@ test "Command: pre exec" { } test "Command: redirect stdout to file" { + if (builtin.os.tag == .windows) return error.SkipZigTest; var td = try TempDir.init(); defer td.deinit(); var stdout = try td.dir.createFile("stdout.txt", .{ .read = true }); @@ -400,6 +402,7 @@ test "Command: redirect stdout to file" { } test "Command: custom env vars" { + if (builtin.os.tag == .windows) return error.SkipZigTest; var td = try TempDir.init(); defer td.deinit(); var stdout = try td.dir.createFile("stdout.txt", .{ .read = true }); @@ -430,6 +433,7 @@ test "Command: custom env vars" { } test "Command: custom working directory" { + if (builtin.os.tag == .windows) return error.SkipZigTest; var td = try TempDir.init(); defer td.deinit(); var stdout = try td.dir.createFile("stdout.txt", .{ .read = true }); diff --git a/src/Pty.zig b/src/Pty.zig index 3bbcb713a..c1e8efd18 100644 --- a/src/Pty.zig +++ b/src/Pty.zig @@ -128,6 +128,7 @@ pub fn childPreExec(self: Pty) !void { } test { + if (builtin.os.tag == .windows) return error.SkipZigTest; var ws: winsize = .{ .ws_row = 50, .ws_col = 80, diff --git a/src/os/desktop.zig b/src/os/desktop.zig index f7717cf65..fdc714c65 100644 --- a/src/os/desktop.zig +++ b/src/os/desktop.zig @@ -40,6 +40,7 @@ pub fn launchedFromDesktop() bool { break :linux gio_pid == pid; }, + .windows => false, else => @compileError("unsupported platform"), }; diff --git a/src/os/passwd.zig b/src/os/passwd.zig index 32d93da3b..b3e3dcc12 100644 --- a/src/os/passwd.zig +++ b/src/os/passwd.zig @@ -138,6 +138,7 @@ pub fn get(alloc: Allocator) !Entry { } test { + if (builtin.os.tag == .windows) return error.SkipZigTest; const testing = std.testing; var arena = ArenaAllocator.init(testing.allocator); defer arena.deinit(); diff --git a/src/os/xdg.zig b/src/os/xdg.zig index cab8aab3d..4c0b465db 100644 --- a/src/os/xdg.zig +++ b/src/os/xdg.zig @@ -20,7 +20,8 @@ pub const Options = struct { /// Get the XDG user config directory. The returned value is allocated. pub fn config(alloc: Allocator, opts: Options) ![]u8 { - if (std.os.getenv("XDG_CONFIG_HOME")) |env| { + if (std.process.getEnvVarOwned(alloc, "XDG_CONFIG_HOME")) |env| { + defer alloc.free(env); // If we have a subdir, then we use the env as-is to avoid a copy. if (opts.subdir) |subdir| { return try std.fs.path.join(alloc, &[_][]const u8{ @@ -30,6 +31,11 @@ pub fn config(alloc: Allocator, opts: Options) ![]u8 { } return try alloc.dupe(u8, env); + } else |err| { + switch (err) { + error.EnvironmentVariableNotFound => {}, + else => return err, + } } // If we have a cached home dir, use that. From 00c24e4ae66b791d7ba71a634d173222ff995db6 Mon Sep 17 00:00:00 2001 From: Krzysztof Wolicki Date: Thu, 19 Oct 2023 10:52:09 +0200 Subject: [PATCH 2/8] Fix option name passed to fontconfig dep --- build.zig | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/build.zig b/build.zig index 4850027a5..c1b3c27d2 100644 --- a/build.zig +++ b/build.zig @@ -625,7 +625,7 @@ fn addDeps( const fontconfig_dep = b.dependency("fontconfig", .{ .target = step.target, .optimize = step.optimize, - .iconv_win_enabled = iconv_win_enabled, + .@"enable-iconv-win" = iconv_win_enabled, }); const freetype_dep = b.dependency("freetype", .{ .target = step.target, From cf9f025a5b8dd08f8193a267968826f1a58d1c8a Mon Sep 17 00:00:00 2001 From: Krzysztof Wolicki Date: Thu, 19 Oct 2023 17:10:24 +0200 Subject: [PATCH 3/8] Don't enable iconv for fontconfig dep on windows --- build.zig | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/build.zig b/build.zig index c1b3c27d2..9ceb27ba3 100644 --- a/build.zig +++ b/build.zig @@ -621,11 +621,11 @@ fn addDeps( const js_dep = b.dependency("zig_js", .{ .target = step.target, .optimize = step.optimize }); const libxev_dep = b.dependency("libxev", .{ .target = step.target, .optimize = step.optimize }); const objc_dep = b.dependency("zig_objc", .{ .target = step.target, .optimize = step.optimize }); - const iconv_win_enabled = b.option(bool, "enable-iconv-win", "Build libxml2 for fontconfig with iconv on Windows") orelse false; + const fontconfig_dep = b.dependency("fontconfig", .{ .target = step.target, .optimize = step.optimize, - .@"enable-iconv-win" = iconv_win_enabled, + .@"enable-iconv-win" = false, }); const freetype_dep = b.dependency("freetype", .{ .target = step.target, From b830deb8a93a0a9599c5ffc0abf08b6e98b77dcc Mon Sep 17 00:00:00 2001 From: Krzysztof Wolicki Date: Thu, 19 Oct 2023 17:21:23 +0200 Subject: [PATCH 4/8] pkg/fontconfig: windows check more readable --- pkg/fontconfig/build.zig | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/fontconfig/build.zig b/pkg/fontconfig/build.zig index c5350235d..2f9a578cc 100644 --- a/pkg/fontconfig/build.zig +++ b/pkg/fontconfig/build.zig @@ -29,7 +29,7 @@ pub fn build(b: *std.Build) !void { const libxml2_dep = b.dependency("libxml2", .{ .target = target, .optimize = optimize, - .iconv = !(target.getOsTag() == .windows) or iconv_win_enabled, + .iconv = target.getOsTag() != .windows or iconv_win_enabled, }); lib.linkLibrary(libxml2_dep.artifact("xml2")); } From f4a2273ad90ef8d447c202323e4cfe3c51680e67 Mon Sep 17 00:00:00 2001 From: Krzysztof Wolicki Date: Thu, 19 Oct 2023 17:22:11 +0200 Subject: [PATCH 5/8] os/xdg: Avoid allocations on non-Windows systems. os/desktop: Add TODO --- src/os/desktop.zig | 1 + src/os/xdg.zig | 37 +++++++++++++++++++++++++------------ 2 files changed, 26 insertions(+), 12 deletions(-) diff --git a/src/os/desktop.zig b/src/os/desktop.zig index fdc714c65..084c86e8d 100644 --- a/src/os/desktop.zig +++ b/src/os/desktop.zig @@ -40,6 +40,7 @@ pub fn launchedFromDesktop() bool { break :linux gio_pid == pid; }, + //TODO: maybe find a way to check that .windows => false, else => @compileError("unsupported platform"), diff --git a/src/os/xdg.zig b/src/os/xdg.zig index 4c0b465db..36a4caade 100644 --- a/src/os/xdg.zig +++ b/src/os/xdg.zig @@ -20,22 +20,35 @@ pub const Options = struct { /// Get the XDG user config directory. The returned value is allocated. pub fn config(alloc: Allocator, opts: Options) ![]u8 { - if (std.process.getEnvVarOwned(alloc, "XDG_CONFIG_HOME")) |env| { - defer alloc.free(env); - // If we have a subdir, then we use the env as-is to avoid a copy. - if (opts.subdir) |subdir| { - return try std.fs.path.join(alloc, &[_][]const u8{ - env, - subdir, - }); - } + if (builtin.os.tag == .windows) { + if (std.process.getEnvVarOwned(alloc, "XDG_CONFIG_HOME")) |env| { + // If we have a subdir, then we use the env as-is to avoid a copy. + if (opts.subdir) |subdir| { + defer alloc.free(env); + return try std.fs.path.join(alloc, &[_][]const u8{ + env, + subdir, + }); + } - return try alloc.dupe(u8, env); - } else |err| { - switch (err) { + // We don't need to dupe since it's already allocated + return env; + } else |err| switch (err) { error.EnvironmentVariableNotFound => {}, else => return err, } + } else { + if (std.os.getenv("XDG_CONFIG_HOME")) |env| { + // If we have a subdir, then we use the env as-is to avoid a copy. + if (opts.subdir) |subdir| { + return try std.fs.path.join(alloc, &[_][]const u8{ + env, + subdir, + }); + } + + return try alloc.dupe(u8, env); + } } // If we have a cached home dir, use that. From b9b33ab25d8899cd7d49a1dee6c0b4e3a8ac2987 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Mon, 23 Oct 2023 08:40:25 -0700 Subject: [PATCH 6/8] pkg/fontconfig: make iconv build param non-windows specific --- build.zig | 1 - pkg/fontconfig/build.zig | 8 ++++++-- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/build.zig b/build.zig index 9ceb27ba3..a455b672d 100644 --- a/build.zig +++ b/build.zig @@ -625,7 +625,6 @@ fn addDeps( const fontconfig_dep = b.dependency("fontconfig", .{ .target = step.target, .optimize = step.optimize, - .@"enable-iconv-win" = false, }); const freetype_dep = b.dependency("freetype", .{ .target = step.target, diff --git a/pkg/fontconfig/build.zig b/pkg/fontconfig/build.zig index 2f9a578cc..0a927d66f 100644 --- a/pkg/fontconfig/build.zig +++ b/pkg/fontconfig/build.zig @@ -6,7 +6,11 @@ pub fn build(b: *std.Build) !void { const optimize = b.standardOptimizeOption(.{}); const libxml2_enabled = b.option(bool, "enable-libxml2", "Build libxml2") orelse true; - const iconv_win_enabled = b.option(bool, "enable-iconv-win", "Build libxml2 with iconv on Windows") orelse false; + const libxml2_iconv_enabled = b.option( + bool, + "enable-libxml2-iconv", + "Build libxml2 with iconv", + ) orelse (target.getOsTag() != .windows); const freetype_enabled = b.option(bool, "enable-freetype", "Build freetype") orelse true; _ = b.addModule("fontconfig", .{ .source_file = .{ .path = "main.zig" } }); @@ -29,7 +33,7 @@ pub fn build(b: *std.Build) !void { const libxml2_dep = b.dependency("libxml2", .{ .target = target, .optimize = optimize, - .iconv = target.getOsTag() != .windows or iconv_win_enabled, + .iconv = libxml2_iconv_enabled, }); lib.linkLibrary(libxml2_dep.artifact("xml2")); } From 3226cbf61b9d76943dfb0bc9e8ac644bf82066b2 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Mon, 23 Oct 2023 08:47:16 -0700 Subject: [PATCH 7/8] os: remove some duplication in the env var check for xdg config --- src/os/xdg.zig | 46 +++++++++++++++++++++------------------------- 1 file changed, 21 insertions(+), 25 deletions(-) diff --git a/src/os/xdg.zig b/src/os/xdg.zig index 36a4caade..f193cdf19 100644 --- a/src/os/xdg.zig +++ b/src/os/xdg.zig @@ -20,35 +20,31 @@ pub const Options = struct { /// Get the XDG user config directory. The returned value is allocated. pub fn config(alloc: Allocator, opts: Options) ![]u8 { - if (builtin.os.tag == .windows) { - if (std.process.getEnvVarOwned(alloc, "XDG_CONFIG_HOME")) |env| { - // If we have a subdir, then we use the env as-is to avoid a copy. - if (opts.subdir) |subdir| { - defer alloc.free(env); - return try std.fs.path.join(alloc, &[_][]const u8{ - env, - subdir, - }); + // First check the env var. On Windows we have to allocate so this tracks + // both whether we have the env var and whether we own it. + const env_, const owned = switch (builtin.os.tag) { + else => .{ std.os.getenv("XDG_CONFIG_HOME"), false }, + .windows => windows: { + if (std.process.getEnvVarOwned(alloc, "XDG_CONFIG_HOME")) |env| { + break :windows .{ env, true }; + } else |err| switch (err) { + error.EnvironmentVariableNotFound => break :windows .{ null, false }, + else => return err, } + }, + }; + defer if (owned) if (env_) |v| alloc.free(v); - // We don't need to dupe since it's already allocated - return env; - } else |err| switch (err) { - error.EnvironmentVariableNotFound => {}, - else => return err, + if (env_) |env| { + // If we have a subdir, then we use the env as-is to avoid a copy. + if (opts.subdir) |subdir| { + return try std.fs.path.join(alloc, &[_][]const u8{ + env, + subdir, + }); } - } else { - if (std.os.getenv("XDG_CONFIG_HOME")) |env| { - // If we have a subdir, then we use the env as-is to avoid a copy. - if (opts.subdir) |subdir| { - return try std.fs.path.join(alloc, &[_][]const u8{ - env, - subdir, - }); - } - return try alloc.dupe(u8, env); - } + return try alloc.dupe(u8, env); } // If we have a cached home dir, use that. From 4a199448a262feca496c2a0357a34983b500954d Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Mon, 23 Oct 2023 08:48:53 -0700 Subject: [PATCH 8/8] os: update todo about subsystem for windows --- src/os/desktop.zig | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/os/desktop.zig b/src/os/desktop.zig index 084c86e8d..ed4f977d9 100644 --- a/src/os/desktop.zig +++ b/src/os/desktop.zig @@ -40,7 +40,8 @@ pub fn launchedFromDesktop() bool { break :linux gio_pid == pid; }, - //TODO: maybe find a way to check that + + // TODO: This should have some logic to detect this. Perhaps std.builtin.subsystem .windows => false, else => @compileError("unsupported platform"),