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 stack_t on FreeBSD #1222

Merged
merged 1 commit into from Jan 23, 2019
Merged

fix stack_t on FreeBSD #1222

merged 1 commit into from Jan 23, 2019

Conversation

ghost
Copy link

@ghost ghost commented Jan 21, 2019

FreeBSD 10.x is EOL, in FreeBSD 11 and later, ss_sp is actually a void* [1]
dragonflybsd still uses c_char [2]

[1] https://svnweb.freebsd.org/base/releng/11.2/sys/sys/signal.h?revision=334459&view=markup#l438
[2] https://github.com/DragonFlyBSD/DragonFlyBSD/blob/master/sys/sys/signal.h#L339

should be committed with rust-lang/rust#57810

@rust-highfive
Copy link

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @alexcrichton (or someone else) soon.

If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes.

Please see the contribution instructions for more information.

@ghost ghost changed the title FreeBSD 10.x is EOL, in FreeBSD 11 and later, ss_sp is actually a voi… fix stack_t on FreeBSD Jan 22, 2019
@alexcrichton
Copy link
Member

@gnzlbg would you be willing to spearhead landing this? This is a breaking change so it'll take some care in landing

@gnzlbg
Copy link
Contributor

gnzlbg commented Jan 22, 2019

@alexcrichton it looks like we just have to land this and then land the PR usptream right ?

I'd expect breakage from this to be minimal, so I'd say its acceptable (we can always do a release and yank the version if it breaks too much code).

In any case, we don't really have to do this. ABI-wise a pointer is a pointer, so this just makes the API on the libc side nicer (I'd guess the nix crate could do this fix without breaking the world).

@alexcrichton
Copy link
Member

Yes functionally changes just need to land, the hazard is dealing with breakage. If you'd like to land this and deal with possible fallout I'm ok with that.

@gnzlbg
Copy link
Contributor

gnzlbg commented Jan 23, 2019

So I'm going to land this in the libc repo, do a new release, and wait a bit to see if somebody complains - if this breaks something maybe we can fix that with minimal impact. If all goes smoothly, we should update the upstream PR to contain this change and land it there. If this causes to much breakage, I'll just yank the released libc and revert this.

AFAICT doing any sort of crater runs for this change is pointless, because the crater runs won't cover these architectures, so we don't really have a way to try this change before we land it.

@bors: r+

@bors
Copy link
Contributor

bors commented Jan 23, 2019

📌 Commit 5e18756 has been approved by gnzlbg

bors added a commit that referenced this pull request Jan 23, 2019
fix stack_t on FreeBSD

FreeBSD 10.x is EOL, in FreeBSD 11 and later, ss_sp is actually a void* [1]
dragonflybsd still uses c_char [2]

[1] https://svnweb.freebsd.org/base/releng/11.2/sys/sys/signal.h?revision=334459&view=markup#l438
[2] https://github.com/DragonFlyBSD/DragonFlyBSD/blob/master/sys/sys/signal.h#L339

should be committed with rust-lang/rust#57810
@bors
Copy link
Contributor

bors commented Jan 23, 2019

⌛ Testing commit 5e18756 with merge e979fff...

@bors
Copy link
Contributor

bors commented Jan 23, 2019

☀️ Test successful - checks-cirrus, checks-travis, status-appveyor
Approved by: gnzlbg
Pushing e979fff to master...

@bors bors merged commit 5e18756 into rust-lang:master Jan 23, 2019
@gnzlbg gnzlbg mentioned this pull request Jan 23, 2019
bors added a commit that referenced this pull request Jan 23, 2019
Bump to 0.2.48

Bumps libc version to 0.2.48. This release contains the breaking changes performed in #1222 .
bors added a commit that referenced this pull request Jan 23, 2019
Bump to 0.2.48

Bumps libc version to 0.2.48. This release contains the breaking changes performed in #1222 .
@ghost ghost deleted the stack_t_bsd branch February 7, 2019 10:42
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.

4 participants