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

#444 Switch to 'stable' rust toolchain channel #463

Merged
merged 15 commits into from
May 15, 2024

Conversation

b-yap
Copy link
Contributor

@b-yap b-yap commented May 13, 2024

Closes #444.

There is a problem with using the Rust nightly-2024-02-09:

error[E0449]: visibility qualifiers are not permitted here
   --> /Users/b.carlayap/.cargo/git/checkouts/substrate-7e08433d4c370a21/ff24c60/frame/contracts/src/lib.rs:975:3
    |
975 |         environmental!(executing_contract: bool);
    |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    |
    = note: trait items always share the visibility of their trait
    = note: this error originates in the macro `environmental` (in Nightly builds, run with -Z macro-backtrace for more info)

I tried a lower version (nightly-2023-12-28) but errors still abound:

error[E0658]: use of unstable library feature 'stdsimd'
  --> /home/runner/.cargo/registry/src/index.crates.io-6f17d22bba15001f/curve25519-dalek-4.1.2/src/backend/vector/ifma/field.rs:26:5
   |
26 |     _mm256_madd52lo_epu64(z.into(), x.into(), y.into()).into()
   |     ^^^^^^^^^^^^^^^^^^^^^
   |
   = note: see issue #48556 <https://github.com/rust-lang/rust/issues/48556> for more information
   = help: add `#![feature(stdsimd)]` to the crate attributes to enable

Found this comment to choose a newer version, so I picked nightly-2024-04-18.


How to begin the review:

  1. .github/actions/shared/action.yml:
    • we have to separate the jobs for building project using the stable version, and running the tests with nightly. But both jobs have similar steps. The shared steps are transferred in this file.
    • the file name action.yml is a must. See documentation:

      The metadata filename must be either action.yml or action.yaml.

  2. .github/workflows/test-code.yml:
    • split into 2 jobs:
      • build-check - to run cargo check with stable version
      • test-code - to run cargo test with nightly version
  3. README.md:
    • add explanation on why we override the toolchain for testing
  4. rust-toolchain.toml:
    • change to stable 1.77.0
  5. the other files are to clear up build warnings.

@b-yap b-yap force-pushed the 444-switch-to-stable-rust-toolchain-channel branch from 53f3237 to c2139da Compare May 14, 2024 08:04
@b-yap b-yap requested a review from a team May 14, 2024 13:58
@b-yap b-yap marked this pull request as ready for review May 15, 2024 07:15
Copy link
Member

@ebma ebma left a comment

Choose a reason for hiding this comment

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

Looks good to me 👍 A bit annoying that here we again have to do some extra handling for nightly due to the mocktopus crate but doing it this way is probably easier than refactoring the code to remove the mocktopus crate entirely. And in the future we might encounter other crates that we want to have in tests but need nightly so IMO it's okay to have this stable vs nightly setup you added @b-yap.

@b-yap b-yap merged commit cafe9df into main May 15, 2024
3 checks passed
@b-yap b-yap deleted the 444-switch-to-stable-rust-toolchain-channel branch May 15, 2024 10:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Switch to 'stable' rust toolchain channel
2 participants