Update Rust toolchain to nightly-2025-11-15 and enhance WASM build co…#717
Merged
Conversation
…nfiguration - Changed Rust toolchain version in CI workflow and configuration files to nightly-2025-11-15. - Updated WASM build flags in `.cargo/config.toml` to include shared memory and various export options, while adjusting the maximum memory limit. - Removed obsolete package version patching from `web.rs` to streamline the build process. These changes ensure compatibility with the latest Rust features and improve the WASM build process.
|
Note Reviews pausedIt 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 Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughUpdates CI and build configuration for the new nightly toolchain, changes wasm linker flags and web build invocation, adds iOS module-map shim generation, and bumps crate and dependency versions. ChangesToolchain, wasm build, version, and iOS build alignment
Estimated code review effort: 3 (Moderate) | ~20 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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 |
Deploying mopro with
|
| Latest commit: |
b907d64
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://8f65fcf5.mopro.pages.dev |
| Branch Preview URL: | https://fix-wasm-2.mopro.pages.dev |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
mopro-ffi/src/app_config/web.rs (1)
43-43: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueToolchain version hardcoded in multiple places.
"nightly-2025-11-15"is now duplicated across this file,mopro-ffi/rust-toolchain.toml, andbuild-and-test.yml(2 occurrences). Consider reading it fromrust-toolchain.toml(or a shared env var) to avoid future drift when bumping again.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@mopro-ffi/src/app_config/web.rs` at line 43, The Rust toolchain version is hardcoded in multiple places and is already drifting risk; update the web config code in app_config::web to stop embedding "nightly-2025-11-15" directly. Read the version from rust-toolchain.toml or a shared environment/config source in the same flow used by the builder logic, and keep build-and-test.yml and the Rust toolchain file aligned so the version is defined once and reused.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@mopro-ffi/.cargo/config.toml`:
- Around line 7-14: Restore the missing wasm threads/TLS flag by adding
+mutable-globals alongside the existing target features in the wasm build
configuration. Update the Rust target settings in .cargo/config.toml and keep
them consistent with app_config::web settings so the shared-memory/TLS exports
are linked with the full required feature set.
---
Nitpick comments:
In `@mopro-ffi/src/app_config/web.rs`:
- Line 43: The Rust toolchain version is hardcoded in multiple places and is
already drifting risk; update the web config code in app_config::web to stop
embedding "nightly-2025-11-15" directly. Read the version from
rust-toolchain.toml or a shared environment/config source in the same flow used
by the builder logic, and keep build-and-test.yml and the Rust toolchain file
aligned so the version is defined once and reused.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 883a1bb8-2afd-40dd-b9a9-574d0fb7d92c
📒 Files selected for processing (4)
.github/workflows/build-and-test.ymlmopro-ffi/.cargo/config.tomlmopro-ffi/rust-toolchain.tomlmopro-ffi/src/app_config/web.rs
- Added `+mutable-globals` to the WASM build flags in `.cargo/config.toml` for enhanced functionality. - Introduced a new constant `WASM_NIGHTLY_TOOLCHAIN` in `constants.rs` to centralize the toolchain versioning. - Updated `web.rs` to utilize the new constant for improved maintainability and clarity in the build command. These changes streamline the configuration and ensure consistency across the project.
- Bump versions of several dependencies in `Cargo.lock`, including `askama`, `askama_derive`, `bytes`, `camino`, `cargo-platform`, `cargo_metadata`, `fs-err`, `serde`, and `serde_json` to their latest releases for improved functionality and security. - Introduce `askama_macros` and `serde_core` as new dependencies to enhance the project structure. - Update `uniffi` version in `Cargo.toml` to `0.32.0` to align with the latest changes in the dependency tree. - Rearrange constants in `web.rs` for better readability and maintainability. These updates ensure compatibility with the latest features and improvements across the project.
- Bump the version of `mopro-cli` and `mopro-ffi` to `0.3.7-alpha.0` in their respective `Cargo.toml` files. - Update dependency versions in `Cargo.lock` to reflect the new version for `mopro-ffi`. - Ensure compatibility across the project with the updated versioning. These changes prepare the project for the next development phase and maintain consistency in dependency management.
- Added a step to the GitHub Actions workflow to install Kotlin version 1.9.24, ensuring the environment is set up with the required Kotlin compiler for the project. - This update improves the build process by providing the necessary tools for Kotlin-based tasks. These changes support the ongoing development and testing efforts within the project.
There was a problem hiding this comment.
🧹 Nitpick comments (3)
.github/workflows/build-and-test.yml (3)
122-127: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winCache the downloaded Kotlin compiler across the matrix.
This step downloads and unzips the ~90MB Kotlin compiler on every job in the
cli_template_testsmatrix (2 OS × 5 adapters = 10 jobs), with no caching. This mirrors the wasm-pack caching pattern already used elsewhere in this file (lines 498-502).♻️ Proposed fix using actions/cache
+ - name: Restore cached Kotlin compiler + id: cache-kotlinc + uses: actions/cache@v4 + with: + path: ~/kotlinc + key: ${{ runner.os }}-kotlinc-1.9.24 + - name: Install Kotlin 1.9.24 + if: steps.cache-kotlinc.outputs.cache-hit != 'true' run: | curl -fL https://github.com/JetBrains/kotlin/releases/download/v1.9.24/kotlin-compiler-1.9.24.zip -o kotlin.zip unzip -q kotlin.zip -d "$HOME" - echo "$HOME/kotlinc/bin" >> "$GITHUB_PATH" + - name: Add Kotlin to PATH + run: echo "$HOME/kotlinc/bin" >> "$GITHUB_PATH"🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/build-and-test.yml around lines 122 - 127, The “Install Kotlin 1.9.24” step in the build-and-test workflow always re-downloads the compiler for every cli_template_tests matrix job. Add an actions/cache-backed cache around the Kotlin zip or extracted kotlinc directory, keyed by the Kotlin version and runner OS, and restore it before the curl/unzip step so the same compiler is reused across the matrix. Mirror the caching pattern already used elsewhere in this workflow and keep the install step as a fallback only when the cache misses.
518-518: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDuplicated hardcoded nightly toolchain string.
nightly-2025-11-15is hardcoded independently at Line 518 and Line 554, and it's also embedded inmopro-ffi/rust-toolchain.tomlandmopro-ffi/src/app_config/constants.rs::WASM_NIGHTLY_TOOLCHAIN. Four places must stay in sync for future nightly bumps; consider consolidating within this workflow via a top-levelenv:var (e.g.WASM_NIGHTLY_TOOLCHAIN) referenced in both jobs to reduce drift risk.Also applies to: 554-554
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/build-and-test.yml at line 518, The nightly toolchain string is duplicated in the workflow and should be centralized to avoid drift. Update the build-and-test workflow so both jobs reference a single top-level env value (for example, WASM_NIGHTLY_TOOLCHAIN) instead of hardcoding nightly-2025-11-15 directly in rustup invocations; use that shared variable in the relevant steps so future bumps only need one workflow edit.
122-127: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winAdd checksum verification for the Kotlin download.
In
.github/workflows/build-and-test.yml, check the published SHA256 forkotlin-compiler-1.9.24.zipwithsha256sum -cbefore unzipping so the workflow doesn’t trust the archive blindly.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/build-and-test.yml around lines 122 - 127, The Kotlin install step in the build-and-test workflow downloads and unzips the compiler archive without verifying integrity first. Update the “Install Kotlin 1.9.24” step to fetch the published SHA256 for kotlin-compiler-1.9.24.zip, validate the downloaded kotlin.zip with sha256sum -c before unzip, and only then continue adding kotlinc/bin to GITHUB_PATH. Keep the fix localized to that workflow job/step so the existing install flow remains unchanged apart from the added verification.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In @.github/workflows/build-and-test.yml:
- Around line 122-127: The “Install Kotlin 1.9.24” step in the build-and-test
workflow always re-downloads the compiler for every cli_template_tests matrix
job. Add an actions/cache-backed cache around the Kotlin zip or extracted
kotlinc directory, keyed by the Kotlin version and runner OS, and restore it
before the curl/unzip step so the same compiler is reused across the matrix.
Mirror the caching pattern already used elsewhere in this workflow and keep the
install step as a fallback only when the cache misses.
- Line 518: The nightly toolchain string is duplicated in the workflow and
should be centralized to avoid drift. Update the build-and-test workflow so both
jobs reference a single top-level env value (for example,
WASM_NIGHTLY_TOOLCHAIN) instead of hardcoding nightly-2025-11-15 directly in
rustup invocations; use that shared variable in the relevant steps so future
bumps only need one workflow edit.
- Around line 122-127: The Kotlin install step in the build-and-test workflow
downloads and unzips the compiler archive without verifying integrity first.
Update the “Install Kotlin 1.9.24” step to fetch the published SHA256 for
kotlin-compiler-1.9.24.zip, validate the downloaded kotlin.zip with sha256sum -c
before unzip, and only then continue adding kotlinc/bin to GITHUB_PATH. Keep the
fix localized to that workflow job/step so the existing install flow remains
unchanged apart from the added verification.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 575f2ea8-f5a0-48e9-8028-ac30a0c4e53c
📒 Files selected for processing (1)
.github/workflows/build-and-test.yml
- Introduced a `module/` directory alongside the xcframework to include a copy of the FFI header and a `module.modulemap`, addressing Xcode 26's change in module map registration for static-library XCFrameworks. - Updated the `project.pbxproj` file to include the `OTHER_SWIFT_FLAGS` for module map file location, ensuring proper integration with the new structure. These changes enhance compatibility with Xcode 26 and streamline the build process for iOS projects.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
cli/src/template/ios/MoproApp.xcodeproj/project.pbxproj (1)
480-480: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueHardcoded bindings folder name duplicates a Rust constant.
MoproiOSBindingshere must stay in sync withIOS_BINDINGS_DIR(used asbindings_dir_nameinmopro-ffi/src/app_config/ios.rs). No compile-time link enforces this; renaming the constant would silently break the module map path.Also applies to: 509-509, 527-527, 546-546
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@cli/src/template/ios/MoproApp.xcodeproj/project.pbxproj` at line 480, The Xcode project is hardcoding the iOS bindings folder name in multiple OTHER_SWIFT_FLAGS entries, which can drift from the Rust IOS_BINDINGS_DIR/bindings_dir_name constant. Update the project.pbxproj references used by the iOS app template so they derive from the same bindings directory name used by MoproiOSBindings/IOS_BINDINGS_DIR, and make sure every occurrence in the affected build settings stays consistent. Keep the module map path and any related flags synchronized across all listed entries so a rename in the Rust constant does not silently break the build.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@cli/src/template/ios/MoproApp.xcodeproj/project.pbxproj`:
- Line 480: The shim module-map flag is being applied unconditionally in the iOS
project settings, which can expose both the bundled xcframework module map and
the extra shim map to older toolchains. Update the project configuration around
OTHER_SWIFT_FLAGS so the MoproApp targets only pass the -Xcc -fmodule-map-file
setting for Xcode 26+ or otherwise ensure a single module map is visible at
build time. Use the existing MoproApp target build settings in the
project.pbxproj to scope the flag correctly.
---
Nitpick comments:
In `@cli/src/template/ios/MoproApp.xcodeproj/project.pbxproj`:
- Line 480: The Xcode project is hardcoding the iOS bindings folder name in
multiple OTHER_SWIFT_FLAGS entries, which can drift from the Rust
IOS_BINDINGS_DIR/bindings_dir_name constant. Update the project.pbxproj
references used by the iOS app template so they derive from the same bindings
directory name used by MoproiOSBindings/IOS_BINDINGS_DIR, and make sure every
occurrence in the affected build settings stays consistent. Keep the module map
path and any related flags synchronized across all listed entries so a rename in
the Rust constant does not silently break the build.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: ebc69cfa-69c5-43ab-a0e6-592411ab503d
📒 Files selected for processing (2)
cli/src/template/ios/MoproApp.xcodeproj/project.pbxprojmopro-ffi/src/app_config/ios.rs
- Removed the embedded module map from the FFI header artifacts, as the new `module/` shim directory is now the sole definition for all Xcode versions. - Updated comments to clarify the changes in module map registration for static-library XCFrameworks, ensuring compatibility with Xcode 26 and avoiding duplicate-module errors. These modifications streamline the build process and enhance compatibility with the latest Xcode versions.
- Simplified the file removal logic for the module map source in the `regroup_header_artifacts` function, ensuring clearer context handling during the removal process. - This change enhances code readability and maintains the existing functionality without introducing any new behavior. These modifications contribute to a cleaner and more maintainable codebase.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
nightly-2025-02-20tonightly-2025-11-15acrossrust-toolchain.toml,web.rs,and CI workflow
RUSTFLAGSwithCARGO_TARGET_WASM32_UNKNOWN_UNKNOWN_RUSTFLAGSin the web build so wasm-specific linker flagsdon't leak into native builds (fixes
clang: error: unknown argumentwhen wasm-pack installswasm-bindgen-cli).cargo/config.tomlto enable SharedArrayBuffer support: add--shared-memory,--import-memory, TLS exports; reduce max-memory from 4 GB → 1 GB; drop+mutable-globalspatch_package_versionworkarounds forindexmap,backtrace,blake2b_simd,wasip2— no longer needed withthe newer nightly
Test plan
mopro build --mode release --platforms webwith a halo2 project and confirm it compiles successfullyclang: error: unknown argumenterrors duringwasm-bindgen-cliinstallation'+atomics' is not a recognized featurewarnings from native compilation steps🤖 Generated with Claude Code
Summary by CodeRabbit
0.32.0.mopro-ffiand CLI to0.3.7-alpha.0, and updated the init TOML template.