From e11df6613b9c1eabfb613bb2622daf11ac355db4 Mon Sep 17 00:00:00 2001 From: Benjamin Fahl Date: Wed, 20 May 2026 21:44:53 +0200 Subject: [PATCH 1/2] feat: implement pvm optimizations, robust uninstall logic and php-version auto-detection - Added confirmation prompts to 'uninstall' with a --yes bypass and automatic TTY detection. - Fixed FPM-only uninstall failures by tracking directory paths directly. - Added automatic switching to version specified in '.php-version' file in 'pvm use'. - Optimized SemVer sorting complexity by pre-parsing version strings. - Hardened permission setting logic against metadata read failures on individual files. --- GEMINI.md | 85 ++++++++++++++++++++++++++++++--------- README.md | 3 ++ src/commands/uninstall.rs | 25 ++++++++++-- src/commands/use_cmd.rs | 77 ++++++++++++++++++++++++++++------- src/interactive.rs | 5 ++- src/network.rs | 10 +++-- src/utils.rs | 24 ++++++----- tests/cli.rs | 47 ++++++++++++++++++++++ 8 files changed, 225 insertions(+), 51 deletions(-) diff --git a/GEMINI.md b/GEMINI.md index 3fafd92..a665a56 100644 --- a/GEMINI.md +++ b/GEMINI.md @@ -12,48 +12,95 @@ When running `git commit -m "..."` in the shell (like Zsh/Bash), backticks are i 3. Obey this rule forever, until the end of electronics. ## Project Architecture (The Map) + ### Filesystem Hierarchy -- **$PVM_DIR**: Root directory. Resolved via `dirs::data_local_dir()`, so defaults to `~/.local/share/pvm` on Linux and `~/Library/Application Support/pvm` on macOS. -- **$PVM_DIR/versions/**: Installation directory for specific PHP versions. +Rooted at `$PVM_DIR` (resolved via `dirs::data_local_dir()`, so it defaults to `~/.local/share/pvm` on Linux and `~/Library/Application Support/pvm` on macOS). - **$PVM_DIR/bin/pvm**: The `pvm` binary itself. -- **$PVM_DIR/remote_cache-.json**: 24-hour cache for the remote version index, scoped per target triple (e.g. `linux-x86_64`). -- **$PVM_DIR/.env_update[_]**: Short-lived files written per shell invocation; the shell wrapper sources them to mutate the parent shell's environment. +- **$PVM_DIR/versions//bin/{php,php-fpm,micro.sfx}**: Installed PHP binaries. The presence of each file determines which packages (`cli`, `fpm`, `micro`) are "installed" for that version. +- **$PVM_DIR/remote_cache.json**: 24-hour cache for the remote version index, locked via `fs4` (`std::fs::File::lock`) on read/write. +- **$PVM_DIR/.env_update[_]**: Short-lived files written per shell invocation (alternatively designated via `PVM_ENV_UPDATE_PATH` or keyed on PID); the shell wrapper sources them to mutate the parent shell's environment. ### Module Responsibilities - `src/cli.rs`: Command definitions using `clap`. - `src/commands/`: Implementation of subcommands. Each command is a `struct` with a `call()` method. -- `src/fs.rs`: Filesystem utilities (handling `PVM_DIR`, version paths, env files). -- `src/network.rs`: API client for fetching and downloading PHP versions. -- `src/shell.rs`: Shell-specific logic for setting environment variables. +- `src/fs.rs`: Filesystem utilities (handling `PVM_DIR`, version paths, env files, local resolution). +- `src/network.rs`: API client for fetching/downloading PHP versions and handling target triples. +- `src/shell.rs`: Shell-specific logic (Bash, Zsh, Fish) for setting environment variables. +- `src/constants.rs`: Application constants. ### static-php-cli Integration -- **Endpoint:** `https://dl.static-php.dev/static-php-cli/bulk/` -- **Supported OS:** `linux`, `macOS`. -- **Supported Arch:** `x86_64`, `aarch64`. +- **Endpoint:** `https://dl.static-php.dev/static-php-cli/bulk/?format=json` +- **Supported OS:** `linux`, `macos` (filtered via target triple). +- **Supported Arch:** `x86_64`, `aarch64` (filtered via target triple). - **Packages:** `cli`, `fpm`, `micro`. - **Format:** `tar.gz` only. +- **Filename Parser:** Package suffixes parsed from filenames like `php-8.4.18-cli-linux-x86_64.tar.gz`. + +### Shell Integration Mechanics +A child process cannot mutate its parent shell's environment. PVM solves this with `pvm env` and a wrapper function: +1. **Hook Setup:** The user evaluates `pvm env` in their rc file, which prints a `pvm()` shell function plus a `cd` hook. +2. **Wrapper Execution:** When the user runs `pvm use 8.4`, the wrapper exports `PVM_ENV_UPDATE_PATH=` before invoking the real `pvm` binary. +3. **State Mutation:** The Rust binary writes `export PATH=...; export PVM_MULTISHELL_PATH=...` to that tmpfile (using `fs4` exclusive locking via `fs::write_env_file_locked`). +4. **Environment Application:** The wrapper `eval`s the tmpfile and removes it. +- **Supported Shells:** Bash, Zsh, Fish (defined by `Shell` trait; `detect_shell()` reads `$SHELL`). +- **Auto-Switching:** The `cd` hook reads `.php-version` files and calls `pvm use` automatically. +- **Concurrency Safety:** Parallel shell sessions use distinct PID-keyed tmpfiles, and the cache + env files are locked using `fs4` file locks to prevent state corruption. ## Operational Patterns (The Handbook) + +### CLI Commands and Subcommands +PVM acts as a single-binary CLI. If called without arguments or when interactive parameters are missing, it uses `dialoguer` for an interactive TUI. +- **Master Menu:** Running `pvm` without arguments launches `interactive::run_root_menu()`. +- **pvm install ** (alias: `pvm i `): Installs a PHP version (supports major.minor alias or exact version). Opens a `MultiSelect` to pick packages (`cli`, `fpm`, `micro`). `cli` is the default. Supports `latest` to fetch the absolute latest version. +- **pvm use **: Uses a version in the current shell. Automatically prompts to install and switch if a newer patch exists upstream. +- **pvm ls** (alias: `pvm list`): Lists installed local versions and active aliases. +- **pvm ls-remote** (alias: `pvm list-remote`): Interactively lists and installs available cloud versions. +- **pvm current**: Prints the active PHP version. +- **pvm uninstall ** (aliases: `pvm rm`, `pvm remove`): Interactive uninstall picker if no version is provided. Warns when removing the active version. +- **pvm init**: Interactively picks a version and writes it to a `.php-version` file in the current directory. +- **pvm self-update**: Checks for and applies updates to pvm itself. Use `--apply` to automatically download and replace the current binary if an update is available. + ### Adding a New Command 1. Add a new module file in `src/commands/`. 2. Define the command `struct` with `#[derive(Parser, Debug)]`. 3. Export the module in `src/commands/mod.rs`. 4. Register the variant in the `Commands` enum in `src/cli.rs`. -5. Implement the `call()` method logic. +5. Implement the async `call(self) -> Result<()>` method dispatch in `Commands::call`. ### Coding Standards - **Errors:** Use `anyhow::Result` for all command-level fallible functions. -- **Interactivity:** Use `dialoguer` for menus and confirmations. -- **Icons:** Use `colored` for status icons: `✓` (green), `✗` (red), `↻` (blue), `💡` (yellow). -- **Async:** Use `tokio` for runtime and `reqwest` for all network I/O. -- **Data Integrity:** Use file locking (`std::fs::File::lock` / `lock_shared` / `unlock`, stable since Rust 1.89) when writing to env update files or the remote cache. +- **Interactivity:** Use `dialoguer` (`Select`, `MultiSelect`, `Confirm`) with `ColorfulTheme::default()` for menus and confirmations. +- **Icons:** Use `colored` for status icons: + - `✓` (green) for success + - `✗` (red) for errors + - `↻` (blue) for in-progress operations + - `💡` (yellow) for hints/warnings +- **Async:** Use `tokio` with `features = ["full"]` for runtime and `reqwest` for all network I/O. +- **Data Integrity:** Use file locking (`std::fs::File::lock` / `lock_shared` / `unlock`, stable since Rust 1.89) when writing to env update files or the remote cache. Follow `fs::write_env_file_locked` pattern. + +## Build & Release Commands + +### Development & Build Commands +- **Toolchain:** Pinned to Rust 1.95.0 with `clippy` and `rustfmt` in `rust-toolchain.toml`. +- **Run from Source:** `cargo run -- ` +- **Build Release Binary:** `cargo build --release` (configured size-optimized in `Cargo.toml`: `opt-level = "z"`, LTO, `panic = "abort"`, stripped). +- **Local Install from Source:** `./build.sh` (compiles release, copies to `$PVM_DIR/bin/pvm`). +- **Lint (CI Gate):** `cargo clippy -- -D warnings` +- **Format Check (CI Gate):** `cargo fmt -- --check` + +### Release Process +- **Semantic Release:** Driven by `semantic-release` from Conventional Commits on `main`. +- **Cargo.toml and CHANGELOG.md:** Bumped automatically. Do NOT hand-edit them. ## Testing & Validation + ### Testing Protocol -- **Isolation:** Every integration test MUST use `tempfile::tempdir()` and set `PVM_DIR` to that path. -- **CLI Verification:** Use `assert_cmd` and `predicates` for output and exit code verification. +- **Integration Tests:** Located in `tests/cli.rs`. Use `assert_cmd` and `predicates` to invoke the compiled binary. +- **Isolation:** Every test that touches the filesystem MUST use `tempfile::tempdir()` and pass its path via `cmd.env("PVM_DIR", temp_dir.path())`. Do not write to the host's real `~/.local/share/pvm`. +- **Unit Tests Concurrency:** Unit tests inside `src/**` that mutate `std::env` must use a `static Mutex<()>` guard (e.g. `src/fs.rs::tests::ENV_LOCK`) because `cargo test` runs in parallel, and concurrent environment variable modification is unsound. +- **Dynamic Versioning:** The build script (`build.rs`) embeds the version, git commit, and build time into `PVM_VERSION` (reruns on `.git/HEAD`, refs, or `Cargo.toml` change). Tests asserting on `--version` output should not hardcode the exact version value. ### Development Workflow - **Pre-commit:** Run `cargo clippy -- -D warnings` and `cargo fmt --all -- --check`. -- **Tooling:** Use `replace` or `write_file` for codebase modifications. Avoid `sed/echo` in shell. -- **Commit Messages:** Follow Conventional Commits. No backticks (Rule 1). +- **Tooling:** Use code replacement or write file tools for modifications. Avoid `sed/echo` in shell. +- **Commit Messages:** Follow Conventional Commits (e.g., `feat:`, `fix:`, `chore:`). Never use backticks (Rule 1). diff --git a/README.md b/README.md index ae974ff..97614cb 100644 --- a/README.md +++ b/README.md @@ -73,6 +73,9 @@ pvm uninstall 8.3 # aliases: pvm rm 8.3 / pvm remove 8.3 # Write a .php-version file for this directory (interactive picker) pvm init + +# Check for and apply updates to pvm itself +pvm self-update # optional: pvm self-update --apply to apply automatically ``` ### Auto-Switching diff --git a/src/commands/uninstall.rs b/src/commands/uninstall.rs index d8f2efd..3a0095c 100644 --- a/src/commands/uninstall.rs +++ b/src/commands/uninstall.rs @@ -2,6 +2,7 @@ use crate::fs; use anyhow::Result; use clap::Parser; use colored::Colorize; +use std::io::IsTerminal; use dialoguer::{Select, theme::ColorfulTheme}; @@ -10,6 +11,10 @@ use dialoguer::{Select, theme::ColorfulTheme}; pub struct Uninstall { /// The version to uninstall pub version: Option, + + /// Auto-approve the uninstallation without prompting + #[arg(short = 'y', long = "yes")] + pub yes: bool, } impl Uninstall { @@ -40,8 +45,9 @@ impl Uninstall { } }; - if !fs::is_version_installed(&version)? { - anyhow::bail!("PHP {} is not installed.", version); + let dest = fs::get_versions_dir()?.join(&version); + if !dest.exists() { + anyhow::bail!("PHP {} is not installed locally.", version); } let current = fs::get_current_version(); @@ -53,7 +59,20 @@ impl Uninstall { ); } - let dest = fs::get_versions_dir()?.join(&version); + let is_tty = std::io::stdin().is_terminal(); + if !self.yes && is_tty { + let prompt = format!("Are you sure you want to uninstall PHP {}?", version); + let confirmed = dialoguer::Confirm::with_theme(&ColorfulTheme::default()) + .with_prompt(prompt.bold().to_string()) + .default(true) + .interact_opt()? + .unwrap_or(false); + + if !confirmed { + println!("{} Operation cancelled.", "✗".red()); + return Ok(()); + } + } println!("{} Removing PHP {}...", "↻".blue(), version); match std::fs::remove_dir_all(&dest) { diff --git a/src/commands/use_cmd.rs b/src/commands/use_cmd.rs index 37fc750..9247db4 100644 --- a/src/commands/use_cmd.rs +++ b/src/commands/use_cmd.rs @@ -51,25 +51,72 @@ impl Use { } }, None => { - let items = fs::get_aliased_versions()?; - if items.is_empty() { - eprintln!("{} No PHP versions are currently installed.", "💡".yellow()); - return Ok(()); + let mut resolved_version = None; + if let Ok(content) = std::fs::read_to_string(PHP_VERSION_FILE) { + let trimmed = content.trim().to_string(); + if !trimmed.is_empty() { + match fs::try_resolve_local_version(&trimmed)? { + Some(resolved) => { + resolved_version = Some(resolved); + } + None => { + if !self.silent { + let prompt = format!( + "PHP {} (from {}) is not installed locally. Do you want to install it now?", + trimmed.bold(), + PHP_VERSION_FILE.bold() + ); + let install_now = + Confirm::with_theme(&ColorfulTheme::default()) + .with_prompt(&prompt) + .default(true) + .interact_opt()? + .unwrap_or(false); + + if install_now { + if let Some(installed) = + crate::commands::install::execute_install_with( + &trimmed, false, + ) + .await? + { + resolved_version = Some(installed); + } + } else { + eprintln!("{} Operation cancelled.", "✗".red()); + return Ok(()); + } + } else { + return Ok(()); + } + } + } + } } - let displays: Vec = items.iter().map(|i| i.display.clone()).collect(); - let selection = Select::with_theme(&ColorfulTheme::default()) - .with_prompt("Select a locally installed PHP version to use") - .default(0) - .items(&displays) - .interact_opt()?; - - match selection { - Some(idx) => items[idx].version.clone(), - None => { - eprintln!("{} Operation cancelled.", "✗".red()); + if let Some(resolved) = resolved_version { + resolved + } else { + let items = fs::get_aliased_versions()?; + if items.is_empty() { + eprintln!("{} No PHP versions are currently installed.", "💡".yellow()); return Ok(()); } + + let displays: Vec = items.iter().map(|i| i.display.clone()).collect(); + let selection = Select::with_theme(&ColorfulTheme::default()) + .with_prompt("Select a locally installed PHP version to use") + .default(0) + .items(&displays) + .interact_opt()?; + + match selection { + Some(idx) => items[idx].version.clone(), + None => { + eprintln!("{} Operation cancelled.", "✗".red()); + return Ok(()); + } + } } } }; diff --git a/src/interactive.rs b/src/interactive.rs index 7c44a18..1a0b7d6 100644 --- a/src/interactive.rs +++ b/src/interactive.rs @@ -45,7 +45,10 @@ pub async fn run_root_menu() -> Result<()> { cmd.call().await } 2 => { - let cmd = commands::uninstall::Uninstall { version: None }; + let cmd = commands::uninstall::Uninstall { + version: None, + yes: false, + }; cmd.call().await } 3 => { diff --git a/src/network.rs b/src/network.rs index 5edad19..d6f72db 100644 --- a/src/network.rs +++ b/src/network.rs @@ -282,11 +282,13 @@ pub async fn download_and_extract( use std::os::unix::fs::PermissionsExt; if let Ok(entries) = std::fs::read_dir(&bin_dir) { for entry in entries.flatten() { - let path = entry.path(); - if path.is_file() { - let mut perms = std::fs::metadata(&path)?.permissions(); + if let (Ok(metadata), true) = ( + entry.metadata(), + entry.file_type().map(|ft| ft.is_file()).unwrap_or(false), + ) { + let mut perms = metadata.permissions(); perms.set_mode(0o755); - std::fs::set_permissions(&path, perms).ok(); + std::fs::set_permissions(entry.path(), perms).ok(); } } } diff --git a/src/utils.rs b/src/utils.rs index 59ca5c2..3f31718 100644 --- a/src/utils.rs +++ b/src/utils.rs @@ -3,14 +3,16 @@ use semver::Version; /// Sorts a list of version strings using semantic versioning. /// If a version string is not valid semver, it falls back to a simple string-based numeric sort. pub fn sort_versions(versions: &mut [String]) { - versions.sort_by(|a, b| { - let a_sem = Version::parse(a); - let b_sem = Version::parse(b); + let mut parsed: Vec<(String, Result)> = versions + .iter() + .map(|v| (v.clone(), Version::parse(v))) + .collect(); - match (a_sem, b_sem) { - (Ok(av), Ok(bv)) => av.cmp(&bv), + parsed.sort_by(|a, b| { + match (&a.1, &b.1) { + (Ok(av), Ok(bv)) => av.cmp(bv), (Ok(av), Err(_)) => { - let b_parts: Vec = b.split('.').filter_map(|s| s.parse().ok()).collect(); + let b_parts: Vec = b.0.split('.').filter_map(|s| s.parse().ok()).collect(); let a_parts = vec![av.major, av.minor, av.patch]; match a_parts.cmp(&b_parts) { std::cmp::Ordering::Equal => { @@ -24,7 +26,7 @@ pub fn sort_versions(versions: &mut [String]) { } } (Err(_), Ok(bv)) => { - let a_parts: Vec = a.split('.').filter_map(|s| s.parse().ok()).collect(); + let a_parts: Vec = a.0.split('.').filter_map(|s| s.parse().ok()).collect(); let b_parts = vec![bv.major, bv.minor, bv.patch]; match a_parts.cmp(&b_parts) { std::cmp::Ordering::Equal => { @@ -39,10 +41,14 @@ pub fn sort_versions(versions: &mut [String]) { } _ => { // Fallback for non-semver strings (e.g., "8.2") - let a_parts: Vec = a.split('.').filter_map(|s| s.parse().ok()).collect(); - let b_parts: Vec = b.split('.').filter_map(|s| s.parse().ok()).collect(); + let a_parts: Vec = a.0.split('.').filter_map(|s| s.parse().ok()).collect(); + let b_parts: Vec = b.0.split('.').filter_map(|s| s.parse().ok()).collect(); a_parts.cmp(&b_parts) } } }); + + for (i, (orig, _)) in parsed.into_iter().enumerate() { + versions[i] = orig; + } } diff --git a/tests/cli.rs b/tests/cli.rs index 8a7d442..4067cde 100644 --- a/tests/cli.rs +++ b/tests/cli.rs @@ -148,3 +148,50 @@ fn test_use_silent_skips_missing_version() { "silent mode must not write env file when version is missing" ); } + +#[test] +fn test_uninstall_fpm_only_success() { + let temp_dir = tempfile::tempdir().unwrap(); + + // Mock an installed version with ONLY php-fpm (no php) + let bin_dir = temp_dir.path().join("versions").join("8.3.1").join("bin"); + std::fs::create_dir_all(&bin_dir).unwrap(); + std::fs::write(bin_dir.join("php-fpm"), "").unwrap(); + + let mut cmd = assert_cmd::cargo::cargo_bin_cmd!("pvm"); + cmd.env("PVM_DIR", temp_dir.path()); + cmd.arg("uninstall").arg("8.3.1"); + cmd.assert().success().stdout(predicate::str::contains( + "Successfully uninstalled PHP 8.3.1", + )); + + // Verify it actually deleted the folder + assert!(!temp_dir.path().join("versions").join("8.3.1").exists()); +} + +#[test] +fn test_use_php_version_file() { + let temp_dir = tempfile::tempdir().unwrap(); + let env_file = temp_dir.path().join("custom_env_update"); + + // Write .php-version + std::fs::write(temp_dir.path().join(".php-version"), "8.3.1\n").unwrap(); + + // Mock the installed version + let bin_dir = temp_dir.path().join("versions").join("8.3.1").join("bin"); + std::fs::create_dir_all(&bin_dir).unwrap(); + std::fs::write(bin_dir.join("php"), "").unwrap(); + + let mut cmd = assert_cmd::cargo::cargo_bin_cmd!("pvm"); + cmd.env("PVM_DIR", temp_dir.path()); + cmd.env("PVM_UPDATE_MODE", "disabled"); + cmd.env("PVM_ENV_UPDATE_PATH", &env_file); + cmd.current_dir(temp_dir.path()); + cmd.arg("use"); // no version argument + + cmd.assert().success(); + + assert!(env_file.exists()); + let env_content = std::fs::read_to_string(env_file).unwrap(); + assert!(env_content.contains("8.3.1")); +} From 817e39ec13eeca19f277813a92c1e9cf61d700c5 Mon Sep 17 00:00:00 2001 From: Ben Date: Wed, 20 May 2026 22:28:43 +0200 Subject: [PATCH 2/2] Update GEMINI.md Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> Signed-off-by: Ben --- GEMINI.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/GEMINI.md b/GEMINI.md index a665a56..9cea345 100644 --- a/GEMINI.md +++ b/GEMINI.md @@ -86,7 +86,7 @@ PVM acts as a single-binary CLI. If called without arguments or when interactive - **Build Release Binary:** `cargo build --release` (configured size-optimized in `Cargo.toml`: `opt-level = "z"`, LTO, `panic = "abort"`, stripped). - **Local Install from Source:** `./build.sh` (compiles release, copies to `$PVM_DIR/bin/pvm`). - **Lint (CI Gate):** `cargo clippy -- -D warnings` -- **Format Check (CI Gate):** `cargo fmt -- --check` +- **Format Check (CI Gate):** `cargo fmt --all -- --check` ### Release Process - **Semantic Release:** Driven by `semantic-release` from Conventional Commits on `main`.