From 1941a440d87a96b1e7f4e0b498c5f15f133d1f99 Mon Sep 17 00:00:00 2001 From: Bryan Lee <38807139+liby@users.noreply.github.com> Date: Sat, 28 Dec 2024 00:02:28 +0800 Subject: [PATCH 1/5] Use `$HOME/Library/Caches` on macOS instead of `$HOME/.cache` --- src/os/xdg.zig | 62 ++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 62 insertions(+) diff --git a/src/os/xdg.zig b/src/os/xdg.zig index 6c7655c226..1d60374efc 100644 --- a/src/os/xdg.zig +++ b/src/os/xdg.zig @@ -30,6 +30,22 @@ pub fn config(alloc: Allocator, opts: Options) ![]u8 { /// Get the XDG cache directory. The returned value is allocated. pub fn cache(alloc: Allocator, opts: Options) ![]u8 { + // On macOS we should use ~/Library/Caches instead of ~/.cache + if (builtin.os.tag == .macos) { + // Get our home dir if not provided + const home = if (opts.home) |h| h else blk: { + var buf: [1024]u8 = undefined; + break :blk try homedir.home(&buf) orelse return error.NoHomeDir; + }; + + return try std.fs.path.join(alloc, &[_][]const u8{ + home, + "Library", + "Caches", + opts.subdir orelse "", + }); + } + return try dir(alloc, opts, .{ .env = "XDG_CACHE_HOME", .windows_env = "LOCALAPPDATA", @@ -143,6 +159,52 @@ test { } } +test "cache directory paths" { + const testing = std.testing; + const alloc = testing.allocator; + const mock_home = "/Users/test"; + + // Test macOS path + if (builtin.os.tag == .macos) { + // Test base path + { + const cache_path = try cache(alloc, .{ .home = mock_home }); + defer alloc.free(cache_path); + try testing.expectEqualStrings("/Users/test/Library/Caches", cache_path); + } + + // Test with subdir + { + const cache_path = try cache(alloc, .{ + .home = mock_home, + .subdir = "ghostty", + }); + defer alloc.free(cache_path); + try testing.expectEqualStrings("/Users/test/Library/Caches/ghostty", cache_path); + } + } + + // Test Linux path (when XDG_CACHE_HOME is not set) + if (builtin.os.tag == .linux) { + // Test base path + { + const cache_path = try cache(alloc, .{ .home = mock_home }); + defer alloc.free(cache_path); + try testing.expectEqualStrings("/Users/test/.cache", cache_path); + } + + // Test with subdir + { + const cache_path = try cache(alloc, .{ + .home = mock_home, + .subdir = "ghostty", + }); + defer alloc.free(cache_path); + try testing.expectEqualStrings("/Users/test/.cache/ghostty", cache_path); + } + } +} + test parseTerminalExec { const testing = std.testing; From 67794d3f6f39b4f765a1780e939dda024c9bf561 Mon Sep 17 00:00:00 2001 From: Bryan Lee <38807139+liby@users.noreply.github.com> Date: Mon, 30 Dec 2024 09:33:53 +0800 Subject: [PATCH 2/5] Respect `XDG_CACHE_HOME` and use`NSFileManager` for cache paths --- src/crash/sentry.zig | 5 +- src/os/macos.zig | 109 ++++++++++++++++++++++++++++++------------- src/os/xdg.zig | 37 ++------------- 3 files changed, 85 insertions(+), 66 deletions(-) diff --git a/src/crash/sentry.zig b/src/crash/sentry.zig index 14f2e484c4..fba20067d8 100644 --- a/src/crash/sentry.zig +++ b/src/crash/sentry.zig @@ -101,7 +101,10 @@ fn initThread(gpa: Allocator) !void { sentry.c.sentry_options_set_before_send(opts, beforeSend, null); // Determine the Sentry cache directory. - const cache_dir = try internal_os.xdg.cache(alloc, .{ .subdir = "ghostty/sentry" }); + const cache_dir = if (builtin.os.tag == .macos) + try internal_os.macos.cacheDir(alloc, "ghostty/sentry") + else + try internal_os.xdg.cache(alloc, .{ .subdir = "ghostty/sentry" }); sentry.c.sentry_options_set_database_path_n( opts, cache_dir.ptr, diff --git a/src/os/macos.zig b/src/os/macos.zig index b3d0a917c5..5cf6ab23a8 100644 --- a/src/os/macos.zig +++ b/src/os/macos.zig @@ -25,43 +25,23 @@ pub fn appSupportDir( alloc: Allocator, sub_path: []const u8, ) AppSupportDirError![]const u8 { - comptime assert(builtin.target.isDarwin()); - - const NSFileManager = objc.getClass("NSFileManager").?; - const manager = NSFileManager.msgSend( - objc.Object, - objc.sel("defaultManager"), - .{}, - ); - - const url = manager.msgSend( - objc.Object, - objc.sel("URLForDirectory:inDomain:appropriateForURL:create:error:"), - .{ - NSSearchPathDirectory.NSApplicationSupportDirectory, - NSSearchPathDomainMask.NSUserDomainMask, - @as(?*anyopaque, null), - true, - @as(?*anyopaque, null), - }, - ); - - // I don't think this is possible but just in case. - if (url.value == null) return error.AppleAPIFailed; - - // Get the UTF-8 string from the URL. - const path = url.getProperty(objc.Object, "path"); - const c_str = path.getProperty(?[*:0]const u8, "UTF8String") orelse - return error.AppleAPIFailed; - const app_support_dir = std.mem.sliceTo(c_str, 0); - - return try std.fs.path.join(alloc, &.{ - app_support_dir, + return try makeCommonPath(alloc, .NSApplicationSupportDirectory, &.{ build_config.bundle_id, sub_path, }); } +pub const CacheDirError = Allocator.Error || error{AppleAPIFailed}; + +/// Return the path to the system cache directory with the given sub path joined. +/// This allocates the result using the given allocator. +pub fn cacheDir( + alloc: Allocator, + sub_path: []const u8, +) CacheDirError![]const u8 { + return try makeCommonPath(alloc, .NSCachesDirectory, &.{sub_path}); +} + pub const SetQosClassError = error{ // The thread can't have its QoS class changed usually because // a different pthread API was called that makes it an invalid @@ -110,9 +90,74 @@ pub const NSOperatingSystemVersion = extern struct { }; pub const NSSearchPathDirectory = enum(c_ulong) { + NSCachesDirectory = 13, NSApplicationSupportDirectory = 14, }; pub const NSSearchPathDomainMask = enum(c_ulong) { NSUserDomainMask = 1, }; + +fn makeCommonPath( + alloc: Allocator, + directory: NSSearchPathDirectory, + sub_paths: []const []const u8, +) (error{AppleAPIFailed} || Allocator.Error)![]const u8 { + comptime assert(builtin.target.isDarwin()); + + const NSFileManager = objc.getClass("NSFileManager").?; + const manager = NSFileManager.msgSend( + objc.Object, + objc.sel("defaultManager"), + .{}, + ); + + const url = manager.msgSend( + objc.Object, + objc.sel("URLForDirectory:inDomain:appropriateForURL:create:error:"), + .{ + directory, + NSSearchPathDomainMask.NSUserDomainMask, + @as(?*anyopaque, null), + true, + @as(?*anyopaque, null), + }, + ); + + if (url.value == null) return error.AppleAPIFailed; + + const path = url.getProperty(objc.Object, "path"); + const c_str = path.getProperty(?[*:0]const u8, "UTF8String") orelse + return error.AppleAPIFailed; + const base_dir = std.mem.sliceTo(c_str, 0); + + // Create a new array with base_dir as the first element + var paths = try alloc.alloc([]const u8, sub_paths.len + 1); + paths[0] = base_dir; + @memcpy(paths[1..], sub_paths); + defer alloc.free(paths); + + return try std.fs.path.join(alloc, paths); +} + +test "cacheDir paths" { + if (!builtin.target.isDarwin()) return; + + const testing = std.testing; + const alloc = testing.allocator; + + // Test base path + { + const cache_path = try cacheDir(alloc, ""); + defer alloc.free(cache_path); + // We don't test the exact path since it comes from NSFileManager + try testing.expect(std.mem.indexOf(u8, cache_path, "Caches") != null); + } + + // Test with subdir + { + const cache_path = try cacheDir(alloc, "ghostty"); + defer alloc.free(cache_path); + try testing.expect(std.mem.indexOf(u8, cache_path, "Caches/ghostty") != null); + } +} diff --git a/src/os/xdg.zig b/src/os/xdg.zig index 1d60374efc..80645dcb5c 100644 --- a/src/os/xdg.zig +++ b/src/os/xdg.zig @@ -30,18 +30,9 @@ pub fn config(alloc: Allocator, opts: Options) ![]u8 { /// Get the XDG cache directory. The returned value is allocated. pub fn cache(alloc: Allocator, opts: Options) ![]u8 { - // On macOS we should use ~/Library/Caches instead of ~/.cache - if (builtin.os.tag == .macos) { - // Get our home dir if not provided - const home = if (opts.home) |h| h else blk: { - var buf: [1024]u8 = undefined; - break :blk try homedir.home(&buf) orelse return error.NoHomeDir; - }; - + if (posix.getenv("XDG_CACHE_HOME")) |env| { return try std.fs.path.join(alloc, &[_][]const u8{ - home, - "Library", - "Caches", + env, opts.subdir orelse "", }); } @@ -164,28 +155,8 @@ test "cache directory paths" { const alloc = testing.allocator; const mock_home = "/Users/test"; - // Test macOS path - if (builtin.os.tag == .macos) { - // Test base path - { - const cache_path = try cache(alloc, .{ .home = mock_home }); - defer alloc.free(cache_path); - try testing.expectEqualStrings("/Users/test/Library/Caches", cache_path); - } - - // Test with subdir - { - const cache_path = try cache(alloc, .{ - .home = mock_home, - .subdir = "ghostty", - }); - defer alloc.free(cache_path); - try testing.expectEqualStrings("/Users/test/Library/Caches/ghostty", cache_path); - } - } - - // Test Linux path (when XDG_CACHE_HOME is not set) - if (builtin.os.tag == .linux) { + // Test when XDG_CACHE_HOME is not set + { // Test base path { const cache_path = try cache(alloc, .{ .home = mock_home }); From 9f44ec7c21cc13985d5e6be06dccda0362932f12 Mon Sep 17 00:00:00 2001 From: Bryan Lee <38807139+liby@users.noreply.github.com> Date: Mon, 30 Dec 2024 14:13:22 +0800 Subject: [PATCH 3/5] Use bundle ID for macOS cache directory path --- src/crash/sentry.zig | 2 +- src/os/macos.zig | 16 ++++++++++++---- 2 files changed, 13 insertions(+), 5 deletions(-) diff --git a/src/crash/sentry.zig b/src/crash/sentry.zig index fba20067d8..f381a88405 100644 --- a/src/crash/sentry.zig +++ b/src/crash/sentry.zig @@ -102,7 +102,7 @@ fn initThread(gpa: Allocator) !void { // Determine the Sentry cache directory. const cache_dir = if (builtin.os.tag == .macos) - try internal_os.macos.cacheDir(alloc, "ghostty/sentry") + try internal_os.macos.cacheDir(alloc, "sentry") else try internal_os.xdg.cache(alloc, .{ .subdir = "ghostty/sentry" }); sentry.c.sentry_options_set_database_path_n( diff --git a/src/os/macos.zig b/src/os/macos.zig index 5cf6ab23a8..918dde9af4 100644 --- a/src/os/macos.zig +++ b/src/os/macos.zig @@ -39,7 +39,10 @@ pub fn cacheDir( alloc: Allocator, sub_path: []const u8, ) CacheDirError![]const u8 { - return try makeCommonPath(alloc, .NSCachesDirectory, &.{sub_path}); + return try makeCommonPath(alloc, .NSCachesDirectory, &.{ + build_config.bundle_id, + sub_path, + }); } pub const SetQosClassError = error{ @@ -150,14 +153,19 @@ test "cacheDir paths" { { const cache_path = try cacheDir(alloc, ""); defer alloc.free(cache_path); - // We don't test the exact path since it comes from NSFileManager try testing.expect(std.mem.indexOf(u8, cache_path, "Caches") != null); + try testing.expect(std.mem.indexOf(u8, cache_path, build_config.bundle_id) != null); } // Test with subdir { - const cache_path = try cacheDir(alloc, "ghostty"); + const cache_path = try cacheDir(alloc, "test"); defer alloc.free(cache_path); - try testing.expect(std.mem.indexOf(u8, cache_path, "Caches/ghostty") != null); + try testing.expect(std.mem.indexOf(u8, cache_path, "Caches") != null); + try testing.expect(std.mem.indexOf(u8, cache_path, build_config.bundle_id) != null); + + const bundle_path = try std.fmt.allocPrint(alloc, "{s}/test", .{build_config.bundle_id}); + defer alloc.free(bundle_path); + try testing.expect(std.mem.indexOf(u8, cache_path, bundle_path) != null); } } From 6fca26972bbe4407f6d0c4c76f13efa8eeda212c Mon Sep 17 00:00:00 2001 From: Bryan Lee <38807139+liby@users.noreply.github.com> Date: Fri, 3 Jan 2025 00:21:44 +0800 Subject: [PATCH 4/5] Remove the redundant check and directly use `dir()` --- src/os/xdg.zig | 7 ------- 1 file changed, 7 deletions(-) diff --git a/src/os/xdg.zig b/src/os/xdg.zig index 80645dcb5c..a5b29abe43 100644 --- a/src/os/xdg.zig +++ b/src/os/xdg.zig @@ -30,13 +30,6 @@ pub fn config(alloc: Allocator, opts: Options) ![]u8 { /// Get the XDG cache directory. The returned value is allocated. pub fn cache(alloc: Allocator, opts: Options) ![]u8 { - if (posix.getenv("XDG_CACHE_HOME")) |env| { - return try std.fs.path.join(alloc, &[_][]const u8{ - env, - opts.subdir orelse "", - }); - } - return try dir(alloc, opts, .{ .env = "XDG_CACHE_HOME", .windows_env = "LOCALAPPDATA", From 57af5f31067dedd8aff07cb576b30044da14e22f Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Thu, 2 Jan 2025 11:50:12 -0800 Subject: [PATCH 5/5] crash: prefer XDG cache dir if available --- src/crash/sentry.zig | 21 +++++++++++++++++---- src/os/macos.zig | 20 +++++++++++--------- 2 files changed, 28 insertions(+), 13 deletions(-) diff --git a/src/crash/sentry.zig b/src/crash/sentry.zig index f381a88405..9e05b427d2 100644 --- a/src/crash/sentry.zig +++ b/src/crash/sentry.zig @@ -101,10 +101,23 @@ fn initThread(gpa: Allocator) !void { sentry.c.sentry_options_set_before_send(opts, beforeSend, null); // Determine the Sentry cache directory. - const cache_dir = if (builtin.os.tag == .macos) - try internal_os.macos.cacheDir(alloc, "sentry") - else - try internal_os.xdg.cache(alloc, .{ .subdir = "ghostty/sentry" }); + const cache_dir = cache_dir: { + // On macOS, we prefer to use the NSCachesDirectory value to be + // a more idiomatic macOS application. But if XDG env vars are set + // we will respect them. + if (comptime builtin.os.tag == .macos) macos: { + if (std.posix.getenv("XDG_CACHE_HOME") != null) break :macos; + break :cache_dir try internal_os.macos.cacheDir( + alloc, + "sentry", + ); + } + + break :cache_dir try internal_os.xdg.cache( + alloc, + .{ .subdir = "ghostty/sentry" }, + ); + }; sentry.c.sentry_options_set_database_path_n( opts, cache_dir.ptr, diff --git a/src/os/macos.zig b/src/os/macos.zig index 918dde9af4..a956d25e2b 100644 --- a/src/os/macos.zig +++ b/src/os/macos.zig @@ -25,10 +25,11 @@ pub fn appSupportDir( alloc: Allocator, sub_path: []const u8, ) AppSupportDirError![]const u8 { - return try makeCommonPath(alloc, .NSApplicationSupportDirectory, &.{ - build_config.bundle_id, - sub_path, - }); + return try commonDir( + alloc, + .NSApplicationSupportDirectory, + &.{ build_config.bundle_id, sub_path }, + ); } pub const CacheDirError = Allocator.Error || error{AppleAPIFailed}; @@ -39,10 +40,11 @@ pub fn cacheDir( alloc: Allocator, sub_path: []const u8, ) CacheDirError![]const u8 { - return try makeCommonPath(alloc, .NSCachesDirectory, &.{ - build_config.bundle_id, - sub_path, - }); + return try commonDir( + alloc, + .NSCachesDirectory, + &.{ build_config.bundle_id, sub_path }, + ); } pub const SetQosClassError = error{ @@ -101,7 +103,7 @@ pub const NSSearchPathDomainMask = enum(c_ulong) { NSUserDomainMask = 1, }; -fn makeCommonPath( +fn commonDir( alloc: Allocator, directory: NSSearchPathDirectory, sub_paths: []const []const u8,