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

ci: updates for Rust 1.84 #4846

Merged
merged 11 commits into from
Jan 10, 2025
Merged

ci: updates for Rust 1.84 #4846

merged 11 commits into from
Jan 10, 2025

Conversation

Owen-CH-Leung
Copy link
Contributor

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

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

@LilyFoote LilyFoote added the CI-skip-changelog Skip checking changelog entry label Jan 9, 2025
@LilyFoote
Copy link
Contributor

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

@Owen-CH-Leung
Copy link
Contributor Author

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

Thanks. Removed changelog

@ngoldbaum
Copy link
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
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
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
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?

Copy link

codspeed-hq bot commented Jan 9, 2025

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
Copy link
Contributor

Icxolu commented Jan 9, 2025

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
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
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
Copy link
Contributor

Icxolu commented Jan 9, 2025

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
Copy link
Contributor

ngoldbaum commented Jan 9, 2025

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

@Icxolu
Copy link
Contributor

Icxolu commented Jan 9, 2025

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
Copy link
Contributor

Icxolu commented Jan 9, 2025

Doesn't look like that worked...

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

Icxolu commented Jan 9, 2025

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
79 of 81 checks passed
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.

4 participants