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

Use an empty vendored file system in Ruff #13436

Merged
merged 2 commits into from
Sep 21, 2024

Conversation

MichaReiser
Copy link
Member

@MichaReiser MichaReiser commented Sep 21, 2024

Summary

This PR changes removes the typeshed stubs from the vendored file system shipped with ruff
and instead ships an empty "typeshed".

Making the typeshed files optional required extracting the typshed files into a new ruff_vendored crate. I do like this even if all our builds always include typeshed because it means red_knot_python_semantic contains less code that needs compiling.

This also allows us to use deflate because the compression algorithm doesn't matter for an archive containing a single, empty file.

Test Plan

cargo test

I verified with cargo tree -f "{p} {f}" -p <package> that:

  • red_knot_wasm: enables deflate compression
  • red_knot: enables zstd compression
  • ruff: uses stored

I'm not quiet sure how to build the binary that maturin builds but comparing the release artifact size with strip = true shows a 1.5MB size reduction

@MichaReiser MichaReiser force-pushed the micha/ruff-empty-vendored-fs branch 2 times, most recently from 5fc44df to 551ed4d Compare September 21, 2024 12:50
@MichaReiser
Copy link
Member Author

@charliermarsh this is a potential short term fix for the conda forge build

@MichaReiser MichaReiser force-pushed the micha/ruff-empty-vendored-fs branch 2 times, most recently from 99ecfef to eebff71 Compare September 21, 2024 13:13
Copy link
Contributor

github-actions bot commented Sep 21, 2024

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.

@MichaReiser MichaReiser marked this pull request as ready for review September 21, 2024 13:42
@charliermarsh
Copy link
Member

This appears to be working: conda-forge/ruff-feedstock#220

@charliermarsh
Copy link
Member

I'm happy to release it today if you want

@MichaReiser MichaReiser changed the base branch from main to micha/use-vendored-fs-from-db September 21, 2024 14:08
@MichaReiser
Copy link
Member Author

Nice. Could you try running the new analyze method over a real project? Just to make sure we don't show annoying warnings when the versions file is empty 😆

Base automatically changed from micha/use-vendored-fs-from-db to main September 21, 2024 14:35
An error occurred while trying to automatically change base from micha/use-vendored-fs-from-db to main September 21, 2024 14:35
@MichaReiser MichaReiser force-pushed the micha/ruff-empty-vendored-fs branch from eebff71 to 23b3b0f Compare September 21, 2024 14:37
@MichaReiser
Copy link
Member Author

MichaReiser commented Sep 21, 2024

But yeah. Feel free to merge and release the changes to unblock conda forge.

@MichaReiser MichaReiser added the internal An internal refactor or improvement label Sep 21, 2024
@MichaReiser
Copy link
Member Author

For changelog: Fix conda forge build.

@MichaReiser
Copy link
Member Author

This appears to be working: conda-forge/ruff-feedstock#220

I love your confidence 😆

Copy link
Member

Choose a reason for hiding this comment

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

This file shouldn't be here

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks. I don't understand why it sometimes creates those files but doesn't delete them when the test is done.

@charliermarsh
Copy link
Member

Just ran, no warnings :)

@charliermarsh charliermarsh enabled auto-merge (squash) September 21, 2024 16:27
@charliermarsh charliermarsh merged commit 653c090 into main Sep 21, 2024
18 checks passed
@charliermarsh charliermarsh deleted the micha/ruff-empty-vendored-fs branch September 21, 2024 16:31
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.

3 participants