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 Trusty OS support #3942

Merged
merged 6 commits into from
Nov 6, 2024
Merged

Add Trusty OS support #3942

merged 6 commits into from
Nov 6, 2024

Conversation

randomPoison
Copy link
Contributor

@randomPoison randomPoison commented Sep 24, 2024

This PR adds support for the Trusty secure operating system. This upstreams the patch that we have been using internally, cleaned up to meet the project's coding conventions.

This is a revival of #2987.

Relevant headers:

  • PROT_READ, PROT_WRITE, and MAP_FAILED come from mman.h.
  • CLOCK_BOOTTIME comes from time.h.
  • STDOUT_FILENO and STDERR_FILE_NO come from unistd.h.
  • AT_PAGESZ comes from elf.h.

@rustbot
Copy link
Collaborator

rustbot commented Sep 24, 2024

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @tgross35 (or someone else) some time within the next two weeks.

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 (S-waiting-on-review and S-waiting-on-author) stays updated, invoking these commands when appropriate:

  • @rustbot author: the review is finished, PR author should check the comments and take action accordingly
  • @rustbot review: the author is ready for a review, this PR will be queued again in the reviewer's queue

@randomPoison
Copy link
Contributor Author

Addressing a comment from the original PR, @JohnTitor asked:

I wonder if we could move the module under linux_like or somewhere else similar to that. I'm not familiar with Trusty but is it completely different from Linux-like OS (i.e. exposing linux-like items on Trusty is harmful)?

Yeah, Trusty is not linux-like, nor is it a Unix. It has some overlap with those platforms, but isn't part of the same family of systems. I think the right thing to do is have Trusty be its own module.

@randomPoison
Copy link
Contributor Author

I think this would also be a good candidate for merging over to the 0.2 branch. The diff should apply cleanly, I've tested it against both branches.

@rustbot label stable-nominated

@rustbot rustbot added the stable-nominated This PR should be considered for cherry-pick to libc's stable release branch label Sep 24, 2024
@randomPoison
Copy link
Contributor Author

What should I do about the unknown target_os = "trusty" cfg in CI? The targets aren't yet in the stable compiler. I've got this Cargo.toml snippet I've been using locally

[lints.rust.unexpected_cfgs]
level = "warn"
check-cfg = [
    'cfg(target_os, values("trusty"))',
]

Is that appropriate to add in the PR as a temporary way to suppress the lint? Or is there a better way to handle this?

@tgross35
Copy link
Contributor

What should I do about the unknown target_os = "trusty" cfg in CI? The targets aren't yet in the stable compiler. I've got this Cargo.toml snippet I've been using locally

We should have that handled in build.rs -

libc/build.rs

Lines 24 to 36 in fbec928

const CHECK_CFG_EXTRA: &'static [(&'static str, &'static [&'static str])] = &[
(
"target_os",
&[
"switch", "aix", "ohos", "hurd", "rtems", "visionos", "nuttx",
],
),
("target_env", &["illumos", "wasi", "aix", "ohos"]),
(
"target_arch",
&["loongarch64", "mips32r6", "mips64r6", "csky"],
),
];

Overall this looks pretty good to me, please just link to the relevant headers providing these definitions if possible.

@rustbot author (just comment @rustbot review when it is ready for that again)

@randomPoison
Copy link
Contributor Author

@tgross35 We're using musl as our libc implementation, so those definitions come from the musl headers.

  • PROT_READ, PROT_WRITE, and MAP_FAILED come from mman.h.
  • CLOCK_BOOTTIME comes from time.h.
  • STDOUT_FILENO and STDERR_FILE_NO come from unistd.h.
  • AT_PAGESZ comes from elf.h.

Also it looks like a rebase to the latest main fixed the unknown platform error, so I didn't need to update build.rs.

@randomPoison
Copy link
Contributor Author

oh I forgot to say the magic word

@rustbot review

Copy link
Contributor

@tgross35 tgross35 left a comment

Choose a reason for hiding this comment

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

Added the headers to the PR description so they make it into git history. Thanks!

@tgross35 tgross35 added this pull request to the merge queue Nov 6, 2024
Merged via the queue into rust-lang:main with commit 94c7cae Nov 6, 2024
41 checks passed
tgross35 added a commit to tgross35/rust-libc that referenced this pull request Nov 6, 2024
Add Trusty OS support

(backport <rust-lang#3942>)
(cherry picked from commit 94c7cae)
@tgross35 tgross35 mentioned this pull request Nov 6, 2024
tgross35 added a commit to tgross35/rust-libc that referenced this pull request Nov 6, 2024
…patch"

Add Trusty OS support

(backport <rust-lang#3942>)
(cherry picked from commit 94c7cae)
@github-actions github-actions bot mentioned this pull request Nov 7, 2024
@tgross35 tgross35 added stable-applied This PR has been cherry-picked to libc's stable release branch and removed stable-nominated This PR should be considered for cherry-pick to libc's stable release branch labels Nov 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-review stable-applied This PR has been cherry-picked to libc's stable release branch
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants