feat(ampup): add ampsql to downloadable binaries#11
Open
LNSD wants to merge 1 commit into
Open
Conversation
Treat ampd as the only required binary while installing ampctl and ampsql on a best-effort basis, so releases or source trees that omit them no longer fail the whole install. - Add `ampsql` to download, build-from-source, activation, and uninstall flows alongside `version_ampsql_path`/`active_ampsql_path` config helpers - Fetch release metadata once via `fetch_release_assets` and resolve each asset in-memory with `ReleaseAssets::resolve`, pairing tasks to assets without zipping parallel collections - Filter unavailable optional artifacts in `DownloadManager` with a warning; only `ampd` is required - Build `ampd`, `ampctl`, and `ampsql` separately so a missing optional crate is skipped rather than aborting the build - Symlink `ampctl`/`ampsql` only when their binaries exist and update docs and mock fixtures Signed-off-by: Lorenzo Delgado <lorenzo@edgeandnode.com>
LNSD
force-pushed
the
lnsd/add-ampsql-binary
branch
from
cd126bc to
733fbcb
Compare
mitchhs12
approved these changes
Jun 19, 2026
mitchhs12
left a comment
mitchhs12
left a comment
Contributor
There was a problem hiding this comment.
Approved but now that ampctl and ampsql are optional, there is a stale optional binary activation bug where rebuilding the same --name/version can leave behind stale binaries.
|
|
||
| // Create version directory | ||
| let version_dir = config.versions_dir.join(version_label); | ||
| fs::create_dir_all(&version_dir).context("Failed to create version directory")?; |
Contributor
There was a problem hiding this comment.
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.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Distribute
ampsqlalongsideampd, and make bothampctlandampsqloptional so installs and builds succeed against releases or source trees that omit them.ampsql-{platform}-{arch}from releases;ampdstays required whileampctl/ampsqlskip gracefully onAssetNotFoundampctl/ampsqlonly when present and clear stale symlinks on version switch inVersionManager::activateampdas required andampctl/ampsqlbest-effort via separatecargo buildinvocations inbuild_and_installampsqltoconfigpath helpers, mock test fixtures, and update feature/pattern docs