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

Add Fuchsia support #37313

Merged
merged 4 commits into from
Oct 24, 2016
Merged

Add Fuchsia support #37313

merged 4 commits into from
Oct 24, 2016

Conversation

raphlinus
Copy link
Contributor

Adds support for the x86_64-unknown-fuchsia target, which covers the
Fuchsia operating system.

@rust-highfive
Copy link
Collaborator

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

@raphlinus
Copy link
Contributor Author

@alexcrichton Note that this PR is dependent on rust-lang/libc#432 to actually work. It also requires a clang wrapper which has not yet been added to the fuchsia repository.

Comments welcome. One missing piece is aarch64 support (it's x86_64 only for now), one of the two officially supported architectures for Fuchsia. But I figure it might be useful to get feedback at this stage.

Copy link
Member

@alexcrichton alexcrichton left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great to me! Let's get the libc PR merged as well yeah so it can be included here.

@@ -312,6 +312,7 @@ pub trait DirEntryExt {
}

#[stable(feature = "dir_entry_ext", since = "1.1.0")]
#[cfg(not(target_os = "fuchsia"))]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We chatted on IRC, but perhaps this can be removed now?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yup, fixed.

pub use libc::{off_t, ino_t, nlink_t, blksize_t, blkcnt_t, stat, time_t};
}

#[cfg(target_arch = "aarch64")]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can probably simplify a bunch of this right as only x86_64 is needed for now?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I chose these based on what's plausibly supported by the Magenta kernel. aarch64 is an officially supported Fuchsia target, and I'd like to get that in soon (possibly as an update to his PR). Magenta kernel also supports 32 bit targets, and I don't want to necessarily preclude Rust on those targets either.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah never mind me then!

@alexcrichton
Copy link
Member

Oh looks like make tidy is also complaining about a few locations?

@alexcrichton
Copy link
Member

Want to update the libc submodule now as well?

@alexcrichton
Copy link
Member

@bors: r+

@bors
Copy link
Contributor

bors commented Oct 22, 2016

☔ The latest upstream changes (presumably #37337) made this pull request unmergeable. Please resolve the merge conflicts.

Adds support for the x86_64-unknown-fuchsia target, which covers the
Fuchsia operating system.
The DirEntryExt::ino() implementation was omitted from the first
iteration of this patch, because a dependency needed to be
configured. The fix is straightforward enough.
Prefer FIXME to TODO
Also trim os::fuchsia::raw architectures.
@alexcrichton
Copy link
Member

@bors: r+

@bors
Copy link
Contributor

bors commented Oct 24, 2016

📌 Commit cea6140 has been approved by alexcrichton

@bors
Copy link
Contributor

bors commented Oct 24, 2016

⌛ Testing commit cea6140 with merge 3caf63c...

bors added a commit that referenced this pull request Oct 24, 2016
Add Fuchsia support

Adds support for the x86_64-unknown-fuchsia target, which covers the
Fuchsia operating system.
@bors bors merged commit cea6140 into rust-lang:master Oct 24, 2016
@raphlinus raphlinus deleted the fuchsia branch October 24, 2016 23:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants