Increase recommended Rust toolchain to 1.98.0

[mulkieran]

Related stratis-storage/project#888

[mulkieran]

@coderabbitai review

[coderabbitai]

✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 2650434d-9346-4e4d-b4d4-c8ad64d4d6c2

📥 Commits

Reviewing files that changed from the base of the PR and between 1d985b2 and f8b6689.

📒 Files selected for processing (2)

• build.rs
• src/systemd/bindings.rs

💤 Files with no reviewable changes (1)

• src/systemd/bindings.rs

Included review availability: Your plan provides up to 4 included reviews per hour; 3 remain after this review.

---

Walkthrough

The pull request updates the pinned Rust toolchain from 1.97.0 to 1.98.0 and applies compatibility changes to validation errors, procedural macro wrapping, and systemd binding generation.

Changes

Rust compatibility updates

Layer / File(s)	Summary
Toolchain version update rust-toolchain.toml	The pinned Rust toolchain changes from 1.97.0 to 1.98.0.
Source and binding generation adjustments src/engine/strat_engine/pool/inspection.rs, src/systemd/bindings.rs, stratisd_proc_macros/src/lib.rs, build.rs	The validation error uses to_string(). wrap_method borrows method statements instead of draining them. Systemd binding generation allowlists sd_notify and syslog and removes the max_align_t blocklist. The generated bindings file no longer contains crate-level lint allowances.

Estimated code review effort: 2 (Simple) | ~10 minutes

Merge Risk: ⚪ Minimal · up to f8b66

This change updates the recommended Rust toolchain and related compatibility code; no actionable merge-blocking risk remains beyond normal checks and review.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Docstring Coverage	✅ Passed	Docstring check was indeterminate for this PR — some files could not be analyzed in time. Not blocking.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and concisely describes the primary change: increasing the recommended Rust toolchain to version 1.98.0.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

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

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

🔇 Additional comments (4)

rust-toolchain.toml (1)

2-2: LGTM!

src/engine/strat_engine/pool/inspection.rs (1)

281-281: LGTM!

src/systemd/bindings.rs (1)

10-10: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

⚠️ Unverified finding
Sandbox verification was unavailable.

Preserve the declared Rust 1.87 MSRV.

Cargo.toml declares Rust 1.87.0 as the lowest supported toolchain. The suspicious_runtime_symbol_definitions lint was added in Rust 1.98, while older compilers report unknown lint attributes as warnings. (github.com)

If an MSRV build enables systemd_compat and denies warnings, this attribute can fail compilation. Add #![allow(unknown_lints)] before this line, or otherwise gate the allowance, then verify the Rust 1.87 build.

Verification

stratisd_proc_macros/src/lib.rs (1)

155-155: LGTM!

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: f903f060-2aef-4bbd-96b7-c845bd211bee

📥 Commits

Reviewing files that changed from the base of the PR and between a202e96 and 1d985b2.

📒 Files selected for processing (4)

• rust-toolchain.toml
• src/engine/strat_engine/pool/inspection.rs
• src/systemd/bindings.rs
• stratisd_proc_macros/src/lib.rs

Included review availability: Your plan provides up to 4 included reviews per hour; 3 remain after this review.

[mulkieran]

🔇 Additional comments (4)

ℹ️ Review info

@coderabbitai I believe that the suspicious_runtime_symbol_definition lint was added earlier than 1.87.0. Our CI compiles a bunch of things with the MSRV and they do seem to be compiling. Also I'm worried that the thing flagged by that lint are actually bad. They were auto-generated by bindgen, and maybe it is auto-generating incorrect code.

[coderabbitai]

Tip

For best results, initiate chat on the files or code changes.

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo '== PR diff for relevant file =='
git diff --find-renames origin/main...HEAD -- src/systemd/bindings.rs || true

echo '== bindings.rs outline =='
ast-grep outline src/systemd/bindings.rs --items all || true

echo '== crate attributes and suspicious symbols =='
rg -n -C 4 'suspicious_runtime_symbol_definition|no_mangle|export_name|link_name|#[[:space:]]*\[unsafe' src/systemd/bindings.rs || true

echo '== bindgen-related configuration and locked version =='
rg -n -i -C 3 'bindgen|systemd' Cargo.toml Cargo.lock build.rs src stratisd_proc_macros 2>/dev/null || true

Length of output: 33012

---

🤖 web_search executed:

Clippy suspicious_runtime_symbol_definition lint documentation: what does it detect, and which Rust or Clippy version introduced it?

Length of output: 16421

---

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo '== src/systemd/bindings.rs =='
cat -n src/systemd/bindings.rs

echo '== systemd binding generator =='
sed -n '1,80p' build.rs

echo '== current commit metadata and relevant parent diff =='
git show --no-ext-diff --format=fuller --no-renames HEAD -- src/systemd/bindings.rs
git diff --no-ext-diff --no-renames HEAD^ HEAD -- src/systemd/bindings.rs || true

echo '== generated binding include and declarations referenced by wrapper =='
rg -n -C 5 'include!|strlen|memcpy|memmove|memset|memcmp|bcmp|open|read|write|close|malloc|realloc|free|exit' src/systemd build.rs

Length of output: 5412

---

@mulkieran You are correct to question this allowance. My earlier claim about the lint version was incorrect.

suspicious_runtime_symbol_definitions is a rustc lint introduced in Rust 1.98.0. It is not a Clippy lint. The allowance is therefore required for the new recommended toolchain, not for the MSRV build.

src/systemd/bindings.rs only includes bindgen output from OUT_DIR. The allowance applies only to that generated-bindings module. It does not suppress the lint for handwritten code.

The lint can flag an imported C declaration such as strlen when bindgen maps C char differently from the Rust runtime signature. This can be ABI-compatible because the pointer representation is unchanged. It does not indicate that bindgen generated a replacement implementation of a runtime symbol.

I could not inspect the exact generated declaration because build.rs creates it in OUT_DIR, which is not present in the source tree. However, the structure matches the known Rust 1.98 bindgen false-positive case. The allowance is appropriate here.

---

✏️ Learnings added

Learnt from: mulkieran
URL: https://github.com/stratis-storage/stratisd/pull/4075

Timestamp: 2026-08-20T21:16:49.633Z
Learning: In this Rust project, `src/systemd/bindings.rs` is a wrapper that includes bindgen-generated systemd bindings from `OUT_DIR`. Rust 1.98.0 introduced the rustc lint `suspicious_runtime_symbol_definitions`, which can flag ABI-compatible C function imports in bindgen output. The module-level `#![allow(suspicious_runtime_symbol_definitions)]` applies only to that generated binding module.

Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.

_{You are interacting with an AI system.}

[packit-as-a-service]

Congratulations! One of the builds has completed. 🍾

You can install the built RPMs by following these steps:

• sudo dnf install -y 'dnf*-command(copr)'
• dnf copr enable packit/stratis-storage-stratisd-4075-copr_pull
• And now you can install the packages.

Please note that the RPMs should be used only in a testing environment.

[mulkieran]

Tip

For best results, initiate chat on the files or code changes.
🧩 Analysis chain

@mulkieran You are correct to question this allowance. My earlier claim about the lint version was incorrect.

suspicious_runtime_symbol_definitions is a rustc lint introduced in Rust 1.98.0. It is not a Clippy lint. The allowance is therefore required for the new recommended toolchain, not for the MSRV build.

src/systemd/bindings.rs only includes bindgen output from OUT_DIR. The allowance applies only to that generated-bindings module. It does not suppress the lint for handwritten code.

The lint can flag an imported C declaration such as strlen when bindgen maps C char differently from the Rust runtime signature. This can be ABI-compatible because the pointer representation is unchanged. It does not indicate that bindgen generated a replacement implementation of a runtime symbol.

I could not inspect the exact generated declaration because build.rs creates it in OUT_DIR, which is not present in the source tree. However, the structure matches the known Rust 1.98 bindgen false-positive case. The allowance is appropriate here.
✏️ Learnings added

You are interacting with an AI system.

@coderabbitai You were correct, suspicious_runtime_symbols_definitions was introduced after Rust 1.87.0. However, you are the bearer of bad news: since you are correct that proves that the jobs that are supposed to compile with the MSRV are compiling with a newer version instead, i.e., there is a bug in our CI and that needs to be fixed.

[coderabbitai]

Tip

For best results, initiate chat on the files or code changes.

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo '== Candidate CI workflow files =='
fd -HI -t f -E .git '.*\.(yml|yaml)$|.*\.toml$' .github . 2>/dev/null | sort || true

echo '== MSRV and toolchain references in workflow/configuration files =='
rg -n -i -C 4 \
  'msrv|minimum.*rust|rust-version|rustup.*toolchain|toolchain:|1\.87\.0|cargo \+|RUSTUP_TOOLCHAIN|rust-toolchain' \
  .github Cargo.toml Cargo.lock rust-toolchain.toml 2>/dev/null || true

echo '== Warning and lint policy =='
rg -n -i -C 4 \
  'unknown_lints|warnings|deny\(warnings\)|-D\s*warnings|--deny[= ]warnings|RUSTFLAGS|cap-lints' \
  .github Cargo.toml rust-toolchain.toml src stratisd_proc_macros 2>/dev/null || true

echo '== Relevant changed configuration =='
cat -n rust-toolchain.toml
printf '\n== bindings crate attributes ==\n'
sed -n '1,50p' src/systemd/bindings.rs

Length of output: 18290

---

@mulkieran The CI configuration does select the MSRV correctly.

Cargo.toml defines the MSRV as 1.87.0. In .github/workflows/main.yml, the MSRV jobs read that value and pass it to actions-rust-lang/setup-rust-toolchain@v1 as the explicit toolchain.

The MSRV build succeeds because src/systemd/bindings.rs already contains:

#![allow(unknown_lints)]

This attribute precedes:

#![allow(suspicious_runtime_symbol_definitions)]

Rust 1.87.0 does not know suspicious_runtime_symbol_definitions. It would normally emit an unknown_lints warning, and this project denies warnings. The existing allow(unknown_lints) suppresses that warning for the generated bindings module.

Therefore, the passing MSRV jobs do not indicate that they use Rust 1.98.0. The new allowance remains compatible with Rust 1.87.0.

_{You are interacting with an AI system.}

[mulkieran]

@coderabbitai review

[coderabbitai]

✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

[mulkieran]

Tip

For best results, initiate chat on the files or code changes.
🧩 Analysis chain

@mulkieran The CI configuration does select the MSRV correctly.

Cargo.toml defines the MSRV as 1.87.0. In .github/workflows/main.yml, the MSRV jobs read that value and pass it to actions-rust-lang/setup-rust-toolchain@v1 as the explicit toolchain.

The MSRV build succeeds because src/systemd/bindings.rs already contains:

#![allow(unknown_lints)]

This attribute precedes:

#![allow(suspicious_runtime_symbol_definitions)]

Rust 1.87.0 does not know suspicious_runtime_symbol_definitions. It would normally emit an unknown_lints warning, and this project denies warnings. The existing allow(unknown_lints) suppresses that warning for the generated bindings module.

Therefore, the passing MSRV jobs do not indicate that they use Rust 1.98.0. The new allowance remains compatible with Rust 1.87.0.

You are interacting with an AI system.

@coderabbitai I decided just to allow the functions stratisd uses, of which there are exactly two which use fundamental types. So none of the clippy or compiler lints that previously affected the generated code are triggered and the generated code is only about 8 lines worth. Seems like an improvement. What do you think?

[coderabbitai]

Tip

For best results, initiate chat on the files or code changes.

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo '== Relevant PR diff =='
git diff --no-ext-diff --unified=80 origin/main...HEAD -- \
  build.rs src/systemd/bindings.rs src/systemd rust-toolchain.toml Cargo.toml || true

echo '== build.rs structure =='
ast-grep outline build.rs --items all || true

echo '== systemd module structure =='
ast-grep outline src/systemd --items all || true

echo '== bindgen configuration and binding inclusion =='
rg -n -C 6 \
  'bindgen|allowlist_function|blocklist|generate\(|write_to_file|include!' \
  build.rs src/systemd

echo '== Declarations and call sites for allowed systemd functions =='
rg -n -C 5 '\b(sd_notify|syslog)\s*\(' src build.rs

echo '== Lint attributes in generated-binding wrapper =='
rg -n -C 4 '#!\[(allow|deny|warn)' src/systemd/bindings.rs || true

Length of output: 5426

---

@mulkieran Yes. This is a clear improvement.

build.rs now allowlists only sd_notify and syslog. These are the only two functions called through src/systemd/bindings.rs.

This change has three benefits:

• It reduces the generated binding surface to the API that stratisd uses.
• It removes the unused declarations that triggered the compiler and Clippy lint diagnostics.
• It removes the need for allow(unknown_lints) and allow(suspicious_runtime_symbol_definitions) in src/systemd/bindings.rs.

The direct call sites match the allowlist:

• src/systemd/mod.rs calls bindings::sd_notify.
• src/systemd/mod.rs calls bindings::syslog.

The smaller generated file also makes future binding changes easier to inspect. I support this approach.

---

✏️ Learnings added

Learnt from: mulkieran
URL: https://github.com/stratis-storage/stratisd/pull/4075

Timestamp: 2026-08-24T17:22:56.225Z
Learning: In the Rust `stratisd` project, `build.rs` generates systemd bindings only for `sd_notify` and `syslog` with bindgen `.allowlist_function(...)`. `src/systemd/mod.rs` contains the only direct calls to these generated bindings. This restricted generated API removes the prior lint diagnostics and avoids lint allowances in `src/systemd/bindings.rs`.

Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.

_{You are interacting with an AI system.}