mirror of
https://github.com/l0ng-ai/tty7.git
synced 2026-09-22 00:02:23 +00:00
fix(cli): make the PATH install reversible, honest, and safe to migrate
Follow-up on the review of #277. Seven fixes, no change to what the feature is for. An AppImage copy is now claimed with a marker file instead of being inferred from "am I an AppImage right now". Keying off the runtime meant that a user who moved from the AppImage to the tarball hit their own copy, read it as somebody else's binary, and never got another install for as long as that file sat there. The Windows uninstaller takes {app} back out of HKCU\Environment. Nothing did before: the entry is written by the app at runtime, so Inno never knew it existed and every uninstall grew the user's PATH by one dead entry. Unix has no equivalent hook and still leaves its symlink behind; that is now stated in the module docs rather than left to be discovered. An occupied candidate directory no longer ends the scan, and every platform now reports whether the install actually wins the lookup. `Occupied` on /opt/homebrew/bin used to mean giving up while ~/.local/bin sat free, and Windows — which appends to PATH and so never collides — reported `Installed` even when an existing tty7 earlier on PATH kept beating it. A new `InstalledShadowed` names the winner. `cargo run --release` no longer repoints the developer's real tty7 at a build tree. `cfg!(debug_assertions)` only covered the debug half of that. The Windows registry PATH is read, matched, and written as UTF-16 throughout. It went through `to_string_lossy` before, so a value the registry holds but Rust cannot represent as a String would have been written back with U+FFFD in place of its characters — the exact PATH corruption the surrounding code is careful to avoid. Two tests mutated $HOME and $PATH while the rest of the binary's tests ran beside them, and src/ui/home.rs mutates $HOME too. `candidate_dirs` takes home as a parameter, `place` takes its mode, and the PATH-joining and registry- joining rules are pure functions — so no test in this module touches the environment any more. 5 tests become 11, and the Windows joining logic is covered on every platform. Also: the config flag reaches Settings → About and both features docs instead of being config.json-only, startup reads config.json once instead of twice, and the CLI's strip failure warns like its sibling instead of being swallowed.
This commit is contained in:
@@ -36,7 +36,7 @@ chmod +x "$STAGE/tty7"
|
||||
# Release builds keep symbols (thin LTO, no profile strip); drop them here so
|
||||
# the archive isn't ~100 MB of debug info.
|
||||
strip "$STAGE/tty7-app" || echo "⚠️ strip unavailable — shipping unstripped binary"
|
||||
strip "$STAGE/tty7" || true
|
||||
strip "$STAGE/tty7" || echo "⚠️ strip unavailable — shipping unstripped CLI"
|
||||
mkdir -p "$STAGE/completions"
|
||||
cp assets/completions/*.json "$STAGE/completions/"
|
||||
cp LICENSE "$STAGE/LICENSE"
|
||||
|
||||
@@ -74,6 +74,7 @@ Source: "{#StageDir}\tty7-app.exe"; DestDir: "{app}"; Flags: ignoreversion
|
||||
; The CLI. `core::cli_install` adds {app} to the user's PATH at first launch,
|
||||
; so this is not registered as an [Env] change here — the portable zip has no
|
||||
; installer to do it, and one code path serving both is one behaviour to debug.
|
||||
; The uninstaller takes that entry back out; see RemoveAppDirFromUserPath below.
|
||||
Source: "{#StageDir}\tty7.exe"; DestDir: "{app}"; Flags: ignoreversion
|
||||
Source: "{#StageDir}\completions\*"; DestDir: "{app}\completions"; Flags: ignoreversion recursesubdirs
|
||||
Source: "{#StageDir}\LICENSE.txt"; DestDir: "{app}"; Flags: ignoreversion
|
||||
@@ -116,3 +117,67 @@ begin
|
||||
SW_HIDE, ewWaitUntilTerminated, ResultCode);
|
||||
Result := '';
|
||||
end;
|
||||
|
||||
(* Take {app} back out of the user's PATH.
|
||||
|
||||
`core::cli_install` puts it there at first launch rather than the installer
|
||||
doing it, because the portable zip has no installer — but that leaves nobody
|
||||
to undo it, and an uninstall that permanently grows the user's PATH by one
|
||||
dead entry is not an uninstall. So the removal lives here, on the one install
|
||||
shape that has an uninstaller at all. (Unix has no equivalent hook: deleting
|
||||
the .app or the tarball leaves the symlink behind for the user to remove.)
|
||||
|
||||
HKCU even for an all-users install: cli_install only ever writes the user
|
||||
hive, so that is the only place an entry can be. On a machine where several
|
||||
users ran tty7, this clears the one uninstalling — the others keep a dead
|
||||
entry, which is the price of not needing elevation to install in the first
|
||||
place.
|
||||
|
||||
Entries are compared case-insensitively and ignoring a trailing backslash,
|
||||
and every other entry is written back verbatim: this is somebody's PATH, and
|
||||
we are here to remove one thing from it, not to tidy it. No
|
||||
WM_SETTINGCHANGE broadcast — the entry now names a deleted directory, so
|
||||
nothing is waiting on the news, and it is gone from new shells at next
|
||||
sign-in regardless. *)
|
||||
procedure RemoveAppDirFromUserPath();
|
||||
var
|
||||
Existing, Rebuilt, Raw, Entry, Target: String;
|
||||
P: Integer;
|
||||
begin
|
||||
if not RegQueryStringValue(HKEY_CURRENT_USER, 'Environment', 'Path', Existing) then
|
||||
exit;
|
||||
|
||||
Target := RemoveBackslashUnlessRoot(ExpandConstant('{app}'));
|
||||
Rebuilt := '';
|
||||
(* The trailing ';' makes the last entry look like every other one. *)
|
||||
Existing := Existing + ';';
|
||||
repeat
|
||||
P := Pos(';', Existing);
|
||||
Raw := Copy(Existing, 1, P - 1);
|
||||
Existing := Copy(Existing, P + 1, Length(Existing));
|
||||
Entry := Trim(Raw);
|
||||
if (Entry <> '') and
|
||||
(CompareText(RemoveBackslashUnlessRoot(Entry), Target) <> 0) then
|
||||
begin
|
||||
if Rebuilt <> '' then
|
||||
Rebuilt := Rebuilt + ';';
|
||||
Rebuilt := Rebuilt + Raw;
|
||||
end;
|
||||
until Existing = '';
|
||||
|
||||
if Rebuilt = '' then
|
||||
RegDeleteValue(HKEY_CURRENT_USER, 'Environment', 'Path')
|
||||
(* Inno cannot read a value's type back, so infer it: a PATH holding a '%' is
|
||||
one that has to stay expandable, and rewriting it as REG_SZ would freeze
|
||||
every other entry's variable at today's value. *)
|
||||
else if Pos('%', Rebuilt) > 0 then
|
||||
RegWriteExpandStringValue(HKEY_CURRENT_USER, 'Environment', 'Path', Rebuilt)
|
||||
else
|
||||
RegWriteStringValue(HKEY_CURRENT_USER, 'Environment', 'Path', Rebuilt);
|
||||
end;
|
||||
|
||||
procedure CurUninstallStepChanged(CurUninstallStep: TUninstallStep);
|
||||
begin
|
||||
if CurUninstallStep = usUninstall then
|
||||
RemoveAppDirFromUserPath();
|
||||
end;
|
||||
|
||||
Reference in New Issue
Block a user