-
Notifications
You must be signed in to change notification settings - Fork 65
kernel: distribute the kernel via per-platform bindings modules (go get, no build step, all 5 platforms) #440
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.
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
msrathore-db
wants to merge
9
commits into
main
Choose a base branch
from
kernel-nested-modules
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
Show all changes
9 commits
Select commit
Hold shift + click to select a range
a9c3e8a
kernel: distribute darwin/arm64 lib as nested per-platform module
msrathore-db a11b8e2
ci: sync committed kernel C-ABI libs from kernel release
msrathore-db d743db3
ci: drop pull-model sync workflow in favor of kernel push
msrathore-db 5957419
kernel: version nested modules for consumer go get (real versions + r…
msrathore-db 09f75a4
kernel: consume external bindings repo; support all 5 platforms
msrathore-db a2b7600
kernel: verify kernel_abi_version() at connect
msrathore-db 7d47600
kernel: drop the ABI-version check
msrathore-db 328b756
kernel: pin to kernel main c403bfb; sync header, fix stale retry test…
msrathore-db 65fef44
Merge remote-tracking branch 'origin/main' into kernel-nested-modules
msrathore-db File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1 +1 @@ | ||
| eff8950428f4e6cc9975c663ec919f334962f7d0 | ||
| c403bfb8ff2361b5919ced1a45feb4cf3b63ab60 |
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
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
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,142 @@ | ||
| # Releasing the driver (with the kernel/SEA backend) | ||
|
|
||
| The kernel/SEA backend is delivered as **prebuilt, per-platform static archives** | ||
| carried by `go get` — no `make kernel-lib` step for consumers, no Rust toolchain. | ||
| The archives do **not** live in this repo: they live in the separate | ||
|
|
||
| ``` | ||
| github.com/databricks/databricks-sql-kernel-bindings | ||
| ``` | ||
|
|
||
| repository, which this driver `require`s. This document explains how the archives | ||
| are versioned and published so that a | ||
| `go get github.com/databricks/databricks-sql-go@vX.Y.Z` resolves the matching | ||
| kernel archives automatically, and how to cut a coordinated release across the | ||
| two repos. | ||
|
|
||
| ## The module layout | ||
|
|
||
| The archives are distributed as nested Go modules in the **bindings** repo: | ||
|
|
||
| ``` | ||
| github.com/databricks/databricks-sql-kernel-bindings (root module) | ||
| ├── include/databricks_kernel.h (C header, at the pinned kernel rev) | ||
| ├── prebuilt_<os>_<arch>.go (root shim: blank-imports lib/<platform>) | ||
| └── lib/<os>_<arch>/ (one NESTED module per platform) | ||
| ├── go.mod → github.com/databricks/databricks-sql-kernel-bindings/lib/<os>_<arch> | ||
| ├── prebuilt.go → //go:build cgo && <os> && <arch> (+ #cgo LDFLAGS) | ||
| └── libdatabricks_sql_kernel.a (the committed prebuilt archive for this platform) | ||
| ``` | ||
|
|
||
| Each `lib/<platform>` directory is **its own Go module**. This is deliberate: Go | ||
| downloads a module's zip only when a build compiles a file from it, and each | ||
| `prebuilt.go` is build-tag-gated to one platform. In THIS driver, the | ||
| `internal/backend/kernel/cgo_<os>[_<arch>].go` files blank-import the matching | ||
| `lib/<platform>` module under the same constraint. So: | ||
|
|
||
| - a **Thrift build** (`CGO_ENABLED=0`, no tag) downloads **none** of them; | ||
| - a **kernel build** for, say, darwin/arm64 downloads **only** the | ||
| `lib/darwin_arm64` module's zip — never the other platforms' archives. The | ||
| other four appear in `go.mod`/`go.sum` (their tiny `go.mod` hashes) but their | ||
| multi-megabyte zips are never fetched. | ||
|
|
||
| The driver owns the cgo call layer (`internal/backend/kernel/cgo.go` does | ||
| `import "C"` and `#include "databricks_kernel.h"`), so the C **header is committed | ||
| in this repo** at `internal/backend/kernel/include/databricks_kernel.h`. The | ||
| bindings modules provide only the archives and their `#cgo LDFLAGS`. The header | ||
| here and the archives there MUST come from the **same kernel revision** — the one | ||
| recorded in the repo-root `KERNEL_REV` file (the bindings repo records the same | ||
| rev in its release notes/commit). | ||
|
|
||
| ## How versioning works | ||
|
|
||
| The driver's `go.mod` `require`s each platform module at a **real version**, with | ||
| **no `replace`** (the modules are external): | ||
|
|
||
| ``` | ||
| require ( | ||
| github.com/databricks/databricks-sql-kernel-bindings/lib/darwin_amd64 vX.Y.Z | ||
| github.com/databricks/databricks-sql-kernel-bindings/lib/darwin_arm64 vX.Y.Z | ||
| github.com/databricks/databricks-sql-kernel-bindings/lib/linux_amd64 vX.Y.Z | ||
| github.com/databricks/databricks-sql-kernel-bindings/lib/linux_arm64 vX.Y.Z | ||
| github.com/databricks/databricks-sql-kernel-bindings/lib/windows_amd64 vX.Y.Z | ||
| ) | ||
| ``` | ||
|
|
||
| - The **`require` version pins the kernel.** `go get .../databricks-sql-go@vX.Y.Z` | ||
| reads that tag's `go.mod`, sees the pinned bindings versions, and resolves those | ||
| exact archive versions from the module proxy. **Upgrading the driver is what | ||
| moves the kernel version** — deterministic, per-driver-version pinning. | ||
| - **Local development** against a checkout of the bindings repo uses a `go.work` | ||
| (or a temporary `replace`), never a committed `replace` — so a downstream | ||
| `go get` always resolves the published versions from the proxy. | ||
|
|
||
| > The bindings repo is a **private** repo today. External `go get` from the public | ||
| > proxy requires it to be made **public** (OSS review + third-party NOTICE). Until | ||
| > then, internal builds fetch it with `GOPRIVATE=github.com/databricks/*` over | ||
| > authenticated git. | ||
|
|
||
| ## Publishing: path-prefixed tags (in the bindings repo) | ||
|
|
||
| Go publishes a nested module using a **tag whose name is the module's | ||
| subdirectory path plus the version**. To release all platforms at `vX.Y.Z` in the | ||
| bindings repo: | ||
|
|
||
| ``` | ||
| git tag lib/darwin_amd64/vX.Y.Z | ||
| git tag lib/darwin_arm64/vX.Y.Z | ||
| git tag lib/linux_amd64/vX.Y.Z | ||
| git tag lib/linux_arm64/vX.Y.Z | ||
| git tag lib/windows_amd64/vX.Y.Z | ||
| git tag vX.Y.Z # the bindings root module | ||
| git push origin --tags | ||
| ``` | ||
|
|
||
| > Module versions are **immutable** once a public proxy has served them. While the | ||
| > repo is private and un-proxied you can re-cut a tag; once public, bump to a new | ||
| > version instead of moving a tag. | ||
|
|
||
| ## Release steps (coordinated, kernel → bindings → driver) | ||
|
|
||
| 1. **Pick the kernel revision** and set it in this repo's `KERNEL_REV`. Sync the | ||
| committed header here (`internal/backend/kernel/include/databricks_kernel.h`) | ||
| to that exact kernel commit's `include/databricks_kernel.h`. | ||
| 2. **Build the archives** for all five platforms at that revision — via the kernel | ||
| repo's `build-c-abi-libs` workflow (native `linux_amd64` + `windows_amd64` | ||
| today; see databricks-sql-kernel#244) and/or a local cross-build for the | ||
| platforms CI cannot yet produce (`darwin_*`, `linux_arm64`). Build flags: | ||
| `cargo build --release --locked --no-default-features --features tls-rustls | ||
| --target <triple>`; set `MACOSX_DEPLOYMENT_TARGET` for the darwin targets so | ||
| the archive links cleanly on older macOS. Windows must be the **`-gnu`** triple | ||
| (Go cgo links a GNU `.a`, never an MSVC `.lib`). | ||
| 3. **Commit the archives** into `lib/<platform>/libdatabricks_sql_kernel.a` in the | ||
| bindings repo, and sync that repo's `include/databricks_kernel.h` to the same | ||
| kernel rev. | ||
| 4. **Tag the bindings repo** with the path-prefixed tags above plus the root tag. | ||
| 5. **Bump the `require` versions** in this driver's `go.mod` to the new bindings | ||
| version. | ||
| 6. **Refresh `go.sum`:** `GOFLAGS=-mod=mod GOPRIVATE=github.com/databricks/* go mod tidy` | ||
| so the new per-platform module checksums land in `go.sum`. Consumers verify | ||
| against these. | ||
| 7. **Tag the driver** `vX.Y.Z` and push. `go get @vX.Y.Z` now resolves the driver | ||
| and, transitively, the matching per-platform kernel archives. | ||
|
|
||
| ## Adding a new platform | ||
|
|
||
| 1. In the **bindings** repo: create `lib/<os>_<arch>/` with its own `go.mod`, a | ||
| build-tag-gated `prebuilt.go` (matching `//go:build` + `#cgo LDFLAGS`), and the | ||
| committed archive; add a root `prebuilt_<os>_<arch>.go` shim; tag it alongside | ||
| the others. | ||
| 2. In this **driver** repo: add a `require` for the new module in `go.mod`, add a | ||
| build-tagged `internal/backend/kernel/cgo_<os>_<arch>.go` that blank-imports it, | ||
| and drop the platform from the exclusion list in `cgo_unsupported.go`. | ||
|
|
||
| ## Consumer experience (for reference) | ||
|
|
||
| - **Thrift (default):** `go get ...` + `go build` — pure Go, no cgo, no archive | ||
| downloaded. | ||
| - **Kernel/SEA:** `go get ...` + `CGO_ENABLED=1 go build -tags databricks_kernel` | ||
| — pulls only the target platform's archive at the driver-pinned version; no | ||
| `make kernel-lib`, no Rust. | ||
| - **Cloning the bindings repo directly** (contributors/CI): use | ||
| `git clone --filter=blob:none` to skip the committed-archive history. |
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
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
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
Oops, something went wrong.
Oops, something went wrong.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🟡 Medium — These five modules are now direct requirements of the driver, so Go must resolve each one's
go.modto compute the build list for any consumer build — including the default Thrift CUJ (CGO_ENABLED=0, no build tag). The build-tag gating on the blank imports (cgo_*.go) only prevents fetching the multi-megabyte archive zips; it does not exempt these modules from module-graph resolution. A fresh consumer with an empty module cache still fetches the (tiny)go.modfiles and verifies them againstgo.sum.Consequence: while
databricks-sql-kernel-bindingsremains private (asdocs/RELEASING.mdnotes), an externalgo get github.com/databricks/databricks-sql-go@<this version>followed bygo buildwill fail for Thrift-only consumers too — not just the kernel opt-in — because the proxy cannot serve the privatego.modfiles. This contradicts the README's framing that the default Thrift build is "completely unchanged" / pulls "no kernel binaries at all": no binaries, correct, but a new hard module-graph dependency on a currently-unreachable repo now gates every consumer.Suggest: (a) do not tag a public driver release with these requires until the bindings repo is public, and (b) soften the README claim to make clear the go.mod dependency applies to all consumers, so the gating condition isn't buried in RELEASING.md.