-
Notifications
You must be signed in to change notification settings - Fork 12.8k
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
added support for GNU/Hurd #115230
added support for GNU/Hurd #115230
Conversation
Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @joshtriplett (or someone else) soon. Please see the contribution instructions for more information. Namely, in order to ensure the minimum review times lag, PR authors and assigned reviewers should ensure that the review label (
|
These commits modify compiler targets. |
This comment has been minimized.
This comment has been minimized.
def87fe
to
a62e97f
Compare
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.
Could you address the requirements of the target tier policy in the PR description (Tier 3 in your case). As well as in the src/doc/rustc/src/platform-support.md. Example here.
Also, you may want to split your PR in two: the first adding the compiler target and the second adding support for the Standard Library.
r? compiler
@@ -133,7 +133,7 @@ const EXTRA_CHECK_CFGS: &[(Option<Mode>, &'static str, Option<&[&'static str]>)] | |||
// #[cfg(bootstrap)] | |||
(Some(Mode::Std), "target_vendor", Some(&["unikraft"])), | |||
(Some(Mode::Std), "target_env", Some(&["libnx"])), | |||
(Some(Mode::Std), "target_os", Some(&["teeos"])), | |||
(Some(Mode::Std), "target_os", Some(&["teeos", "hurd"])), |
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.
(Some(Mode::Std), "target_os", Some(&["teeos", "hurd"])), | |
// #[cfg(bootstrap)] hurd | |
(Some(Mode::Std), "target_os", Some(&["teeos", "hurd"])), |
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.
right, done so!
This comment has been minimized.
This comment has been minimized.
84130e6
to
b8f3604
Compare
This comment has been minimized.
This comment has been minimized.
b8f3604
to
cc454b5
Compare
This comment has been minimized.
This comment has been minimized.
cc454b5
to
4c48e84
Compare
We added myself as target maintainer.
We used the same target named as used in llvm: i686-unknown-hurd-gnu
GNU tools use i686-unknown-gnu and that indeed sometimes bring confusion, that's why we used the llvm triplet.
That's alright here.
The target legal terms are basically like the Linux ecosystem.
Agreed.
GNU/Hurd uses the GNU toolchain, like Linux.
The whole point of the GNU project is exactly that :)
This is the same situation as linux, since we use the GNU toolchain and libc.
That is also the kind of goals of the GNU project :)
We do have core/alloc/std.
The documentation was added. Running tests can be done in a qemu VM for instance, pre-built installed images are available.
Ok, sure!
We believe our additions didn't break anything, and we will watch CI anyway.
Sure!
Ok! |
This comment has been minimized.
This comment has been minimized.
4c48e84
to
2cfa544
Compare
This comment has been minimized.
This comment has been minimized.
2cfa544
to
3918e18
Compare
This comment has been minimized.
This comment has been minimized.
3918e18
to
e704fdb
Compare
This comment has been minimized.
This comment has been minimized.
e704fdb
to
fc585c2
Compare
💔 Test failed - checks-actions |
This comment has been minimized.
This comment has been minimized.
? How can that happen? Isn't this
supposed to make |
Or do we need to first add the declaration in |
It doesn't make sense to me why we would get that error if CI already compiles successfully. Looking at how the builtin condition values for cfg are constructed all the values for @bors retry |
@@ -133,7 +133,8 @@ const EXTRA_CHECK_CFGS: &[(Option<Mode>, &str, Option<&[&'static str]>)] = &[ | |||
// #[cfg(bootstrap)] | |||
(Some(Mode::Std), "target_vendor", Some(&["unikraft"])), | |||
(Some(Mode::Std), "target_env", Some(&["libnx"])), | |||
(Some(Mode::Std), "target_os", Some(&["teeos"])), | |||
// #[cfg(bootstrap)] hurd | |||
(Some(Mode::Std), "target_os", Some(&["teeos", "hurd"])), |
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.
You need one for Mode::Rustc
too if you want to use it in rustc.
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.
Ok, done so, thanks!
3541306
to
dcea770
Compare
@bors r=b-naber |
☀️ Test successful - checks-actions |
Finished benchmarking commit (f73d376): comparison URL. Overall result: ✅ improvements - no action needed@rustbot label: -perf-regression Instruction countThis is a highly reliable metric that was used to determine the overall result at the top of this comment.
Max RSS (memory usage)ResultsThis is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.
CyclesThis benchmark run did not return any relevant results for this metric. Binary sizeThis benchmark run did not return any relevant results for this metric. Bootstrap: 633.468s -> 632.84s (-0.10%) |
adding support for i686-unknown-hurd-gnu