Repository navigation
feat!: add transactional package installation foundation - #502
Merged
Merged
Conversation
3 tasks
eitsupi
marked this pull request as draft
September 30, 2026 22:42
Member
Author
|
Based on discussions in the chat, I refactored to remove support for manifest-only drivers, which had not been documented. |
eitsupi
marked this pull request as ready for review
October 1, 2026 01:32
zeroshade
pushed a commit
that referenced
this pull request
Oct 23, 2026
Fix unsafe Windows tests discovered while reviewing #502 by isolating registry and driver directories in process-private temporary fixtures.
Adopt the shared Windows test isolation from main: route Windows tests through winregroot and systempath, replace the process-global HKCU override in the config test main with wintest, and drop the build tag for registry tests.
Member
Author
|
This is ready for another look. Changes since the last review:
The PR description is updated to match. |
zeroshade
approved these changes
Oct 28, 2026
zeroshade
left a comment
Member
There was a problem hiding this comment.
Thanks — every item from the last round is fixed, or deferred in a way we agreed on. The os.Root pinning, the digest check before deletion, and keeping external registrations registration-only all hold up against the earlier reproductions, and the legacy cleanup works on the real test-driver-1.tar.gz layout.
Non-blocking:
registration_namespace.go:144–167: other roots are scanned without their locks, so aCreateManifestin another root that lands between the scan and deletion can end up pointing at a deleted library. dbc doesn't create such references itself, so a note in the deferred-work list is enough.install_package_test.go:72–78: this still expects0755after the umask change, and fails underumask 077. Please set the umask in the test or expect the umask-adjusted mode.package_cleanup.go:218–228: consider reporting stale-generation cleanup errors instead of dropping them.- The HKCU/
%AppData%lock split is only noted in a code comment; please track it in an issue.
The generation directory mode follows the process umask, which is covered by TestInstallPackageGenerationModeHonorsUmask. The fixed 0755 expectation failed under a restrictive umask such as 077.
Member
Author
|
Thanks for your review.
|
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
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.
Summary
This PR extracts the transactional package installation foundation from #497 for dbc 0.4.0. Direct install, sync, and
Client.Installprepare a validated, dbc-owned driver payload before replacing the runtime registration. The previous installation remains usable until commit.It also removes manifest-only dbc package installation, an undocumented feature with no known usage. Every installed package must contain an actual
Files.driverpayload;Driver.sharedis no longer a fallback for a missing payload. This requirement applies even when signature verification is disabled.Ordinary ADBC runtime manifests that reference externally managed libraries remain supported. Removing manifest-only packages does not remove external runtime registration discovery or parsing.
Transaction contract
InstallPackage(ctx, ...)supports cancellation during lock waits and safe pre-commit boundaries. Registration mutation completes once started, including any rollback.Behavior changes in 0.4.0
Files.driversource = "dbc"and a safe same-root legacy layout; delete only the exact registered driver binary, then remove its directory if empty. Metadata may remain.config.UninstallDriverSharedConfig.UninstallDriverorClient.Uninstall.Legacy ownership is a compatibility assumption based on the old install layout, not a provenance proof. Unsafe paths, uncertain filesystem identity, conflicting references, or unknown/corrupt receipts prevent payload deletion. No additional heuristics reconstruct legacy ownership.
Compatibility and scope
Same-version direct install still exits before download or verification.
Client.Installdoes not gain mandatory signature or MANIFEST-version checks.InstallDriverremains a deprecated, non-transactional API, with the same owned-payload requirement.Native locks coordinate processes; Wasm locks are runtime-local. Wasm uninstall retains payloads because directory identity cannot be pinned safely. Post-uninstall cleanup errors identify retained paths; recovery of registration-less generations is deferred. Stable Windows registry lock identity remains follow-up work. With
ADBC_DRIVER_PATH, cleanup scans other configured roots without taking their locks, so a manifest created there concurrently could reference a just-removed library; dbc never creates such cross-root references itself. Stale-generation cleanup errors after a successful install or uninstall are currently dropped; reporting them is follow-up work.Owned payloads are stored in hidden
.dbc-package-g-*directories next to the driver manifest, created with the process umask applied. On Unix, lock files are intentionally left in place after release.Lockfile v2, Packslip, a new package format, external registration commands, aliases, and replay/repair are outside this PR. This foundation accepts prepared owned payloads so future package decoding and reproducible-artifact locking can build on the same transaction model.
Validation
CI passes on Linux, macOS, Windows, and Wasm (Node and Deno). Windows tests now use the shared registry and directory isolation from #508.