Skip to content
Merged
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
118 changes: 73 additions & 45 deletions ampup/src/builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -510,7 +510,7 @@ impl<'a> GitRepo<'a> {
}
}

/// Build and install the ampd and ampctl binaries
/// Build and install the ampd, ampctl, and ampsql binaries
fn build_and_install(
version_manager: &VersionManager,
repo_path: &Path,
Expand All @@ -519,50 +519,40 @@ fn build_and_install(
) -> Result<()> {
check_command_exists("cargo")?;

ui::info!("Building ampd and ampctl");
let jobs_str = jobs.map(|j| j.to_string());

let mut args = vec!["build", "--release", "-p", "ampd", "-p", "ampctl"];
let config = version_manager.config();

// Create version directory
let version_dir = config.versions_dir.join(version_label);
fs::create_dir_all(&version_dir).context("Failed to create version directory")?;
Comment thread
LNSD marked this conversation as resolved.

// Build ampd (required)
ui::info!("Building ampd");

let jobs_str;
if let Some(j) = jobs {
jobs_str = j.to_string();
args.extend(["-j", &jobs_str]);
let mut ampd_args = vec!["build", "--release", "-p", "ampd"];
if let Some(ref j) = jobs_str {
ampd_args.extend(["-j", j]);
}

let status = Command::new("cargo")
.args(&args)
let ampd_status = Command::new("cargo")
.args(&ampd_args)
.current_dir(repo_path)
.status()
.context("Failed to execute cargo build")?;

if !status.success() {
if !ampd_status.success() {
return Err(BuildError::CargoBuildFailed.into());
}

// Find the built binaries
let ampd_source = repo_path.join("target/release/ampd");
let ampctl_source = repo_path.join("target/release/ampctl");

if !ampd_source.exists() {
return Err(BuildError::BinaryNotFound {
path: ampd_source.clone(),
}
.into());
}

if !ampctl_source.exists() {
return Err(BuildError::BinaryNotFound {
path: ampctl_source.clone(),
}
.into());
}

let config = version_manager.config();

// Create version directory
let version_dir = config.versions_dir.join(version_label);
fs::create_dir_all(&version_dir).context("Failed to create version directory")?;

// Copy ampd binary
let ampd_dest = version_dir.join("ampd");
fs::copy(&ampd_source, &ampd_dest).context("Failed to copy ampd binary")?;
Expand All @@ -578,29 +568,67 @@ fn build_and_install(
.context("Failed to set executable permissions on ampd")?;
}

// Copy ampctl binary
let ampctl_dest = version_dir.join("ampctl");
fs::copy(&ampctl_source, &ampctl_dest).context("Failed to copy ampctl binary")?;

#[cfg(unix)]
{
use std::os::unix::fs::PermissionsExt;
let mut perms = fs::metadata(&ampctl_dest)
.context("Failed to get ampctl metadata")?
.permissions();
perms.set_mode(0o755);
fs::set_permissions(&ampctl_dest, perms)
.context("Failed to set executable permissions on ampctl")?;
}
// Build the optional binaries best-effort
build_optional_binary("ampctl", repo_path, &version_dir, jobs_str.as_deref())?;
build_optional_binary("ampsql", repo_path, &version_dir, jobs_str.as_deref())?;

// Activate this version
version_manager.activate(version_label)?;

ui::success!(
"Built and installed ampd and ampctl {}",
ui::version(version_label)
);
ui::detail!("Run 'ampd --version' and 'ampctl --version' to verify installation");
ui::success!("Built and installed amp {}", ui::version(version_label));
ui::detail!("Run 'ampd --version' to verify installation");

Ok(())
}

/// Build an optional binary best-effort and install it into `version_dir`.
///
/// On success the freshly built binary is copied in (and made executable on Unix).
/// When the build does not produce a binary, any stale copy from a previous build
/// of the same version is removed so `activate()` cannot symlink an outdated binary.
fn build_optional_binary(
name: &str,
repo_path: &Path,
version_dir: &Path,
jobs_str: Option<&str>,
) -> Result<()> {
ui::info!("Building {name}");

let mut args = vec!["build", "--release", "-p", name];
if let Some(j) = jobs_str {
args.extend(["-j", j]);
}

let status = Command::new("cargo")
.args(&args)
.current_dir(repo_path)
.status()
.context("Failed to execute cargo build")?;

let source = repo_path.join("target/release").join(name);
let dest = version_dir.join(name);

if status.success() && source.exists() {
fs::copy(&source, &dest).with_context(|| format!("Failed to copy {name} binary"))?;

#[cfg(unix)]
{
use std::os::unix::fs::PermissionsExt;
let mut perms = fs::metadata(&dest)
.with_context(|| format!("Failed to get {name} metadata"))?
.permissions();
perms.set_mode(0o755);
fs::set_permissions(&dest, perms)
.with_context(|| format!("Failed to set executable permissions on {name}"))?;
}
} else {
// Remove any stale copy from a previous build so activate() won't symlink it.
if dest.exists() {
fs::remove_file(&dest)
.with_context(|| format!("Failed to remove stale {name} binary"))?;
}
ui::warn!("Skipping {name}: build did not produce a binary");
}

Ok(())
}
Expand Down
6 changes: 3 additions & 3 deletions ampup/src/commands/install.rs
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ pub async fn run(
ui::info!("Switching to version {}", ui::version(&version));
crate::commands::use_version::switch_to_version(&version_manager, &version)?;
ui::success!("Switched to version {}", ui::version(&version));
ui::detail!("Run 'ampd --version' and 'ampctl --version' to verify installation");
ui::detail!("Run 'ampd --version' to verify installation");
return Ok(());
}

Expand Down Expand Up @@ -94,8 +94,8 @@ pub async fn run(
.install_from_release(&version, platform, arch)
.await?;

ui::success!("Installed ampd and ampctl {}", ui::version(&version));
ui::detail!("Run 'ampd --version' and 'ampctl --version' to verify installation");
ui::success!("Installed amp {}", ui::version(&version));
ui::detail!("Run 'ampd --version' to verify installation");

Ok(())
}
10 changes: 10 additions & 0 deletions ampup/src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,16 @@ impl Config {
self.bin_dir.join("ampctl")
}

/// Get the ampsql binary path for a specific version
pub fn version_ampsql_path(&self, version: &str) -> PathBuf {
self.versions_dir.join(version).join("ampsql")
}

/// Get the active ampsql binary symlink path
pub fn active_ampsql_path(&self) -> PathBuf {
self.bin_dir.join("ampsql")
}

/// Ensure all required directories exist
pub fn ensure_dirs(&self) -> Result<()> {
fs::create_dir_all(&self.amp_dir).context("Failed to create amp directory")?;
Expand Down
88 changes: 78 additions & 10 deletions ampup/src/download_manager.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ use tokio::{sync::Semaphore, task::JoinSet};
use crate::{
github::{GitHubClient, ResolvedAsset},
progress::ProgressReporter,
ui,
};

// ---------------------------------------------------------------------------
Expand All @@ -22,6 +23,9 @@ pub struct DownloadTask {
pub artifact_name: String,
/// Destination filename inside the version directory (e.g., "ampd")
pub dest_filename: String,
/// Whether the artifact may be absent from the release. Optional artifacts
/// missing from the release are skipped; required ones fail the install.
pub optional: bool,
}

/// Errors that occur during bounded-concurrent download operations.
Expand Down Expand Up @@ -173,13 +177,24 @@ impl DownloadManager {
version_dir: PathBuf,
reporter: Arc<dyn ProgressReporter>,
) -> Result<()> {
// Resolve all asset metadata with a single API call so that each
// spawned task can download directly without re-fetching the release.
let asset_names: Vec<&str> = tasks.iter().map(|t| t.artifact_name.as_str()).collect();
let resolved = self
.github
.resolve_release_assets(version, &asset_names)
.await?;
// Fetch release metadata once, then resolve each task's asset in the
// same iteration that owns the task. Pairing the task with its resolved
// asset structurally avoids zipping two parallel collections whose
// alignment would only be guaranteed by convention.
let release = self.github.fetch_release_assets(version).await?;

// Drop optional artifacts the release does not provide; a missing
// required artifact fails fast via `resolve`.
let mut planned: Vec<(DownloadTask, ResolvedAsset)> = Vec::with_capacity(tasks.len());
for task in tasks {
match release.resolve(&task.artifact_name, task.optional)? {
Some(asset) => planned.push((task, asset)),
None => ui::warn!(
"Skipping {}: not available in this release",
task.artifact_name
),
}
}

let parent = version_dir.parent().ok_or_else(|| {
anyhow::anyhow!("version_dir has no parent: {}", version_dir.display())
Expand All @@ -189,13 +204,16 @@ impl DownloadManager {
let staging_dir =
tempfile::tempdir_in(parent).context("Failed to create staging directory")?;

let names: Vec<String> = tasks.iter().map(|t| t.artifact_name.clone()).collect();
reporter.set_total(tasks.len(), names);
let names: Vec<String> = planned
.iter()
.map(|(task, _)| task.artifact_name.clone())
.collect();
reporter.set_total(planned.len(), names);

let semaphore = Arc::new(Semaphore::new(self.max_concurrent));
let mut join_set: JoinSet<std::result::Result<String, DownloadError>> = JoinSet::new();

for (task, asset) in tasks.into_iter().zip(resolved) {
for (task, asset) in planned {
let github = self.github.clone();
let sem = semaphore.clone();
let staging_path = staging_dir.path().to_path_buf();
Expand Down Expand Up @@ -786,10 +804,12 @@ mod tests {
DownloadTask {
artifact_name: "ampd-linux-x86_64".to_string(),
dest_filename: "ampd".to_string(),
optional: false,
},
DownloadTask {
artifact_name: "ampctl-linux-x86_64".to_string(),
dest_filename: "ampctl".to_string(),
optional: false,
},
]
}
Expand Down Expand Up @@ -860,6 +880,52 @@ mod tests {
);
}

/// A missing *optional* asset is skipped; required artifacts still install.
#[tokio::test]
async fn download_all_with_missing_optional_asset_skips_it() {
//* Given — release only contains ampd; ampctl is optional and absent
let fixture = TestFixture::new(
&["ampd-linux-x86_64"],
vec![Route::ok(
"download/ampd-linux-x86_64",
b"fake-ampd-binary".to_vec(),
)],
4,
)
.await;

let tasks = vec![
DownloadTask {
artifact_name: "ampd-linux-x86_64".to_string(),
dest_filename: "ampd".to_string(),
optional: false,
},
DownloadTask {
artifact_name: "ampctl-linux-x86_64".to_string(),
dest_filename: "ampctl".to_string(),
optional: true,
},
];

//* When
let result = fixture.download(tasks).await;

//* Then
assert!(
result.is_ok(),
"a missing optional asset should not fail the install: {:?}",
result.err()
);
assert!(
fixture.version_dir.join("ampd").exists(),
"required ampd should be installed"
);
assert!(
!fixture.version_dir.join("ampctl").exists(),
"absent optional ampctl should be skipped, not installed"
);
}

/// `-j 1` (sequential) mode still produces a correct install.
#[tokio::test]
async fn download_all_with_sequential_mode_succeeds() {
Expand Down Expand Up @@ -913,6 +979,7 @@ mod tests {
let tasks = vec![DownloadTask {
artifact_name: "ampd-linux-x86_64".to_string(),
dest_filename: "ampd".to_string(),
optional: false,
}];

//* When
Expand Down Expand Up @@ -952,6 +1019,7 @@ mod tests {
let tasks = vec![DownloadTask {
artifact_name: "ampd-linux-x86_64".to_string(),
dest_filename: "ampd".to_string(),
optional: false,
}];

//* When
Expand Down
Loading
Loading