Skip to content

ci: updates for Rust 1.84 - #4846

Merged
Icxolu merged 11 commits into
PyO3:mainfrom
Owen-CH-Leung:Fix-ruff-fmt-error
Jan 10, 2025
Merged

ci: updates for Rust 1.84#4846
Icxolu merged 11 commits into
PyO3:mainfrom
Owen-CH-Leung:Fix-ruff-fmt-error

Conversation

@Owen-CH-Leung

@Owen-CH-Leung Owen-CH-Leung commented Jan 9, 2025

Copy link
Copy Markdown
Contributor

See this

ruff has released 0.9.0 few hours ago and the CI didn't pin the ruff version, hence ruff checks that noxfile.py is malformed, and the CI returns this error. Since the fmt CI errors out, the remaining CI pipelines are marked as cancelled and not running

This MR is created by running ruff format . which reformats the noxfile.py

@LilyFirefly LilyFirefly added the CI-skip-changelog Skip checking changelog entry label Jan 9, 2025
@LilyFirefly

Copy link
Copy Markdown
Contributor

Given this is purely internal formatting we don't need the newsfragment for the changelog.

@Owen-CH-Leung

Copy link
Copy Markdown
Contributor Author

Given this is purely internal formatting we don't need the newsfragment for the changelog.

Thanks. Removed changelog

@ngoldbaum

Copy link
Copy Markdown
Contributor

Well that's fun, rustup update stable is broken right now. I can trigger it on my local dev setup.

@Owen-CH-Leung

Copy link
Copy Markdown
Contributor Author

Well that's fun, rustup update stable is broken right now. I can trigger it on my local dev setup.

Same here on my mac m1. I got :

info: syncing channel updates for 'stable-aarch64-apple-darwin'
info: latest update on 2025-01-09, rust version 1.84.0 (9fc6b4312 2025-01-07)
error: component 'rust-std' for target 'wasm32-wasi' is unavailable for download for channel 'stable'

@ngoldbaum

Copy link
Copy Markdown
Contributor

You can fix it locally with

rustup target remove wasm32-wasi
rustup update
rustup target add wasm32-wasip1

I'll open a separate PR to (hopefully) fix CI.

@ngoldbaum

Copy link
Copy Markdown
Contributor

I'll open a separate PR to (hopefully) fix CI.

Actually it looks like both fixes need to go in simultaneously. Can you apply the change from my other PR to this PR?

@codspeed-hq

codspeed-hq Bot commented Jan 9, 2025

Copy link
Copy Markdown

CodSpeed Performance Report

Merging #4846 will improve performances by 15.87%

Comparing Owen-CH-Leung:Fix-ruff-fmt-error (408d14b) with main (c0f08c2)

Summary

⚡ 1 improvements
✅ 83 untouched benchmarks

Benchmarks breakdown

Benchmark main Owen-CH-Leung:Fix-ruff-fmt-error Change
extract_float_downcast_fail 425.8 ns 367.5 ns +15.87%

@Icxolu

Icxolu commented Jan 9, 2025

Copy link
Copy Markdown
Member

We will also need to bless the ui tests for CI to pass again. (I've done that locally, I may push those here as well)

@ngoldbaum

Copy link
Copy Markdown
Contributor

I've done that locally, I may push those here as well

please do so, I just pushed the fix the failing target

sorry @Owen-CH-Leung - didn't want to trample on your branch but you happened to have unlucky timing coinciding with a rust release and I'd like to fix this quickly.

@Icxolu Icxolu changed the title Fix failing ruff fmt test ci: updates for Rust 1.84 Jan 9, 2025
@ngoldbaum

Copy link
Copy Markdown
Contributor

Looks like a UI test change is missing:

EXPECTED:
  ┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈
  error[E0277]: the trait bound `PyErr: From<MyError>` is not satisfied
    --> tests/ui/invalid_result_conversion.rs:22:25
     |
  22 | fn should_not_work() -> Result<(), MyError> {
     |                         ^^^^^^ the trait `From<MyError>` is not implemented for `PyErr`, which is required by `MyError: Into<PyErr>`
     |
     = help: the following other types implement trait `From<T>`:
               `PyErr` implements `From<AddrParseError>`
               `PyErr` implements `From<DecodeUtf16Error>`
               `PyErr` implements `From<DowncastError<'_, '_>>`
               `PyErr` implements `From<DowncastIntoError<'_>>`
               `PyErr` implements `From<FromUtf16Error>`
               `PyErr` implements `From<FromUtf8Error>`
               `PyErr` implements `From<Infallible>`
               `PyErr` implements `From<IntoInnerError<W>>`
             and $N others
     = note: required for `MyError` to implement `Into<PyErr>`
  ┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈
  
  ACTUAL OUTPUT:
  ┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈
  error[E0277]: the trait bound `PyErr: From<MyError>` is not satisfied
    --> tests/ui/invalid_result_conversion.rs:22:25
     |
  22 | fn should_not_work() -> Result<(), MyError> {
     |                         ^^^^^^ the trait `From<MyError>` is not implemented for `PyErr`
     |
     = help: the following other types implement trait `From<T>`:
               `PyErr` implements `From<AddrParseError>`
               `PyErr` implements `From<DecodeUtf16Error>`
               `PyErr` implements `From<DowncastError<'_, '_>>`
               `PyErr` implements `From<DowncastIntoError<'_>>`
               `PyErr` implements `From<FromUtf16Error>`
               `PyErr` implements `From<FromUtf8Error>`
               `PyErr` implements `From<Infallible>`
               `PyErr` implements `From<IntoInnerError<W>>`
             and $N others
     = note: required for `MyError` to implement `Into<PyErr>`
  ┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈
  note: If the actual output is the correct output you can bless it by rerunning
        your test with the environment variable TRYBUILD=overwrite

@Icxolu

Icxolu commented Jan 9, 2025

Copy link
Copy Markdown
Member

Yeah, that one does not run on windows, that's why it slipped through 😓 . Will fix it shortly.

@Icxolu
Icxolu enabled auto-merge January 9, 2025 17:40
@Icxolu
Icxolu added this pull request to the merge queue Jan 9, 2025
@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Jan 9, 2025
@ngoldbaum

ngoldbaum commented Jan 9, 2025

Copy link
Copy Markdown
Contributor

Maybe we should temporarily allow the WASM emscripten CI to fail?

@Icxolu

Icxolu commented Jan 9, 2025

Copy link
Copy Markdown
Member

It's probably this one I would guess...

The wasm32-unknown-emscripten target’s binary release of the standard library is now built with the latest emsdk 3.1.68, which fixes an ABI-incompatibility with Emscripten >= 3.1.42. If you are locally using a version of emsdk with an incompatible ABI (e.g. before 3.1.42 or a future one), you should build your code with -Zbuild-std to ensure that std uses the correct ABI.

It looks like we build with 3.1.13, but I'm not sure whats decides which version gets used.

@ngoldbaum
ngoldbaum enabled auto-merge January 9, 2025 21:55
@Icxolu

Icxolu commented Jan 9, 2025

Copy link
Copy Markdown
Member

Doesn't look like that worked...

@ngoldbaum
ngoldbaum disabled auto-merge January 9, 2025 22:07
@Icxolu

Icxolu commented Jan 9, 2025

Copy link
Copy Markdown
Member

Looks like that worked 🎉

@Icxolu
Icxolu enabled auto-merge January 9, 2025 23:16
@Icxolu
Icxolu added this pull request to the merge queue Jan 9, 2025
Merged via the queue into PyO3:main with commit 1840bc5 Jan 10, 2025
davidhewitt pushed a commit that referenced this pull request Jan 10, 2025
* Fix failing ruff fmt test

* Add newsfragments

* remove changelog

* use wasm32-wasip1 in CI

* update ui tests

* use `wasm32-wasip1` in noxfile

* update one more ui test

* fix clippy beta

* bump `EMSCRIPTEN_VERSION`

* emscripten link `sqlite3`

* emscipten update node

---------

Co-authored-by: Nathan Goldbaum <nathan.goldbaum@gmail.com>
Co-authored-by: Icxolu <10486322+Icxolu@users.noreply.github.com>
davidhewitt pushed a commit that referenced this pull request Jan 11, 2025
* Fix failing ruff fmt test

* Add newsfragments

* remove changelog

* use wasm32-wasip1 in CI

* update ui tests

* use `wasm32-wasip1` in noxfile

* update one more ui test

* fix clippy beta

* bump `EMSCRIPTEN_VERSION`

* emscripten link `sqlite3`

* emscipten update node

---------

Co-authored-by: Nathan Goldbaum <nathan.goldbaum@gmail.com>
Co-authored-by: Icxolu <10486322+Icxolu@users.noreply.github.com>
@clin1234

clin1234 commented Jan 17, 2025

Copy link
Copy Markdown
Contributor

clippy broke in CI with beta Rust toolchains:

  error: this looks like a formatting argument but it is not part of a formatting macro
   --> src/lib.rs:3:22
    |
  3 |       feature = "nightly",
    |  ______________________^
  4 | |     feature(auto_traits, negative_impls, try_trait_v2)
    | |_^
    |
    = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#literal_string_with_formatting_args
    = note: `-D clippy::literal-string-with-formatting-args` implied by `-D warnings`
    = help: to override `-D warnings` add `#[allow(clippy::literal_string_with_formatting_args)]`

@Icxolu

Icxolu commented Jan 17, 2025

Copy link
Copy Markdown
Member

That's a current upstream clippy bug

@clin1234

Copy link
Copy Markdown
Contributor

That's a current upstream clippy bug

Is there an issue tracking this?

@Icxolu

Icxolu commented Jan 17, 2025

Copy link
Copy Markdown
Member

I believe it's this one: rust-lang/rust-clippy#13885

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CI-build-full CI-skip-changelog Skip checking changelog entry

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants