feat(adbc): optional ADBC driver install/list/uninstall in ampup - #14
Draft
incrypto32 wants to merge 18 commits into
Draft
feat(adbc): optional ADBC driver install/list/uninstall in ampup#14incrypto32 wants to merge 18 commits into
incrypto32 wants to merge 18 commits into
Conversation
Add an `adbc` subcommand (install/list/uninstall) mirroring the existing `self` subcommand structure. Runners are stubs that fail loudly until the download/placement path lands. Part of edgeandnode/amp#2600 (optional ADBC driver support in ampup).
Add an `adbc` module with a `Driver` catalog (postgresql) and `asset_name`, mapping a driver + platform/arch to the release asset `adbc-driver-<driver>-<platform>-<arch>.tar.gz`. Wire the command runners to validate the driver name against the catalog. Part of edgeandnode/amp#2600.
Read the digest GitHub advertises for each release asset and verify the downloaded bytes against it, failing with ChecksumMismatch on a mismatch. Applies to every download (binaries and, later, drivers). Previously only a non-empty check ran. - github: Asset/ResolvedAsset carry an optional digest. - download_manager: verify_artifact checks SHA-256 when a digest is present. - tests: verify_artifact match/mismatch/prefix/no-digest, plus end-to-end matching and mismatched-digest cases through download_all. Part of edgeandnode/amp#2600.
Wire `ampup adbc install <driver>` to resolve the active amp version, download the pinned driver archive, verify its SHA-256 digest, and extract the validated members to a staging dir. Adds a flat-tar.gz extractor (traversal-guarded) and the driver runtime-lib naming. Placement into the version dir is not yet wired. Part of edgeandnode/amp#2600.
Install now moves the extracted driver into a self-contained ~/.amp/versions/<v>/drivers/<driver>/ and writes an ADBC manifest.toml (typed via the toml crate) whose Driver.shared is the absolute library path, so ampd can load the driver by manifest. Staging happens inside the drivers dir so the final placement is a same-filesystem atomic rename; reinstalls replace the existing dir. Part of edgeandnode/amp#2600.
extract_and_validate checked only the entry name, so a symlink or hardlink named as an allowed member would be materialized by unpack as a link instead of the archived bytes. Reject any entry that is not a regular file before unpacking. Part of edgeandnode/amp#2600.
adbc install downloads a single asset directly, so the -j/--jobs flag was accepted and silently ignored. Remove it from the subcommand and the install signature. Part of edgeandnode/amp#2600.
list shows the complete drivers installed for the active amp version (directories that match the catalog and hold a manifest.toml, so leftover staging dirs and partial installs are skipped). uninstall removes a driver's directory and prunes an emptied drivers dir. Part of edgeandnode/amp#2600.
The driver download called download_resolved_asset directly, so it lacked the single application-level retry that binary downloads get via download_with_retry. Reuse that wrapper (now pub(crate)) so a driver fetch survives a transient failure the same way binaries do. Part of edgeandnode/amp#2600.
Split install into a thin wrapper plus install_driver so a test can inject a GitHubClient pointed at a mock server. Add an in-process mock GitHub server and integration tests: install drives the full fetch -> verify -> extract -> place -> manifest path against the mock, and uninstall removes a placed driver and prunes the emptied drivers dir. Part of edgeandnode/amp#2600.
mitchhs12
reviewed
Jul 22, 2026
verify_artifact only checks that the download is non-empty when no digest is advertised. Driver assets always carry one, so treat a missing digest as a malformed release and refuse rather than install a library that ampd would load unverified. Part of edgeandnode/amp#2600.
The adbc commands were pinned to the active version, unlike `ampup install` which takes one. Add --version to install, list, and uninstall. Resolving the version now also requires it to be installed. Installing amp replaces the whole version directory, so drivers placed under a version whose binaries are not there yet would be destroyed by the next `ampup install`. Part of edgeandnode/amp#2600.
Requiring installed binaries for list and uninstall left an orphaned driver directory impossible to inspect or remove, and made a read-only query fail on a stale .version. Split the check out of version resolution so it guards installation only, and reuse VersionError so the message matches the rest of the CLI. Also name the flag's value VERSION in help, and cover installing for an explicit non-active version plus uninstalling from a version whose binaries are gone. Part of edgeandnode/amp#2600.
build_from_local_path_with_custom_name and rebuild_removes_stale_optional_binaries both replace the process-global PATH to install a mock cargo. Run concurrently, one restores the original value while the other is mid-build, so the mock disappears and the real cargo fails against the fake repo. Share a mutex so they cannot overlap.
incrypto32
marked this pull request as ready for review
July 23, 2026 09:13
Drivers were installed into a self-contained drivers/<driver>/ directory
holding an ADBC manifest.toml beside the library. Install them directly
into the version directory instead, alongside the amp binaries, and drop
the manifest: with the library at a known path, a manifest naming its own
neighbour is indirection amp does not need. That also removes the toml
dependency.
The library is renamed to amp-adbc-driver-<driver>.{so,dylib} on install,
so its filename is what distinguishes it from ampd/ampctl/ampsql now that
they share a directory. License files are namespaced the same way.
list and uninstall check the catalog's expected filenames rather than
walking a directory, and cover every supported platform rather than the
host's: `adbc install --platform` can place a library this host would not
look for, which would otherwise strand a file no ampup command could
remove.
Placement is now per-file renames rather than one directory rename, so a
failed move rolls back the files that already landed; without that a
partial install would leave the library behind and list would report a
driver whose installation had errored.
Part of edgeandnode/amp#2600.
Contributor
|
Quick heads-up: the library inside each archive is now |
The release archive now ships the driver library under its final name
(amp-adbc-driver-<driver>.{so,dylib}), so ampup places it as-is instead
of renaming it. LICENSE/NOTICE are validated in the archive but no longer
extracted onto disk; redistribution is satisfied by the release tarball.
Add ADBC driver usage to the README and the ampup feature doc: the subcommands, per-version behavior, the version directory layout, and the install flow. Uses the generic amp-adbc-driver-<driver> filename form.
incrypto32
marked this pull request as draft
July 27, 2026 07:53
The digest is already in the release metadata, so a missing one can be caught before spending the download instead of after.
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.
Adds optional ADBC driver support to ampup, tracking edgeandnode/amp#2600.
ampup adbc install <driver>downloads the pinned driver archive from the matching amp release, verifies its sha256 against the asset digest, and places the library into the version directory besideampd/ampctl/ampsql. Also addslistanduninstall.Currently supports the
postgresqldriver.--version,--platform, and--archoverride the defaults (active version, host platform/arch).Layout. The driver library lands directly in
~/.amp/versions/<version>/asamp-adbc-driver-<driver>.{so,dylib}, beside the amp binaries. No per-driver subdirectory, no manifest — the local cache mirrors the release's flat artifact list.Naming. The release pipeline (edgeandnode/amp#2599, PR #2714) names the library inside each archive with the final
amp-adbc-driver-<driver>prefix, so ampup places it as-is. The.tar.gzasset names are unchanged.Notes:
Tested against an in-process mock GitHub server: full install path (fetch → download → verify → extract → place), digest rejection, explicit-version targeting, and uninstall across platforms.