Skip to content

feat!: add transactional package installation foundation - #502

Open
eitsupi wants to merge 53 commits into
columnar-tech:mainfrom
eitsupi:feat/install-foundation
Open

eitsupi wants to merge 53 commits into
columnar-tech:mainfrom
eitsupi:feat/install-foundation

Conversation

@eitsupi

@eitsupi eitsupi commented Sep 29, 2026 •

Copy link
Copy Markdown
Member

Summary

This PR extracts the transactional package installation foundation from #497 for dbc 0.4.0. Direct install, sync, and Client.Install prepare 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.driver payload; Driver.shared is 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

private staging
  -> archive validation and optional verification
  -> ownership receipt
  -> unique owned-payload generation publication
  -> runtime registration replacement [commit]
  -> conservative post-commit cleanup
  • Pre-commit failures preserve the previous registration and payload. Post-commit cleanup failures do not roll back a successful installation.
  • Unix registrations use atomic file replacement. Windows restores previous raw registry values after ordinary partial failure; rollback failure preserves the candidate because registration state is uncertain.
  • Driver-scoped locks serialize cooperating mutations. Install/update writes only the primary root; lower-priority installations remain in place.
  • Cleanup retains filesystem identity, symlink/TOCTOU, cross-root reference, and external-registration borrower checks. Receipts describe owned payloads only and are deleted last.
  • 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

Case Previous behavior Behavior in this PR
Package without Files.driver Could install only a manifest pointing to an external library; this was undocumented. Rejected before publication or registration changes.
Replacing an installed version Some paths uninstalled the working version before the replacement was ready. Prepare and validate the replacement first, then switch registration and clean up.
Receipt-backed dbc package uninstall No transactional ownership receipt. Remove registration, then delete validated owned files conservatively.
Normal pre-0.4 dbc package uninstall Could recursively remove the package directory. Recognize source = "dbc" and a safe same-root legacy layout; delete only the exact registered driver binary, then remove its directory if empty. Metadata may remain.
External or ambiguous registration uninstall External library bytes could be deleted in some cases. Remove registration and retain library bytes.
config.UninstallDriverShared Public API for shared-library removal. Removed; use Config.UninstallDriver or Client.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.Install does not gain mandatory signature or MANIFEST-version checks. InstallDriver remains 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.

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

The known Windows test-isolation failure awaits integration of #508 after it merges.

@eitsupi eitsupi changed the title feat: add transactional package installation foundation feat!: add transactional package installation foundation Sep 29, 2026
@eitsupi

eitsupi commented Sep 30, 2026

Copy link
Copy Markdown
Member Author

Thanks for the detailed review!
I'll address the cleanup safety, reference handling, legacy compatibility, test payloads, and Wasm locking issues here, along with the smaller fixes.
I'll also check the macOS case-sensitivity concern before deciding whether to defer it.

I'd like to handle the Windows HKCU lock identity in a follow-up and document the current limitation here.
Does that sound reasonable?

My preference is to address these issues in this PR first, then revisit splitting if you still think it would help.

@eitsupi

eitsupi commented Sep 30, 2026

Copy link
Copy Markdown
Member Author

I looked at splitting this more carefully against the current diff.

The only cleanly independent split I see is the low-level fslock / atomicfile / hostpath / Wasm infrastructure, which is about ~ 2k added lines.
That would still leave roughly 10k lines here, so it does not reduce the main review very much.

A larger split could make a stacked series of roughly ~2k infrastructure / ~6k registration+transaction / ~5k cleanup.
The downside is that the transaction part is not really independently mergeable: once installs move to unique generations and receipts, ownership-aware cleanup is needed to make replacement and uninstall complete.
The issues from this review also mostly sit exactly at that boundary.

So I think further splitting would mainly help as a review-only stack, while adding rebasing/interface churn and making the already-reviewed changes harder to follow.
Unless you still strongly prefer that, I’d prefer to keep this PR together.

@eitsupi

eitsupi commented Sep 30, 2026

Copy link
Copy Markdown
Member Author

The CI failure should be addressed by #508, which can be merged independently.

@eitsupi
eitsupi marked this pull request as ready for review September 30, 2026 13:10
@eitsupi
eitsupi marked this pull request as draft September 30, 2026 22:42
@eitsupi

eitsupi commented Oct 1, 2026

Copy link
Copy Markdown
Member Author

Based on discussions in the chat, I refactored to remove support for manifest-only drivers, which had not been documented.

@eitsupi
eitsupi marked this pull request as ready for review October 1, 2026 01:32
@eitsupi
eitsupi requested a review from zeroshade October 1, 2026 01:32

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants