-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
add definitions for s390x musl targets #2071
Conversation
Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @Amanieu (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. |
CI is mostly blocking on rust-lang/rust#82166 being merged, which adds the s390x-unknown-linux-musl target. |
LGTM. Let's wait on rust-lang/rust#82166 so we can have CI set up. I'm not too familiar with the libc CI, but basically just copy what we're already doing for other architectures on musl (e.g. mips, arm). The relevant files are all in |
Yeah that's why I decided to go with #82166 first. Then handle the libc bits and any std bits afterward. |
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.
These all are destined to go into fn ioctl which is expecting a c_int.
Based on conversation in Zulip, I wonder if it makes sense to move a lot of these constants to the common part of the musl support code. |
Yes, I think so, that's kind of how the libc crate is "supposed" to be designed (according to https://github.com/rust-lang/libc/blob/master/CONTRIBUTING.md anyways!). Probably using a similar structure as the "generic bits" vs. "arch-specific bits" pattern that musl uses for its own organization. |
Co-authored-by: Jubilee <46493976+workingjubilee@users.noreply.github.com>
…constants Co-authored-by: Jubilee <46493976+workingjubilee@users.noreply.github.com>
@Amanieu this should be good to go now. rust#82166 has landed. |
Is ci/s390x-linux-musl.json still needed? |
git, how does it even work? |
You need to add the target to .github/workflows/bors.yml so it gets picked up by CI. |
@bors r+ |
📌 Commit f230862 has been approved by |
add definitions for s390x musl targets Add support for s390x musl targets to libc. I haven't added CI because I am not familiar with the pipelines, but would be glad to do so if somebody outlines what needs to be done.
💔 Test failed - checks-actions |
oh, technically the s390x-unknown-linux-musl target is tier 3 atm. can we just skip the CI? or how do you want to tackle it? |
Ah good point, let's just skip it for now. |
@bors r=Amanieu |
📌 Commit 41cda2a has been approved by |
☀️ Test successful - checks-actions, checks-cirrus-freebsd-11, checks-cirrus-freebsd-12, checks-cirrus-freebsd-13 |
@bors r+ |
Add support for s390x musl targets to libc.
I haven't added CI because I am not familiar with the pipelines, but would be glad to do so if somebody outlines what needs to be done.