Skip to content

Update Rust toolchain to 1.88 and MSRV to 1.86 #19011

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

Merged
merged 3 commits into from
Jun 28, 2025
Merged

Conversation

MichaReiser
Copy link
Member

Summary

The most important new feature for us is that Rust now supports dyn upcasting. That allows us to remove the Upcast trait.

Test Plan

cargo build

@MichaReiser MichaReiser added the internal An internal refactor or improvement label Jun 28, 2025
Copy link
Contributor

github-actions bot commented Jun 28, 2025

mypy_primer results

No ecosystem changes detected ✅

Copy link
Contributor

github-actions bot commented Jun 28, 2025

ruff-ecosystem results

Linter (stable)

✅ ecosystem check detected no linter changes.

Linter (preview)

✅ ecosystem check detected no linter changes.

Formatter (stable)

✅ ecosystem check detected no format changes.

Formatter (preview)

✅ ecosystem check detected no format changes.

@AlexWaygood
Copy link
Member

hmm, some windows path filtering stopped working?

@MeGaGiGaGon
Copy link
Contributor

MeGaGiGaGon commented Jun 28, 2025

Looks like it's because <path>.path().canonicalize() and insta's path representation now disagree: The paths that are passed as a filter have a starting \\?\ both on main and on this PR:
On main:

---- requires_python_pyproject_toml_above_with_tool stdout ----
[crates\ruff\tests\lint.rs:19:5] path.as_ref().to_str() = Some(
    "\\\\?\\C:\\Users\\GiGaGon\\AppData\\Local\\Temp\\.tmp8VZVtR\\foo\\test.py",
)
[crates\ruff\tests\lint.rs:19:5] path.as_ref().to_str() = Some(
    "\\\\?\\C:\\Users\\GiGaGon\\AppData\\Local\\Temp\\.tmp8VZVtR",
)

On this PR

---- requires_python_pyproject_toml_above_with_tool stdout ----
[crates\ruff\tests\lint.rs:19:5] path.as_ref().to_str() = Some(
    "\\\\?\\C:\\Users\\GiGaGon\\AppData\\Local\\Temp\\.tmpUGvoNc\\foo\\test.py",
)
[crates\ruff\tests\lint.rs:19:5] path.as_ref().to_str() = Some(
    "\\\\?\\C:\\Users\\GiGaGon\\AppData\\Local\\Temp\\.tmpUGvoNc",
)

But insta's output doesn't have a starting \\?\ as of this PR:
On main with filter removed

   78-  "[TMP]/foo",
   79-  "[TMP]/foo/src",
         78+  "\\?\C:\Users\GiGaGon\AppData\Local\Temp\.tmpAuA7Xq\foo",
         79+  "\\?\C:\Users\GiGaGon\AppData\Local\Temp\.tmpAuA7Xq\foo\src",

On this PR

   78-  "[TMP]/foo",
   79-  "[TMP]/foo/src",
         78+  "C:\Users\GiGaGon\AppData\Local\Temp\.tmpAQDo8w\foo",
         79+  "C:\Users\GiGaGon\AppData\Local\Temp\.tmpAQDo8w\foo\src",

And this is supported by the fact that all of the lint tests pass if I add a .trim_start_matches(r"\\?\") to tempdir_filter

@MichaReiser
Copy link
Member Author

MichaReiser commented Jun 28, 2025

Thanks for the analysis. The paths in your analysis look the same to me (ignoring the temp folder name) and I couldn't find any mention of changes to path handling in the Rust 1.88 changelog

@MeGaGiGaGon
Copy link
Contributor

Sorry if I was unclear on the issue (I need to get better at that)
On main: path passed to tempdir_filter starts with \\?\, path insta outputs starts with \\?\, regex matches and everything works
On this pr: path passed to tempdir_filter starts with \\?\, path insta outputs does not start with \\?\, regex fails to match and path is not replaced with [TMP] causing the test failure

@ntBre
Copy link
Contributor

ntBre commented Jun 28, 2025

This might have actually changed in 1.88? rust-lang/rust#139683 I didn't see it in the release notes either, though.

I found this in the blame for canonicalize but it might not be related.

@MichaReiser
Copy link
Member Author

I think the culprint is rust-lang/rust#138869

canonicalize returns an UNC path on both branches. What's different is that we only see a UNC path in the spawned Ruff process on main but not with Rust 1.88.

This is now a bit annoying to fix because downstream users might run the tests with both Rust 1.86 and 1.88. I think the easiest is to use dunce to avoid UNC paths in the first place

@MichaReiser MichaReiser merged commit 29927f2 into main Jun 28, 2025
36 checks passed
@MichaReiser MichaReiser deleted the micha/rust-1.88 branch June 28, 2025 18:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
internal An internal refactor or improvement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants