build(huntsman): Manage crate dependency versions centrally through the Cargo workspace; Set all crates' version to 0.1.0-dev. - #421
Conversation
WalkthroughThe root Cargo workspace now owns shared package metadata and dependency configuration. Component, example, and test manifests inherit those settings. The Helm chart version is updated to ChangesCargo workspace migration
Helm chart metadata
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
sitaowang1998
left a comment
There was a problem hiding this comment.
Shall we create issues to track the pending upgrade and the removal of bincode?
|
@coderabbitai Based on the PR description, create issues to track the pending upgrades for the following dependencies:
|
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
repo="y-scope/spider"
for query in \
'bincode in:title,body is:issue is:open' \
'sqlx in:title,body is:issue is:open' \
'libloading in:title,body is:issue is:open' \
'syn in:title,body is:issue is:open'
do
echo "=== $query ==="
gh issue list --repo "$repo" --state open --search "$query" \
--json number,title,url,body,assignees \
--jq '.[] | {number, title, url, assignees: [.assignees[].login]}'
doneLength of output: 920 🏁 Script executed: #!/bin/bash
set -euo pipefail
printf '%s\n' '=== Dependency declarations ==='
rg -n -i -C 2 '^\s*(bincode|sqlx|libloading|syn)\s*=' \
-g 'Cargo.toml' -g 'Cargo.lock' .
printf '%s\n' '=== Direct Rust usage ==='
rg -n -i -C 2 '\b(bincode|sqlx|libloading|syn)\b' \
-g '*.rs' .Length of output: 66988 🏁 Script executed: #!/bin/bash
set -euo pipefail
repo="y-scope/spider"
pr_url="https://github.com/y-scope/spider/pull/421"
requester="LinZhihao-723"
comment_url="$(
gh api "repos/$repo/issues/421/comments" --paginate \
--jq '.[] | select(.body | contains("create issues to track the pending upgrades")) | .html_url' \
| head -n 1
)"
if [[ -z "$comment_url" ]]; then
comment_url="$pr_url#issuecomment-unavailable"
fi
create_issue() {
local title="$1"
local body="$2"
gh issue create \
--repo "$repo" \
--title "$title" \
--assignee "$requester" \
--body "$body"
}
bincode_body=$(cat <<EOF
## Summary
Replace the unmaintained \`bincode\` dependency with a maintained serialization alternative.
## Rationale
The workspace currently centralizes \`bincode = "1.3.3"\`. Because the library is no longer maintained, the executor IPC serialization should move to a supported alternative.
## Affected areas
- Workspace dependency declaration: \`Cargo.toml\`
- Executor process-pool request/response framing: \`components/spider-execution-manager/src/process_pool.rs\`
- Executor subprocess protocol: \`components/spider-task-executor/src/bin/spider_task_executor.rs\` and \`components/spider-task-executor/src/protocol.rs\`
- Shared executor test harness: \`tests/huntsman/test-utils/src/executor.rs\`
- Related integration and protocol tests
## Required changes
1. Evaluate maintained serialization formats/libraries that meet the executor IPC protocol requirements.
2. Select and document the replacement.
3. Migrate all request/response encoding and decoding together so parent and subprocess remain wire-compatible.
4. Remove the direct \`bincode\` dependency and update the lockfile.
## Acceptance criteria
- No workspace member directly depends on \`bincode\`.
- Parent/executor request-response behavior and error handling remain covered by tests.
- The selected format and any compatibility or migration constraints are documented.
Requested by @$requester.
Backlinks: $pr_url · $comment_url
EOF
)
sqlx_body=$(cat <<EOF
## Summary
Upgrade \`sqlx\` across the workspace and adapt Spider to its breaking API changes.
## Rationale
The centralized workspace dependency is currently \`sqlx = "0.8.6"\` with MySQL support. The pending major upgrade changes APIs, so it was intentionally deferred from PR `#421`.
## Affected areas
- Workspace dependency declaration: \`Cargo.toml\`
- Custom SQLx trait implementations: \`components/spider-core/src/types/id.rs\` and \`components/spider-derive/src/mysql.rs\`
- MariaDB implementation and database error types: \`components/spider-storage/src/db/\`
- SQLx-related derive and integration test coverage
## Required changes
1. Upgrade to the targeted supported \`sqlx\` release and preserve required MySQL/runtime features.
2. Update custom \`Type\`, \`Encode\`, and \`Decode\` implementations plus storage queries/transactions for API changes.
3. Refresh the lockfile and validate database-facing tests.
## Acceptance criteria
- The workspace uses the selected upgraded \`sqlx\` version centrally.
- All compile errors and behavioral changes caused by the upgrade are addressed.
- MariaDB storage workflows and custom ID/enum SQL mappings remain covered by tests.
Requested by @$requester.
Backlinks: $pr_url · $comment_url
EOF
)
libloading_body=$(cat <<EOF
## Summary
Upgrade \`libloading\` and adapt task-executor dynamic-library loading code to its breaking API changes.
## Rationale
The workspace currently centralizes \`libloading = "0.8.9"\`. Its pending major upgrade has API changes and was deferred from PR `#421`.
## Affected areas
- Workspace dependency declaration: \`Cargo.toml\`
- Dynamic library and symbol loading: \`components/spider-task-executor/src/manager.rs\`
- Dynamic-loading error conversion: \`components/spider-task-executor/src/error.rs\`
- Task-executor integration tests that load task packages
## Required changes
1. Upgrade \`libloading\` to the intended supported major release.
2. Adapt library/symbol lifetime, unsafe-operation, and error-handling code to the new API.
3. Refresh the lockfile and test loading valid and invalid task libraries.
## Acceptance criteria
- The workspace centrally resolves the selected upgraded \`libloading\` version.
- Task packages can still be loaded and executed successfully.
- Invalid or incompatible libraries produce the expected \`ExecutorError\` behavior.
- Relevant tests cover the updated loading path.
Requested by @$requester.
Backlinks: $pr_url · $comment_url
EOF
)
syn_body=$(cat <<EOF
## Summary
Assess and align the \`syn\` dependency graph, which currently resolves both 2.x and 3.x versions.
## Rationale
The workspace directly declares \`syn = "2.0.119"\`, while other resolved dependencies use \`syn 3.x\`. PR `#421` intentionally did not make the breaking alignment change; its lockfile adds \`syn 3.0.3\` as a host dependency.
## Affected areas
- Workspace dependency declaration: \`Cargo.toml\`
- \`spider-derive\` procedural macro: \`components/spider-derive/src/\`
- \`spider-tdl-derive\` procedural macro: \`components/spider-tdl-derive/src/\`
- \`Cargo.lock\` and proc-macro dependency graph
## Required changes
1. Identify which direct and transitive dependencies require each \`syn\` major version.
2. Determine whether the workspace's direct procedural-macro dependencies can migrate to \`syn 3.x\`.
3. Update macro parsing/AST code for breaking API changes where applicable.
4. Document any remaining multiple-major-version resolution that is unavoidable because it is transitive.
## Acceptance criteria
- The direct workspace \`syn\` strategy is documented and implemented.
- Spider procedural macros and their tests work with the selected \`syn\` version.
- The lockfile contains no avoidable duplicate \`syn\` major versions; any unavoidable versions are documented with their dependency owners.
Requested by @$requester.
Backlinks: $pr_url · $comment_url
EOF
)
printf 'bincode: %s\n' "$(create_issue "Replace unmaintained bincode serialization dependency" "$bincode_body")"
printf 'sqlx: %s\n' "$(create_issue "Upgrade sqlx and adapt to breaking API changes" "$sqlx_body")"
printf 'libloading: %s\n' "$(create_issue "Upgrade libloading and adapt task library loading" "$libloading_body")"
printf 'syn: %s\n' "$(create_issue "Assess and align syn 2.x/3.x dependency versions" "$syn_body")"Length of output: 6719
You are interacting with an AI system. |
Description
Summary
This PR introduces workspace-level dependency and package inheritance so that every external crate is declared exactly once, in the root
Cargo.toml, instead of being pinned independently in each of the 21 member manifests. Before this change a version bump was an N-file edit, and five crates had already drifted apart across members (tokiowas simultaneously1.50.0and1.52.3,anyhow1.0.98and1.0.102,tokio-util0.7and0.7.18,tracing0.1.41and0.1.44,tracing-subscriber0.3.19and0.3.23). All drift is resolved and every dependency is aligned to the newest semver-compatible release.Every bump stays within its crate's current major, so no Rust source file is modified by this PR — it is confined to manifests and
Cargo.lock. Where a crate has a breaking major available, it is pinned to the last release before that break rather than upgraded; those upgrades are deferred to individual PRs (see below).The change is intended to be behavior-neutral. It was verified as such by diffing the fully resolved feature graph (
cargo metadata) before and after: of roughly 318 packages common to both graphs, exactly one differs, and that difference comes from an upstream manifest change rather than from this migration (see Notes).All member crates additionally move from
version = "0.1.0"to a sharedversion = "0.1.0-dev", and the Helm chart'sappVersionis updated to match.Workspace inheritance (root
Cargo.toml)[workspace.package]holding the two fields every member repeated identically:version = "0.1.0-dev"andedition = "2024".[workspace.dependencies]with 39 external crates and the 13 internal path dependencies. Internal crates are now referenced by name rather than by a relative path, which removes path spellings like../../../../components/spider-tdlfrom the example crates.resolver,members, anddefault-membersare unchanged.Member manifests (21 crates)
[package]now usesversion = { workspace = true }andedition = { workspace = true }.[dependencies],[dev-dependencies], and[build-dependencies]becomes{ workspace = true }. Members retain only the features they need in addition to what the workspace entry already provides, so the common case collapses to a single bare inheritance.{ workspace = true }inline-table form is used throughout rather than the dotteddep.workspace = trueform, for consistency and to avoid tripping thedotted-keys-out-of-orderlint rule configured intombi.toml.sqlxaddsruntime-tokioinspider-storage;synaddsfullinspider-tdl-derive;tracingaddsattributesinspider-storage;spider-tdladdsderiveinhuntsman-complex,huntsman-nn-tasks, andintegration-test-tasks. Thetokioandtokio-utilworkspace entries deliberately carry no features, because the per-member feature sets differ substantially, so each member keeps its full list.spider-tdl-deriveremains an optional dependency ofspider-tdl({ workspace = true, optional = true }).default-features = falsemoves up to the workspacefutures-util,tracing, andtracing-subscriberwere each declared withdefault-features = falsein individual members. Cargo ignores a member'sdefault-features = falsewhen the workspace entry leaves defaults enabled, so this key now lives on the workspace entry instead; leaving it in the members would have silently re-enabled default features.For
tracingthis meant restating the intent slightly: the workspace entry isdefault-features = false, features = ["std"], andspider-storage— which previously took defaults plusattributes— now declaresattributesexplicitly. The resolved feature set is unchanged (verified below).Dependency version alignment
Aligned to the newest compatible release
Where a crate had drifted, the drifted requirements are listed together in "Before".
anyhow1.0.98,1.0.1021.0.104async-channel2.3.12.5.0async-trait0.1.890.1.91bytes1.101.12.1clap4.6.14.6.4const_format0.2.350.2.36dashmap6.1.06.2.1futures-util0.3.310.3.33libloading0.8.50.8.9proc-macro21.0.1061.0.107quote1.0.451.0.47rand0.9.10.9.5semver1.0.271.0.28serde1.0.2281.0.229serde_json1.0.1491.0.151serial_test3.2.03.5.0syn2.0.1172.0.119thiserror2.0.182.0.19tokio1.50.0,1.52.31.53.1tokio-util0.7,0.7.180.7.19tracing0.1.41,0.1.440.1.44tracing-subscriber0.3.19,0.3.230.3.23Every entry in
[workspace.dependencies]is now patch-exact, per the repository convention.bytesandtokio-utilpreviously named no patch version, and those two pre-existing deviations are corrected here.Breaking major available, deliberately not taken
Each of these is pinned to the newest release within its current major — the last version before the API break — so that this PR needs no source changes. Each upgrade is deferred to its own PR so it can be reviewed against its own migration guide.
bincode1.3.33.0.03.0.0is a tombstone, not a usable release; crate is unmaintainedlibloading0.8.90.9.0dlerrortext moves out ofDisplay, degrading load-failure diagnosticsrand0.9.50.10.2Rngrenamed toRngExt;SeedableRng::from_os_rngremovedserial_test3.5.04.0.1syn3 only; no user-facing API changesqlx0.8.60.9.0ArgumentBufferloses its lifetime;query*()narrowed; MySQL behavior changessyn2.0.1193.0.3syn2.x deliberately; see Notestabled0.20.00.21.0Charset,TabSize, andColumnNamesreworked; none used heresqlxandlibloadingare worth calling out, because their majors would each require code changes that this pinning avoids:sqlx0.9.0removes the lifetime parameter fromArgumentBuffer(which theEncodeimpls inspider-core/src/types/id.rsand theMySqlEnumderive inspider-derive/src/mysql.rsboth name) and narrowsquery*()toimpl SqlSafeStr(which the dynamically built statements inspider-storage/src/db/mariadb.rswould need to opt out of). It also changes MySQL connection behavior —SET NAMES utf8mb4 COLLATE utf8_general_ciis no longer sent by default — and touches how database errors are surfaced, which matters here because duplicate-key and foreign-key handling inmariadb.rsclassifies errors by downcasting toMySqlDatabaseErrorand matching on the error number. Staying on0.8.6keeps all of that as-is.libloading0.9.0moves the underlyingdlerrortext out ofError'sDisplayimplementation and behindError::source(). BecauseExecutorError'sFrom<libloading::Error>impl formats viato_string(), taking that major without also rewriting the conversion would silently reduce every TDL package load failure to"dlopen failed". Staying on0.8.9preserves the current diagnostics.bincodeis a special case in that table: it is not merely deferred. The crate is unmaintained, and its3.0.0release is a deliberate tombstone whosesrc/lib.rsis a barecompile_error!, so it cannot be depended on at all —2.0.1is the last functional release. Migrating offbincodeentirely is tracked separately; this repo already usesrmp-serdefor every other serialization path.Crate version scheme
All 21 members now report
0.1.0-devthrough workspace inheritance.0.1.0-devis a valid SemVer 2.0.0 pre-release (devis a well-formed alphanumeric identifier) and orders below0.1.0. No member declares a version requirement on another member — internal dependencies are path-only — so the suffix has no effect on resolution.Helm chart (
tools/deployment/spider-helm/Chart.yaml)appVersionmoves from"0.1.0"to"0.1.0-dev"so the deployed application version tracks the workspace crate version.appVersionis surfaced only as theapp.kubernetes.io/versionlabel viaspider.labelsin_helpers.tpl; container image tags come fromvalues.yamlthroughspider.imageRefand are unaffected.versionis bumped0.1.5→0.1.6, matching the convention that every change to the chart carries a patch bump.Notes
cargo metadataoutput between a pristine checkout and this branch, exactly one package's feature set differs.tokio-utilgainslibc, becausetokio-util0.7.19declarescodec = ["libc"]where0.7.18did not; the workspace already enabledcodec, so this follows mechanically from the upstream bump. No package loses a feature.futures-util,tracing,tracing-subscriber,tokio,sqlx,serde,clap,syn, andspider-tdlall resolve to byte-identical feature sets.Cargo.lockgains exactly one package,syn 3.0.3, pulled in by the bumpedasync-trait,clap_derive,serde_derive, andthiserror-impl. It resolves only as a host dependency, so it adds one proc-macro-side compilation unit and contributes nothing to the target artifact.syn 2.xis unaffected and still required by 23 crates in the graph, including this repo's two proc-macro crates, andsyn 1.0.109remains reachable throughnon-empty-string→delegate, so a single syn version is not achievable at present and the workspace intentionally stays onsyn 2.x.default-features = falseontracingis effectively inert in this workspace, sincesqlx-coreenables tracing's default features regardless. It is declared for correctness of intent rather than for effect. The same is true offutures-util, whose defaults are enabled transitively byserial_test,hyper-util,sqlx, andtonic.test-utilspreviously declaredserdewith no features and now inheritsderivefrom the workspace entry. This is inherent to Cargo's feature unioning and has no resolved effect, asserde/deriveis already enabled by 12 other members. Avoiding it would mean removingderivefrom the workspace entry and repeating it in those 12 members.Checklist
breaking change.
Validation performed
Summary by CodeRabbit
Chores
Deployment