Skip to content
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

Fixes #1454 by setting the msrv #1463

Merged
merged 11 commits into from
Dec 28, 2023
38 changes: 34 additions & 4 deletions .github/workflows/ci.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,30 @@ jobs:
- run: ./frb_internal generate-website

# ----------------------------------- codegen -----------------------------------
# instead of running this explicit test for frb_codegen and frb_rust only, we run the test for
# all rust packages in the job "test_rust". While this would not be needed for these other packages,
# we avoid a ci dependency to cargo-msrv.
# However, the code is left commented here in case this decission changes.
# msrv:
# name: 'Test :: FRB Codegen :: MSRV'
# runs-on: ubuntu-latest
# steps:
# - uses: actions/checkout@v2

# - name: Install Rust
# uses: actions-rs/toolchain@v1
# with:
# toolchain: stable

# - name: Verify MSRV for frb_codegen
# run: |
# cargo install cargo-msrv
# cargo msrv --path frb_codegen verify

# - name: Verify MSRV for frb_rust
# run: |
# cargo install cargo-msrv
# cargo msrv --path frb_rust verify

generate_run_frb_codegen_command_generate:
name: 'Generate :: FRB Codegen :: Command Generate'
Expand Down Expand Up @@ -146,7 +170,7 @@ jobs:
flutter-version: ${{ env.FRB_MAIN_FLUTTER_VERSION }}
architecture: x64
- uses: taiki-e/install-action@cargo-llvm-cov

# execute
- run: ./frb_internal generate-run-frb-codegen-command-generate --set-exit-if-changed --package ${{ matrix.package }} --coverage

Expand Down Expand Up @@ -323,6 +347,8 @@ jobs:
test_rust:
name: 'Test :: Rust'
runs-on: ${{ matrix.info.image }}
outputs:
FRB_MSRV_RUST_VERSION: ${{ env.FRB_MSRV_RUST_VERSION }}
patmuk marked this conversation as resolved.
Show resolved Hide resolved
strategy:
fail-fast: false
matrix:
Expand All @@ -339,7 +365,11 @@ jobs:
version: nightly
- image: ubuntu-latest
version: nightly-2023-12-16
# TODO(@fzyzcjy, @gutenfries): add MSRV here, as well as other locked versions as needed
# tests the MSRV
# run on all rust packages, though only neede for frb_rust and frb_codegen
patmuk marked this conversation as resolved.
Show resolved Hide resolved
- image: ubuntu-latest
# update this, if a later MSVR is needed
patmuk marked this conversation as resolved.
Show resolved Hide resolved
version: 1.74.0

steps:
# setup
Expand Down Expand Up @@ -367,8 +397,8 @@ jobs:
- uses: actions/upload-artifact@v4
with:
name: ${{ github.job }}--${{ matrix.info.image }}--${{ matrix.info.version }}--coverage
path: target/coverage

path: target/coverage
test_dart_native:
name: 'Test :: Dart :: Native'
runs-on: ${{ matrix.image }}
Expand Down
1 change: 1 addition & 0 deletions frb_codegen/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
categories.workspace = true
description.workspace = true
edition.workspace = true
rust-version = "1.74.0"
Copy link
Owner

Choose a reason for hiding this comment

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

It may be great if we can lower the MSRV by fixing some trivial compile errors ;)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hmmm - I personally don't mind, as I like to use the latest version.
But I suggest opening another issue for that. In #1454 you can get an impression of what kind of changes would have to be made (that was with rust 1.71., the compiler complained about pup (crate) mostly).
I assume that would require a redesign of the modularization.
IMHO: Not worth it, would not do that unless somebody complains.

I got to the MSRV of 1.74.0 by using cargo msrv, which performs a bisect search. So it won't compile to anything lower without changes to the code.
I would leave it as it, and only of somebody raises an issue needing a lower version work on specifically supporting from that version on.

Copy link
Owner

Choose a reason for hiding this comment

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

I suggest opening another issue for that

Sure! Then let's keep this PR simple and atomic.

keywords.workspace = true
license.workspace = true
name = "flutter_rust_bridge_codegen"
Expand Down
1 change: 1 addition & 0 deletions frb_rust/Cargo.toml
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
[package]
name = "flutter_rust_bridge"
edition.workspace = true
rust-version = "1.70.0"
version.workspace = true
description.workspace = true
license.workspace = true
Expand Down
Loading