-
Notifications
You must be signed in to change notification settings - Fork 12.7k
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
redox: convert to target_family unix #60547
Conversation
r? @shepmaster (rust_highfive has picked a reviewer for you, use r? to override) |
So once try is green here, we can merge the libc PR upstream, and update rust-lang/rust libc. IIRC, |
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.
Looks reasonable to me! I don't have an opinion one way or another on what target_family it's considered under, but for the existing sys/unix
code the primary goal I think is to absolute reduce the amount of #[cfg]
as much as possible. It's needed in some cases for sure, but each additional #[cfg]
tends to add more complexity we have to manage in the future...
@gnzlbg @mati865 @alexcrichton thanks for the review, I will be updating to address your comments soon |
It has been a pretty long time that we have had to carry a forked version of Rust along with a complicated build process in the Redox repository due to this one change (#52331) not being present in upstream. What is the best way to move forward on it? I am open to anything that would make a path like "file:/home" parse into components like this: Prefix("file:") It can require a feature flag to match on the Redox-specific prefix and only be available on Redox, whatever it takes to make checks for a prefix return true. |
The fact that Windows-specific API components leaked into the cross-platform public part of the The solution proposed on that issue sounds like it would work quite well, and we could discuss in the libs team as well if we want to be more aggressive about deprecation warnings to change the Windows idioms there as well. |
For now, I will remove the Prefix change. I hope I can address it in a way everyone is happy with soon. |
Ok, the comments from @alexcrichton and @mati865 should now be addressed. I also rebased on master. |
The job Click to expand the log.
I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
The job Click to expand the log.
I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
The job Click to expand the log.
I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
The job Click to expand the log.
I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
The job Click to expand the log.
I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
8850362
to
46eddf7
Compare
I got the build mostly under control. The last error was this:
Which does not appear to be caused by any changes I made |
Ok @gnzlbg, it builds for Linux at least. I have rebased everything so I can revert to only the first commit when the libc patch lands |
@bors: try |
@joelpalmer, this is still waiting on #61393. I will rebase when that or something similar is merged |
I have rebased this PR, waiting on review from @alexcrichton |
The job Click to expand the log.
I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
@bors: r+ |
📌 Commit ebb648d has been approved by |
redox: convert to target_family unix This is the second step to supporting #60139. In order to have a smooth transition, there will need to be a change made in liblibc at the same time, switching Redox over to the unix target family. See rust-lang/libc#1332
☀️ Test successful - checks-azure |
This is the second step to supporting #60139.
In order to have a smooth transition, there will need to be a change made in liblibc at the same time, switching Redox over to the unix target family. See rust-lang/libc#1332