Skip to content

Update serde, so TiKV can update time crate#390

Merged
ti-chi-bot[bot] merged 12 commits intotikv:masterfrom
v01dstar:update-serde
Feb 12, 2026
Merged

Update serde, so TiKV can update time crate#390
ti-chi-bot[bot] merged 12 commits intotikv:masterfrom
v01dstar:update-serde

Conversation

@v01dstar
Copy link
Member

@v01dstar v01dstar commented Feb 9, 2026

image

Summary by CodeRabbit

  • Chores

    • Updated Rust toolchain edition to 2024.
    • Relaxed the serde dependency to the broader 1.0 series.
  • Style

    • Reordered imports and adjusted public export ordering for consistency.
    • Reflowed formatting and wrapped assertions for readability.
  • Tests / Refactor

    • Reformatted test assertions and benches without behavior changes.
    • Improved test panic-hook handling to a safer, shared implementation.

Signed-off-by: Yang Zhang <yang.zhang@pingcap.com>
@ti-chi-bot ti-chi-bot bot added dco-signoff: yes Indicates the PR's author has signed the dco. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Feb 9, 2026
@coderabbitai
Copy link

coderabbitai bot commented Feb 9, 2026

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

Updated crate edition to "2024", loosened the serde version, and applied many non-functional edits (import reorderings, public export reorderings, test formatting and RNG API tweaks). One internal test utility changed panic hook storage from a raw pointer to an Arc, updating PanicGuard's prev_hook field.

Changes

Cohort / File(s) Summary
Cargo manifest
Cargo.toml
Bumped edition from "2018" to "2024"; changed serde from =1.0.194 (features ["derive"]) to 1.0 (features ["derive"]).
Test utility (panic hook)
src/test_util.rs
Replaced raw pointer storage for the panic hook with Arc<dyn Fn(&panic::PanicHookInfo) + Sync + Send + 'static>; removed unsafe PointerHolder and unsafe impls; updated PanicGuard.prev_hook field signature.
Log batch & benches
src/log_batch.rs, tests/benches/bench_recovery.rs
Switched RNG APIs in benches/tests, adjusted test formatting and Display formatting; import/formatting tweaks only.
Public re-exports
src/lib.rs, src/file_pipe_log/mod.rs
Reordered items in pub use lines (moved PerfContext, reordered FileNameExt/parse_reserved_file_name); exported items unchanged.
File pipe log modules
src/file_pipe_log/pipe.rs, src/file_pipe_log/pipe_builder.rs, src/file_pipe_log/reader.rs
Grouped/reordered imports and minor test assertion reflowing; no behavioral or signature changes.
Engine & core modules
src/engine.rs, src/config.rs, src/consistency.rs, src/memtable.rs, src/purge.rs, src/swappy_allocator.rs, src/fork.rs
Import-order adjustments, duplicate import removal, and test assertion reformatting; no public API changes.
Env & platform files
src/env/log_fd/unix.rs, src/env/obfuscated.rs
Reordered and deduplicated imports; no logic or signature changes.
Filter & utilities
src/filter.rs, src/file_pipe_log/..., src/test_util.rs
Mostly stylistic reordering of use-item lists; notable functional/internal change only in src/test_util.rs as above.
Tests & failpoints
tests/*, tests/failpoints/*
Reformatted assertions (multiline wrapping), minor RNG/import reorders; behavior preserved.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐰 I hopped through crates and polished a nook,
Edition raised and serde given a look.
Imports danced, tests tidied in a whirl,
I swapped a raw pointer for a safer curl.
A quiet rabbit’s tweak — a gentler hook.

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately describes the main change: updating the serde dependency version from 1.0.194 to 1.0 (and edition from 2018 to 2024), which enables TiKV to update the time crate. The title is concise and specific.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

Tip

Issue Planner is now in beta. Read the docs and try it out! Share your feedback on Discord.


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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@v01dstar v01dstar changed the title Update serde, so TiKV can update time create Update serde, so TiKV can update time crate Feb 9, 2026
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Fix all issues with AI agents
In `@Cargo.toml`:
- Line 61: The serde dependency is pinned to an exact version via version =
"=1.0" which normalizes to =1.0.0; update the serde entry in Cargo.toml (the
serde dependency line) to remove the '=' prefix so the version becomes a
caret-compatible range (e.g., version = "1.0") and keep features = ["derive"]
unchanged so the resolver can pick any compatible 1.0.x release.

Signed-off-by: Yang Zhang <yang.zhang@pingcap.com>
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Fix all issues with AI agents
In `@Cargo.toml`:
- Around line 5-6: You bumped the Cargo.toml edition to "2024" while setting
rust-version = "1.85.0"; confirm this change aligns with project and downstream
requirements by running full CI and integration tests, validate downstream
consumers (e.g., TiKV) can consume 2024-edition artifacts, scan the codebase for
patterns affected by the edition (search for unsafe functions that may trigger
the unsafe_op_in_unsafe_fn lint and uses of impl Trait with inferred lifetime
captures), and if any incompatibilities appear either fix the offending code or
revert the edition change until downstream/CI compatibility is ensured.

Comment on lines +5 to 6
edition = "2024"
rust-version = "1.85.0"
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

# First, let's look at the repository structure
git ls-files | head -20

Repository: tikv/raft-engine

Length of output: 360


🏁 Script executed:

# Check for 'gen' usage as identifier
rg -n --type rust '\bgen\b' -g '!target/' -g '!Cargo.toml' | head -50

Repository: tikv/raft-engine

Length of output: 250


🏁 Script executed:

# Check unsafe fn patterns that might be affected by 2024 edition changes
ast-grep --pattern 'unsafe fn $NAME($$$) { $$$ }'

Repository: tikv/raft-engine

Length of output: 1751


🏁 Script executed:

# Let's also check the full Cargo.toml context
cat Cargo.toml

Repository: tikv/raft-engine

Length of output: 2619


Edition jump from 2018 → 2024 is significant — confirm this aligns with project requirements and downstream consumers.

The 2024 edition introduces breaking changes across two skipped editions (2021 and 2024), including the unsafe_op_in_unsafe_fn lint and new lifetime capture rules in impl Trait. The MSRV of 1.85.0 correctly matches the 2024 edition requirement.

A code scan shows no gen identifier conflicts (only .gen() method calls from rand) and no problematic unsafe function patterns, so the migration itself appears sound. Ensure CI passes cleanly with this edition, and verify compatibility with downstream consumers like TiKV that will depend on 2024-edition artifacts.

🤖 Prompt for AI Agents
In `@Cargo.toml` around lines 5 - 6, You bumped the Cargo.toml edition to "2024"
while setting rust-version = "1.85.0"; confirm this change aligns with project
and downstream requirements by running full CI and integration tests, validate
downstream consumers (e.g., TiKV) can consume 2024-edition artifacts, scan the
codebase for patterns affected by the edition (search for unsafe functions that
may trigger the unsafe_op_in_unsafe_fn lint and uses of impl Trait with inferred
lifetime captures), and if any incompatibilities appear either fix the offending
code or revert the edition change until downstream/CI compatibility is ensured.

Copy link
Member

@Connor1996 Connor1996 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@ti-chi-bot ti-chi-bot bot added needs-1-more-lgtm Indicates a PR needs 1 more LGTM. approved labels Feb 9, 2026
Signed-off-by: Yang Zhang <yang.zhang@pingcap.com>
@ti-chi-bot ti-chi-bot bot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Feb 9, 2026
Signed-off-by: Yang Zhang <yang.zhang@pingcap.com>
@ti-chi-bot ti-chi-bot bot added size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Feb 9, 2026
Signed-off-by: Yang Zhang <yang.zhang@pingcap.com>
Signed-off-by: Yang Zhang <yang.zhang@pingcap.com>
@ti-chi-bot ti-chi-bot bot added lgtm and removed needs-1-more-lgtm Indicates a PR needs 1 more LGTM. labels Feb 10, 2026
Signed-off-by: Yang Zhang <yang.zhang@pingcap.com>
@ti-chi-bot ti-chi-bot bot removed the lgtm label Feb 10, 2026
Signed-off-by: Yang Zhang <yang.zhang@pingcap.com>
Signed-off-by: Yang Zhang <yang.zhang@pingcap.com>
Signed-off-by: Yang Zhang <yang.zhang@pingcap.com>
@ti-chi-bot ti-chi-bot bot added the needs-1-more-lgtm Indicates a PR needs 1 more LGTM. label Feb 10, 2026
EXTRA_CARGO_ARGS: '--verbose'
# The macOS nightly CI job was removed because grpc-io 0.13 fails with the
# macOS C++ toolchain. Without a committed Cargo.lock, we cannot pin
# grpc-io to 0.10 as a workaround.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can't we pin grpc-io in Cargo.toml?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

grpc-io is not a direct dependency. raft-engine depend on kvproto which then depend on grpc-io, I don't think we want to pin it in this repo

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I will pin grpc-io there

Signed-off-by: Yang Zhang <yang.zhang@pingcap.com>
@Connor1996
Copy link
Member

/approve

@ti-chi-bot ti-chi-bot bot added lgtm and removed needs-1-more-lgtm Indicates a PR needs 1 more LGTM. labels Feb 11, 2026
Signed-off-by: Yang Zhang <yang.zhang@pingcap.com>
@ti-chi-bot ti-chi-bot bot added the needs-1-more-lgtm Indicates a PR needs 1 more LGTM. label Feb 12, 2026
@ti-chi-bot ti-chi-bot bot added the lgtm label Feb 12, 2026
@ti-chi-bot
Copy link

ti-chi-bot bot commented Feb 12, 2026

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: Connor1996, LykxSassinator

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details Needs approval from an approver in each of these files:
  • OWNERS [Connor1996,LykxSassinator]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@ti-chi-bot ti-chi-bot bot removed the needs-1-more-lgtm Indicates a PR needs 1 more LGTM. label Feb 12, 2026
@ti-chi-bot
Copy link

ti-chi-bot bot commented Feb 12, 2026

[LGTM Timeline notifier]

Timeline:

  • 2026-02-09 22:29:17.983155608 +0000 UTC m=+223973.677295458: ☑️ agreed by Connor1996.
  • 2026-02-10 06:01:20.772125686 +0000 UTC m=+251096.466265526: ☑️ agreed by LykxSassinator.
  • 2026-02-10 07:30:38.113990297 +0000 UTC m=+256453.808130167: ✖️🔁 reset by v01dstar.
  • 2026-02-10 08:53:16.4848269 +0000 UTC m=+261412.178966740: ☑️ agreed by LykxSassinator.
  • 2026-02-11 22:19:25.972897449 +0000 UTC m=+56919.052888144: ☑️ agreed by Connor1996.
  • 2026-02-11 22:24:07.660915785 +0000 UTC m=+57200.740906480: ✖️🔁 reset by v01dstar.
  • 2026-02-12 00:48:10.787976852 +0000 UTC m=+65843.867967547: ☑️ agreed by Connor1996.
  • 2026-02-12 02:13:09.291581143 +0000 UTC m=+70942.371571838: ☑️ agreed by LykxSassinator.

@ti-chi-bot ti-chi-bot bot merged commit b6e85af into tikv:master Feb 12, 2026
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved dco-signoff: yes Indicates the PR's author has signed the dco. lgtm size/XL Denotes a PR that changes 500-999 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants