-
Notifications
You must be signed in to change notification settings - Fork 13.8k
std: Add Motor OS std library port #147000
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
base: master
Are you sure you want to change the base?
Conversation
These commits modify the If this was unintentional then you should revert the changes before this PR is merged. The list of allowed third-party dependencies may have been modified! You must ensure that any new dependencies have compatible licenses before merging. |
Is there any notable std functionality that isn't supported on this platform? Just skimming it, it seems like things are pretty complete (threads, systemtime, process, etc) Nominating for libs as this adds a new std target. @rustbot label +I-libs-nominated |
I believe all major pieces are supported/implemented, including networking. Some specific things are not (yet) fully implemented, like FS links, or DNS lookup (at the moment, only "localhost" resolves to an IP address). On the other hand, tokio is ported via a mio port, which was not completely trivial... |
#[unstable(feature = "motor_ext", issue = "none")] | ||
pub fn map_motor_error(err: moto_rt::ErrorCode) -> crate::io::Error { |
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.
Probably doesn't need to be crate public?
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.
If a Rust program calls into moto-sys (the Motor OS kernel API) or moto-rt (the Motor OS runtime API), they get moto_rt::ErrorCode
. To convert it into io::Error (e.g. for Tokio) they need access to a similar function. So either the function should be exported here as pub
, or duplicated elsewhere, from where std can't import (due to io::Error being in std). If io::Error is moved to core (which I've seen being discussed), I'll move this function to moto-rt.
I prefer to avoid code duplication and just export it here. If it is better to duplicate it elsewhere, I can do that also, let me know.
#[cfg(not(test))] | ||
#[unsafe(no_mangle)] | ||
pub extern "C" fn motor_start() -> ! { |
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.
vroom vroom 🙂
moto_rt::process::exit(-1) | ||
} | ||
|
||
pub(crate) use crate::os::motor::map_motor_error; |
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.
Why not just define the function here? This is the module for cvt
on others
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.
I replied to the previous comment on this function. I can move it here and re-export in os::motor; or just move it here if you don't buy my argument re: why it is nice to have it exported. Please let me know.
} | ||
|
||
pub fn current_exe() -> io::Result<PathBuf> { | ||
Ok(crate::sys::args::args().next().unwrap().into()) |
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.
If you run say ls foo
on Motor OS, is the first argument ls
(like on Unix and Windows) or /path/to/ls
? If the former, this is not a correct implementation. std::env::current_exe()
is supposed to return an absolute path to the executable file.
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.
In many modern systems, even unices, there is often no global "absolute path", as programs run in their own jails/containers/whatever. Also just by looking at how other platforms deal with it, it is all over the place:
- aix just "infers" the location if args[0] is not an absolute path (which is buggy and thus worse, I'd say);
- openbsd does the same thing as Motor OS, only additionally calls
canonicalize()
if args[0] starts with a dot or contains a slash; if not, args[0] is returned unmodified; - same for vxworks.
So I'd say Motor OS is not unique here. I can add a call to canonicalize()
, if you insist, but I don't think it will make things better: I'd rather have an unmodified args[0] rather than have it massaged in some cases and not in others.
☔ The latest upstream changes (presumably #147019) made this pull request unmergeable. Please resolve the merge conflicts. |
As part of work to add stdlib support for Motor OS.
e2e23a1
to
f3884e2
Compare
This PR was rebased onto a different master commit. Here's a range-diff highlighting what actually changed. Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers. |
Motor OS was added as a no-std Tier-3 target in rust-lang#146848 as x86_64-unknown-motor. This patch/PR adds the std library for Motor OS. While the patch may seem large, all it does is proxy std pal calls to moto-rt. When there is some non-trivial code (e.g. thread::spawn), it is quite similar, often identical, to what other platforms do.
f3884e2
to
d8da6c0
Compare
Motor OS was added as a no-std Tier-3 target in
PR 146848 as x86_64-unknown-motor.
This PR adds the std library for Motor OS.
While the PR may seem large, all it does is proxy
std pal calls to moto-rt. Where there is some non-trivial
code (e.g. thread::spawn), it is quite similar, often
identical, to what other platforms do.