diff --git a/src/cli/install/mappings.zig b/src/cli/install/mappings.zig index c14bbce..349f28b 100644 --- a/src/cli/install/mappings.zig +++ b/src/cli/install/mappings.zig @@ -218,15 +218,6 @@ pub fn writeBinding( } } - if (existing != null and (conflict_mode == .force or conflict_mode == .interactive)) { - backupFile(allocator, config_path) catch |err| { - var errbuf: [256]u8 = undefined; - const msg = std.fmt.bufPrint(&errbuf, "error: cannot create backup of {s}: {}, aborting overwrite\n", .{ config_path, err }) catch "error: backup failed, aborting\n"; - _ = std.posix.write(std.posix.STDERR_FILENO, msg) catch {}; - return err; - }; - } - var has_target = false; if (devices) |devs| { for (devs) |d| { @@ -236,6 +227,17 @@ pub fn writeBinding( } } } + + // Only back up when an existing entry is actually being overwritten; a + // pure add (no matching entry) changes nothing that needs a backup. + if (has_target and (conflict_mode == .force or conflict_mode == .interactive)) { + backupFile(allocator, config_path) catch |err| { + var errbuf: [256]u8 = undefined; + const msg = std.fmt.bufPrint(&errbuf, "error: cannot create backup of {s}: {}, aborting overwrite\n", .{ config_path, err }) catch "error: backup failed, aborting\n"; + _ = std.posix.write(std.posix.STDERR_FILENO, msg) catch {}; + return err; + }; + } const old_count = if (devices) |d| d.len else 0; const new_count = if (has_target) old_count else old_count + 1; var new_devices = try allocator.alloc(user_config_mod.DeviceEntry, new_count); diff --git a/src/cli/install/phase.zig b/src/cli/install/phase.zig index b3c2174..fa0c277 100644 --- a/src/cli/install/phase.zig +++ b/src/cli/install/phase.zig @@ -14,7 +14,7 @@ pub var test_probe_alive_override: ?*const fn (path: []const u8) bool = null; /// Test hook: when non-null, uninstall prefixes runtime paths /// (/run/padctl/padctl.sock, .pid) with this root instead of "" so the -/// PR-2 daemon-stop probe path can be exercised against a tmpdir without +/// daemon-stop probe path can be exercised against a tmpdir without /// flipping the lifecycle scope to .package via opts.destdir. pub var test_runtime_root_override: ?[]const u8 = null; diff --git a/src/cli/install/plan.zig b/src/cli/install/plan.zig index a39facc..a7170b1 100644 --- a/src/cli/install/plan.zig +++ b/src/cli/install/plan.zig @@ -79,31 +79,8 @@ pub fn writeAll(fd: std.posix.fd_t, s: []const u8) void { _ = std.posix.write(fd, s) catch {}; } -pub fn ensureDir(path: []const u8) !void { - std.fs.makeDirAbsolute(path) catch |err| switch (err) { - error.PathAlreadyExists => {}, - else => return err, - }; -} - -pub fn ensureDirAll(allocator: std.mem.Allocator, path: []const u8) !void { - var components = std.ArrayList([]const u8){}; - defer components.deinit(allocator); - - var remaining = path; - while (remaining.len > 1) { - try components.append(allocator, remaining); - remaining = std.fs.path.dirname(remaining) orelse break; - } - - var i: usize = components.items.len; - while (i > 0) { - i -= 1; - ensureDir(components.items[i]) catch |err| switch (err) { - error.PathAlreadyExists => {}, - else => return err, - }; - } +pub fn ensureDirAll(_: std.mem.Allocator, path: []const u8) !void { + try std.fs.cwd().makePath(path); } pub fn dirExistsAbsolute(path: []const u8) bool { @@ -122,19 +99,9 @@ pub fn dirIsNonEmpty(path: []const u8) bool { return false; } +// Atomic copy (temp + rename) preserving source mode. No partial dst on failure. pub fn copyFile(src: []const u8, dst: []const u8) !void { - var src_file = try std.fs.openFileAbsolute(src, .{}); - defer src_file.close(); - const stat = try src_file.stat(); - var dst_file = try std.fs.createFileAbsolute(dst, .{ .truncate = true }); - defer dst_file.close(); - try dst_file.chmod(stat.mode & 0o777); - var buf: [65536]u8 = undefined; - while (true) { - const n = try src_file.read(&buf); - if (n == 0) break; - try dst_file.writeAll(buf[0..n]); - } + try std.fs.copyFileAbsolute(src, dst, .{}); } // Write to {dst}.new then rename(2) over dst — avoids ETXTBSY when dst is currently executing. @@ -251,7 +218,8 @@ pub fn hostHasInputGroup() bool { pub fn userInGroup(name: []const u8) bool { const target_gid = groupGid(name) orelse return false; if (std.os.linux.getegid() == target_gid) return true; - var gids: [64]std.os.linux.gid_t = undefined; + // 1024 covers heavy LDAP/AD memberships; getgroups fails (-1) if exceeded. + var gids: [1024]std.os.linux.gid_t = undefined; const ret = std.os.linux.getgroups(gids.len, &gids[0]); if (ret > gids.len) return false; for (gids[0..ret]) |g| { @@ -334,9 +302,8 @@ pub const EnvSnapshot = struct { /// Single source of truth for every decision `install.run()` makes before it /// starts touching the filesystem. All derived axes are computed exactly once -/// in `compute()` so later phases read a plain struct instead of re-deriving. -/// All derived axes are computed exactly once so later phases read a plain -/// struct instead of re-deriving, preventing decision drift between call sites. +/// in `compute()` so later phases read a plain struct instead of re-deriving, +/// preventing decision drift between call sites. pub const InstallPlan = struct { // --- inputs (captured, not re-derived) --- opts: InstallOptions, @@ -348,7 +315,7 @@ pub const InstallPlan = struct { // --- single source of truth --- scope: LifecycleScope, - // --- derived axes (computed-from-scope shims; PR-4/PR-5 will remove) --- + // --- derived axes --- staging_mode: bool, effective_user_service: bool, immutable_kind: ImmutableKind, @@ -387,8 +354,7 @@ pub const InstallPlan = struct { const staging_mode = scope == .package; - // user-service routing follows scope. Explicit --user-service still wins - // for transitional callers; PR-5 removes the orthogonal axis. + // user-service routing follows scope. Explicit --user-service still wins. const effective_user_service = opts.user_service orelse (scope == .user); const immutable_kind = detectImmutableOs(allocator, if (staging_mode) destdir else ""); diff --git a/src/cli/install/services.zig b/src/cli/install/services.zig index ee4e647..ad9bcf3 100644 --- a/src/cli/install/services.zig +++ b/src/cli/install/services.zig @@ -255,7 +255,10 @@ pub fn buildSystemctlUserArgv( if (plan.mode == .skip) return null; var argv = std.ArrayList([]const u8){}; - errdefer argv.deinit(allocator); + errdefer { + for (argv.items) |s| allocator.free(s); + argv.deinit(allocator); + } if (plan.mode == .sudo_hop) { try argv.append(allocator, try allocator.dupe(u8, "sudo")); @@ -386,11 +389,16 @@ pub fn stopDaemonScope(allocator: std.mem.Allocator, scope: SystemctlScope) !voi fn buildSystemctlSystemArgv(allocator: std.mem.Allocator, verbs: []const []const u8) ![][]const u8 { var argv = try allocator.alloc([]const u8, 1 + verbs.len); - errdefer allocator.free(argv); + var filled: usize = 0; + errdefer { + for (argv[0..filled]) |s| allocator.free(s); + allocator.free(argv); + } argv[0] = try allocator.dupe(u8, "systemctl"); - errdefer allocator.free(argv[0]); + filled = 1; for (verbs, 0..) |v, i| { argv[1 + i] = try allocator.dupe(u8, v); + filled = 2 + i; } return argv; } diff --git a/src/cli/install/tests.zig b/src/cli/install/tests.zig index 7876d10..386e44e 100644 --- a/src/cli/install/tests.zig +++ b/src/cli/install/tests.zig @@ -978,7 +978,7 @@ test "install: uninstall removes runtime state paths under system scope" { defer f.close(); } - // PR-3: runtime paths are touched only in non-package scopes. Force + // runtime paths are touched only in non-package scopes. Force // scope=.system and redirect the path root to the staging tmpdir. phase_mod.test_runtime_root_override = staging; defer phase_mod.test_runtime_root_override = null; @@ -2505,6 +2505,45 @@ test "install: writeBinding conflict with force - backup + overwrite" { try std.testing.expect(found_bak); } +test "install: writeBinding force pure-add does not create backup" { + const testing_alloc = std.testing.allocator; + + var tmp = std.testing.tmpDir(.{}); + defer tmp.cleanup(); + const destdir = try tmp.dir.realpathAlloc(testing_alloc, "."); + defer testing_alloc.free(destdir); + + // Pre-existing config with an UNRELATED device entry. + try writeBinding(testing_alloc, destdir, "Vader", "vader_map", .skip, mockPromptKeep); + + // Force-bind a NEW device: pure add, no entry overwritten. + try writeBinding(testing_alloc, destdir, "DualSense", "ds_map", .force, mockPromptKeep); + + const config_path = try std.fmt.allocPrint(testing_alloc, "{s}/etc/padctl/config.toml", .{destdir}); + defer testing_alloc.free(config_path); + const content = try std.fs.cwd().readFileAlloc(testing_alloc, config_path, 64 * 1024); + defer testing_alloc.free(content); + + // New entry added, old entry preserved. + try std.testing.expect(std.mem.indexOf(u8, content, "default_mapping = \"ds_map\"") != null); + try std.testing.expect(std.mem.indexOf(u8, content, "default_mapping = \"vader_map\"") != null); + + // No backup file: nothing was overwritten. + const etc_dir = try std.fmt.allocPrint(testing_alloc, "{s}/etc/padctl", .{destdir}); + defer testing_alloc.free(etc_dir); + var dir = try std.fs.openDirAbsolute(etc_dir, .{ .iterate = true }); + defer dir.close(); + var found_bak = false; + var it = dir.iterate(); + while (try it.next()) |entry| { + if (std.mem.startsWith(u8, entry.name, "config.toml.bak.")) { + found_bak = true; + break; + } + } + try std.testing.expect(!found_bak); +} + test "install: installMapping errors on missing source" { const testing = std.testing; const allocator = testing.allocator; @@ -3951,7 +3990,7 @@ test "uninstall: removes 61-padctl-driver-block.rules" { } // --------------------------------------------------------------------------- -// PR-3: LifecycleScope integration tests +// LifecycleScope integration tests // --------------------------------------------------------------------------- const scope_mod = @import("scope.zig"); @@ -4122,7 +4161,7 @@ test "uninstall: user scope routes to user systemctl only" { try uninstall(allocator, opts); } - // scope=.user must NOT trigger a system-scope stop. The PR-2 probe-and-stop + // scope=.user must NOT trigger a system-scope stop. The probe-and-stop // path only fires when probeSocketAlive returns true, and there's no socket // here, so stopDaemonScope is never called — calls list stays empty. try testing.expectEqual(@as(usize, 0), ProbeRig.calls.items.len); diff --git a/src/cli/install/udev.zig b/src/cli/install/udev.zig index e2a14a4..233c220 100644 --- a/src/cli/install/udev.zig +++ b/src/cli/install/udev.zig @@ -86,7 +86,7 @@ pub fn writeServiceSentinel(allocator: std.mem.Allocator, plan: *const InstallPl const ds = es.getDaySeconds(); const iso = std.fmt.bufPrint(&buf, "{d:0>4}-{d:0>2}-{d:0>2}T{d:0>2}:{d:0>2}:{d:0>2}Z", .{ yd.year, - @as(u32, @intFromEnum(md.month)) + 1, + @as(u32, @intFromEnum(md.month)), @as(u32, md.day_index) + 1, ds.getHoursIntoDay(), ds.getMinutesIntoHour(),