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

1.0 release planning #753

Open
18 of 21 tasks
sunfishcode opened this issue Jul 19, 2023 · 21 comments
Open
18 of 21 tasks

1.0 release planning #753

sunfishcode opened this issue Jul 19, 2023 · 21 comments
Labels
semver bump Issues that will require a semver-incompatible fix

Comments

@sunfishcode
Copy link
Member

sunfishcode commented Jul 19, 2023

Here's a planning issue for a 1.0 release, similar to the one for the 0.38 release. Hopefully we won't need a semver bump for a good long while, but when we do, here's the list of changes we can make:

Deferring for now:

  • move rustix::thread sleep functions into a new rustix::sleep module, because they're usable from single-threaded programs too
  • Move Dir into its own crate, and have rustix export getdents directly?
  • Split open and openat into separate functions for "create" which needs a mode vs. "open existing" which doesn't need a mode?
  • Make RecvAncillaryBuffer::drain always remove all messages, even if the iterator isn't exhausted. With that, we can remove the read_and_length in AncillaryDrain.
  • Replace rustix::cstr! with something like rustix::nativestr! so that users don't hard code "native strings are C strings" in their code?
  • rename Arg::as_str to avoid "a method with this name may be added to the standard library in the future" warnings
  • rustix::process::waitpid has no way to wait for specific process groups (Pid now requires non-negative values). waitpgid has been added as a temporary workaround.
  • remove libc from the public API (Port 1.0-staging to main #1152) (Remove most linux-raw-sys types from the public API #1277)

Don't do these:

  • set_ip_add_membership/get_ip_add_membership should take address and ifindex arguments and *_with_ifindex should be removed.
  • Rename ClockId::Uptime on FreeBSD to Boottime, since it's an alias
@notgull

This comment was marked as resolved.

@sunfishcode sunfishcode added the semver bump Issues that will require a semver-incompatible fix label Sep 29, 2023
@sunfishcode

This comment was marked as resolved.

@sunfishcode sunfishcode changed the title 0.39 release planning 1.0 release planning Oct 22, 2023
@sunfishcode

This comment was marked as resolved.

@notgull
Copy link
Contributor

notgull commented Oct 22, 2023

If this is going to be a stable release, we should make sure that libc and linux-raw-sys aren't exposed in the public API. I know there are still a few places where it is.

In my opinion a stable release is a bad idea, as the API is still somewhat volatile. There should at least be an "unstable" feature flag to test new APIs for inclusion, like Tokio has.

@sunfishcode
Copy link
Member Author

I already consider 0.38 to be stable. For example, I maintain an older version of linux-raw-sys to preserve compatibility with rustix 0.38. And I'm not aware of any significant volatility in the 0.38 API. It's added lots of features, and even deprecated a handful of things, but ideas for breaking changes are collected here in this issue, specifically to keep 0.38 stable.

So my idea for 1.0 is just to better communicate what's already happening.

It would be nice to better encapsulate libc/linux-raw-sys types, and I'm open to adding an "unstable" feature flag if there's a need for it. I'm under the impression that these aren't urgent right now, though I'd be happy to learn more about how these impact users.

@SUPERCILEX

This comment was marked as resolved.

@morr0ne

This comment was marked as resolved.

@sunfishcode

This comment was marked as resolved.

@SUPERCILEX
Copy link
Contributor

I'd like to propose making the timeout param in https://docs.rs/rustix/latest/rustix/event/fn.poll.html be an Option<u32> where None passes -1 to act as an infinite timeout. That feels like a clearer interface than anything negative is infinity.

@SUPERCILEX

This comment was marked as resolved.

@sunfishcode

This comment was marked as resolved.

@SUPERCILEX

This comment has been minimized.

@SUPERCILEX
Copy link
Contributor

From #1110 (comment):
epoll appears to be inconsistent in that it expects users to use the module prefix (i.e. epoll::foo) instead of using the free function directly. This is neat, but it's inconsistent with the rest of rustix's APIs: inotify, port, eventfd, io_uring, mount, shm, and probably others I didn't have time to look at. @sunfishcode thoughts on a PR that restores the epoll_ prefix with some deprecations? We could go the other direction, but that interacts poorly with auto-complete and would be much more breaking so not worth it IMO.

Sidenote: the deprecations won't actually work due to rust-lang/rust#30827. (For example the fs::*mount APIs are deprecated but there's no warning about it.)

@sunfishcode

This comment has been minimized.

@raftario

This comment has been minimized.

@sunfishcode

This comment has been minimized.

@sunfishcode

This comment has been minimized.

@sunfishcode
Copy link
Member Author

All todo items here are now either fixed, have posted PRs, or are tentatively recategorized to "Defering for now". I'll give these some time, while I work on documenting the changes in CHANGELOG.md and do extra testing and tidying, but the end is now in sight.

From #1105 (comment), let's add to the todo list that we need to fix the occurrences of cargo clippy --workspace --all-targets --features all-apis -- -Aclippy::all -Dclippy::impl_trait_in_params (and use-libc).

A PR doing this for all public APIs is posted at #1283.

From #1110 (comment): epoll appears to be inconsistent in that it expects users to use the module prefix (i.e. epoll::foo) instead of using the free function directly. This is neat, but it's inconsistent with the rest of rustix's APIs: inotify, port, eventfd, io_uring, mount, shm, and probably others I didn't have time to look at. @sunfishcode thoughts on a PR that restores the epoll_ prefix with some deprecations? We could go the other direction, but that interacts poorly with auto-complete and would be much more breaking so not worth it IMO.

In my experience the autocomplete is ok once one has the appropriate use of the API module in scope.

I've now filed #1284 to use this style in the port API, and I've ensured that all modules using this style have examples showing the appropriate use, and doc aliases.

eventfd is just one function and one type so I don't think it needs a pub mod. That leaves io_uring and mount, which are a little harder to fit into this style. mount::un feels weird, IoringSendFlags would clash with SendFlags without its prefix, and subjectively they don't seem to fit this style quite as well.

I'd like to propose making the timeout param in https://docs.rs/rustix/latest/rustix/event/fn.poll.html be an Option<u32> where None passes -1 to act as an infinite timeout. That feels like a clearer interface than anything negative is infinity.

I haven't yet made this change. I do like how it makes the interface clearer, but I'm unsure about the range check it introduces.

If this is going to be a stable release, we should make sure that libc and linux-raw-sys aren't exposed in the public API. I know there are still a few places where it is.

With #1152 now landed, #1277 and #1282 remove the last linux-raw-sys types from the public API.

I haven't yet removed all the libc types when using the libc backend. That would require duplicating a lot of platform-specific types like stat which gratuitously varies in all kinds of details across architectures and OS's. Perhaps just avoiding linux-raw-sys is good enough for now, because the libc crate doesn't have semver changes nearly as often.

None of this is final at this point; I'm open to feedback here.

@SUPERCILEX
Copy link
Contributor

I haven't yet made this change. I do like how it makes the interface clearer, but I'm unsure about the range check it introduces.

By this do you mean that we're limiting possible inputs the caller can supply (namely no negative numbers). I feel like that's reasonable as we probably want a different API if negative numbers start meaning something else.


I'd really like to see the story around MaybeUninit buffers resolved for 1.0 (or a clear expectation that there will be a 2.0 addressing the issue). I'm still on team dependent types (as in the input to the function determines the return type), but I'm not sure how we should hash this out.

@sunfishcode
Copy link
Member Author

sunfishcode commented Jan 23, 2025

I haven't yet made this change. I do like how it makes the interface clearer, but I'm unsure about the range check it introduces.

By this do you mean that we're limiting possible inputs the caller can supply (namely no negative numbers). I feel like that's reasonable as we probably want a different API if negative numbers start meaning something else.

If it's Option<u32>, then the u32's range includes values greater than poll's c_int supports. Maybe it means we should just go all the way to Option<Timespec> and use ppoll in the implementation when we can (Linux, most BSDs).

I'd really like to see the story around MaybeUninit buffers resolved for 1.0 (or a clear expectation that there will be a 2.0 addressing the issue). I'm still on team dependent types (as in the input to the function determines the return type), but I'm not sure how we should hash this out.

Is that #1108 and #1110? Thanks for the reminder; I'll take another look at those before the release.

@SUPERCILEX
Copy link
Contributor

we should just go all the way to Option and use ppoll

If we can, that sounds even better!


Yup, mostly #1110 as I think it could have ripple effects (e.g. getting rid of io::read_uninit).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
semver bump Issues that will require a semver-incompatible fix
Projects
None yet
Development

No branches or pull requests

5 participants