Skip to content

Conversation

joshtriplett
Copy link
Member

This is a straightforward wrapper that uses the existing helpers for C
string handling and errno handling.

Having this available is convenient for UNIX utility programs written in
Rust, and avoids having to call the unsafe libc::chroot directly and
handle errors manually, in a program that may otherwise be entirely safe
code.

@rust-highfive
Copy link
Contributor

r? @m-ou-se

(rust-highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Apr 29, 2021
@joshtriplett joshtriplett added O-unix Operating system: Unix-like T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. Libs-Small Libs issues that are considered "small" or self-contained labels Apr 29, 2021
Copy link
Member

@dtolnay dtolnay left a comment

Choose a reason for hiding this comment

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

Seems good to me! :)

@dtolnay dtolnay assigned dtolnay and unassigned m-ou-se Apr 29, 2021
@dtolnay
Copy link
Member

dtolnay commented Apr 29, 2021

@bors r+

@bors
Copy link
Collaborator

bors commented Apr 29, 2021

📌 Commit 2fb2f0b has been approved by dtolnay

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Apr 29, 2021
@rust-log-analyzer

This comment has been minimized.

jackh726 added a commit to jackh726/rust that referenced this pull request Apr 29, 2021
Add std::os::unix::fs::chroot to change the root directory of the current process

This is a straightforward wrapper that uses the existing helpers for C
string handling and errno handling.

Having this available is convenient for UNIX utility programs written in
Rust, and avoids having to call the unsafe `libc::chroot` directly and
handle errors manually, in a program that may otherwise be entirely safe
code.
@jackh726
Copy link
Member

Failed in #84718

@bors r-

@bors bors added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Apr 29, 2021
@joshtriplett
Copy link
Member Author

Fixed the doc link.

@bors r=dtolnay rollup

@bors
Copy link
Collaborator

bors commented Apr 29, 2021

📌 Commit 3b414c3 has been approved by dtolnay

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Apr 29, 2021
/// This typically requires privileges, such as root or a specific capability.
///
/// This does not change the current working directory; you should call
/// [`std::env::set_current_dir`][`crate::env::set_current_dir`] afterwards.
Copy link
Member

Choose a reason for hiding this comment

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

Adding extern crate self as std; in std's lib.rs would avoid having to do the link target explicitly here (and definitely elsewhere too).

Copy link
Member Author

Choose a reason for hiding this comment

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

According to @jyn514, doing that currently produces an ICE.

Copy link
Member

Choose a reason for hiding this comment

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

This is #73445. I haven't tried it in a while, it may work now.

Copy link
Member

Choose a reason for hiding this comment

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

Also, you can shorten a bit like so:

Suggested change
/// [`std::env::set_current_dir`][`crate::env::set_current_dir`] afterwards.
/// [`std::env::set_current_dir`](crate::env::set_current_dir) afterwards.

Copy link
Member

Choose a reason for hiding this comment

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

If you go the extern crate route, you might want to do

#[cfg(doc)]
extern crate self as std;

to ensure it's only used for docs.

Copy link
Member

Choose a reason for hiding this comment

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

@camelid that will break any links that are re-exported, cfg(doc) isn't set for dependencies.

Copy link
Member

Choose a reason for hiding this comment

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

I haven't tried it in a while, it may work now.

Just tried it, extern crate self as core works but not extern crate self as std (#84755, #73445 (comment)).

@rust-log-analyzer

This comment has been minimized.

jackh726 added a commit to jackh726/rust that referenced this pull request Apr 29, 2021
Add std::os::unix::fs::chroot to change the root directory of the current process

This is a straightforward wrapper that uses the existing helpers for C
string handling and errno handling.

Having this available is convenient for UNIX utility programs written in
Rust, and avoids having to call the unsafe `libc::chroot` directly and
handle errors manually, in a program that may otherwise be entirely safe
code.
@bors
Copy link
Collaborator

bors commented Apr 30, 2021

⌛ Testing commit eda658dc7c95c69e8c1f7191b0ba7bebb6e6be2e with merge fe71c7f6e0f60886eff3b39268916552115b74e1...

@rust-log-analyzer

This comment has been minimized.

@bors
Copy link
Collaborator

bors commented Apr 30, 2021

💔 Test failed - checks-actions

@bors bors added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Apr 30, 2021
@joshtriplett
Copy link
Member Author

joshtriplett commented Apr 30, 2021

Well, that's interesting. Either Fuchsia doesn't have chroot, or libc isn't providing a binding to it. I'll investigate.

cc @tmandry for Fuchsia.

@joshtriplett
Copy link
Member Author

The answer appears to be that Fuchsia doesn't have chroot. I'll add a cfg.

…rent process

This is a straightforward wrapper that uses the existing helpers for C
string handling and errno handling.

Having this available is convenient for UNIX utility programs written in
Rust, and avoids having to call the unsafe `libc::chroot` directly and
handle errors manually, in a program that may otherwise be entirely safe
code.
@joshtriplett
Copy link
Member Author

@bors try

@bors
Copy link
Collaborator

bors commented Apr 30, 2021

⌛ Trying commit ffb874a with merge 7ede84235ed7c851bb5bc27e1f7d6f438d6b78d5...

@bors
Copy link
Collaborator

bors commented Apr 30, 2021

☀️ Try build successful - checks-actions
Build commit: 7ede84235ed7c851bb5bc27e1f7d6f438d6b78d5 (7ede84235ed7c851bb5bc27e1f7d6f438d6b78d5)

@joshtriplett
Copy link
Member Author

@bors r=dtolnay rollup=never

@bors
Copy link
Collaborator

bors commented Apr 30, 2021

📌 Commit ffb874a has been approved by dtolnay

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Apr 30, 2021
@bors
Copy link
Collaborator

bors commented Apr 30, 2021

⌛ Testing commit ffb874a with merge 7506228...

@bors
Copy link
Collaborator

bors commented Apr 30, 2021

☀️ Test successful - checks-actions
Approved by: dtolnay
Pushing 7506228 to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Apr 30, 2021
@bors bors merged commit 7506228 into rust-lang:master Apr 30, 2021
@rustbot rustbot added this to the 1.53.0 milestone Apr 30, 2021
@joshtriplett joshtriplett deleted the chroot branch April 30, 2021 22:09
@CDirkx CDirkx mentioned this pull request May 19, 2021
bors added a commit to rust-lang-ci/rust that referenced this pull request May 23, 2021
Fix `vxworks`

Some PRs made the `vxworks` target not build anymore. This PR fixes that:

- rust-lang#82973: copy `ExitStatusError` implementation from `unix`.
- rust-lang#84716: no `libc::chroot` available on `vxworks`, so for now don't implement `os::unix::fs::chroot`.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Libs-Small Libs issues that are considered "small" or self-contained merged-by-bors This PR was explicitly merged by bors. O-unix Operating system: Unix-like S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

10 participants