Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 2 additions & 5 deletions crates/clickhousectl/src/version_manager/install.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,17 +9,14 @@ use crate::version_manager::spec::VersionSpec;
use std::os::unix::fs::PermissionsExt;

/// Install a version spec, trying installed versions first before any remote call.
/// Matches the UX of `install_resolved`'s post-resolve local check, but avoids the
/// network round-trip when a local match exists.
/// An installed match is a successful no-op regardless of whether the spec is
/// exact or partial.
pub async fn install_local_first(
spec: &VersionSpec,
platform: &Platform,
force: bool,
) -> Result<String> {
if !force && let Some(local) = try_resolve_local(spec) {
if matches!(spec, VersionSpec::Exact(_)) {
return Err(Error::VersionAlreadyInstalled(local));
}
eprintln!("ClickHouse {} is already installed as {}", spec, local);
eprintln!("Use --force to re-download the latest build");
return Ok(local);
Expand Down
60 changes: 50 additions & 10 deletions crates/clickhousectl/tests/local_install_local_first_test.rs
Original file line number Diff line number Diff line change
@@ -1,8 +1,10 @@
//! Regression test for issue #217: `local install <concrete-spec>` must satisfy
//! the request from already-installed versions without any remote call.
//! Regression tests for local-first installs. Issue #217 requires
//! `local install <concrete-spec>` to satisfy the request from already-installed
//! versions without any remote call, while issue #339 requires exact and partial
//! spellings of an installed version to both be successful no-ops.
//!
//! Strategy: spawn the binary with `HOME=<tempdir>`, pre-seed a fake installed
//! version, run `local install 25.12 --json`, and assert that:
//! version, run both partial and exact `local install` commands, and assert that:
//! - the command exits 0,
//! - stderr says "already installed",
//! - stderr does NOT say "Resolving" (which only prints on the remote path).
Expand All @@ -13,17 +15,19 @@

use std::os::unix::fs::PermissionsExt;
use std::path::PathBuf;
use std::process::Command;
use std::process::{Command, Output};

fn clickhousectl_binary() -> PathBuf {
PathBuf::from(env!("CARGO_BIN_EXE_clickhousectl"))
}

#[test]
fn local_install_minor_with_existing_match_does_not_hit_network() {
fn install_with_existing_version(installed_version: &str, requested_spec: &str) -> Output {
let tempdir = tempfile::tempdir().expect("create tempdir");

let version_dir = tempdir.path().join(".clickhouse/versions/25.12.9.61");
let version_dir = tempdir
.path()
.join(".clickhouse/versions")
.join(installed_version);
std::fs::create_dir_all(&version_dir).expect("create version dir");

let binary = version_dir.join("clickhouse");
Expand All @@ -32,12 +36,17 @@ fn local_install_minor_with_existing_match_does_not_hit_network() {
perms.set_mode(0o755);
std::fs::set_permissions(&binary, perms).unwrap();

let output = Command::new(clickhousectl_binary())
Command::new(clickhousectl_binary())
.env("DO_NOT_TRACK", "1")
.env("HOME", tempdir.path())
.args(["local", "install", "25.12", "--json"])
.args(["local", "install", requested_spec, "--json"])
.output()
.expect("run clickhousectl");
.expect("run clickhousectl")
}

#[test]
fn local_install_minor_with_existing_match_does_not_hit_network() {
let output = install_with_existing_version("25.12.9.61", "25.12");

let stderr = String::from_utf8_lossy(&output.stderr);
assert!(
Expand All @@ -57,3 +66,34 @@ fn local_install_minor_with_existing_match_does_not_hit_network() {
stderr
);
}

#[test]
fn local_install_exact_with_existing_match_is_a_successful_no_op() {
let version = "25.12.9.61";
let output = install_with_existing_version(version, version);

let stderr = String::from_utf8_lossy(&output.stderr);
assert!(
output.status.success(),
"expected success, got status {:?}\nstderr: {}",
output.status,
stderr
);
assert!(
stderr.contains(&format!(
"ClickHouse {version} is already installed as {version}"
)),
"expected exact-version no-op message, got: {}",
stderr
);
assert!(
stderr.contains("Use --force to re-download the latest build"),
"expected --force hint, got: {}",
stderr
);
assert!(
!stderr.contains("Resolving"),
"expected no remote-resolve message, got: {}",
stderr
);
}