From f812b3699a15b76306c055cdf6f8d8b9663089a9 Mon Sep 17 00:00:00 2001 From: foxnne Date: Sun, 18 Oct 2026 15:46:20 -0500 Subject: [PATCH 1/2] store: a plugin id that differs only in case is refused safely Uninstall deletes plugins/ only when the filesystem lists a folder of exactly that name that no other entry shares an inode with; anything else (a case variant standing there, an alias, an unreadable listing, an id that is not a plain name) keeps the folder and drops only the card. On macOS this stopped a refused build's card from deleting an installed, unloaded plugin's folder. A refused store install or update becomes a failed job (Retry, or the update offer marked failed) instead of a row left started; plugins/ is read once per session for the clash check and again after an install or uninstall; the clash names come back borrowed, never truncated; and one card names every case variant of a folder. Part of #530 Co-Authored-By: Claude Opus 5.5 --- app/App.zig | 94 ++++++++++++++++++++++++++++----------- app/store/PluginStore.zig | 37 ++++++++++++++- src/editor/Editor.zig | 52 ++++++++++++---------- tests/integration.zig | 26 +++++++++-- 4 files changed, 154 insertions(+), 55 deletions(-) diff --git a/app/App.zig b/app/App.zig index 15cd17ef5..a0dc61031 100644 --- a/app/App.zig +++ b/app/App.zig @@ -81,6 +81,9 @@ palette_folder: []const u8, /// `plugin_data_lock`: a plugin may ask from a worker thread (an indexer, an LSP client). plugin_data_folders: std.StringHashMapUnmanaged(PluginDataFolder) = .empty, plugin_data_lock: std.Io.Mutex = .init, +/// The folders in `plugins/` by the names the filesystem keeps (`pluginCaseClash`), read on first +/// need and kept until something is installed or removed (`forgetPluginFolders`). Owned. +plugin_folders: ?std.ArrayListUnmanaged([]u8) = null, /// Registered plugins whose entry could not be made when they registered (out of memory), by the /// plugin's own id (its memory, good while it is registered): made on their next ask instead. plugin_data_missed: [8]?[]const u8 = @splat(null), @@ -960,32 +963,83 @@ pub fn deinit(app: *App) void { app.undecided_plugin_ids.deinit(app.gpa); for (app.auto_update_off_ids.items) |id| app.gpa.free(id); app.auto_update_off_ids.deinit(app.gpa); + app.forgetPluginFolders(); } /// 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]; - }; - 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; +/// *is* `plugins/graphl`, so writing one would overwrite the other. Borrowed: the loaded plugin's +/// id, or a name in `plugin_folders`, until `forgetPluginFolders`. +pub fn pluginCaseClash(app: *App, id: []const u8) ?[]const u8 { + if (app.host.pluginByIdIgnoringCase(id)) |p| if (!std.mem.eql(u8, p.id, id)) return p.id; + for (app.pluginFolders()) |name| { + if (!std.mem.eql(u8, name, id) and std.ascii.eqlIgnoreCase(name, id)) return name; + } + return null; +} + +/// The folders in `plugins/`, by the names the filesystem keeps for them: read once and kept +/// (`plugin_folders`), so asking about a clash for every plugin in an update pass reads the disk +/// once, not once each. Empty on the web, or when the folder can't be read. +fn pluginFolders(app: *App) []const []const u8 { + if (app.plugin_folders) |names| return names.items; + var names: std.ArrayListUnmanaged([]u8) = .empty; + defer app.plugin_folders = names; + if (comptime builtin.target.cpu.arch == .wasm32) return &.{}; + const plugins_dir = std.fs.path.join(app.gpa, &.{ app.config_folder, "plugins" }) catch return &.{}; defer app.gpa.free(plugins_dir); - var dir = std.Io.Dir.cwd().openDir(dvui.io, plugins_dir, .{ .iterate = true }) catch return null; + var dir = std.Io.Dir.cwd().openDir(dvui.io, plugins_dir, .{ .iterate = true }) catch return &.{}; 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]; + const name = app.gpa.dupe(u8, entry.name) catch continue; + names.append(app.gpa, name) catch app.gpa.free(name); } - return null; + return names.items; +} + +/// Read `plugins/` again on the next ask: something was installed or removed there. +pub fn forgetPluginFolders(app: *App) void { + const names = app.plugin_folders orelse return; + for (names.items) |name| app.gpa.free(name); + var list = names; + list.deinit(app.gpa); + app.plugin_folders = null; +} + +/// Whether uninstalling `id` may delete `plugins/`: only when it is a folder of exactly that +/// name and no other plugin's folder is the same one. On macOS and Windows `plugins/graphl` *is* +/// `plugins/Graphl`, so a refused build's card (`recordCaseClash`) must not take the installed +/// plugin with it, whether that plugin is loaded or not. +/// +/// Decided by what the filesystem lists, read now: the names it keeps, and each folder's inode. A +/// name not listed exactly (a case variant stands there), or another folder with the same inode, +/// keeps it. Every entry is in the one `plugins/` folder, so the inodes are on one device unless +/// a plugin folder is itself a mount point, where a coincidence only keeps a folder, never deletes +/// one. Anything that cannot be read keeps it too. +pub fn pluginFolderDeletable(app: *App, id: []const u8) bool { + if (comptime builtin.target.cpu.arch == .wasm32) return false; + // An id that is not a plain name could reach outside `plugins/`. + if (!isValidPluginId(id)) return false; + const plugins_dir = std.fs.path.join(app.gpa, &.{ app.config_folder, "plugins" }) catch return false; + defer app.gpa.free(plugins_dir); + var dir = std.Io.Dir.cwd().openDir(dvui.io, plugins_dir, .{ .iterate = true }) catch return false; + defer dir.close(dvui.io); + var mine: ?std.Io.File.INode = null; + var others: std.ArrayListUnmanaged(std.Io.File.INode) = .empty; + defer others.deinit(app.gpa); + var it = dir.iterate(); + while (it.next(dvui.io) catch return false) |entry| { + if (std.mem.eql(u8, entry.name, id)) { + if (entry.kind != .directory) return false; + mine = entry.inode; + } else others.append(app.gpa, entry.inode) catch return false; + } + const own = mine orelse return false; + for (others.items) |inode| if (inode == own) return false; + return true; } /// The card for a build of `id` refused because `other`, the same id in another case, is loaded or @@ -1000,16 +1054,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/store/PluginStore.zig b/app/store/PluginStore.zig index e6335cfae..1480c8417 100644 --- a/app/store/PluginStore.zig +++ b/app/store/PluginStore.zig @@ -1698,10 +1698,12 @@ 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 row fails as a failed download does: Retry on a card, or the update offer + // marked failed for the automatic pass (`tick`), never left "Installing…". + if (app.refuseCaseClash(id)) return refuseDownload(id, url, sha256, opts); const job = buildJob(id, url, sha256, opts) catch { reportError("could not prepare download for '{s}'", .{id}); @@ -1720,6 +1722,37 @@ fn startDownloadUrl(id: []const u8, url: []const u8, sha256: []const u8, opts: D }; } +/// A download refused before it started (`PluginManager.refuseCaseClash`): a job already failed, +/// with no worker and nothing on disk, so the row shows it as `tick` and `drawCardControls` show +/// any failed download. +fn refuseDownload(id: []const u8, url: []const u8, sha256: []const u8, opts: DownloadOptions) void { + const a = app.gpa; + const job = a.create(Job) catch return; + const reason = "id differs only in case"; + job.* = .{ + .status = .init(@backingInt(JobStatus.failed)), + .id = a.dupe(u8, id) catch return a.destroy(job), + .url = a.dupe(u8, url) catch return freeRefused(job, 1), + .sha256 = a.dupe(u8, sha256) catch return freeRefused(job, 2), + .dest = a.dupe(u8, "") catch return freeRefused(job, 3), + .is_update = opts.is_update, + .quiet = opts.quiet, + .win = dvui.currentWindow(), + }; + @memcpy(job.err_buf[0..reason.len], reason); + job.err_len = reason.len; + jobs.put(a, job.id, job) catch freeJob(job); +} + +/// Unwind a `refuseDownload` that ran out of memory after its first `n` strings. +fn freeRefused(job: *Job, n: u2) void { + const a = app.gpa; + if (n > 0) a.free(job.id); + if (n > 1) a.free(job.url); + if (n > 2) a.free(job.sha256); + a.destroy(job); +} + /// Allocate a `Job` with all strings owned; `errdefer` unwinds every partial allocation so a /// mid-build OOM never leaks. pub const DownloadOptions = struct { diff --git a/src/editor/Editor.zig b/src/editor/Editor.zig index c94fba659..d4f97046d 100644 --- a/src/editor/Editor.zig +++ b/src/editor/Editor.zig @@ -741,16 +741,23 @@ pub fn loadUserPlugins(editor: *Editor, config_folder: []const u8) void { // 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. + const twins = for (ids.items) |other| { + if (other.ptr != plugin_id.ptr and std.ascii.eqlIgnoreCase(other, plugin_id)) break true; + } else false; + if (twins) { + const first = for (ids.items) |other| { + if (std.ascii.eqlIgnoreCase(other, plugin_id)) break other; + } else plugin_id; + if (first.ptr == plugin_id.ptr) { + var names: std.Io.Writer.Allocating = .init(editor.app.arena.allocator()); + for (ids.items) |other| if (std.ascii.eqlIgnoreCase(other, plugin_id)) { + names.writer.print("{s}'{s}'", .{ if (names.written().len > 0) ", " else "", other }) 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, .{}); } @@ -1462,8 +1469,7 @@ 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| { + if (editor.app.pluginCaseClash(id)) |other| { editor.app.recordCaseClash(id, other, path); editor.app.gpa.free(path); return error.DuplicatePluginId; @@ -1810,6 +1816,8 @@ fn takeWebFileTypePrompt(gpa: std.mem.Allocator, id: []const u8) bool { pub fn installAndLoadPlugin(editor: *Editor, id: []const u8) !void { if (isBundledPluginId(id)) return error.NotUnloadable; if (editor.app.host.pluginById(id) != null) return; // already loaded + // The download just made `plugins/`. + editor.app.forgetPluginFolders(); editor.app.untrackDisabledPlugin(id); try editor.setPluginEnabledPersisted(id, true); try editor.loadUserPluginById(id); @@ -2072,16 +2080,13 @@ 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 what the filesystem lists, never by the name asked for. + if (editor.app.pluginFolderDeletable(id)) { + 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) }); + } else dvui.log.info("uninstallPlugin '{s}': {s} is not this plugin's own folder alone; kept", .{ id, plugin_dir }); + editor.app.forgetPluginFolders(); // 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); @@ -7101,8 +7106,7 @@ const plugin_manager_vtable: PluginManager.VTable = .{ .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; + const other = app.pluginCaseClash(id) orelse return false; app.recordCaseClash(id, other, null); return true; } diff --git a/tests/integration.zig b/tests/integration.zig index dc36c1b40..6cfe6e71e 100644 --- a/tests/integration.zig +++ b/tests/integration.zig @@ -7674,13 +7674,31 @@ test "headless: the whole editor comes up, opens a file, plays the tour and goes try tmp.dir.deleteTree(dvui.io, "plugins/text"); } // A folder already in `plugins/` in another case clashes too, before a store install - // writes anything. + // writes anything: an installed plugin that is not loaded. { + // `plugins/` is read once until something is installed or removed: a folder made + // behind the app's back after that read is not seen until then. + try std.testing.expect(editor.app.pluginCaseClash("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); - try std.testing.expect(editor.app.pluginCaseClash(&clash_buf, "Graphl") == null); + try std.testing.expect(editor.app.pluginCaseClash("graphl") == null); + editor.app.forgetPluginFolders(); + try std.testing.expectEqualStrings("Graphl", editor.app.pluginCaseClash("graphl") orelse return error.TestUnexpectedResult); + try std.testing.expect(editor.app.pluginCaseClash("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. From ab71c3a20b107c36fcff9994dfb05774b4758ee1 Mon Sep 17 00:00:00 2001 From: foxnne Date: Sun, 18 Oct 2026 16:13:02 -0500 Subject: [PATCH 2/2] store: case clashes read plugins/ fresh and fail closed Review of the case-clash fix. Uninstall deletes plugins/ only when an entry of exactly that name exists and no other entry is the id in another case (store.folders.deletable, a new std-only file with unit tests); an exact-name symlink loses only the link. No inode rule: Zig fills Windows directory-entry inodes from a FileIndex NTFS leaves 0. The session cache is gone: each install, update, uninstall and load reads plugins/ fresh, the automatic update pass reads it once for the pass, and an unreadable listing refuses the install and keeps the folder. A refusal is built through buildJob with its reason, shown on the card with a Dismiss beside the plugin's own controls; startup groups case variants in one pass. Part of #530 Co-Authored-By: Claude Opus 5.5 --- app/App.zig | 105 ++++++++----------------- app/root.zig | 2 + app/store/PluginManager.zig | 9 --- app/store/PluginStore.zig | 121 +++++++++++++++++++---------- app/store/plugin_folders.zig | 145 +++++++++++++++++++++++++++++++++++ build/app.zig | 3 + src/editor/Editor.zig | 75 ++++++++++-------- tests/integration.zig | 16 ++-- 8 files changed, 314 insertions(+), 162 deletions(-) create mode 100644 app/store/plugin_folders.zig diff --git a/app/App.zig b/app/App.zig index a0dc61031..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; @@ -81,9 +82,6 @@ palette_folder: []const u8, /// `plugin_data_lock`: a plugin may ask from a worker thread (an indexer, an LSP client). plugin_data_folders: std.StringHashMapUnmanaged(PluginDataFolder) = .empty, plugin_data_lock: std.Io.Mutex = .init, -/// The folders in `plugins/` by the names the filesystem keeps (`pluginCaseClash`), read on first -/// need and kept until something is installed or removed (`forgetPluginFolders`). Owned. -plugin_folders: ?std.ArrayListUnmanaged([]u8) = null, /// Registered plugins whose entry could not be made when they registered (out of memory), by the /// plugin's own id (its memory, good while it is registered): made on their next ask instead. plugin_data_missed: [8]?[]const u8 = @splat(null), @@ -963,83 +961,42 @@ pub fn deinit(app: *App) void { app.undecided_plugin_ids.deinit(app.gpa); for (app.auto_update_off_ids.items) |id| app.gpa.free(id); app.auto_update_off_ids.deinit(app.gpa); - app.forgetPluginFolders(); -} - -/// 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. Borrowed: the loaded plugin's -/// id, or a name in `plugin_folders`, until `forgetPluginFolders`. -pub fn pluginCaseClash(app: *App, id: []const u8) ?[]const u8 { - if (app.host.pluginByIdIgnoringCase(id)) |p| if (!std.mem.eql(u8, p.id, id)) return p.id; - for (app.pluginFolders()) |name| { - if (!std.mem.eql(u8, name, id) and std.ascii.eqlIgnoreCase(name, id)) return name; - } - return null; } -/// The folders in `plugins/`, by the names the filesystem keeps for them: read once and kept -/// (`plugin_folders`), so asking about a clash for every plugin in an update pass reads the disk -/// once, not once each. Empty on the web, or when the folder can't be read. -fn pluginFolders(app: *App) []const []const u8 { - if (app.plugin_folders) |names| return names.items; - var names: std.ArrayListUnmanaged([]u8) = .empty; - defer app.plugin_folders = names; - if (comptime builtin.target.cpu.arch == .wasm32) return &.{}; - const plugins_dir = std.fs.path.join(app.gpa, &.{ app.config_folder, "plugins" }) catch return &.{}; - defer app.gpa.free(plugins_dir); - var dir = std.Io.Dir.cwd().openDir(dvui.io, plugins_dir, .{ .iterate = true }) catch return &.{}; - defer dir.close(dvui.io); - var it = dir.iterate(); - while (it.next(dvui.io) catch null) |entry| { - if (entry.kind != .directory) continue; - const name = app.gpa.dupe(u8, entry.name) catch continue; - names.append(app.gpa, name) catch app.gpa.free(name); - } - return names.items; -} +/// 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, +}; -/// Read `plugins/` again on the next ask: something was installed or removed there. -pub fn forgetPluginFolders(app: *App) void { - const names = app.plugin_folders orelse return; - for (names.items) |name| app.gpa.free(name); - var list = names; - list.deinit(app.gpa); - app.plugin_folders = null; +/// `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 }; } -/// Whether uninstalling `id` may delete `plugins/`: only when it is a folder of exactly that -/// name and no other plugin's folder is the same one. On macOS and Windows `plugins/graphl` *is* -/// `plugins/Graphl`, so a refused build's card (`recordCaseClash`) must not take the installed -/// plugin with it, whether that plugin is loaded or not. -/// -/// Decided by what the filesystem lists, read now: the names it keeps, and each folder's inode. A -/// name not listed exactly (a case variant stands there), or another folder with the same inode, -/// keeps it. Every entry is in the one `plugins/` folder, so the inodes are on one device unless -/// a plugin folder is itself a mount point, where a coincidence only keeps a folder, never deletes -/// one. Anything that cannot be read keeps it too. -pub fn pluginFolderDeletable(app: *App, id: []const u8) bool { - if (comptime builtin.target.cpu.arch == .wasm32) return false; - // An id that is not a plain name could reach outside `plugins/`. - if (!isValidPluginId(id)) return false; - const plugins_dir = std.fs.path.join(app.gpa, &.{ app.config_folder, "plugins" }) catch return false; +/// 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 .unreadable; defer app.gpa.free(plugins_dir); - var dir = std.Io.Dir.cwd().openDir(dvui.io, plugins_dir, .{ .iterate = true }) catch return false; - defer dir.close(dvui.io); - var mine: ?std.Io.File.INode = null; - var others: std.ArrayListUnmanaged(std.Io.File.INode) = .empty; - defer others.deinit(app.gpa); - var it = dir.iterate(); - while (it.next(dvui.io) catch return false) |entry| { - if (std.mem.eql(u8, entry.name, id)) { - if (entry.kind != .directory) return false; - mine = entry.inode; - } else others.append(app.gpa, entry.inode) catch return false; - } - const own = mine orelse return false; - for (others.items) |inode| if (inode == own) return false; - return true; + 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 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 1480c8417..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); @@ -1701,9 +1720,17 @@ fn startDownloadUrl(id: []const u8, url: []const u8, sha256: []const u8, opts: D 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. - // Refused, the row fails as a failed download does: Retry on a card, or the update offer - // marked failed for the automatic pass (`tick`), never left "Installing…". - if (app.refuseCaseClash(id)) return refuseDownload(id, url, sha256, opts); + // 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}); @@ -1722,35 +1749,26 @@ fn startDownloadUrl(id: []const u8, url: []const u8, sha256: []const u8, opts: D }; } -/// A download refused before it started (`PluginManager.refuseCaseClash`): a job already failed, -/// with no worker and nothing on disk, so the row shows it as `tick` and `drawCardControls` show -/// any failed download. -fn refuseDownload(id: []const u8, url: []const u8, sha256: []const u8, opts: DownloadOptions) void { - const a = app.gpa; - const job = a.create(Job) catch return; - const reason = "id differs only in case"; - job.* = .{ - .status = .init(@backingInt(JobStatus.failed)), - .id = a.dupe(u8, id) catch return a.destroy(job), - .url = a.dupe(u8, url) catch return freeRefused(job, 1), - .sha256 = a.dupe(u8, sha256) catch return freeRefused(job, 2), - .dest = a.dupe(u8, "") catch return freeRefused(job, 3), - .is_update = opts.is_update, - .quiet = opts.quiet, - .win = dvui.currentWindow(), - }; - @memcpy(job.err_buf[0..reason.len], reason); - job.err_len = reason.len; - jobs.put(a, job.id, job) catch freeJob(job); -} - -/// Unwind a `refuseDownload` that ran out of memory after its first `n` strings. -fn freeRefused(job: *Job, n: u2) void { - const a = app.gpa; - if (n > 0) a.free(job.id); - if (n > 1) a.free(job.url); - if (n > 2) a.free(job.sha256); - a.destroy(job); +/// 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 @@ -1760,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 { @@ -1769,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); @@ -1784,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, @@ -3146,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 }); @@ -3432,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 { @@ -3477,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 { @@ -3506,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 d4f97046d..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,6 +749,15 @@ 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; @@ -744,25 +765,17 @@ pub fn loadUserPlugins(editor: *Editor, config_folder: []const u8) void { // 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. - const twins = for (ids.items) |other| { - if (other.ptr != plugin_id.ptr and std.ascii.eqlIgnoreCase(other, plugin_id)) break true; - } else false; - if (twins) { - const first = for (ids.items) |other| { - if (std.ascii.eqlIgnoreCase(other, plugin_id)) break other; - } else plugin_id; - if (first.ptr == plugin_id.ptr) { + // 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 (ids.items) |other| if (std.ascii.eqlIgnoreCase(other, plugin_id)) { - names.writer.print("{s}'{s}'", .{ if (names.written().len > 0) ", " else "", other }) catch {}; - }; + 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)) { @@ -1469,8 +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. - if (editor.app.pluginCaseClash(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; } @@ -1816,8 +1834,6 @@ fn takeWebFileTypePrompt(gpa: std.mem.Allocator, id: []const u8) bool { pub fn installAndLoadPlugin(editor: *Editor, id: []const u8) !void { if (isBundledPluginId(id)) return error.NotUnloadable; if (editor.app.host.pluginById(id) != null) return; // already loaded - // The download just made `plugins/`. - editor.app.forgetPluginFolders(); editor.app.untrackDisabledPlugin(id); try editor.setPluginEnabledPersisted(id, true); try editor.loadUserPluginById(id); @@ -2081,12 +2097,17 @@ pub fn uninstallPlugin(editor: *Editor, id: []const u8, force: bool) !void { defer editor.app.gpa.free(plugin_dir); // A build refused for its case (`App.recordCaseClash`) names a folder that, on macOS and // Windows, is another installed plugin's own, loaded or not: its card drops, and that folder - // stays. Told apart by what the filesystem lists, never by the name asked for. - if (editor.app.pluginFolderDeletable(id)) { - 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) }); - } else dvui.log.info("uninstallPlugin '{s}': {s} is not this plugin's own folder alone; kept", .{ id, plugin_dir }); - editor.app.forgetPluginFolders(); + // 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); @@ -7103,14 +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; - const other = app.pluginCaseClash(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 6cfe6e71e..cd8f0bf39 100644 --- a/tests/integration.zig +++ b/tests/integration.zig @@ -7673,19 +7673,17 @@ 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: an installed plugin that is not loaded. + // A folder already in `plugins/` in another case clashes too, read fresh at each ask: an + // installed plugin that is not loaded. { - // `plugins/` is read once until something is installed or removed: a folder made - // behind the app's back after that read is not seen until then. - try std.testing.expect(editor.app.pluginCaseClash("graphl") == null); + 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 {}; - try std.testing.expect(editor.app.pluginCaseClash("graphl") == null); - editor.app.forgetPluginFolders(); - try std.testing.expectEqualStrings("Graphl", editor.app.pluginCaseClash("graphl") orelse return error.TestUnexpectedResult); - try std.testing.expect(editor.app.pluginCaseClash("Graphl") == null); + 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);