-
-
Notifications
You must be signed in to change notification settings - Fork 1.7k
chroot: use execvp directly instead of process::Command #9013
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
base: main
Are you sure you want to change the base?
Conversation
|
Rather than using
|
CodSpeed Performance ReportMerging #9013 will not alter performanceComparing Summary
Footnotes
|
|
GNU testsuite comparison: |
I made some revisions based on your feedback. |
|
GNU testsuite comparison: |
|
I confirm this change fixes #9010 Nice job! :) |
|
would it be possible to add a test? thanks |
|
GNU testsuite comparison: |
Done |
|
GNU testsuite comparison: |
|
GNU testsuite comparison: |
|
@mattsu2020 I suggest replacing the word "refactor" with "chroot" in the title of this PR. |
src/uu/chroot/src/chroot.rs
Outdated
| use std::os::unix::process::CommandExt; | ||
| use std::path::{Path, PathBuf}; | ||
| use std::process; | ||
| use std::process::Command as ProcessCommand; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it's better to not mix cosmetic changes (ProcessCommand instead of process::Command) with functional fixes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I restored it to its original state.
|
GNU testsuite comparison: |
ff6f08b to
55a3100
Compare
|
GNU testsuite comparison: |
fix this issue
#9010