From c66f8e0159bb1b33f299f68189edd8039aace072 Mon Sep 17 00:00:00 2001 From: Carlos Granados Date: Sat, 25 Jul 2026 22:27:59 +0200 Subject: [PATCH] Existing interpreter handler --- internal/cli/install.go | 1 + internal/ini/ini.go | 27 +++ internal/ini/ini_test.go | 39 +++++ internal/installer/backup.go | 45 +++++ internal/installer/iniconfig.go | 147 +++++++++++++++++ internal/installer/install.go | 152 +++++++++++++++-- internal/installer/install_test.go | 257 ++++++++++++++++++++++++----- internal/manifest/manifest.go | 4 + internal/php/parse.go | 22 ++- internal/php/parse_test.go | 23 +++ 10 files changed, 650 insertions(+), 67 deletions(-) create mode 100644 internal/installer/backup.go create mode 100644 internal/installer/iniconfig.go diff --git a/internal/cli/install.go b/internal/cli/install.go index 70560cd..4d0ee89 100644 --- a/internal/cli/install.go +++ b/internal/cli/install.go @@ -40,6 +40,7 @@ func newInstallCmd() *cobra.Command { ZTS: opts.ZTS, AssumeYes: globalOpts.Yes, Out: cmd.OutOrStdout(), + In: cmd.InOrStdin(), }) }, } diff --git a/internal/ini/ini.go b/internal/ini/ini.go index 1ae269c..81adcc7 100644 --- a/internal/ini/ini.go +++ b/internal/ini/ini.go @@ -33,6 +33,33 @@ func StripXdebugLoaders(content string) (string, []string) { return strings.Join(kept, "\n"), removed } +// CommentExtensionLoaders comments out every active extension= / zend_extension= +// directive by prefixing it with "; ", returning the rewritten content and the +// list of lines that were commented. Already-commented loaders and non-loader +// lines are left unchanged. +// +// This is used when copying an existing php's config to a self-contained +// debugger interpreter, which cannot load foreign .so files (they are built for +// a specific PHP ABI). Commenting keeps them visible but inert. Note xdebug +// loaders are removed entirely by StripXdebugLoaders and never reach here. +func CommentExtensionLoaders(content string) (string, []string) { + lines := strings.Split(content, "\n") + var commented []string + for i, ln := range lines { + isComment, key, _, ok := parseDirective(ln) + if !ok || isComment || (key != "extension" && key != "zend_extension") { + continue + } + commented = append(commented, strings.TrimRight(ln, "\r")) + body, cr := ln, "" + if strings.HasSuffix(body, "\r") { + body, cr = body[:len(body)-1], "\r" + } + lines[i] = "; " + body + cr + } + return strings.Join(lines, "\n"), commented +} + // DisallowedModes returns the de-duplicated set of xdebug.mode tokens present in // xdebug.mode directives (active or commented out) that are not in AllowedModes, // preserving first-seen order. It is empty if xdebug.mode is absent or already diff --git a/internal/ini/ini_test.go b/internal/ini/ini_test.go index 63a0dcd..3b6e57e 100644 --- a/internal/ini/ini_test.go +++ b/internal/ini/ini_test.go @@ -80,6 +80,45 @@ func TestStripXdebugLoaders(t *testing.T) { } } +func TestCommentExtensionLoaders(t *testing.T) { + tests := []struct { + name string + in string + want string + wantCommented []string + }{ + { + name: "comments active extension and zend_extension", + in: "extension=mysqli.so\nzend_extension=/opt/opcache.so\nmemory_limit=256M\n", + want: "; extension=mysqli.so\n; zend_extension=/opt/opcache.so\nmemory_limit=256M\n", + wantCommented: []string{"extension=mysqli.so", "zend_extension=/opt/opcache.so"}, + }, + { + name: "leaves already-commented and non-loaders alone", + in: ";extension=foo.so\ndisplay_errors=On\n", + want: ";extension=foo.so\ndisplay_errors=On\n", + wantCommented: nil, + }, + { + name: "preserves CRLF", + in: "extension=mysqli.so\r\nmemory_limit=256M\r\n", + want: "; extension=mysqli.so\r\nmemory_limit=256M\r\n", + wantCommented: []string{"extension=mysqli.so"}, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got, commented := CommentExtensionLoaders(tt.in) + if got != tt.want { + t.Errorf("content = %q, want %q", got, tt.want) + } + if !reflect.DeepEqual(commented, tt.wantCommented) { + t.Errorf("commented = %#v, want %#v", commented, tt.wantCommented) + } + }) + } +} + func TestDisallowedModes(t *testing.T) { tests := []struct { name string diff --git a/internal/installer/backup.go b/internal/installer/backup.go new file mode 100644 index 0000000..59f1c8d --- /dev/null +++ b/internal/installer/backup.go @@ -0,0 +1,45 @@ +package installer + +import ( + "fmt" + "os" + "path/filepath" +) + +// backupExisting moves the existing interpreter at srcPath into backupDir under a +// unique name, preserving whatever it is (a real binary or a symlink). It falls +// back to copy+remove if the move crosses filesystems. Returns the backup path. +func backupExisting(srcPath, backupDir, key string, nowNanos int64) (string, error) { + if err := os.MkdirAll(backupDir, 0o755); err != nil { + return "", fmt.Errorf("creating backup dir: %w", err) + } + dst := filepath.Join(backupDir, fmt.Sprintf("php-%s-%d", key, nowNanos)) + + if err := os.Rename(srcPath, dst); err == nil { + return dst, nil + } + // Cross-device or other rename failure: copy the resolved binary, then remove + // the original. + if err := copyFile(srcPath, dst, 0o755); err != nil { + return "", fmt.Errorf("backing up %s: %w", srcPath, err) + } + if err := os.Remove(srcPath); err != nil { + os.Remove(dst) + return "", fmt.Errorf("removing original %s after backup: %w", srcPath, err) + } + return dst, nil +} + +// restoreBackup moves a backup back to its original path. +func restoreBackup(backupPath, originalPath string) error { + if err := os.MkdirAll(filepath.Dir(originalPath), 0o755); err != nil { + return err + } + if err := os.Rename(backupPath, originalPath); err == nil { + return nil + } + if err := copyFile(backupPath, originalPath, 0o755); err != nil { + return err + } + return os.Remove(backupPath) +} diff --git a/internal/installer/iniconfig.go b/internal/installer/iniconfig.go new file mode 100644 index 0000000..40797ab --- /dev/null +++ b/internal/installer/iniconfig.go @@ -0,0 +1,147 @@ +package installer + +import ( + "fmt" + "os" + "path/filepath" + "sort" + "strings" + + "github.com/php-debugger/installer/internal/ini" + "github.com/php-debugger/installer/internal/php" +) + +// configPair is a source ini file and where its (sanitized) copy is written. +type configPair struct{ src, dst string } + +// copyConfig copies the existing interpreter's ini files into the new +// interpreter's compiled-in config path (so the new php loads the same +// configuration), sanitizing each on the way: xdebug loader lines are stripped, +// and disallowed xdebug.mode tokens are removed after confirmation. +// +// It registers undo steps on rb (restoring overwritten files / removing created +// ones) and returns the list of destination files written. +func copyConfig(existing, target *php.Info, rb *rollback, opts Options) ([]string, error) { + pairs := configPairs(existing, target) + if len(pairs) == 0 { + if target.Ini.ConfigPath == "" && target.Ini.ScanDir == "" { + opts.logf("Note: the interpreter reports no config path; skipping ini copy.") + } + return nil, nil + } + + stripModes, err := decideStripModes(pairs, opts) + if err != nil { + return nil, err + } + + var written []string + for _, pr := range pairs { + data, err := os.ReadFile(pr.src) + if err != nil { + return written, fmt.Errorf("reading ini %s: %w", pr.src, err) + } + content, removedLoaders := ini.StripXdebugLoaders(string(data)) + content, commentedLoaders := ini.CommentExtensionLoaders(content) + if stripModes { + content, _, _ = ini.SanitizeXdebugMode(content) + } + + if err := registerConfigUndo(pr.dst, rb); err != nil { + return written, err + } + if err := os.MkdirAll(filepath.Dir(pr.dst), 0o755); err != nil { + return written, fmt.Errorf("creating config dir: %w", err) + } + if err := os.WriteFile(pr.dst, []byte(content), 0o644); err != nil { + return written, fmt.Errorf("writing ini %s: %w", pr.dst, err) + } + written = append(written, pr.dst) + opts.logf(" wrote %s%s", pr.dst, loaderNote(len(removedLoaders), len(commentedLoaders))) + } + return written, nil +} + +// configPairs builds the (source, destination) list: the existing main php.ini +// goes to the new interpreter's ConfigPath, and each additional .ini goes to its +// ScanDir (by base name). +func configPairs(existing, target *php.Info) []configPair { + var pairs []configPair + if existing.Ini.LoadedFile != "" && target.Ini.ConfigPath != "" { + pairs = append(pairs, configPair{ + src: existing.Ini.LoadedFile, + dst: filepath.Join(target.Ini.ConfigPath, "php.ini"), + }) + } + if target.Ini.ScanDir != "" { + for _, f := range existing.Ini.AdditionalFiles { + pairs = append(pairs, configPair{ + src: f, + dst: filepath.Join(target.Ini.ScanDir, filepath.Base(f)), + }) + } + } + return pairs +} + +// decideStripModes scans the source ini files for disallowed xdebug.mode tokens +// and, if any are found, asks the user whether to remove them (auto-yes under +// --yes). Returns true if the caller should sanitize xdebug.mode. +func decideStripModes(pairs []configPair, opts Options) (bool, error) { + seen := map[string]bool{} + var disallowed []string + for _, pr := range pairs { + data, err := os.ReadFile(pr.src) + if err != nil { + return false, fmt.Errorf("reading ini %s: %w", pr.src, err) + } + for _, m := range ini.DisallowedModes(string(data)) { + if !seen[m] { + seen[m] = true + disallowed = append(disallowed, m) + } + } + } + if len(disallowed) == 0 { + return false, nil + } + sort.Strings(disallowed) + return opts.confirm(fmt.Sprintf( + "xdebug.mode lists disallowed mode(s): %s. Remove them (keeping only off/debug)?", + strings.Join(disallowed, ", "))), nil +} + +// registerConfigUndo records how to undo writing dst: restore its prior contents +// if it existed, otherwise remove it on rollback. +func registerConfigUndo(dst string, rb *rollback) error { + if prev, err := os.ReadFile(dst); err == nil { + rb.add(func() error { return os.WriteFile(dst, prev, 0o644) }) + } else if os.IsNotExist(err) { + rb.add(func() error { return removeIfExists(dst) }) + } else { + return fmt.Errorf("inspecting %s: %w", dst, err) + } + return nil +} + +func removeIfExists(path string) error { + if err := os.Remove(path); err != nil && !os.IsNotExist(err) { + return err + } + return nil +} + +// loaderNote summarizes what happened to extension loaders in a copied ini file. +func loaderNote(removed, commented int) string { + var parts []string + if removed > 0 { + parts = append(parts, fmt.Sprintf("removed %d xdebug loader(s)", removed)) + } + if commented > 0 { + parts = append(parts, fmt.Sprintf("commented %d extension loader(s)", commented)) + } + if len(parts) == 0 { + return "" + } + return " (" + strings.Join(parts, ", ") + ")" +} diff --git a/internal/installer/install.go b/internal/installer/install.go index a92d4dd..069e35c 100644 --- a/internal/installer/install.go +++ b/internal/installer/install.go @@ -4,11 +4,13 @@ package installer import ( + "bufio" "context" "fmt" "io" "os" "path/filepath" + "strings" "time" "github.com/php-debugger/installer/internal/manifest" @@ -26,6 +28,9 @@ type Options struct { // Out receives human-readable progress output (may be nil). Out io.Writer + // In is read for interactive confirmations (defaults to os.Stdin via the CLI; + // may be nil, in which case prompts default to "no" unless AssumeYes). + In io.Reader // Client and Env are optional overrides for testing. When nil, real ones are // constructed. @@ -55,10 +60,28 @@ func (o Options) clock() time.Time { return time.Now().UTC() } +// confirm asks the user a yes/no question. Returns true under --yes; otherwise +// reads a line from In (defaulting to "no" if there is no input). +func (o Options) confirm(question string) bool { + if o.AssumeYes { + return true + } + if o.In == nil { + return false + } + if o.Out != nil { + fmt.Fprintf(o.Out, "%s [y/N]: ", question) + } + line, _ := bufio.NewReader(o.In).ReadString('\n') + line = strings.ToLower(strings.TrimSpace(line)) + return line == "y" || line == "yes" +} + // InstallInterpreter installs a self-contained PHP interpreter with the debugger -// compiled in, and activates it. This is the clean-host path: it does not yet -// detect or back up a pre-existing interpreter (that is layered on in a later -// step). On any failure after the first filesystem change, it rolls back. +// compiled in, and activates it. If an existing interpreter is found it is backed +// up and replaced at its location, and its ini configuration is copied (minus +// xdebug) into the new interpreter's config path. On any failure after the first +// filesystem change, it rolls back. func InstallInterpreter(ctx context.Context, opts Options) error { env, err := opts.env() if err != nil { @@ -70,16 +93,11 @@ func InstallInterpreter(ctx context.Context, opts Options) error { if err != nil { return err } - binDir, err := platform.SelectBinDir(layout.BinCandidates) - if err != nil { - return fmt.Errorf("%w\ntry --user for a per-user install, or re-run with elevated privileges", err) - } client := opts.Client if client == nil { client = release.NewClient() } - rel, err := client.LatestRelease(ctx) if err != nil { return err @@ -92,7 +110,6 @@ func InstallInterpreter(ctx context.Context, opts Options) error { return err } } - asset, err := release.SelectAsset(rel.Assets, release.Selector{ Kind: release.Interpreter, Series: series, @@ -104,6 +121,17 @@ func InstallInterpreter(ctx context.Context, opts Options) error { return err } + // Detect a pre-existing interpreter before we change anything. Ignore one + // that is our own previous install (avoid backing up our own symlink). + existing := detectExisting(ctx, opts, layout.Root) + + // Decide where the active `php` goes: replace the existing interpreter at its + // location when possible, else our scope's bin dir. + linkDir, replaceExisting, err := chooseLinkDir(existing, layout) + if err != nil { + return err + } + opts.logf("Installing php-debugger interpreter: php %s (%s) %s, release %s", series, threading(opts.ZTS), p, rel.TagName) @@ -133,8 +161,9 @@ func InstallInterpreter(ctx context.Context, opts Options) error { return fmt.Errorf("querying downloaded interpreter: %w", err) } - // --- place into the versioned directory --- rb := &rollback{} + + // --- place the binary into the versioned directory --- versionDir := layout.VersionDir(series, opts.ZTS) binTarget := filepath.Join(versionDir, "bin", phpBinaryName(p.OS)) if err := installFile(dlPath, binTarget); err != nil { @@ -142,19 +171,46 @@ func InstallInterpreter(ctx context.Context, opts Options) error { } rb.add(func() error { return os.RemoveAll(versionDir) }) - // --- activate (symlink/shim into the bin dir) --- - prevTarget, _, hadPrev := platform.ReadActive(binDir, "php") - activePath, kind, err := platform.Activate(binDir, "php", binTarget) + // --- copy the existing interpreter's ini config into the new one --- + var configFiles []string + if existing != nil { + opts.logf("Copying existing PHP configuration (removing xdebug) ...") + configFiles, err = copyConfig(existing, info, rb, opts) + if err != nil { + rb.run() + return fmt.Errorf("copying ini configuration; rolled back: %w", err) + } + } + + // --- back up and replace an existing interpreter at its location --- + var backup *manifest.Backup + if replaceExisting { + opts.logf("Backing up existing interpreter at %s ...", existing.Path) + backupPath, err := backupExisting(existing.Path, layout.BackupsDir(), + platform.VersionDirName(series, opts.ZTS), time.Now().UnixNano()) + if err != nil { + rb.run() + return fmt.Errorf("backing up existing interpreter; rolled back: %w", err) + } + origPath := existing.Path + rb.add(func() error { return restoreBackup(backupPath, origPath) }) + backup = &manifest.Backup{OriginalPath: origPath, BackupPath: backupPath, CreatedAt: opts.clock()} + maybeWarnManaged(opts, existing) + } + + // --- activate (symlink into the chosen bin dir) --- + prevTarget, _, hadPrev := platform.ReadActive(linkDir, "php") + activePath, kind, err := platform.Activate(linkDir, "php", binTarget) if err != nil { rb.run() - return fmt.Errorf("activating interpreter: %w", err) + return fmt.Errorf("activating interpreter; rolled back: %w", err) } rb.add(func() error { if hadPrev { - _, _, e := platform.Activate(binDir, "php", prevTarget) + _, _, e := platform.Activate(linkDir, "php", prevTarget) return e } - return platform.RemoveActive(binDir, "php") + return platform.RemoveActive(linkDir, "php") }) // --- post-verify via the activated entry --- @@ -182,7 +238,7 @@ func InstallInterpreter(ctx context.Context, opts Options) error { } key := platform.VersionDirName(series, opts.ZTS) m.InstallRoot = layout.Root - m.BinDir = binDir + m.BinDir = linkDir m.SetInterpreter(key, manifest.Interpreter{ Series: series, PHPVersion: info.Version, @@ -190,18 +246,78 @@ func InstallInterpreter(ctx context.Context, opts Options) error { ReleaseTag: rel.TagName, Dir: versionDir, InstalledAt: opts.clock(), + ConfigFiles: configFiles, }) m.SetActive(key) + if backup != nil { + m.SetBackup(key, *backup) + } if err := m.Save(layout.ManifestPath()); err != nil { rb.run() return fmt.Errorf("saving manifest; rolled back: %w", err) } opts.logf("Installed php %s (%s). Active php -> %s", info.Version, kind, activePath) - warnIfNotOnPATH(opts, p.OS, binDir) + if !replaceExisting { + warnIfNotOnPATH(opts, p.OS, linkDir) + } return nil } +// detectExisting finds a pre-existing php on PATH and queries it. It returns nil +// if none is found, if it cannot be queried, or if it is our own previous +// install under root (which must not be treated as a foreign interpreter). +func detectExisting(ctx context.Context, opts Options, root string) *php.Info { + path, err := php.Detect() + if err != nil { + return nil + } + if resolved, e := filepath.EvalSymlinks(path); e == nil && isUnderRoot(resolved, root) { + return nil // our own previously-installed interpreter + } + info, err := php.Query(ctx, path) + if err != nil { + opts.logf("Note: found php at %s but could not query it (%v); treating as clean host.", path, err) + return nil + } + return info +} + +// chooseLinkDir decides where the active `php` entry goes. When an existing +// interpreter is found and its directory is writable, we replace it in place; +// otherwise we use the scope's first writable bin dir. +func chooseLinkDir(existing *php.Info, layout platform.Layout) (dir string, replace bool, err error) { + if existing != nil { + d := filepath.Dir(existing.Path) + if platform.IsDirWritable(d) { + return d, true, nil + } + } + binDir, err := platform.SelectBinDir(layout.BinCandidates) + if err != nil { + return "", false, fmt.Errorf("%w\ntry --user for a per-user install, or re-run with elevated privileges", err) + } + return binDir, false, nil +} + +func isUnderRoot(path, root string) bool { + rel, err := filepath.Rel(root, path) + if err != nil { + return false + } + return rel != ".." && !strings.HasPrefix(rel, ".."+string(os.PathSeparator)) +} + +// maybeWarnManaged warns when the replaced interpreter appears to be managed by a +// package manager (e.g. Homebrew), since a future upgrade may recreate it. +func maybeWarnManaged(opts Options, existing *php.Info) { + target, _ := filepath.EvalSymlinks(existing.Path) + low := strings.ToLower(existing.Path + " " + target) + if strings.Contains(low, "cellar") || strings.Contains(low, "homebrew") { + opts.logf("Note: %s looks package-manager-managed; a future upgrade (e.g. `brew upgrade`) may recreate it and shadow this install.", existing.Path) + } +} + func threading(zts bool) string { if zts { return "zts" diff --git a/internal/installer/install_test.go b/internal/installer/install_test.go index dac1d86..687df36 100644 --- a/internal/installer/install_test.go +++ b/internal/installer/install_test.go @@ -17,24 +17,40 @@ import ( "github.com/php-debugger/installer/internal/release" ) +// isolatePATH points PATH at an empty dir so php.Detect() finds no pre-existing +// interpreter (unless a test adds one). Prevents tests from touching the real +// system php. +func isolatePATH(t *testing.T) { + t.Helper() + t.Setenv("PATH", t.TempDir()) +} + // fakePHP is a /bin/sh script that impersonates a php binary well enough for the // installer's smoke test, info query and module check. hasDebugger controls -// whether `-m` lists the debugger module (to exercise the rollback path). -func fakePHP(hasDebugger bool) string { +// whether `-m` lists the debugger module. cfgDir/scanDir, when non-empty, are +// reported by `--ini` as the compiled config paths. +func fakePHP(version string, hasDebugger bool, cfgDir, scanDir string) string { modules := "[PHP Modules]\\nCore\\ndate\\n" if hasDebugger { modules += "php_debugger\\n" } - return `#!/bin/sh + series := version + if i := strings.LastIndex(version, "."); i >= 0 { + series = version[:i] + } + return fmt.Sprintf(`#!/bin/sh case "$1" in - -v) echo "PHP 8.3.7 (cli) (built: Jan 1 2026) (NTS)" ;; - -m) printf '` + modules + `' ;; - --ini) echo "Loaded Configuration File: (none)" ;; - -r) printf 'version=8.3.7\nseries=8.3\nzts=0\nextension_dir=/fake/ext\n' ;; + -v) echo "PHP %s (cli) (built: Jan 1 2026) (NTS)" ;; + -m) printf '%s' ;; + --ini) + echo "Configuration File (php.ini) Path: \"%s\"" + echo "Loaded Configuration File: (none)" + echo "Scan for additional .ini files in: \"%s\"" ;; + -r) printf 'version=%s\nseries=%s\nzts=0\nextension_dir=/fake/ext\n' ;; *) : ;; esac exit 0 -` +`, version, modules, cfgDir, scanDir, version, series) } // newFakeReleaseServer serves a latest-release payload pointing at a single @@ -67,12 +83,23 @@ func linuxUserEnv(home string) platform.Env { } } +func writeExec(t *testing.T, path, content string) { + t.Helper() + if err := os.MkdirAll(filepath.Dir(path), 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(path, []byte(content), 0o755); err != nil { + t.Fatal(err) + } +} + func TestInstallInterpreterCleanHost(t *testing.T) { if runtime.GOOS == "windows" { t.Skip("fake php is a /bin/sh script") } + isolatePATH(t) home := t.TempDir() - srv := newFakeReleaseServer(t, fakePHP(true)) + srv := newFakeReleaseServer(t, fakePHP("8.3.7", true, "", "")) client := release.NewClient() client.BaseURL = srv.URL @@ -80,10 +107,7 @@ func TestInstallInterpreterCleanHost(t *testing.T) { env := linuxUserEnv(home) var out bytes.Buffer err := InstallInterpreter(context.Background(), Options{ - Scope: platform.User, - Out: &out, - Client: client, - Env: &env, + Scope: platform.User, Out: &out, Client: client, Env: &env, }) if err != nil { t.Fatalf("InstallInterpreter: %v\n--- output ---\n%s", err, out.String()) @@ -94,16 +118,11 @@ func TestInstallInterpreterCleanHost(t *testing.T) { if _, err := os.Stat(binTarget); err != nil { t.Errorf("interpreter binary not placed at %s: %v", binTarget, err) } - link := filepath.Join(home, ".local", "bin", "php") got, err := os.Readlink(link) - if err != nil { - t.Fatalf("active symlink missing: %v", err) - } - if got != binTarget { - t.Errorf("symlink -> %q, want %q", got, binTarget) + if err != nil || got != binTarget { + t.Fatalf("active symlink -> %q (err %v), want %q", got, err, binTarget) } - m, err := manifest.Load(filepath.Join(root, "manifest.json")) if err != nil { t.Fatal(err) @@ -111,15 +130,105 @@ func TestInstallInterpreterCleanHost(t *testing.T) { if m.Active() != "8.3" { t.Errorf("manifest active = %q, want 8.3", m.Active()) } - it, ok := m.Interpreter("8.3") + if _, ok := m.Backup("8.3"); ok { + t.Error("clean host should not record a backup") + } +} + +func TestInstallInterpreterReplacesExisting(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("fake php is a /bin/sh script") + } + home := t.TempDir() + + // The new interpreter reports a config path/scan dir we control. + newCfg := filepath.Join(t.TempDir(), "newcfg") + newScan := filepath.Join(newCfg, "conf.d") + srv := newFakeReleaseServer(t, fakePHP("8.3.7", true, newCfg, newScan)) + + // A pre-existing php on PATH, with ini files that load xdebug and set a + // disallowed xdebug.mode. + existBin := filepath.Join(t.TempDir(), "bin") + existIni := t.TempDir() + existConfd := filepath.Join(existIni, "conf.d") + writeExec(t, filepath.Join(existBin, "php"), existingPHPScript("8.2.9", + filepath.Join(existIni, "php.ini"), existConfd, + filepath.Join(existConfd, "20-xdebug.ini"))) + if err := os.WriteFile(filepath.Join(existIni, "php.ini"), + []byte("zend_extension=xdebug.so\nextension=mysqli.so\nmemory_limit=128M\nxdebug.mode=develop,debug\n"), 0o644); err != nil { + t.Fatal(err) + } + if err := os.MkdirAll(existConfd, 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(existConfd, "20-xdebug.ini"), + []byte("zend_extension=/opt/xdebug.so\n"), 0o644); err != nil { + t.Fatal(err) + } + t.Setenv("PATH", existBin) // php.Detect() finds our fake existing php + + client := release.NewClient() + client.BaseURL = srv.URL + env := linuxUserEnv(home) + + var out bytes.Buffer + err := InstallInterpreter(context.Background(), Options{ + Scope: platform.User, AssumeYes: true, Out: &out, Client: client, Env: &env, + }) + if err != nil { + t.Fatalf("InstallInterpreter: %v\n--- output ---\n%s", err, out.String()) + } + + root := filepath.Join(home, ".local", "share", "php-debugger") + binTarget := filepath.Join(root, "8.3", "bin", "php") + + // The active php is placed at the existing interpreter's location. + link := filepath.Join(existBin, "php") + if tgt, err := os.Readlink(link); err != nil || tgt != binTarget { + t.Errorf("existing php not replaced by symlink: -> %q (err %v), want %q", tgt, err, binTarget) + } + + // A backup of the original was recorded and exists on disk. + m, err := manifest.Load(filepath.Join(root, "manifest.json")) + if err != nil { + t.Fatal(err) + } + b, ok := m.Backup("8.3") if !ok { - t.Fatal("interpreter 8.3 not recorded in manifest") + t.Fatal("no backup recorded for replaced interpreter") + } + if b.OriginalPath != link { + t.Errorf("backup OriginalPath = %q, want %q", b.OriginalPath, link) + } + if _, err := os.Stat(b.BackupPath); err != nil { + t.Errorf("backup file missing: %v", err) + } + + // Config was copied to the new interpreter's config path and sanitized. + mainIni, err := os.ReadFile(filepath.Join(newCfg, "php.ini")) + if err != nil { + t.Fatalf("main php.ini not copied: %v", err) + } + s := string(mainIni) + if strings.Contains(s, "xdebug.so") || strings.Contains(strings.ToLower(s), "zend_extension") { + t.Errorf("xdebug loader not stripped from php.ini:\n%s", s) + } + if !strings.Contains(s, "memory_limit=128M") { + t.Errorf("non-xdebug settings should be preserved:\n%s", s) + } + // non-xdebug extension loaders are commented out, not removed or left active. + if !strings.Contains(s, "; extension=mysqli.so") { + t.Errorf("mysqli loader should be commented out:\n%s", s) } - if it.PHPVersion != "8.3.7" || it.ReleaseTag != "9.9.9" { - t.Errorf("interpreter record = %+v", it) + if strings.Contains(s, "develop") || !strings.Contains(s, "xdebug.mode=debug") { + t.Errorf("xdebug.mode not sanitized to debug:\n%s", s) } - if it.Dir != filepath.Join(root, "8.3") { - t.Errorf("interpreter dir = %q", it.Dir) + if _, err := os.Stat(filepath.Join(newScan, "20-xdebug.ini")); err != nil { + t.Errorf("additional ini not copied: %v", err) + } + it, _ := m.Interpreter("8.3") + if len(it.ConfigFiles) != 2 { + t.Errorf("expected 2 config files recorded, got %v", it.ConfigFiles) } } @@ -127,19 +236,16 @@ func TestInstallInterpreterRollbackOnMissingModule(t *testing.T) { if runtime.GOOS == "windows" { t.Skip("fake php is a /bin/sh script") } + isolatePATH(t) home := t.TempDir() - srv := newFakeReleaseServer(t, fakePHP(false)) // -m does NOT list php-debugger + srv := newFakeReleaseServer(t, fakePHP("8.3.7", false, "", "")) // no debugger module client := release.NewClient() client.BaseURL = srv.URL - env := linuxUserEnv(home) var out bytes.Buffer err := InstallInterpreter(context.Background(), Options{ - Scope: platform.User, - Out: &out, - Client: client, - Env: &env, + Scope: platform.User, Out: &out, Client: client, Env: &env, }) if err == nil { t.Fatalf("expected install to fail when module is missing\n%s", out.String()) @@ -149,40 +255,76 @@ func TestInstallInterpreterRollbackOnMissingModule(t *testing.T) { } root := filepath.Join(home, ".local", "share", "php-debugger") - - // version dir removed if _, err := os.Stat(filepath.Join(root, "8.3")); !os.IsNotExist(err) { t.Error("version directory should have been rolled back") } - // symlink not left behind if _, err := os.Lstat(filepath.Join(home, ".local", "bin", "php")); !os.IsNotExist(err) { t.Error("active symlink should have been rolled back") } - // manifest never written (save happens only after verification) if _, err := os.Stat(filepath.Join(root, "manifest.json")); !os.IsNotExist(err) { t.Error("manifest should not exist after a rolled-back install") } } -func TestInstallInterpreterSmokeFailureNoChanges(t *testing.T) { +func TestInstallRollbackRestoresReplacedInterpreter(t *testing.T) { if runtime.GOOS == "windows" { t.Skip("fake php is a /bin/sh script") } home := t.TempDir() - // a "binary" that exits non-zero on -v - badPHP := "#!/bin/sh\nexit 3\n" - srv := newFakeReleaseServer(t, badPHP) + // New interpreter LACKS the debugger module -> post-verify fails -> rollback. + newCfg := filepath.Join(t.TempDir(), "newcfg") + srv := newFakeReleaseServer(t, fakePHP("8.3.7", false, newCfg, filepath.Join(newCfg, "conf.d"))) + + existBin := filepath.Join(t.TempDir(), "bin") + existIni := t.TempDir() + existPhp := filepath.Join(existBin, "php") + writeExec(t, existPhp, existingPHPScript("8.2.9", + filepath.Join(existIni, "php.ini"), filepath.Join(existIni, "conf.d"), "")) + if err := os.WriteFile(filepath.Join(existIni, "php.ini"), []byte("memory_limit=99M\n"), 0o644); err != nil { + t.Fatal(err) + } + origContent, _ := os.ReadFile(existPhp) + t.Setenv("PATH", existBin) client := release.NewClient() client.BaseURL = srv.URL + env := linuxUserEnv(home) + var out bytes.Buffer + err := InstallInterpreter(context.Background(), Options{ + Scope: platform.User, AssumeYes: true, Out: &out, Client: client, Env: &env, + }) + if err == nil { + t.Fatal("expected failure due to missing debugger module") + } + + // The original interpreter must be restored at its location. + restored, rerr := os.ReadFile(existPhp) + if rerr != nil { + t.Fatalf("original interpreter not restored: %v", rerr) + } + if !bytes.Equal(restored, origContent) { + t.Error("restored interpreter content differs from original") + } + // It must be a real file again, not our symlink. + if isLink, _ := platform.IsSymlink(existPhp); isLink { + t.Error("restored path should not be a symlink") + } +} +func TestInstallInterpreterSmokeFailureNoChanges(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("fake php is a /bin/sh script") + } + isolatePATH(t) + home := t.TempDir() + srv := newFakeReleaseServer(t, "#!/bin/sh\nexit 3\n") + + client := release.NewClient() + client.BaseURL = srv.URL env := linuxUserEnv(home) var out bytes.Buffer err := InstallInterpreter(context.Background(), Options{ - Scope: platform.User, - Out: &out, - Client: client, - Env: &env, + Scope: platform.User, Out: &out, Client: client, Env: &env, }) if err == nil { t.Fatal("expected smoke-test failure") @@ -190,7 +332,6 @@ func TestInstallInterpreterSmokeFailureNoChanges(t *testing.T) { if !strings.Contains(err.Error(), "--extension-only") { t.Errorf("smoke failure should suggest the extension, got: %v", err) } - // nothing should have been created under the install root if _, err := os.Stat(filepath.Join(home, ".local", "share", "php-debugger", "8.3")); !os.IsNotExist(err) { t.Error("no version dir should be created when the smoke test fails") } @@ -207,3 +348,29 @@ func TestThreadingAndBinaryName(t *testing.T) { t.Error("unix binary should be php") } } + +// existingPHPScript builds a fake pre-existing php that reports the given ini +// file locations via --ini (so the installer copies them). +func existingPHPScript(version, loadedFile, scanDir, additional string) string { + add := "(none)" + if additional != "" { + add = `\"` + additional + `\"` + } + series := version + if i := strings.LastIndex(version, "."); i >= 0 { + series = version[:i] + } + return fmt.Sprintf(`#!/bin/sh +case "$1" in + -v) echo "PHP %s (cli) (built: Jan 1 2026) (NTS)" ;; + -m) printf '[PHP Modules]\nCore\nxdebug\n' ;; + --ini) + echo "Loaded Configuration File: \"%s\"" + echo "Scan for additional .ini files in: \"%s\"" + echo "Additional .ini files parsed: %s" ;; + -r) printf 'version=%s\nseries=%s\nzts=0\nextension_dir=/fake/ext\n' ;; + *) : ;; +esac +exit 0 +`, version, loadedFile, scanDir, add, version, series) +} diff --git a/internal/manifest/manifest.go b/internal/manifest/manifest.go index 6aa168a..448f466 100644 --- a/internal/manifest/manifest.go +++ b/internal/manifest/manifest.go @@ -41,6 +41,10 @@ type Interpreter struct { ReleaseTag string `json:"releaseTag"` Dir string `json:"dir"` // absolute install directory InstalledAt time.Time `json:"installedAt"` + // ConfigFiles are ini files this install wrote into the interpreter's + // compiled-in config path (copied from a replaced interpreter), recorded so + // uninstall can remove them. + ConfigFiles []string `json:"configFiles,omitempty"` } // Backup records an interpreter that was replaced during an install, so it can diff --git a/internal/php/parse.go b/internal/php/parse.go index 7c4e64e..71ee012 100644 --- a/internal/php/parse.go +++ b/internal/php/parse.go @@ -4,6 +4,9 @@ import "strings" // IniPaths captures the ini file locations reported by `php --ini`. type IniPaths struct { + // ConfigPath is the directory php looks in for its main php.ini (the + // "Configuration File (php.ini) Path"). This is compiled into the binary. + ConfigPath string // LoadedFile is the main php.ini actually loaded ("" if none). LoadedFile string // ScanDir is the directory scanned for additional .ini files ("" if none). @@ -36,6 +39,8 @@ func parseIniOutput(out string) IniPaths { lines := strings.Split(out, "\n") for i, ln := range lines { switch { + case strings.HasPrefix(ln, "Configuration File (php.ini) Path:"): + p.ConfigPath = cleanIniValue(afterColon(ln)) case strings.HasPrefix(ln, "Loaded Configuration File:"): p.LoadedFile = cleanIniValue(afterColon(ln)) case strings.HasPrefix(ln, "Scan for additional .ini files in:"): @@ -89,9 +94,10 @@ func afterColon(s string) string { return s } -// cleanIniValue trims a value and normalizes PHP's "(none)" placeholder to "". +// cleanIniValue trims a value, strips surrounding double quotes (newer PHP quotes +// these paths) and normalizes PHP's "(none)" placeholder to "". func cleanIniValue(s string) string { - s = strings.TrimSpace(s) + s = unquotePath(strings.TrimSpace(s)) if s == "(none)" { return "" } @@ -99,11 +105,11 @@ func cleanIniValue(s string) string { } // parseFileList splits a comma/whitespace separated list of file paths, dropping -// empties and "(none)". +// empties and "(none)", stripping surrounding quotes on each entry. func parseFileList(s string) []string { var out []string for _, part := range strings.Split(s, ",") { - part = strings.TrimSpace(part) + part = unquotePath(strings.TrimSpace(part)) if part == "" || part == "(none)" { continue } @@ -111,3 +117,11 @@ func parseFileList(s string) []string { } return out } + +// unquotePath removes a single pair of surrounding double quotes, then trims. +func unquotePath(s string) string { + if len(s) >= 2 && s[0] == '"' && s[len(s)-1] == '"' { + s = s[1 : len(s)-1] + } + return strings.TrimSpace(s) +} diff --git a/internal/php/parse_test.go b/internal/php/parse_test.go index ba62f11..4b10252 100644 --- a/internal/php/parse_test.go +++ b/internal/php/parse_test.go @@ -39,6 +39,7 @@ Additional .ini files parsed: /etc/php/8.3/cli/conf.d/10-opcache.ini, ` got := parseIniOutput(out) want := IniPaths{ + ConfigPath: "/etc/php/8.3/cli", LoadedFile: "/etc/php/8.3/cli/php.ini", ScanDir: "/etc/php/8.3/cli/conf.d", AdditionalFiles: []string{ @@ -52,6 +53,28 @@ Additional .ini files parsed: /etc/php/8.3/cli/conf.d/10-opcache.ini, } } +func TestParseIniOutputQuotedPaths(t *testing.T) { + // Newer PHP (e.g. 8.5) quotes the paths in --ini output. + out := `Configuration File (php.ini) Path: "/opt/homebrew/etc/php/8.5" +Loaded Configuration File: "/opt/homebrew/etc/php/8.5/php.ini" +Scan for additional .ini files in: "/opt/homebrew/etc/php/8.5/conf.d" +Additional .ini files parsed: "/opt/homebrew/etc/php/8.5/conf.d/20-debug.ini" +` + got := parseIniOutput(out) + if got.ConfigPath != "/opt/homebrew/etc/php/8.5" { + t.Errorf("ConfigPath = %q", got.ConfigPath) + } + if got.LoadedFile != "/opt/homebrew/etc/php/8.5/php.ini" { + t.Errorf("LoadedFile = %q", got.LoadedFile) + } + if got.ScanDir != "/opt/homebrew/etc/php/8.5/conf.d" { + t.Errorf("ScanDir = %q", got.ScanDir) + } + if len(got.AdditionalFiles) != 1 || got.AdditionalFiles[0] != "/opt/homebrew/etc/php/8.5/conf.d/20-debug.ini" { + t.Errorf("AdditionalFiles = %v", got.AdditionalFiles) + } +} + func TestParseIniOutputNone(t *testing.T) { out := `Configuration File (php.ini) Path: /usr/local/etc/php Loaded Configuration File: (none)