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

Redox Support Preview #37702

Merged
merged 41 commits into from Dec 15, 2016

Conversation

Projects
None yet
@jackpot51
Copy link
Contributor

jackpot51 commented Nov 11, 2016

Important - This is only a preview of a working sys::redox.

Compiling the Redox default distribution with this libstd results in a fully functioning distribution. As such, all further changes would be cosmetic or implementing features that have not been used by the default distribution (of which there are only a small number).

I do not expect this to be merged, but would like to discuss how it may be improved and get feedback.

There are a few unimplemented!() - cloexec for example. I have documented them below. These would be resolved before desiring a merge.

There are also issues with how the Redox syscall library is called - currently I am using a re-export in libc but that probably would not be desired.

@rust-highfive

This comment has been minimized.

Copy link
Collaborator

rust-highfive commented Nov 11, 2016

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @aturon (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.

@jackpot51

This comment has been minimized.

Copy link
Contributor Author

jackpot51 commented Nov 11, 2016

@alexcrichton @brson @eddyb I am mentioning you here because you have expressed interest in a Redox libstd port

Previous discussion here: https://internals.rust-lang.org/t/redox-support-in-liblibc-and-libstd/3954/17

@@ -34,7 +34,6 @@ pub mod condvar;
pub mod io;
pub mod memchr;
pub mod mutex;
pub mod net;

This comment has been minimized.

@jackpot51

jackpot51 Nov 11, 2016

Author Contributor

Because Redox does networking using files, and does not support the traditional Berkeley Socket API, we must specify an alternative net module, which is defined in sys::redox::net and imported down below

#[inline]
pub fn wait_timeout(&self, _mutex: &Mutex, _dur: Duration) -> bool {
::sys_common::util::dumb_print(format_args!("condvar wait_timeout\n"));
unimplemented!();

This comment has been minimized.

@jackpot51

jackpot51 Nov 11, 2016

Author Contributor

This will be implemented shortly with additional flags to futex


pub fn set_cloexec(&self) -> io::Result<()> {
::sys_common::util::dumb_print(format_args!("{}: set cloexec\n", self.fd));
unimplemented!();

This comment has been minimized.

@jackpot51

jackpot51 Nov 11, 2016

Author Contributor

This will be implemented shortly with an fcntl call


pub fn set_nonblocking(&self, _nonblocking: bool) -> io::Result<()> {
::sys_common::util::dumb_print(format_args!("{}: set nonblocking\n", self.fd));
unimplemented!();

This comment has been minimized.

@jackpot51

jackpot51 Nov 11, 2016

Author Contributor

This will be implemented shortly with an fcntl call


pub fn rename(_old: &Path, _new: &Path) -> io::Result<()> {
::sys_common::util::dumb_print(format_args!("Rename\n"));
unimplemented!();

This comment has been minimized.

@jackpot51

jackpot51 Nov 11, 2016

Author Contributor

This will require a rename syscall


pub fn set_perm(_p: &Path, _perm: FilePermissions) -> io::Result<()> {
::sys_common::util::dumb_print(format_args!("Set perm\n"));
unimplemented!();

This comment has been minimized.

@jackpot51

jackpot51 Nov 11, 2016

Author Contributor

This will require a chmod syscall


pub fn symlink(_src: &Path, _dst: &Path) -> io::Result<()> {
::sys_common::util::dumb_print(format_args!("Symlink\n"));
unimplemented!();

This comment has been minimized.

@jackpot51

jackpot51 Nov 11, 2016

Author Contributor

Symlinks are not supported on Redox at the moment

This comment has been minimized.

@nagisa

nagisa Nov 11, 2016

Contributor

In that case Err of some sort indicating that the operation is not supported should be returned (just like other OSes do when underlying filesystem does not support the operation).


pub fn link(_src: &Path, _dst: &Path) -> io::Result<()> {
::sys_common::util::dumb_print(format_args!("Link\n"));
unimplemented!();

This comment has been minimized.

@jackpot51

jackpot51 Nov 11, 2016

Author Contributor

Hard links are not supported on Redox at the moment

_p2: AnonPipe,
_v2: &mut Vec<u8>) -> io::Result<()> {
::sys_common::util::dumb_print(format_args!("read2\n"));
unimplemented!();

This comment has been minimized.

@jackpot51

jackpot51 Nov 11, 2016

Author Contributor

Due to using set_nonblocking, which is not supported until fcntl is added, this is not implemented yet

@@ -28,7 +28,7 @@
#![panic_runtime]
#![feature(panic_runtime)]
#![cfg_attr(unix, feature(libc))]
#![cfg_attr(windows, feature(core_intrinsics))]
#![cfg_attr(any(target_os = "redox", windows), feature(core_intrinsics))]

This comment has been minimized.

@jackpot51

jackpot51 Nov 11, 2016

Author Contributor

Redox has no libc function for abort, so using the intrinsic is required

@@ -69,6 +69,7 @@ mod imp;

// i686-pc-windows-gnu and all others
#[cfg(any(all(unix, not(target_os = "emscripten")),
target_os = "redox",

This comment has been minimized.

@jackpot51

jackpot51 Nov 11, 2016

Author Contributor

Redox could potentially use libunwind for backtraces. Right now, panic_abort is preferred.

@jackpot51

This comment has been minimized.

Copy link
Contributor Author

jackpot51 commented Dec 2, 2016

@alexcrichton Take, for instance, terminal auto completion. With Prefix the way it is in this patch, the autocompleter can correctly fill in paths that start with / like /bin/cat, relative paths like cat, as well as scheme-absolute paths like file:/bin/cat.

Without this patch, filling in file:/bin/cat will look for a folder named file: in the current directory.

Since scheme-absolute paths are not often used in this manner, I would be ok with putting off the decision until later - but it will introduce potential problems in some applications

@alexcrichton

This comment has been minimized.

Copy link
Member

alexcrichton commented Dec 2, 2016

Well, I suppose it was just an idea. If it's require to have a custom prefix for redox, I don't know how to solve that problem in a good way.

@brson

This comment has been minimized.

Copy link
Contributor

brson commented Dec 12, 2016

I think we're at an impasse for now on the prefix issue. Can we just remove prefixes from this patch to get the bulk of the work landed and circle back to the tough design question?

I'll go post a thread to irlo about expanding Prefix to raise awareness and see if anybody has bright ideas.

@jackpot51

This comment has been minimized.

Copy link
Contributor Author

jackpot51 commented Dec 12, 2016

@brson Sure

@jackpot51

This comment has been minimized.

Copy link
Contributor Author

jackpot51 commented Dec 12, 2016

@brson @alexcrichton The prefix handling has been removed. This must be readded in the future to avoid leaving in a potential security problem, but I would like to see a merge of Redox support before then as you have described.

@brson

This comment has been minimized.

Copy link
Contributor

brson commented Dec 12, 2016

@bors r+

Thanks for your enduring patience.

@bors

This comment has been minimized.

Copy link
Contributor

bors commented Dec 12, 2016

📌 Commit c61baa0 has been approved by brson

@jackpot51

This comment has been minimized.

Copy link
Contributor Author

jackpot51 commented Dec 12, 2016

Thanks @brson. I am going to work on adding the x86_64-unknown-redox target to rustc now.

jackpot51 added some commits Dec 12, 2016

@jackpot51

This comment has been minimized.

Copy link
Contributor Author

jackpot51 commented Dec 14, 2016

@brson, this job failed due to an issue fixed by #38340.

@brson

This comment has been minimized.

Copy link
Contributor

brson commented Dec 14, 2016

@bors r+

@bors

This comment has been minimized.

Copy link
Contributor

bors commented Dec 14, 2016

📌 Commit 3e15dc1 has been approved by brson

@jackpot51 jackpot51 force-pushed the redox-os:redox branch from cf87de9 to 3e15dc1 Dec 15, 2016

@eddyb

This comment has been minimized.

Copy link
Member

eddyb commented Dec 15, 2016

@bors r=brson

@bors

This comment has been minimized.

Copy link
Contributor

bors commented Dec 15, 2016

📌 Commit 3e15dc1 has been approved by brson

@jackpot51

This comment has been minimized.

Copy link
Contributor Author

jackpot51 commented Dec 15, 2016

thanks @eddyb

@bors

This comment has been minimized.

Copy link
Contributor

bors commented Dec 15, 2016

⌛️ Testing commit 3e15dc1 with merge cfa668f...

bors added a commit that referenced this pull request Dec 15, 2016

Auto merge of #37702 - redox-os:redox, r=brson
Redox Support Preview

# Important - This is only a preview of a working `sys::redox`.

Compiling the Redox default distribution with this `libstd` results in a fully functioning distribution. As such, all further changes would be cosmetic or implementing features that have not been used by the default distribution (of which there are only a small number).

I do not expect this to be merged, but would like to discuss how it may be improved and get feedback.

There are a few `unimplemented!()` - `cloexec` for example. I have documented them below. These would be resolved before desiring a merge.

There are also issues with how the Redox syscall library is called - currently I am using a re-export in `libc` but that probably would not be desired.

@bors bors merged commit 3e15dc1 into rust-lang:master Dec 15, 2016

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
homu Test successful
Details
@ticki

This comment has been minimized.

Copy link
Contributor

ticki commented Dec 15, 2016

woohoo!

@jackpot51 jackpot51 deleted the redox-os:redox branch Dec 15, 2016

frewsxcv added a commit to frewsxcv/rust that referenced this pull request Aug 23, 2017

Rollup merge of rust-lang#43983 - ids1024:redox-path-prefix, r=alexcr…
…ichton

Redox: correct is_absolute() and has_root()

This is awkward, but representing schemes properly in `Components` is not easily possible without breaking backwards compatibility, as discussed earlier in rust-lang#37702.

But these methods can be corrected anyway.
@RalfJung

This comment has been minimized.

Copy link
Member

RalfJung commented on b1b35dd Aug 6, 2018

@jackpot51
I am looking at Mutex::init here right now and wondering why it is not a NOP. Seems like The Redox Mutex is completely safe to use after just doing Mutex::new. Why does init write a 0?

This comment has been minimized.

Copy link
Contributor Author

jackpot51 replied Aug 6, 2018

You are probably right, it could be a noop... so long as a mutex is never recycled

This comment has been minimized.

Copy link
Member

RalfJung replied Aug 7, 2018

I don't think recycling is legal. POSIX docs say you must not initialize an already initialized mutex.

This comment has been minimized.

Copy link
Contributor Author

jackpot51 replied Aug 14, 2018

Sounds like it could be a no-op then

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.