diff --git a/app/App.zig b/app/App.zig index 15cd17ef5..dc192026b 100644 --- a/app/App.zig +++ b/app/App.zig @@ -43,6 +43,7 @@ pub const PluginEnabledState = enum { unset, enabled, disabled }; const Layout = @import("layout/Layout.zig"); const Host = sdk.Host; const PluginLoader = @import("root.zig").store.Loader; +const store = @import("root.zig").store; const SettingsWatcher = @import("watch/SettingsWatcher.zig"); const ControlServer = @import("control/Server.zig"); const ControlState = @import("control/routes.zig").State; @@ -962,30 +963,40 @@ pub fn deinit(app: *App) void { app.auto_update_off_ids.deinit(app.gpa); } -/// The id another plugin goes by that `id` is in another case: a loaded plugin's -/// (`Host.pluginByIdIgnoringCase`), or a folder in `plugins/`. Asked before anything keyed by -/// `id` touches the disk (a store install or update, a load): on macOS and Windows `plugins/Graphl` -/// *is* `plugins/graphl`, so writing one would overwrite the other. Written into `buf`. -pub fn pluginCaseClash(app: *App, buf: *[64]u8, id: []const u8) ?[]const u8 { - if (app.host.pluginByIdIgnoringCase(id)) |p| if (!std.mem.eql(u8, p.id, id)) { - const n = @min(p.id.len, buf.len); - @memcpy(buf[0..n], p.id[0..n]); - return buf[0..n]; - }; +/// Why a build of `id` may not be installed or loaded: another plugin goes by `id` in another +/// case, loaded or with a folder in `plugins/` (on macOS and Windows `plugins/Graphl` *is* +/// `plugins/graphl`, so writing one would overwrite the other), or `plugins/` cannot be read to +/// tell, which is refused as a possible clash. +pub const CaseClash = union(enum) { + /// The other plugin's id or folder name. Borrowed from the loaded plugin, or from the + /// listing it was read from. + other: []const u8, + unreadable, +}; + +/// `id`'s clash with what is loaded and with `folders`, a listing of `plugins/` +/// (`store.folders.read`). Asked before anything keyed by `id` touches the disk. +pub fn caseClashIn(app: *App, folders: store.folders.Folders, id: []const u8) ?CaseClash { + if (app.host.pluginByIdIgnoringCase(id)) |p| if (!std.mem.eql(u8, p.id, id)) return .{ .other = p.id }; + return .{ .other = store.folders.clash(folders.names, id) orelse return null }; +} + +/// The clash `recordCaseClash` explains, for a load (`Editor.loadUserPluginById`, the startup +/// scan): `plugins/` read now. The name is copied into `buf`, whole; one that does not fit is +/// refused as unreadable rather than shown cut off. +pub fn pluginCaseClash(app: *App, buf: []u8, id: []const u8) ?CaseClash { if (comptime builtin.target.cpu.arch == .wasm32) return null; - const plugins_dir = std.fs.path.join(app.gpa, &.{ app.config_folder, "plugins" }) catch return null; + const plugins_dir = std.fs.path.join(app.gpa, &.{ app.config_folder, "plugins" }) catch return .unreadable; defer app.gpa.free(plugins_dir); - var dir = std.Io.Dir.cwd().openDir(dvui.io, plugins_dir, .{ .iterate = true }) catch return null; - defer dir.close(dvui.io); - var it = dir.iterate(); - while (it.next(dvui.io) catch null) |entry| { - if (entry.kind != .directory) continue; - if (std.mem.eql(u8, entry.name, id) or !std.ascii.eqlIgnoreCase(entry.name, id)) continue; - const n = @min(entry.name.len, buf.len); - @memcpy(buf[0..n], entry.name[0..n]); - return buf[0..n]; - } - return null; + const folders = store.folders.read(app.gpa, dvui.io, plugins_dir) catch return .unreadable; + defer folders.deinit(app.gpa); + return switch (app.caseClashIn(folders, id) orelse return null) { + .unreadable => .unreadable, + .other => |name| if (name.len <= buf.len) blk: { + @memcpy(buf[0..name.len], name); + break :blk .{ .other = buf[0..name.len] }; + } else .unreadable, + }; } /// The card for a build of `id` refused because `other`, the same id in another case, is loaded or @@ -1000,16 +1011,6 @@ pub fn recordCaseClash(app: *App, id: []const u8, other: []const u8, path: ?[]co app.recordPluginFailure(id, why, null, if (probe) |info| info.plugin_version else null, if (path) |p| .of(p) else .{}); } -/// Whether two paths name one folder: the same file on the same device, so a case-insensitive -/// filesystem's `plugins/Graphl` and `plugins/graphl` are one, and a case-sensitive one's are -/// two. False when either cannot be read. -pub fn sameFolder(a: []const u8, b: []const u8) bool { - if (comptime builtin.target.cpu.arch == .wasm32) return false; - const sa = std.Io.Dir.cwd().statFile(dvui.io, a, .{}) catch return false; - const sb = std.Io.Dir.cwd().statFile(dvui.io, b, .{}) catch return false; - return sa.inode == sb.inode; -} - /// `/plugins//.` — where a user plugin's build lives. Caller frees. pub fn userPluginPath(gpa: std.mem.Allocator, app: *App, id: []const u8) ![]u8 { const file_name = try PluginLoader.pluginFilename(id, gpa); diff --git a/app/root.zig b/app/root.zig index e2baa69d1..a4e932347 100644 --- a/app/root.zig +++ b/app/root.zig @@ -182,6 +182,8 @@ pub const store = struct { else @import("store/PluginLoader.zig"); pub const registry = @import("store/registry/store.zig"); + /// The folders in `plugins/` and what a plugin id may do among them. + pub const folders = @import("store/plugin_folders.zig"); }; // The root of `zig build test-integration`'s `fizzy-app-tests` (`build/app.zig`): the files here diff --git a/app/store/PluginManager.zig b/app/store/PluginManager.zig index 004d242ba..003084612 100644 --- a/app/store/PluginManager.zig +++ b/app/store/PluginManager.zig @@ -125,11 +125,6 @@ pub const VTable = struct { /// Bring the main region into view — on a narrow layout the store's README page is drawn /// somewhere the user cannot see yet. A no-op is a perfectly good implementation. revealMain: *const fn (ctx: *anyopaque) void, - /// Asked before an install or update writes anything into `plugins/`: refuse `id` when it - /// is a plugin already loaded or installed in another case (on macOS and Windows that folder - /// is the other plugin's), saying why where the app shows load failures. True when refused. - /// Null for an app that keeps no such rule. - refuseCaseClash: ?*const fn (ctx: *anyopaque, id: []const u8) bool = null, }; pub fn isDisabled(self: PluginManager, id: []const u8) bool { @@ -198,10 +193,6 @@ pub fn offerUpdates(self: PluginManager) void { pub fn revealMain(self: PluginManager) void { self.vtable.revealMain(self.ctx); } -pub fn refuseCaseClash(self: PluginManager, id: []const u8) bool { - const refuse = self.vtable.refuseCaseClash orelse return false; - return refuse(self.ctx, id); -} /// True when `id` has a dylib on disk that was refused at load. pub fn isFailed(self: PluginManager, id: []const u8) bool { diff --git a/app/store/PluginStore.zig b/app/store/PluginStore.zig index e6335cfae..cbe3c7c19 100644 --- a/app/store/PluginStore.zig +++ b/app/store/PluginStore.zig @@ -23,6 +23,7 @@ const store = @import("registry/store.zig"); const Page = @import("PluginPage.zig"); const PluginLoader = @import("PluginLoader.zig"); const PluginManager = @import("PluginManager.zig"); +const plugin_folders = @import("plugin_folders.zig"); const App = @import("../App.zig"); /// Pretend two installed plugins have store updates waiting, so the update flow (the app's /// offer, or the silent path's logging) can be exercised without an actually out-of-date @@ -103,6 +104,9 @@ const Job = struct { win: *dvui.Window, err_buf: [64]u8 = undefined, err_len: usize = 0, + /// Refused before it started, and why (`refusal`): owned. Such a job never downloads; its card + /// shows the reason with a Dismiss, and the plugin's own controls stay below it. + refused: ?[]u8 = null, /// This job's download worker, as a cancelable `Io` task group rather than a detached /// `std.Thread`. The worker writes `err_buf`/`err_len`/`status` back into the `Job`, so the /// `Job` must not be freed while it is alive — and a detached thread gave `freeJob` no way to @@ -127,6 +131,8 @@ const PendingAction = union(enum) { set_enabled: struct { id: []u8, enabled: bool }, set_auto_update: struct { id: []u8, on: bool }, uninstall: struct { id: []u8 }, + /// A refused job's card dismissed (`Job.refused`). + dismiss: struct { id: []u8 }, }; var pending_actions: std.ArrayListUnmanaged(PendingAction) = .empty; @@ -782,6 +788,7 @@ pub fn deinit() void { .set_enabled => |a| app.gpa.free(a.id), .set_auto_update => |a| app.gpa.free(a.id), .uninstall => |a| app.gpa.free(a.id), + .dismiss => |a| app.gpa.free(a.id), }; pending_actions.deinit(app.gpa); clearPendingUpdates(); @@ -797,6 +804,7 @@ pub fn deinit() void { .set_enabled => |a| app.gpa.free(a.id), .set_auto_update => |a| app.gpa.free(a.id), .uninstall => |a| app.gpa.free(a.id), + .dismiss => |a| app.gpa.free(a.id), }; pending_actions.deinit(app.gpa); for (name_cache.keys()) |k| app.gpa.free(k); @@ -831,6 +839,7 @@ fn freeJob(job: *Job) void { app.gpa.free(job.url); app.gpa.free(job.sha256); app.gpa.free(job.dest); + if (job.refused) |r| app.gpa.free(r); app.gpa.destroy(job); } @@ -1073,6 +1082,16 @@ pub fn updateStatus(id: []const u8) ?[]const u8 { } pub fn applyAllPendingUpdates() void { + // One listing of `plugins/` for the whole pass (`refusal`). Unreadable, each install reads its + // own and is refused on its own. + const folders: ?plugin_folders.Folders = if (comptime builtin.target.cpu.arch == .wasm32) null else read: { + const plugins_dir = std.fs.path.join(app.gpa, &.{ app.config_folder, "plugins" }) catch break :read null; + defer app.gpa.free(plugins_dir); + break :read plugin_folders.read(app.gpa, dvui.io, plugins_dir) catch null; + }; + defer if (folders) |f| f.deinit(app.gpa); + pass_folders = if (folders) |*f| f else null; + defer pass_folders = null; // Index rather than pointer iteration: `applyPendingUpdate` writes back into the list. var i: usize = 0; while (i < pending_updates.items.len) : (i += 1) { @@ -1394,7 +1413,7 @@ pub fn tick() void { // Nobody asked for a quiet job, so it leaves no failed card to retry from. Log it, // drop the job, and put the Update button back on its row as Retry. if (job.quiet) { - dvui.log.warn("plugin updates: '{s}' could not be downloaded ({s})", .{ job.id, job.err_buf[0..job.err_len] }); + dvui.log.warn("plugin updates: '{s}' could not be downloaded ({s})", .{ job.id, job.refused orelse job.err_buf[0..job.err_len] }); markPendingUpdateFailed(job.id); jobs.swapRemoveAt(i); freeJob(job); @@ -1698,10 +1717,20 @@ fn startDownload(id: []const u8, release: store.ShardRelease, opts: DownloadOpti /// wasm *compile* error rather than dead code. fn startDownloadUrl(id: []const u8, url: []const u8, sha256: []const u8, opts: DownloadOptions) void { if (comptime builtin.target.cpu.arch == .wasm32) return; + removeJob(id); // replace any prior failed job // Before anything is written into `plugins/`: on macOS and Windows a case variant's // folder is that folder, and the download would overwrite the plugin installed there. - if (app.refuseCaseClash(id)) return; - removeJob(id); // replace any prior failed job + // Refused, the job fails at once and says why: on its card, or as the update offer marked + // failed for the automatic pass (`tick`), never left "Installing…". + var why_buf: [512]u8 = undefined; + if (refusal(&why_buf, id)) |why| { + var refused = opts; + refused.refused = why; + const job = buildJob(id, url, sha256, refused) catch return reportError("could not refuse the download for '{s}'", .{id}); + jobs.put(app.gpa, job.id, job) catch freeJob(job); + dvui.log.warn("plugin '{s}': not installed: {s}", .{ id, why }); + return; + } const job = buildJob(id, url, sha256, opts) catch { reportError("could not prepare download for '{s}'", .{id}); @@ -1720,6 +1749,28 @@ fn startDownloadUrl(id: []const u8, url: []const u8, sha256: []const u8, opts: D }; } +/// The listing of `plugins/` an automatic update pass reads once and asks about every plugin it +/// updates (`applyAllPendingUpdates`); null outside one, when each install reads its own. +var pass_folders: ?*const plugin_folders.Folders = null; + +/// Why installing `id` now must not go ahead, written into `buf`; null when it may. Another plugin +/// goes by `id` in another case, loaded or installed, or `plugins/` cannot be read to tell: on +/// macOS and Windows that other plugin's folder *is* `plugins/`. A name too long for the +/// message gets the message without it, never cut short. +fn refusal(buf: []u8, id: []const u8) ?[]const u8 { + if (app.host.pluginByIdIgnoringCase(id)) |p| if (!std.mem.eql(u8, p.id, id)) + return std.fmt.bufPrint(buf, "its id differs only in case from the loaded plugin '{s}'", .{p.id}) catch "its id differs only in case from a loaded plugin"; + const read: ?plugin_folders.Folders = if (pass_folders == null) fresh: { + const plugins_dir = std.fs.path.join(app.gpa, &.{ app.config_folder, "plugins" }) catch return "the plugins folder could not be read to check it"; + defer app.gpa.free(plugins_dir); + break :fresh plugin_folders.read(app.gpa, dvui.io, plugins_dir) catch return "the plugins folder could not be read to check it"; + } else null; + defer if (read) |f| f.deinit(app.gpa); + const folders = if (pass_folders) |f| f.* else read.?; + const other = plugin_folders.clash(folders.names, id) orelse return null; + return std.fmt.bufPrint(buf, "its id differs only in case from the installed plugin '{s}'", .{other}) catch "its id differs only in case from an installed plugin"; +} + /// Allocate a `Job` with all strings owned; `errdefer` unwinds every partial allocation so a /// mid-build OOM never leaks. pub const DownloadOptions = struct { @@ -1727,6 +1778,9 @@ pub const DownloadOptions = struct { is_update: bool, /// Suppress the error reporting and Retry card a user-initiated job leaves behind. quiet: bool = false, + /// Refused before it starts, and why (`refusal`): the job is built failed, and nothing is + /// made on disk. + refused: ?[]const u8 = null, }; fn buildJob(id: []const u8, url: []const u8, sha256: []const u8, opts: DownloadOptions) !*Job { @@ -1736,8 +1790,10 @@ fn buildJob(id: []const u8, url: []const u8, sha256: []const u8, opts: DownloadO defer a.free(plugins_dir); const plugin_dir = try std.fs.path.join(a, &.{ plugins_dir, id }); defer a.free(plugin_dir); - std.Io.Dir.createDirAbsolute(dvui.io, plugins_dir, .default_dir) catch {}; // best-effort; exists is fine - std.Io.Dir.createDirAbsolute(dvui.io, plugin_dir, .default_dir) catch {}; // best-effort; exists is fine + if (opts.refused == null) { + std.Io.Dir.createDirAbsolute(dvui.io, plugins_dir, .default_dir) catch {}; // best-effort; exists is fine + std.Io.Dir.createDirAbsolute(dvui.io, plugin_dir, .default_dir) catch {}; // best-effort; exists is fine + } const file_name = try PluginLoader.pluginFilename(id, a); defer a.free(file_name); @@ -1751,9 +1807,12 @@ fn buildJob(id: []const u8, url: []const u8, sha256: []const u8, opts: DownloadO errdefer a.free(sha_dup); const dest = try std.fs.path.join(a, &.{ plugin_dir, file_name }); errdefer a.free(dest); + const refused: ?[]u8 = if (opts.refused) |why| try a.dupe(u8, why) else null; + errdefer if (refused) |r| a.free(r); job.* = .{ - .status = .init(@backingInt(JobStatus.downloading)), + .status = .init(@backingInt(if (refused != null) JobStatus.failed else JobStatus.downloading)), + .refused = refused, .id = id_dup, .url = url_dup, .sha256 = sha_dup, @@ -3113,7 +3172,12 @@ fn drawCardControls(entry: StoreEntry) void { drawInstalling(if (job.is_update) "Updating…" else "Installing…", muted); return; }, - .failed => { + .failed => if (job.refused) |why| { + // Refused, not failed: Retry would only refuse again. Say why, offer to dismiss it, + // and leave the plugin's own controls (Uninstall among them) below. + dvui.labelNoFmt(@src(), why, .{}, .{ .gravity_y = 0.5, .color_text = .{ .color = theme.color(.err, .text) } }); + if (dvui.button(@src(), "Dismiss", .{}, .{ .gravity_y = 0.5 })) queueDismiss(entry.id); + } else { if (selectedRelease(entry)) |rel| { if (dvui.button(@src(), "Retry", .{}, .{ .gravity_y = 0.5, .color_text = .{ .color = theme.color(.err, .text) } })) startDownload(entry.id, rel, .{ .is_update = job.is_update }); @@ -3399,13 +3463,13 @@ fn removePendingForId(id: []const u8) void { .uninstall => |a| std.mem.eql(u8, a.id, id), // A different axis entirely: enabling or uninstalling a plugin must not silently // swallow a queued auto-update toggle for the same id. - .set_auto_update => false, + .set_auto_update, .dismiss => false, }; if (matches) { switch (action) { .set_enabled => |a| app.gpa.free(a.id), .uninstall => |a| app.gpa.free(a.id), - .set_auto_update => unreachable, + .set_auto_update, .dismiss => unreachable, } _ = pending_actions.orderedRemove(i); } else { @@ -3444,6 +3508,12 @@ fn queueSetAutoUpdate(id: []const u8, on: bool) void { }; } +/// Drop a refused job's card in `tick`, not mid-draw: its reason is what the card is drawing. +fn queueDismiss(id: []const u8) void { + const dup = app.gpa.dupe(u8, id) catch return; + pending_actions.append(app.gpa, .{ .dismiss = .{ .id = dup } }) catch app.gpa.free(dup); +} + fn queueUninstall(id: []const u8) void { removePendingForId(id); const dup = app.gpa.dupe(u8, id) catch { @@ -3473,9 +3543,15 @@ fn applyPendingActions() void { app.gpa.free(a.id); }, .uninstall => |a| { + // A refused job's card goes with the plugin it was refused beside. + removeJob(a.id); applyUninstall(a.id); app.gpa.free(a.id); }, + .dismiss => |a| { + removeJob(a.id); + app.gpa.free(a.id); + }, }; pending_actions.clearRetainingCapacity(); } diff --git a/app/store/plugin_folders.zig b/app/store/plugin_folders.zig new file mode 100644 index 000000000..f388354f4 --- /dev/null +++ b/app/store/plugin_folders.zig @@ -0,0 +1,145 @@ +//! The folders in `plugins/`, by the names the filesystem keeps for them, and the two questions a +//! plugin id asks of them before anything touches the disk: does another plugin's folder already +//! stand there in another case, and may uninstalling this id delete what is there. +//! +//! On macOS and Windows `plugins/Graphl` *is* `plugins/graphl`: writing or deleting one writes or +//! deletes the other. So the answers come from the listing the filesystem gives, read fresh at the +//! user's action, and anything that cannot be read answers the safe way: refuse the install, keep +//! the folder. +//! +//! std-only, so its rules are tested without a window (`build/app.zig`'s unit list). +const std = @import("std"); + +/// The names in `plugins/`, owned. +pub const Folders = struct { + names: []const []const u8, + + pub fn deinit(self: Folders, gpa: std.mem.Allocator) void { + for (self.names) |n| gpa.free(n); + gpa.free(self.names); + } +}; + +/// Read `plugins_dir`'s entries (folders and links to them). A folder not made yet holds nothing; +/// one that cannot be read is `error.Unreadable`, which a caller treats as a possible clash. +pub fn read(gpa: std.mem.Allocator, io: std.Io, plugins_dir: []const u8) error{ Unreadable, OutOfMemory }!Folders { + var names: std.ArrayListUnmanaged([]const u8) = .empty; + errdefer { + for (names.items) |n| gpa.free(n); + names.deinit(gpa); + } + var dir = std.Io.Dir.cwd().openDir(io, plugins_dir, .{ .iterate = true }) catch |err| switch (err) { + error.FileNotFound => return .{ .names = try names.toOwnedSlice(gpa) }, + else => return error.Unreadable, + }; + defer dir.close(io); + var it = dir.iterate(); + while (it.next(io) catch return error.Unreadable) |entry| { + if (entry.kind != .directory and entry.kind != .sym_link) continue; + const name = try gpa.dupe(u8, entry.name); + names.append(gpa, name) catch |err| { + gpa.free(name); + return err; + }; + } + return .{ .names = try names.toOwnedSlice(gpa) }; +} + +/// The name in `names` that is `id` in another case, borrowed from `names`: another plugin's +/// folder, which on a case-insensitive filesystem is `plugins/` itself. +pub fn clash(names: []const []const u8, id: []const u8) ?[]const u8 { + for (names) |n| { + if (!std.mem.eql(u8, n, id) and std.ascii.eqlIgnoreCase(n, id)) return n; + } + return null; +} + +/// What uninstalling `id` may delete in `plugins/`. +pub const Deletable = enum { + /// Nothing: no entry of exactly that name, a case variant stands beside it (on a + /// case-insensitive filesystem, the same folder as `plugins/`), the listing cannot be + /// read, or `id` is not a plain name. + keep, + /// Its own folder: delete the tree. + folder, + /// A link of exactly that name (a dev setup's): remove the link, never what it points at. + link, +}; + +/// Whether `plugins/` is `id`'s alone to delete, by the names listed now: an entry of exactly +/// that name, and no other entry `id` in another case. On a case-insensitive filesystem that +/// second rule is what keeps an installed plugin, loaded or not, from going with a refused +/// variant's card. +pub fn deletable(io: std.Io, plugins_dir: []const u8, id: []const u8) Deletable { + if (!plainName(id)) return .keep; + var dir = std.Io.Dir.cwd().openDir(io, plugins_dir, .{ .iterate = true }) catch return .keep; + defer dir.close(io); + var own: ?std.Io.File.Kind = null; + var it = dir.iterate(); + while (it.next(io) catch return .keep) |entry| { + if (std.mem.eql(u8, entry.name, id)) { + own = entry.kind; + } else if (std.ascii.eqlIgnoreCase(entry.name, id)) return .keep; + } + return switch (own orelse return .keep) { + .directory => .folder, + .sym_link => .link, + else => .keep, + }; +} + +/// A name that stays inside `plugins/`: no separators, not `.` or `..`, not empty. +fn plainName(id: []const u8) bool { + if (id.len == 0 or std.mem.eql(u8, id, ".") or std.mem.eql(u8, id, "..")) return false; + for (id) |c| if (c == '/' or c == '\\' or c == 0) return false; + return true; +} + +test "a name in another case clashes, the same name does not" { + const names: []const []const u8 = &.{ "pixi", "Graphl" }; + try std.testing.expectEqualStrings("Graphl", clash(names, "graphl").?); + try std.testing.expect(clash(names, "Graphl") == null); + try std.testing.expect(clash(names, "atlas") == null); +} + +test "uninstall deletes only an entry of exactly the name, with no variant beside it" { + var tmp = std.testing.tmpDir(.{}); + defer tmp.cleanup(); + const io = std.testing.io; + var buf: [std.fs.max_path_bytes]u8 = undefined; + const root = buf[0..try tmp.dir.realPath(io, &buf)]; + + try tmp.dir.createDirPath(io, "solo"); + try tmp.dir.createDirPath(io, "Graphl"); + try std.testing.expectEqual(Deletable.folder, deletable(io, root, "solo")); + // A variant stands there: on this filesystem or not, never deleted for `graphl`. + try std.testing.expectEqual(Deletable.keep, deletable(io, root, "graphl")); + try std.testing.expectEqual(Deletable.folder, deletable(io, root, "Graphl")); + try std.testing.expectEqual(Deletable.keep, deletable(io, root, "absent")); + try std.testing.expectEqual(Deletable.keep, deletable(io, root, "..")); + try std.testing.expectEqual(Deletable.keep, deletable(io, root, "a/b")); + // A folder that cannot be read keeps everything. + try std.testing.expectEqual(Deletable.keep, deletable(io, "/nonexistent/plugins", "solo")); + + // A link of exactly the name is the link to remove, never what it points at. + tmp.dir.symLink(io, "solo", "linked", .{ .is_directory = true }) catch return error.SkipZigTest; + try std.testing.expectEqual(Deletable.link, deletable(io, root, "linked")); +} + +test "a listing reads folders and links, and a folder not made yet holds nothing" { + var tmp = std.testing.tmpDir(.{}); + defer tmp.cleanup(); + const io = std.testing.io; + var buf: [std.fs.max_path_bytes]u8 = undefined; + const root = buf[0..try tmp.dir.realPath(io, &buf)]; + try tmp.dir.createDirPath(io, "pixi"); + try tmp.dir.writeFile(io, .{ .sub_path = "stray.txt", .data = "" }); + const folders = try read(std.testing.allocator, io, root); + defer folders.deinit(std.testing.allocator); + try std.testing.expectEqual(@as(usize, 1), folders.names.len); + try std.testing.expectEqualStrings("pixi", folders.names[0]); + + const none = try read(std.testing.allocator, io, "/nonexistent/plugins/at/all"); + defer none.deinit(std.testing.allocator); + try std.testing.expectEqual(@as(usize, 0), none.names.len); +} diff --git a/build/app.zig b/build/app.zig index bbf87a987..9a8c95194 100644 --- a/build/app.zig +++ b/build/app.zig @@ -829,6 +829,9 @@ pub fn construct(b: *std.Build, target: std.Build.ResolvedTarget, optimize: std. // The command palette's recently used commands: order, re-use, the cap. std-only // (see RecentCommands.zig); `Recents` holds it and stores it in recents.zon. .{ "fizzy-recent-commands-tests", "app/RecentCommands.zig" }, + // Which plugin folders a case variant clashes with, and what an uninstall may delete + // there. std-only (see plugin_folders.zig); the store and the loader ask it. + .{ "fizzy-plugin-folders-tests", "app/store/plugin_folders.zig" }, // Where a floating view's window opens, what keeps it reachable, its name, the floats' // stacking and where a closed float's views go. std-only (see float_rules.zig); // `Floats.zig` and `ViewDrag.zig` apply it, covered by `tests/integration.zig`. diff --git a/src/editor/Editor.zig b/src/editor/Editor.zig index c94fba659..3bb765054 100644 --- a/src/editor/Editor.zig +++ b/src/editor/Editor.zig @@ -689,6 +689,18 @@ pub const view_settings = "fizzy.settings"; /// Stable workbench sidebar view id (matches `workbench.view_files`). pub const workbench_files_view = workbench_mod.view_files; +/// A hash-map context that takes two ids for one when they differ only in ASCII case. +const CaseInsensitive = struct { + pub fn hash(_: CaseInsensitive, key: []const u8) u64 { + var h = std.hash.Wyhash.init(0); + for (key) |c| h.update(&.{std.ascii.toLower(c)}); + return h.final(); + } + pub fn eql(_: CaseInsensitive, a: []const u8, b: []const u8) bool { + return std.ascii.eqlIgnoreCase(a, b); + } +}; + pub fn loadUserPlugins(editor: *Editor, config_folder: []const u8) void { if (comptime builtin.target.cpu.arch == .wasm32 or !App.loads_plugins) return; @@ -737,25 +749,33 @@ pub fn loadUserPlugins(editor: *Editor, config_folder: []const u8) void { } }.lt); + // The folders by their names ignoring case, in sorted order, made once: a group of more than + // one is the same id several times. + var case_groups: std.HashMapUnmanaged([]const u8, std.ArrayListUnmanaged([]const u8), CaseInsensitive, std.hash_map.default_max_load_percentage) = .empty; + for (ids.items) |id| { + const entry = case_groups.getOrPut(editor.app.arena.allocator(), id) catch continue; + if (!entry.found_existing) entry.value_ptr.* = .empty; + entry.value_ptr.append(editor.app.arena.allocator(), id) catch {}; + } + for (ids.items) |plugin_id| { // Skip `.load-tmp` and any other dotfile/directory — not a plugin id. if (plugin_id.len == 0 or plugin_id[0] == '.') continue; - // Two folders whose names differ only in case (a case-sensitive filesystem lets both - // exist) are one id twice: neither loads. Which one was meant cannot be told from the - // folders, and loading whichever sorts first would let byte order choose. One card, on - // the first by that order, names them both. - const twin: ?[]const u8 = for (ids.items) |other| { - if (other.ptr != plugin_id.ptr and std.ascii.eqlIgnoreCase(other, plugin_id)) break other; - } else null; - if (twin) |other| { - if (std.mem.lessThan(u8, plugin_id, other)) { - const why = std.fmt.allocPrint(editor.app.arena.allocator(), "plugin folders '{s}' and '{s}' differ only in case, so neither is loaded: remove one", .{ plugin_id, other }) catch "two plugin folders differ only in case, so neither is loaded"; + // Folders whose names differ only in case (a case-sensitive filesystem lets them all + // exist) are one id several times: none loads. Which one was meant cannot be told from + // the folders, and loading whichever sorts first would let byte order choose. One card, + // on the first by that order, names every one (`case_groups`, made in one pass above). + if (case_groups.get(plugin_id)) |group| if (group.items.len > 1) { + if (group.items[0].ptr == plugin_id.ptr) { + var names: std.Io.Writer.Allocating = .init(editor.app.arena.allocator()); + for (group.items, 0..) |name, i| names.writer.print("{s}'{s}'", .{ @as([]const u8, if (i > 0) ", " else ""), name }) catch {}; + const why = std.fmt.allocPrint(editor.app.arena.allocator(), "plugin folders {s} differ only in case, so none is loaded: keep one", .{names.written()}) catch "plugin folders differ only in case, so none is loaded"; dvui.log.warn("user plugin '{s}': {s}", .{ plugin_id, why }); editor.app.recordPluginFailure(plugin_id, why, null, null, .{}); } continue; - } + }; // User-disabled plugins (store "disable") stay on disk but are not loaded. if (editor.app.isPluginDisabled(plugin_id)) { @@ -1462,9 +1482,13 @@ pub fn loadUserPluginById(editor: *Editor, id: []const u8) !void { if (comptime builtin.target.cpu.arch == .wasm32 or !App.loads_plugins) return error.NotUnloadable; const path = try App.userPluginPath(editor.app.gpa, &editor.app, id); // Asked before the build is opened: a case variant of a loaded or installed id fails saying so. - var clash_buf: [64]u8 = undefined; - if (editor.app.pluginCaseClash(&clash_buf, id)) |other| { - editor.app.recordCaseClash(id, other, path); + var clash_buf: [std.fs.max_name_bytes]u8 = undefined; + if (editor.app.pluginCaseClash(&clash_buf, id)) |clash| { + switch (clash) { + .other => |other| editor.app.recordCaseClash(id, other, path), + // Refused as a possible clash: nothing says the folder is this plugin's alone. + .unreadable => editor.app.recordPluginFailure(id, "the plugins folder could not be read to check for a plugin whose id differs only in case", null, null, .of(path)), + } editor.app.gpa.free(path); return error.DuplicatePluginId; } @@ -2072,16 +2096,18 @@ pub fn uninstallPlugin(editor: *Editor, id: []const u8, force: bool) !void { const plugin_dir = try std.fs.path.join(editor.app.gpa, &.{ editor.app.config_folder, "plugins", id }); defer editor.app.gpa.free(plugin_dir); // A build refused for its case (`App.recordCaseClash`) names a folder that, on macOS and - // Windows, is the loaded plugin's own: its card drops, and that folder stays. Told apart by - // what the folder is on disk, not by its name. - const keep = if (editor.app.host.pluginByIdIgnoringCase(id)) |loaded| keep: { - if (std.mem.eql(u8, loaded.id, id)) break :keep false; - const loaded_dir = try std.fs.path.join(editor.app.gpa, &.{ editor.app.config_folder, "plugins", loaded.id }); - defer editor.app.gpa.free(loaded_dir); - break :keep App.sameFolder(plugin_dir, loaded_dir); - } else false; - if (!keep) std.Io.Dir.cwd().deleteTree(dvui.io, plugin_dir) catch |err| - dvui.log.warn("uninstallPlugin '{s}': could not delete {s}: {s}", .{ id, plugin_dir, @errorName(err) }); + // Windows, is another installed plugin's own, loaded or not: its card drops, and that folder + // stays. Told apart by the names the filesystem lists now, never by the name asked for. + const plugins_dir = try std.fs.path.join(editor.app.gpa, &.{ editor.app.config_folder, "plugins" }); + defer editor.app.gpa.free(plugins_dir); + switch (@import("app").store.folders.deletable(dvui.io, plugins_dir, id)) { + .folder => std.Io.Dir.cwd().deleteTree(dvui.io, plugin_dir) catch |err| + dvui.log.warn("uninstallPlugin '{s}': could not delete {s}: {s}", .{ id, plugin_dir, @errorName(err) }), + // A dev setup's link: the link goes, never what it points at. + .link => std.Io.Dir.cwd().deleteFile(dvui.io, plugin_dir) catch |err| + dvui.log.warn("uninstallPlugin '{s}': could not remove the link {s}: {s}", .{ id, plugin_dir, @errorName(err) }), + .keep => dvui.log.info("uninstallPlugin '{s}': {s} is not this plugin's own folder alone; kept", .{ id, plugin_dir }), + } // A broken (failed-to-load) build can be uninstalled too; clear its failure record so the // card disappears instead of lingering as "Failed". editor.app.clearFailedUserPlugin(id); @@ -7098,15 +7124,6 @@ const plugin_manager_vtable: PluginManager.VTable = .{ pmSelf(ctx).revealCenter(); } }.f, - .refuseCaseClash = struct { - fn f(ctx: *anyopaque, id: []const u8) bool { - const app = &pmSelf(ctx).app; - var buf: [64]u8 = undefined; - const other = app.pluginCaseClash(&buf, id) orelse return false; - app.recordCaseClash(id, other, null); - return true; - } - }.f, }; pub fn pluginManager(editor: *Editor) PluginManager { diff --git a/tests/integration.zig b/tests/integration.zig index dc36c1b40..cd8f0bf39 100644 --- a/tests/integration.zig +++ b/tests/integration.zig @@ -7673,14 +7673,30 @@ test "headless: the whole editor comes up, opens a file, plays the tour and goes _ = try tmp.dir.statFile(dvui.io, "plugins/text/keep", .{}); try tmp.dir.deleteTree(dvui.io, "plugins/text"); } - // A folder already in `plugins/` in another case clashes too, before a store install - // writes anything. + // A folder already in `plugins/` in another case clashes too, read fresh at each ask: an + // installed plugin that is not loaded. { + var clash_buf: [std.fs.max_name_bytes]u8 = undefined; + try std.testing.expect(editor.app.pluginCaseClash(&clash_buf, "graphl") == null); try tmp.dir.createDirPath(dvui.io, "plugins/Graphl"); + try tmp.dir.writeFile(dvui.io, .{ .sub_path = "plugins/Graphl/keep", .data = "" }); defer tmp.dir.deleteTree(dvui.io, "plugins/Graphl") catch {}; - var clash_buf: [64]u8 = undefined; - try std.testing.expectEqualStrings("Graphl", editor.app.pluginCaseClash(&clash_buf, "graphl") orelse return error.TestUnexpectedResult); + const found = editor.app.pluginCaseClash(&clash_buf, "graphl") orelse return error.TestUnexpectedResult; + try std.testing.expectEqualStrings("Graphl", found.other); try std.testing.expect(editor.app.pluginCaseClash(&clash_buf, "Graphl") == null); + // Its refused variant's card, uninstalled, never takes the installed plugin's folder, + // though on macOS and Windows `plugins/graphl` is that folder. + editor.app.recordCaseClash("graphl", "Graphl", null); + try editor.uninstallPlugin("graphl", true); + _ = try tmp.dir.statFile(dvui.io, "plugins/Graphl/keep", .{}); + try std.testing.expectEqual(sdk.services.plugins.Api.State.not_loaded, editor.app.pluginStatus("graphl").state); + } + // A plugin's own folder, alone under its name, is still what uninstalling deletes. + { + try tmp.dir.createDirPath(dvui.io, "plugins/solo"); + try tmp.dir.writeFile(dvui.io, .{ .sub_path = "plugins/solo/build", .data = "" }); + try editor.uninstallPlugin("solo", true); + try std.testing.expectError(error.FileNotFound, tmp.dir.statFile(dvui.io, "plugins/solo", .{})); } } // The app offers its plugins the `automation` service, and nothing is in flight.