Skip to content

refactor(rust): regroup crates under unified rust/cube workspace#10784

Merged
ovr merged 3 commits intomasterfrom
refactor/introduce-cube-workspace
Apr 30, 2026
Merged

refactor(rust): regroup crates under unified rust/cube workspace#10784
ovr merged 3 commits intomasterfrom
refactor/introduce-cube-workspace

Conversation

@ovr
Copy link
Copy Markdown
Member

@ovr ovr commented Apr 30, 2026

Move cubenativeutils, cubeshared, cubeorchestrator, and cubesqlplanner into a single Cargo workspace at rust/cube/ so they share one Cargo.lock and target/, while leaving cubesql and cubestore in place.

Move cubenativeutils, cubeshared, cubeorchestrator, and cubesqlplanner
into a single Cargo workspace at rust/cube/ so they share one Cargo.lock
and target/, while leaving cubesql and cubestore in place.
@github-actions github-actions Bot added cube store Issues relating to Cube Store rust Pull requests that update Rust code data source driver labels Apr 30, 2026
@claude
Copy link
Copy Markdown
Contributor

claude Bot commented Apr 30, 2026

Claude finished @ovr's task in 3m 9s —— View job


PR Review: refactor(rust): regroup crates under unified rust/cube workspace

  • Gather context and understand the workspace restructuring
  • Review new workspace Cargo.toml and .gitignore
  • Review CI/CD workflow changes
  • Review dependency path updates (Cargo.toml files)
  • Check for stale references to old paths
  • Review Cargo.lock changes
  • Verify completeness of the move
  • Post comprehensive review

Summary

Clean workspace consolidation that moves cubenativeutils, cubeshared, cubeorchestrator, and cubesqlplanner (+ nativebridge) into a single Cargo workspace at rust/cube/. The shared Cargo.lock and target/ will eliminate redundant dependency resolution and compilation across these crates. CI workflows are properly simplified from per-crate steps to single workspace-level commands.

What looks good

  • All path references updated correctlypackages/cubejs-backend-native/Cargo.toml, packages/cubejs-cubestore-driver/codegen/generate.sh, CLAUDE.md, and all internal Cargo.toml relative paths (e.g., cubeclient = { path = "../../../cubesql/cubeclient" }) are properly adjusted.
  • No stale references remain — A comprehensive search across all file types (.toml, .yml, .sh, .ts, .js, Dockerfile, Makefile) found zero leftover references to the old rust/cubenativeutils/, rust/cubeorchestrator/, rust/cubeshared/, or rust/cubesqlplanner/ paths.
  • Old per-crate Cargo.lock files properly deleted — The four individual lockfiles are removed and replaced by a single unified rust/cube/Cargo.lock.
  • CI simplification is correctpush.yml collapses 4 separate cargo fmt checks into one cargo fmt --manifest-path rust/cube/Cargo.toml --all -- --check, and 4 separate cargo test runs into one cargo test --manifest-path rust/cube/Cargo.toml --workspace --features cubesqlplanner/integration-postgres -j 4. The --features cubesqlplanner/integration-postgres package-qualified syntax is the correct way to enable per-crate features in a workspace test.
  • Workspace Cargo.toml is minimal and correctresolver = "2" with all five members listed.

Observations (non-blocking)

  1. Broader CI trigger scope — The workflow path filters now use rust/cube/** where before they used individual crate paths. This means a change to, say, only cubeshared will now trigger the rust-cubesql.yml and drivers-tests.yml workflows, which previously wouldn't have fired for those crates. This is technically a trade-off (more CI runs on changes that might not need them) but is the pragmatic choice given the unified workspace — the alternative (listing each sub-crate path) would defeat the purpose of the consolidation.

  2. Missing rust-cache for rust/cube workspace in rust-cubesql.yml — The lint job at .github/workflows/rust-cubesql.yml:38-43 configures Swatinem/rust-cache only for ./rust/cubesql -> target, but now also runs cargo fmt and cargo clippy on the rust/cube workspace. The cube workspace's target/ won't be cached between runs. This was also the case before this PR (the old individual crate builds weren't cached either), so it's pre-existing — but now would be a good time to add a second cache entry since the workspace consolidation means more gets built in one clippy invocation:

    workspaces: |
      ./rust/cubesql -> target
      ./rust/cube -> target

    Fix this →

  3. Stale .gitignore entries in moved cratescubenativeutils/.gitignore and cubesqlplanner/.gitignore contain entries like /cubesql/target, /cubesql/egraph-debug*, upstream, .cargo/config.toml that were relevant when these crates lived as siblings of cubesql in rust/. Now under rust/cube/, these patterns don't match anything. They're harmless but could be cleaned up to avoid confusion. Additionally, the per-crate /target entries in each sub-crate .gitignore are now redundant with the workspace-level rust/cube/.gitignore (which contains target).

  4. No rust-toolchain.toml in workspace root — The rust/cube/ directory inherits rust-toolchain.toml from the filesystem (it exists in the directory and pins 1.90.0). This matches the CI toolchain: 1.90.0 — good. Just noting it was carried over properly.

Verdict

This is a well-executed structural refactoring. All paths are correct, CI is properly updated, no references were missed, and the workspace Cargo.toml is clean. The observations above are all non-blocking improvements that could be done as follow-ups.

LGTM

@ovr ovr marked this pull request as ready for review April 30, 2026 15:21
@ovr ovr requested review from a team as code owners April 30, 2026 15:21
@claude
Copy link
Copy Markdown
Contributor

claude Bot commented Apr 30, 2026

Claude finished @ovr's task in 3m 30s —— View job


Review of PR #10784

  • Gather context and understand the workspace restructuring
  • Review new workspace Cargo.toml and .gitignore
  • Review CI/CD workflow changes
  • Review dependency path updates (Cargo.toml files)
  • Check for stale references to old paths
  • Review Cargo.lock changes
  • Verify completeness of the move
  • Post comprehensive review

Overall Assessment

Clean workspace consolidation — the move of cubenativeutils, cubeshared, cubeorchestrator, and cubesqlplanner into rust/cube/ is well-structured. CI workflows were updated correctly with nice simplifications (4 separate cargo fmt/cargo test steps → 1 each). One issue to address before merge:


Bug: CubeStore Dockerfile will break

rust/cubestore/Dockerfile:5 still references the old path:

COPY cubeshared /build/cubeshared

The Docker build context is ./rust/ (confirmed in rust-cubestore.yml, rust-cubestore-master.yml, and publish.yml). Since cubeshared moved from rust/cubeshared/ to rust/cube/cubeshared/, this COPY will fail — the directory no longer exists at the context root.

Additionally, cubestore/cubestore/Cargo.toml:29 now points to ../../cube/cubeshared, which inside the Docker container resolves from /build/cubestore/cubestore/ to /build/cube/cubeshared — but the COPY puts it at /build/cubeshared.

Fix: Update the Dockerfile COPY to match both the new source path and the expected Cargo.toml resolution:

COPY cube/cubeshared /build/cube/cubeshared

Fix this →


Minor: rustfmt.toml behavior change

The old cubeorchestrator/rustfmt.toml and cubeshared/rustfmt.toml had imports_granularity = "Crate" enabled, while the shared rust/cube/rustfmt.toml (inherited from cubenativeutils) has it commented out:

# TODO: Uncomment, when it will become stable in rustfmt
# imports_granularity = "Crate"

Since this is an unstable option on stable Rust 1.90.0, it was likely silently ignored anyway — so this is probably a no-op in practice. Just flagging for awareness.


What looks good

  • Workspace Cargo.toml (rust/cube/Cargo.toml): Correct resolver = "2", all 5 member crates listed properly including the nested cubesqlplanner/nativebridge.
  • CI simplification (push.yml): 4 separate cargo fmt checks → single cargo fmt --manifest-path rust/cube/Cargo.toml --all -- --check. Same for tests with --workspace --features cubesqlplanner/integration-postgres. Well done.
  • Path trigger consolidation: rust/cubenativeutils/**, rust/cubeorchestrator/**, rust/cubeshared/**, rust/cubesqlplanner/** → single rust/cube/** across all workflow files.
  • Dependency paths: All Cargo.toml references updated correctly — cubeclient path in cubesqlplanner (../../../cubesql/cubeclient), cubeshared in cubestore (../../cube/cubeshared), and backend-native paths all resolve properly.
  • Shared tooling: rust-toolchain.toml consolidation uses the superset of components (rustfmt, rustc-dev, clippy), which is correct.
  • Cargo.lock: Properly consolidated from the old cubesqlplanner-only lock into a unified workspace lock.
  • No stale references: Searched across all .toml, .yml, .sh, .js, .ts, and Dockerfile* files — no leftover references to old paths (except the Dockerfile issue above).
  • CLAUDE.md updated to reflect the new /rust/cube/cubesqlplanner path.

Optional follow-up (not blocking)

The workspace Cargo.toml could benefit from [workspace.dependencies] to deduplicate shared deps across crates (e.g., serde, tokio, neon, async-trait appear in multiple member Cargo.toml files). This is a nice-to-have for a future PR.

Comment thread rust/cubestore/cubestore/Cargo.toml
@codecov
Copy link
Copy Markdown

codecov Bot commented Apr 30, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 78.75%. Comparing base (1f749cf) to head (29adba0).
⚠️ Report is 2 commits behind head on master.

Additional details and impacted files
@@           Coverage Diff           @@
##           master   #10784   +/-   ##
=======================================
  Coverage   78.74%   78.75%           
=======================================
  Files         469      469           
  Lines       92130    92130           
  Branches     3388     3388           
=======================================
+ Hits        72548    72553    +5     
+ Misses      19089    19084    -5     
  Partials      493      493           
Flag Coverage Δ
cube-backend 57.99% <ø> (ø)
cubesql 83.37% <ø> (+<0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@ovr ovr merged commit 54a3351 into master Apr 30, 2026
100 of 102 checks passed
@ovr ovr deleted the refactor/introduce-cube-workspace branch April 30, 2026 16:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cube store Issues relating to Cube Store data source driver rust Pull requests that update Rust code

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants