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

fix: Update rustix to fix the enable_raw_mode() error on WSL/Android #926

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

sxyazi
Copy link

@sxyazi sxyazi commented Sep 6, 2024

Fixes #912

Related upstream PR: bytecodealliance/rustix#1147


Should we limit rustix to just the mirror version, similar to what libc does, so that upstream fixes will automatically apply in the future?

libc = { version = "0.2", default-features = false, optional = true }

Also if possible, could we release a patch version of crossterm that includes this fix?

@joshka
Copy link
Collaborator

joshka commented Sep 6, 2024

Should we limit rustix to just the mirror version, similar to what libc does, so that upstream fixes will automatically apply in the future?

Being explicit about versions like this means that when cargo does dependency resolution, it will not resolve a set of versions that does not work. version = "0.38.36" means any version of 0.38.y where y > = 36. I'd suggest not moving to using version = "0.38" because 0.38.35 is specifically broken and should not be used.

Apps that don't specifically lock the rustix version can just run cargo update to get the updated version of rustix even without this change, so this PR is fine to merge and then release when it makes sense.

Copy link
Collaborator

@joshka joshka left a comment

Choose a reason for hiding this comment

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

LGTM

@sxyazi
Copy link
Author

sxyazi commented Sep 7, 2024

it will not resolve a set of versions that does not work. version = "0.38.36" means any version of 0.38.y where y > = 36

TIL about this, it makes total sense. Thanks for the excellent explanation!

@joshka
Copy link
Collaborator

joshka commented Sep 7, 2024

@TimonPost this is a simple version bump and is pretty safe to merge.

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.

enable_raw_mode() error in version 0.28.0 under WSL and Android
2 participants