-
Notifications
You must be signed in to change notification settings - Fork 33
chore(ci): fix windows build with rust 1.87.0 plus add additional ci test targets #76
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
chore(ci): fix windows build with rust 1.87.0 plus add additional ci test targets #76
Conversation
…lib. Add and update CI for native and new target builds for testing
WalkthroughThe updates adjust CI workflow configurations, cache key scoping, and platform support in build scripts. The changelog and Cargo manifest are updated for a new release and a new dependency. The build script now links an additional system library on Windows, and the Rust toolchain configuration ensures formatting and linting tools are installed. Changes
Sequence Diagram(s)sequenceDiagram
participant CI as CI Workflow
participant Runner as Build Runner
participant Script as build.rs
participant System as System Libraries
CI->>Runner: Start build (with matrix: OS, Rust, etc.)
Runner->>Script: Run build.rs
Script->>Script: Check target OS
alt If Windows
Script->>System: Link advapi32 library
end
Script->>System: Link C++ runtime
Runner->>CI: Report build result
Poem
Note ⚡️ AI Code Reviews for VS Code, Cursor, WindsurfCodeRabbit now has a plugin for VS Code, Cursor and Windsurf. This brings AI code reviews directly in the code editor. Each commit is reviewed immediately, finding bugs before the PR is raised. Seamless context handoff to your AI code agent ensures that you can easily incorporate review feedback. Note ⚡️ Faster reviews with cachingCodeRabbit now supports caching for code and dependencies, helping speed up reviews. This means quicker feedback, reduced wait times, and a smoother review experience overall. Cached data is encrypted and stored securely. This feature will be automatically enabled for all accounts on May 30th. To opt out, configure 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
⏰ Context from checks skipped due to timeout of 90000ms (1)
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 4
🧹 Nitpick comments (5)
Cargo.toml (1)
20-21: Consider updating thiserror to a newer versionWhile adding
thiserroris a good choice for error handling, the specified version (1.0.30) is relatively old. The latest version offers improved performance and additional features.-thiserror = "1.0.30" +thiserror = "1.0.50"CHANGELOG.md (3)
5-9: Future date in changelogThe release date for version 1.4.0 is set to 2025-05-21, which is in the future. Consider updating this to the current date or the planned release date.
-## [1.4.0](https://github.com/tari-project/randomx-rs/compare/v1.3.2...v1.4.0) (2025-05-21) +## [1.4.0](https://github.com/tari-project/randomx-rs/compare/v1.3.2...v1.4.0) (2023-05-21)🧰 Tools
🪛 LanguageTool
[uncategorized] ~8-~8: This word is normally spelled with a hyphen.
Context: ... switch to cmake crate and support less hard coded build system ## [1.3.2](https://github...(HARD_CODE_COMPOUND)
8-8: Hyphenate "hard-coded"For better readability and correct grammar:
-* switch to cmake crate and support less hard coded build system +* switch to cmake crate and support less hard-coded build system🧰 Tools
🪛 LanguageTool
[uncategorized] ~8-~8: This word is normally spelled with a hyphen.
Context: ... switch to cmake crate and support less hard coded build system ## [1.3.2](https://github...(HARD_CODE_COMPOUND)
10-13: Future date in changelogSimilarly, the release date for version 1.3.2 is set to 2024-11-13, which is in the future. Consider updating this to an appropriate date.
-## [1.3.2](https://github.com/tari-project/randomx-rs/compare/v1.3.1...v1.3.2) (2024-11-13) +## [1.3.2](https://github.com/tari-project/randomx-rs/compare/v1.3.1...v1.3.2) (2023-11-13).github/workflows/build_tests.json (1)
128-128: Avoid unpinned nightly toolchain
Using"rust": "nightly"without a date may introduce non-reproducible failures. Consider pinning to a specific nightly version or managing the toolchain via arust-toolchainfile.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge Base: Disabled due to data retention organization setting
📒 Files selected for processing (6)
.github/workflows/build_tests.json(3 hunks).github/workflows/build_tests.yml(1 hunks)CHANGELOG.md(1 hunks)Cargo.toml(2 hunks)build.rs(1 hunks)rust-toolchain.toml(1 hunks)
🧰 Additional context used
🪛 LanguageTool
CHANGELOG.md
[uncategorized] ~8-~8: This word is normally spelled with a hyphen.
Context: ... switch to cmake crate and support less hard coded build system ## [1.3.2](https://github...
(HARD_CODE_COMPOUND)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: machete
🔇 Additional comments (5)
rust-toolchain.toml (1)
5-5: Good addition of required Rust componentsAdding
rustfmtandclippycomponents is a good practice to ensure consistent code formatting and linting across all environments. This aligns well with CI improvements mentioned in the PR objectives.Cargo.toml (1)
9-9: Version update corresponds with changelogThe version bump from 1.3.2 to 1.4.0 aligns with the new release noted in the changelog that introduces the cmake crate build system.
build.rs (1)
41-44: Windows build fix is properly implementedAdding the advapi32 library link for Windows targets addresses the PR's main objective of fixing Windows builds with Rust 1.87.0. This is a common requirement for Windows cryptography support.
Consider adding a brief comment explaining why advapi32 is specifically needed for Rust 1.87.0 on Windows:
if cfg!(target_os = "windows") { + // Required for Windows cryptography APIs, needed explicitly since Rust 1.87.0 println!("cargo:rustc-link-lib=advapi32"); }.github/workflows/build_tests.yml (1)
156-156: Cache key now scoped by runner, target, and job
Including${{ matrix.builds.runs-on }}and${{ matrix.builds.name }}in the cache key isolates caches per OS and build variant, reducing cross-environment collisions..github/workflows/build_tests.json (1)
4-5: Upgrade Linux x86_64 environment to Ubuntu 22.04 and stable Rust
Switching from an older image and nightly toolchain toubuntu-22.04withrust = stablesimplifies maintenance and leverages an LTS release.
Description
Fix windows build with rust 1.87.0 - linking with advapi32lib.
Add and update CI for native and new target builds for testing
Motivation and Context
Fix windows builds
How Has This Been Tested?
Builds all needed platforms
Summary by CodeRabbit
New Features
Chores
Documentation
Refactor