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

Breaking Changes from version 0.2.55 to 0.2.58 #1397

Closed
JoelScarfone opened this issue Jun 13, 2019 · 7 comments
Closed

Breaking Changes from version 0.2.55 to 0.2.58 #1397

JoelScarfone opened this issue Jun 13, 2019 · 7 comments

Comments

@JoelScarfone
Copy link

Two recent breaking changes:

libc::RLIMIT_AS is a u32 on 0.2.58, changed from an i32 on earlier versions.

libc::setrlimit takes in a u32 as its resource parameter on 0.2.56, changed from an i32 on earlier versions.

Both of these changes involve moving from using a libc::c_int (i32) to a libc::__rlimit_resource_t (u32). I found these breaking changes when upgrading from 0.2.55 to 0.2.58. Just looking to see that this is expected or not.

@gnzlbg
Copy link
Contributor

gnzlbg commented Jun 13, 2019

This is expected. If you are passing the libc::RLIMIT_... constants to functions accepting those constants (like libc::setrlimit), you shouldn't have seen any errors, but if you were calling these with your own constants (e.g. defined with a different type), then these errors are expected, and you can migrate your code by adding as _ to the code, e.g., libc::setrlimit(my_const as _).

Could you let us know a bit more about what you were doing ?

@JoelScarfone
Copy link
Author

I had a function that wrapped getting the resource I wanted to set the resource limit on, that function returned a libc::c_int that i passed to setrlimit, hence you can see where it broke as in that function i was expecting a libc::c_int return value on all variants, as well as calling setrlimit with a libc::c_int.

@gnzlbg
Copy link
Contributor

gnzlbg commented Jun 13, 2019

Damn yes, that use case was expected breakage. The best thing you can do is have a type rlimit_t = ...; type alias, and use #[cfg(...)] to set the appropriate type, depending on the target.

@gnzlbg
Copy link
Contributor

gnzlbg commented Jun 13, 2019

Ideally, at some point in the future, the nix crate would expose a portable and safe API for performing the operations that you need.

@JoelScarfone
Copy link
Author

All good, thanks for a hasty response.

@gnzlbg
Copy link
Contributor

gnzlbg commented Jun 13, 2019

Yes, sorry about the breakage. The tests that verified the rlimit APIs were disabled for some reason, and re-enabling them discovered that the types were incorrect in some targets. Sadly, we can't use the crater tool with libc yet to test whether some breaking change actually breaks somebody, so I decided to roll in the fix, and wait and see if the changes actually break somebody (which it did, it broke your code).

I'm not sure whether we should revert the change and yank the released versions, or leave it as is. You are the only user that has reported back about this, yet, but that doesn't mean that more users aren't affected.

How bad is it to fix this on your end ? (e.g. is it possible at all? is it too painful? )

@JoelScarfone
Copy link
Author

It is already fixed and in master on my end, I wouldn't revert; Especially if i'm the only one who brought it up (and since reverting would be another breaking change :D).

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

No branches or pull requests

2 participants