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

Fuchsia's target_family should be None, not "unix" #58590

Open
kulakowski opened this issue Feb 20, 2019 · 23 comments
Open

Fuchsia's target_family should be None, not "unix" #58590

kulakowski opened this issue Feb 20, 2019 · 23 comments
Labels
O-fuchsia Operating system: Fuchsia T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@kulakowski
Copy link
Contributor

Currently, the Fuchsia target is part of the unix target_family. I'd like to suggest that this should not be the case, and that #cfg[unix] should be false for Fuchsia.

First, I want to capture what I think the opposite case is. Fuchsia does support some amount of posix functionality. That subset is currently informal and pragmatic. It has made starting to port exist software a certain amount easier.

"unix" connotes a lot of things. I've singled out some big ones that do not apply to Fuchsia. I think that in aggregate, these outweigh the benefits mentioned above.

Process model: Fuchsia does not have fork and exec, and does not have a process hierarchy. There's no wait(2) or waitpid(2) on Fuchsia.

Signals: Fuchsia does not have unix signals.

Filesystems and users: Fuchsia does not have unix users or groups, and does not implement unix filesystem permissions. Fuchsia does not have a global filesystem.

FDs: Files and file descriptors are central, primitive concepts for unix: "everything is a file". In Fuchsia, they are an abstraction built out of other primitives, and working with those primitives directly is often preferred.

C and ABI: Unix system ABIs are typically deeply intertwined with its C standard library, and C is a de facto standard for specifying ABIs (witness repr(C)). On Fuchsia, almost all ABIs are specified in a language-agnostic IDL. The biggest exception, the system ABI in, we have been careful to describe in terms of ELF dynamic linkage, rather than C per se.

IO: Portable unix IO boils down to synchronous read(2) and write(2). Abstractions for event-driven programming exist, but are not portable. Fuchsia has limited emulation for some of them, preferring instead to use native constructs more directly.

In aggregate, I think defining #cfg[unix] to be true for Fuchsia is tempting, yet a trap. An existing small program which just wants to synchronously manipulate stdin and stdout seems to benefit, for example, until they want to handle ^C with their same unix code.

I'd love for rust programs, existing or not, to be as good as possible as easily as possible when built for Fuchsia. I think not setting #cfg(unix) will help with that.

For some background: I work on Fuchsia. Among other things, I am one of the maintainers for our libc.

cc @cramertj

@jonas-schievink jonas-schievink added the O-fuchsia Operating system: Fuchsia label Feb 20, 2019
@Centril Centril added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Feb 20, 2019
@nagisa
Copy link
Member

nagisa commented Feb 20, 2019

#[cfg(unix)] was historically defined as #[cfg(not(windows))] so that having two configurations of #[cfg(windows)] and #[cfg(unix)] would "technically" cover all the targets.

While the name is misnomer, making Fuchsia neither #[cfg(unix)] nor #[cfg(windows)] may cause more harm than good.

@sfackler
Copy link
Member

Is Redox a unix? That's the other "weird" OS we support IIRC.

@abarth
Copy link

abarth commented Feb 20, 2019

While the name is misnomer, making Fuchsia neither #[cfg(unix)] nor #[cfg(windows)] may cause more harm than good.

That approach would seem limiting. For example, Rust might want to support a future operating system that is quite different from both Unix and Windows some day.

@kulakowski
Copy link
Contributor Author

Redox has no target_family:

target_family: None,

@sanxiyn
Copy link
Member

sanxiyn commented Feb 28, 2019

I think the case of Redox is a good argument for making Fuchsia "not unix". It apparently works.

How do we proceed now?

@ghost

This comment has been minimized.

@sanxiyn

This comment has been minimized.

@spacejam
Copy link

this one has bitten code I've written, as sled tests used to gate some libc calls (fork + kill 9 for crash testing) on cfg(target_family = "unix"), which caused an interesting dive into the fuchsia architecture and rust support when I learned that sled ran on fuchsia but failed to build the tests.

@kulakowski
Copy link
Contributor Author

My specific suggestion is to change target_family from Some("unix") to None. This is what the redox configuration does as well.

@ghost

This comment has been minimized.

@cramertj cramertj changed the title Fuchsia is not unix Fuchsia's target_family should be None, not "unix" Feb 28, 2019
@vi
Copy link
Contributor

vi commented Apr 21, 2019

Meanwhile redox: convert to target_family unix.

@anp
Copy link
Member

anp commented May 4, 2021

An example just cropped up of another mismatch here: Fuchsia doesn't provide a chroot.

@nagisa
Copy link
Member

nagisa commented May 4, 2021

The historical definition that we had before no longer holds. We have plenty of targets that are neither unix nor windows nowadays. target_family also recently changed to have a broader definition in rust-lang/reference#1006. So it seems to me like if the maintainers of this target wish to not say fuchsia is a unix then its perfectly fine to make it not unix.

@ijackson
Copy link
Contributor

Another way that Rust thinking Fuchsia is a kind of Unix is causing trouble:

Its ExitStatus is nothing like a Unix wait status, but still impl unix ExitStatusExt even though the methods make no sense (and the exit status value doesn't fit): 05a88aa#diff-419f54722c391977948bbd36705ade83c33b4f5bea47cd5b02003ed88732eafcR249

i would like to stabilise #![feature(unix_process_wait_more)] (#79982) and this is the only significant open question. Would it be OK to stabilise that bodge on Fuchsia, with the expectation that this will be fixed by withdrawing or deprecating this API on Fuchsia?

Is the into_raw bodge there the best bodge? Would it be better to return the raw Fuchsia code even though it's nothing like the wait status that ExitStatusExt::into_raw usually returns?

For background, see the stdlib docs for unix ExitStatusExt

@kulakowski
Copy link
Contributor Author

This comment reminded me that I filed this bug a long time ago :) I no longer work on Fuchsia and wanted to mention that here. I believe @anp is an appropriate point of contact now.

@anp
Copy link
Member

anp commented May 13, 2021

Thanks @kulakowski! There are two things we're doing on the Fuchsia side here:

  1. Holding some internal discussions right now about what library surface we'd like libstd to use (it currently uses a mix of libc, our system calls, and another support library that's used to implement our libc but isn't itself "stable").
  2. Assessing the ecosystem fallout of making cfg(unix) evaluate to false for Fuchsia. There are a few crates like libc which I think might need to have #[cfg(unix)] -> #[cfg(any(unix, target_os = "fuchsia"))], but this suspicion hasn't been confirmed yet.

Once (1) has progressed to some consensus and we have answers on (2) I'll report back with some proposed next steps for a migration.

@ijackson, re:

Would it be OK to stabilise that bodge on Fuchsia, with the expectation that this will be fixed by withdrawing or deprecating this API on Fuchsia?

As a possible alternative, maybe we should consider disabling the ExitStatusExt trait on Fuchsia altogether? I'm not able to find any uses of it in our code, and if we're going to stop being cfg(unix) eventually then this would match that intent. I believe the chroot example I cited above took a similar path, although it did so by flagging a new API off for the target rather than disabling an API that already works. This seems fine to me given the likelihood of removing our unix target family as that would cause the same trait's removal eventually, but the approach may warrant more discussion.

@ijackson
Copy link
Contributor

ijackson commented May 14, 2021

@anp

As a possible alternative, maybe we should consider disabling the ExitStatusExt trait on Fuchsia altogether?

I'm not a libs team member, so I'm not authoritative, but I would be fine with that. Would you care to prepare an MR to do that (for both ExitStatus and the unstable ExitStatusError), and tag me? I think this may be easier for you than for me since building for Fuchsia is not straightforward for me.

@ijackson
Copy link
Contributor

unstable ExitStatusError

For clarity: that's the branch in #82973 which I am hoping will be merged soon.

@anp
Copy link
Member

anp commented Jun 2, 2021

Sorry for the delay here! Looks like #82973 merged with a stub impl of ExitStatusError for Fuchsia. I think that's fine, and we can clean it up when we land the cfg(unix) removal.

@ShadowJonathan
Copy link

An interesting addition to this conversation; fuchsia doesn't have ..

@anp
Copy link
Member

anp commented Aug 30, 2021

Updating with what I think are more concrete next steps:

  1. Produce an experimental toolchain without cfg(unix) to identify Fuchsia's own crates.io dependencies that are broken
  2. Work with maintainers of dependencies from (1) to get them compiling for Fuchsia without cfg(unix)
  3. Roll dependency updates from (2) through to Fuchsia's source tree
  4. Remove Fuchsia's target_family, leaving file-descriptor-based APIs in libstd in place with cfg(any(unix, target_os = "fuchsia"))
  5. Roll the toolchain from (3) through to Fuchsia's source tree

At this point, libstd will no longer "claim that Fuchsia is a UNIX" but it will still use Fuchsia's POSIX emulation for file descriptors which we'd like to remove. We've had some discussions about the "right" way for libstd to use Fuchsia libraries, and it will definitely require Fuchsia RFCs to change what we're exporting, but I'll summarize my rough understanding here.

A lot of libstd's functionality on Fuchsia comes from a library called fdio, including process spawning, file descriptor emulation, and process-global state like the current working directory and namespace. Fuchsia's SDK will need to offer non-fdio interfaces for

  • process spawning w/o manually reimplementing the procargs protocol
  • manipulating a process namespace without file descriptors
  • implementing the necessary fuchsia.io* protocols on zircon handles
  • ...and a few other things

Once Fuchsia's SDK offers a couple of libraries to do this (say libzxio.a with I/O protocol impls and libnsio.so with the process-global state), we need to migrate libstd to use those APIs. At that point, we can consider exposing traits in std like AsRawHandle that work with the native underlying handle types rather than FDs.

@tmandry
Copy link
Member

tmandry commented Mar 18, 2022

Another difference from #93858 (comment):

I see the failure is on fuchsia. It looks like it only supports a subset of the operations on the UNIX CommandExt (e.g. groups doesn't work either), as per library/std/src/sys/unix/process/process_fuchsia.rs.

Is this something I should tackle here (e.g. return an error when this is set on fuschia?), or is this OK for this to behave like e.g. groups, that is, to do nothing silently?

jhpratt added a commit to jhpratt/rust that referenced this issue Jul 10, 2024
…r=tmandry

Fixup failing fuchsia tests

The Fuchsia platform passes all tests with these changes. Two tests are ignored because they rely on Fuchsia not returning a status code upon a process aborting. See rust-lang#102032 and rust-lang#58590 for more details on that topic.

Many formatting changes are also included in this PR.

r? tmandry
r? erickt
rust-timer added a commit to rust-lang-ci/rust that referenced this issue Jul 10, 2024
Rollup merge of rust-lang#127461 - c6c7:fixup-failing-fuchsia-tests, r=tmandry

Fixup failing fuchsia tests

The Fuchsia platform passes all tests with these changes. Two tests are ignored because they rely on Fuchsia not returning a status code upon a process aborting. See rust-lang#102032 and rust-lang#58590 for more details on that topic.

Many formatting changes are also included in this PR.

r? tmandry
r? erickt
@workingjubilee
Copy link
Member

I should note that Fuchsia was exempted from implementing std::os::unix::fs::chroot which is unprecedented for std::os::unix API, although I don't consider the status quo of "implementing stub-only versions that always return Err" to be much of an improvement.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
O-fuchsia Operating system: Fuchsia T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests