seokjin0414 commented on issue #3688:
URL: https://github.com/apache/iggy/issues/3688#issuecomment-5156005392
@hubcio a build-system question before I start, since the answer changes the
shape of the PR.
`fluss-rs` 0.1.0 (the only non-yanked release) compiles a bundled `.proto`
in its build script, so it needs a system `protoc`. On a clean machine:
```
error: failed to run custom build command for `fluss-rs v0.1.0`
--- stderr
Error: Custom { kind: NotFound, error: "Could not find `protoc`. If
`protoc` is
installed, try setting the `PROTOC` environment variable to the path of the
`protoc` binary. ..." }
```
`prost-build` 0.14 dropped its `vendored` feature and the crate ships the
`.proto` rather than generated code, so there's no way around it from our side.
Nothing in `.github/` or `scripts/` installs protoc today, and
`ubuntu-latest` (24.04) doesn't ship it either. The workspace has no
`default-members`, so both `cargo check --all --all-features` and `cargo clippy
--all-features --all-targets` select every member. Adding
`core/connectors/sources/fluss_source` as a normal member therefore means CI
and local dev both need protoc installed.
Two ways I see:
1. Add it as a normal member, install `protobuf-compiler` in CI, document it
in CONTRIBUTING. One line in each, and the connector stays consistent with the
others. The cost is that contributors need protoc for the standard `cargo
clippy --all-features --all-targets` step.
2. Put it in `exclude` alongside `foreign/cpp`. The workspace build is
untouched, but excluded packages can't inherit `[workspace.dependencies]`
(that's why `foreign/python` hard-pins everything), so every dep gets pinned
locally and the crate falls outside `cargo sort --workspace`, `bump-version.sh`
and the cargo-rail scoping. A dedicated job would still need protoc.
Dependency-wise it's fairly contained: 28 new external packages in
`Cargo.lock`, most of them prost-build's build-time deps, and fluss-rs's `arrow
^57` resolves onto the 57.3.1 tree the workspace already carries alongside
58.3.0. I checked by adding the crate locally and diffing `Cargo.lock`.
I'd go with 1, mainly to keep the connector conventions intact, but adding a
system build dependency to the repo seems like your call rather than mine.
Whichever you pick also decides the integration-test shape, since
testcontainers-modules has no Fluss module and a real cluster needs ZooKeeper +
CoordinatorServer + TabletServer.
Scope note: 0.1.0 is from April and 0.2.0 was yanked on release day, so the
pinned version sits a fair way behind main. The first PR would cover log tables
-> `Schema::Json`. The Arrow `RecordBatch` scanner and column projection are
both in 0.1.0, so `arrow_ipc` payloads are doable, but they're a separate
scanner with a separate offset-tracking path, so I'd rather leave that for a
follow-up. PK changelog scanning is main-only, so phase 2 waits for a later
fluss-rs release.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
