-
Notifications
You must be signed in to change notification settings - Fork 13.2k
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
Remove libuv, librustuv, gyp. #17465
Conversation
Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @nikomatsakis (or someone else) soon. |
process::ExitStatus(..) => {} | ||
process::ExitSignal(..) => fail!() | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How come this test was removed?
I know that @aturon is also planning on working in this area, so you should definitely sync up with him before moving ahead much farther. Also, servo is one of the main users of libgreen right now, and we need to coordinate with them in the removal of libgreen to make sure they know what's coming. Also, this is most certainly a |
Thanks for the PR! I've tried to reach @anchovieshat (and xales) on IRC, but to no avail. I would definitely like to coordinate on the effort here, as I am beginning implementation work on the runtime removal RFC this week. In particular, while Servo does plan to eventually remove all reliance on libgreen, this will take some time, and I don't want to cause unnecessary pain in the meantime. I was envisioning removing the libgreen crate as the last step in this process, based on my discussions with the Servo team. My plan was to start by migrating In any case, please reach out to me on IRC so that we can avoid duplicated or out-of-order work. |
}); | ||
rx.recv(); | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you retain these tests? They can run with a green::basic::event_loop
Thanks @anchovieshat! The tests in the standard library (ones based on |
@@ -41,11 +36,6 @@ pub mod runtest; | |||
pub mod common; | |||
pub mod errors; | |||
|
|||
#[start] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Have you been able to successfully run make check
with this setup? I'm not sure whether/why it was important for the compile tests to previously go through the green scheduler.
Overall this looks pretty good; I left a few additional questions about the tests and benchmarks. Has this successfully gotten through a My biggest concern is that there's not nearly enough documentation/warning about what's happening. The commit message needs to be significantly fleshed out to explain the change, reference the runtime removal RFC, and explain that we will eventually remove libgreen as well. Similarly in doc comments and guide modifications. |
This commit successfully goes through all of the tests in |
@anchovieshat Great! If you can add the docs and address some of the comments/questions about tests, then I think this will be ready to go. |
As per RFC 62 (https://github.com/rust-lang/rfcs/blob/master/active/0062-remove-runtime.md), libgreen is in the process of being removed from Rust. To begin this process, the I/O support in libuv and librustuv is being removed. libgreen will remain in a transitional state so that users of the library will have time to adapt to the removal. [breaking-change]
I'm wondering what the status is here? Have you had any luck removing |
@anchovieshat I'm going to close this PR in favor of a new one that takes care of Thanks for your work on this, and feel free to ping me if you'd like to stay involved with these efforts. |
Remove libuv, librustuv, and gyp in preparation for the removal of libgreen.