Skip to content

Commit d0b346b

Browse files
gok03gok03
authored
fix(install): align shell-rc marker so uninstall fully cleans up (#65)
The curl|sh bootstrap (scripts/install.sh) wrote a `# >>> refuse cli (managed)` block, but the binary (internal/shim/shellrc.go) manages a `# >>> refuse shim (managed)` block. Because the markers differed: - `refuse uninstall` left the curl installer's PATH block behind — an incomplete uninstall that kept ~/.refuse/bin on PATH. - installing both ways stacked two redundant blocks. - install.sh now writes the same canonical marker the binary manages, so a later `refuse install` rewrites it in place instead of stacking. - patchFile strips ALL recognised blocks (canonical + legacy "cli", plus any duplicates), so install consolidates to one block and uninstall removes them all — including legacy blocks from older installs. - Tests: legacy+canonical stripped on uninstall; duplicates consolidated on install. Co-authored-by: gok03 <gok03@users.noreply.github.com>
1 parent 63893db commit d0b346b

3 files changed

Lines changed: 121 additions & 15 deletions

File tree

‎internal/shim/shellrc.go‎

Lines changed: 41 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,20 @@ const (
1616
endMarker = "# <<< refuse shim (managed) <<<"
1717
)
1818

19+
// markerPair is a begin/end delimiter set we recognise in shell-rc files.
20+
type markerPair struct{ begin, end string }
21+
22+
// managedMarkers is every block refuse owns. The first is canonical (what we
23+
// write today); the rest are legacy delimiters from older installers that we
24+
// still strip so install consolidates duplicates and uninstall cleans up fully.
25+
// The curl|sh bootstrap (scripts/install.sh) historically wrote a
26+
// "refuse cli (managed)" block with a different marker than the binary, so an
27+
// uninstall used to leave it behind — this list fixes that.
28+
var managedMarkers = []markerPair{
29+
{beginMarker, endMarker},
30+
{"# >>> refuse cli (managed) >>>", "# <<< refuse cli (managed) <<<"},
31+
}
32+
1933
// updateShellRC writes (`enable`=true) or removes (`enable`=false) the PATH
2034
// block in the user's shell-rc files. Returns the rc files actually touched.
2135
func updateShellRC(binDir string, enable bool) ([]string, error) {
@@ -75,26 +89,40 @@ func managedBlock(binDir string) string {
7589
return b.String()
7690
}
7791

92+
// stripBlock removes every begin..end block (inclusive, plus a trailing
93+
// newline) from content. Loops so duplicate blocks are all removed.
94+
func stripBlock(content, begin, end string) string {
95+
for {
96+
start := strings.Index(content, begin)
97+
if start < 0 {
98+
return content
99+
}
100+
rel := strings.Index(content[start:], end)
101+
if rel < 0 {
102+
return content // unterminated marker — leave it rather than eat the rest
103+
}
104+
stop := start + rel + len(end)
105+
if stop < len(content) && content[stop] == '\n' {
106+
stop++
107+
}
108+
content = content[:start] + content[stop:]
109+
}
110+
}
111+
78112
func patchFile(path, binDir string, enable bool) (bool, error) {
79113
raw, err := os.ReadFile(path)
80114
if err != nil {
81115
return false, err
82116
}
83117
content := string(raw)
84118

85-
startIdx := strings.Index(content, beginMarker)
86-
endIdx := strings.Index(content, endMarker)
87-
88-
var withoutBlock string
89-
if startIdx >= 0 && endIdx > startIdx {
90-
// Strip the existing managed block (and the trailing newline if any).
91-
end := endIdx + len(endMarker)
92-
if end < len(content) && content[end] == '\n' {
93-
end++
94-
}
95-
withoutBlock = content[:startIdx] + content[end:]
96-
} else {
97-
withoutBlock = content
119+
// Strip every managed block we recognise — canonical and legacy, and any
120+
// duplicates of each — so install consolidates to one block and uninstall
121+
// removes them all (including a stray "refuse cli (managed)" block from an
122+
// older curl|sh install).
123+
withoutBlock := content
124+
for _, m := range managedMarkers {
125+
withoutBlock = stripBlock(withoutBlock, m.begin, m.end)
98126
}
99127

100128
var next string

‎internal/shim/shellrc_test.go‎

Lines changed: 75 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,75 @@
1+
package shim
2+
3+
import (
4+
"os"
5+
"path/filepath"
6+
"strings"
7+
"testing"
8+
)
9+
10+
const legacyCliBlock = "# >>> refuse cli (managed) >>>\n" +
11+
"export PATH=\"/home/u/.refuse/bin:$PATH\"\n" +
12+
"# <<< refuse cli (managed) <<<\n"
13+
14+
// Uninstall (enable=false) must remove BOTH the canonical "shim" block and the
15+
// legacy "cli" block written by the old curl|sh installer — and leave the rest
16+
// of the file untouched. Regression for the marker-mismatch that let an
17+
// uninstall strand a refuse PATH export behind.
18+
func TestPatchFileStripsLegacyAndCanonicalOnUninstall(t *testing.T) {
19+
dir := t.TempDir()
20+
rc := filepath.Join(dir, ".zshrc")
21+
content := "export FOO=1\n\n" + legacyCliBlock + "\n" + managedBlock("/home/u/.refuse/bin")
22+
if err := os.WriteFile(rc, []byte(content), 0o644); err != nil {
23+
t.Fatal(err)
24+
}
25+
26+
if _, err := patchFile(rc, "/home/u/.refuse/bin", false); err != nil {
27+
t.Fatal(err)
28+
}
29+
30+
got := readFile(t, rc)
31+
if strings.Contains(got, "refuse cli (managed)") {
32+
t.Errorf("legacy cli block survived uninstall:\n%s", got)
33+
}
34+
if strings.Contains(got, "refuse shim (managed)") {
35+
t.Errorf("canonical shim block survived uninstall:\n%s", got)
36+
}
37+
if !strings.Contains(got, "export FOO=1") {
38+
t.Errorf("uninstall clobbered unrelated content:\n%s", got)
39+
}
40+
}
41+
42+
// Install (enable=true) over a file that already has the legacy block (and even
43+
// a duplicate) must consolidate to exactly one canonical block.
44+
func TestPatchFileConsolidatesDuplicatesOnInstall(t *testing.T) {
45+
dir := t.TempDir()
46+
rc := filepath.Join(dir, ".zshrc")
47+
content := "export FOO=1\n\n" + legacyCliBlock + "\n" + legacyCliBlock
48+
if err := os.WriteFile(rc, []byte(content), 0o644); err != nil {
49+
t.Fatal(err)
50+
}
51+
52+
if _, err := patchFile(rc, "/home/u/.refuse/bin", true); err != nil {
53+
t.Fatal(err)
54+
}
55+
56+
got := readFile(t, rc)
57+
if n := strings.Count(got, ">>> refuse"); n != 1 {
58+
t.Errorf("expected exactly one managed block, found %d:\n%s", n, got)
59+
}
60+
if strings.Contains(got, "refuse cli (managed)") {
61+
t.Errorf("legacy cli block not migrated to canonical:\n%s", got)
62+
}
63+
if !strings.Contains(got, "export FOO=1") {
64+
t.Errorf("install clobbered unrelated content:\n%s", got)
65+
}
66+
}
67+
68+
func readFile(t *testing.T, p string) string {
69+
t.Helper()
70+
b, err := os.ReadFile(p)
71+
if err != nil {
72+
t.Fatal(err)
73+
}
74+
return string(b)
75+
}

‎scripts/install.sh‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -116,8 +116,11 @@ echo "refuse: installed $INSTALL_DIR/refuse"
116116
patch_rc() {
117117
rc=$1
118118
[ -e "$rc" ] || return 0
119-
marker_begin="# >>> refuse cli (managed) >>>"
120-
marker_end="# <<< refuse cli (managed) <<<"
119+
# Must match internal/shim/shellrc.go's canonical marker exactly, so that a
120+
# later `refuse install` rewrites this block in place (instead of stacking a
121+
# second one) and `refuse uninstall` removes it.
122+
marker_begin="# >>> refuse shim (managed) >>>"
123+
marker_end="# <<< refuse shim (managed) <<<"
121124
# Skip if already patched (idempotent re-installs).
122125
if grep -qF "$marker_begin" "$rc" 2>/dev/null; then
123126
return 0

0 commit comments

Comments
 (0)