Skip to content

feat(ampup): add ampsql to downloadable binaries #11

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
LNSD wants to merge 1 commit into
base: main
Choose a base branch
from
Open

feat(ampup): add ampsql to downloadable binaries #11

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
126 changes: 83 additions & 43 deletions ampup/src/builder.rs
View file
Open in desktop
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")?;

@mitchhs12 mitchhs12 Jun 19, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since ampctl/ampsql only print a warning now (they're no longer required), this can leave stale optional binaries behind when rebuilding the same --name/version.

We reuse the final versions/<label> directory here, and because the skip paths below don’t remove any existing ampctl or ampsql, if a previous build produced one of those binaries but the next build skips it, activate() will symlink the stale file by mistake.


// 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,79 @@ 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")?;
// Build ampctl
ui::info!("Building ampctl");

#[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")?;
let mut ampctl_args = vec!["build", "--release", "-p", "ampctl"];
if let Some(ref j) = jobs_str {
ampctl_args.extend(["-j", j]);
}

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

let ampctl_source = repo_path.join("target/release/ampctl");
if ampctl_status.success() && ampctl_source.exists() {
// 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")?;
}
} else {
ui::warn!("Skipping ampctl: build did not produce a binary");
}

// Build ampsql
ui::info!("Building ampsql");

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

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

let ampsql_source = repo_path.join("target/release/ampsql");
if ampsql_status.success() && ampsql_source.exists() {
// Copy ampsql binary
let ampsql_dest = version_dir.join("ampsql");
fs::copy(&ampsql_source, &ampsql_dest).context("Failed to copy ampsql binary")?;

#[cfg(unix)]
{
use std::os::unix::fs::PermissionsExt;
let mut perms = fs::metadata(&ampsql_dest)
.context("Failed to get ampsql metadata")?
.permissions();
perms.set_mode(0o755);
fs::set_permissions(&ampsql_dest, perms)
.context("Failed to set executable permissions on ampsql")?;
}
} else {
ui::warn!("Skipping ampsql: build did not produce a binary");
}

// 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(())
}
Expand Down
6 changes: 3 additions & 3 deletions ampup/src/commands/install.rs
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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