Upgrade command-fds to 0.3.0 am: 72193c9d0b
Original change: https://android-review.googlesource.com/c/platform/external/rust/crates/command-fds/+/3223702
Change-Id: Ibec02a309ee21e1cd3a917b8ce31bf9ee31bbca6
Signed-off-by: Automerger Merge Worker <[email protected]>
diff --git a/.cargo_vcs_info.json b/.cargo_vcs_info.json
index 0562cca..973f7ca 100644
--- a/.cargo_vcs_info.json
+++ b/.cargo_vcs_info.json
@@ -1,6 +1,6 @@
{
"git": {
- "sha1": "425bb5dc189f3a5130bdc80ca47d6da5001157b4"
+ "sha1": "8db37c4e5dc2040af513dfe45b92cc3d970a3c43"
},
"path_in_vcs": ""
}
\ No newline at end of file
diff --git a/.github/dependabot.yml b/.github/dependabot.yml
new file mode 100644
index 0000000..98e44ee
--- /dev/null
+++ b/.github/dependabot.yml
@@ -0,0 +1,10 @@
+version: 2
+updates:
+ - package-ecosystem: "cargo"
+ directory: "/"
+ schedule:
+ interval: "weekly"
+ - package-ecosystem: "github-actions"
+ directory: "/"
+ schedule:
+ interval: "weekly"
diff --git a/.github/workflows/rust.yml b/.github/workflows/rust.yml
index 3d98a16..9212c43 100644
--- a/.github/workflows/rust.yml
+++ b/.github/workflows/rust.yml
@@ -7,13 +7,12 @@
env:
CARGO_TERM_COLOR: always
- grcov-version: 0.8.0
jobs:
build:
runs-on: ubuntu-latest
steps:
- - uses: actions/checkout@v2
+ - uses: actions/checkout@v4
- name: Build
run: cargo build
- name: Run tests
@@ -27,7 +26,7 @@
format:
runs-on: ubuntu-latest
steps:
- - uses: actions/checkout@v2
+ - uses: actions/checkout@v4
- name: Format Rust code
run: cargo fmt --all -- --check
@@ -36,11 +35,9 @@
env:
RUSTC_BOOTSTRAP: 1
steps:
- - uses: actions/checkout@v2
- - name: Install dependencies
- run: sudo apt-get install libdbus-1-dev
+ - uses: actions/checkout@v4
- name: Install grcov
- run: curl -L https://github.com/mozilla/grcov/releases/latest/download/grcov-linux-x86_64.tar.bz2 | tar jxf -
+ run: curl -L https://github.com/mozilla/grcov/releases/download/v0.8.6/grcov-v0.8.6-x86_64-unknown-linux-gnu.tar.gz | tar zxf -
- name: Install llvm-tools
run: rustup component add llvm-tools-preview
- name: Build for coverage
@@ -55,7 +52,7 @@
- name: Convert coverage
run: ./grcov . -s . --binary-path target/debug/ -t lcov --branch --ignore-not-existing -o target/debug/lcov.info
- name: Upload coverage to codecov.io
- uses: codecov/codecov-action@v1
+ uses: codecov/codecov-action@v3
with:
directory: ./target/debug
fail_ci_if_error: true
diff --git a/Android.bp b/Android.bp
index 1b91848..e42d2d3 100644
--- a/Android.bp
+++ b/Android.bp
@@ -3,6 +3,7 @@
package {
default_applicable_licenses: ["external_rust_crates_command-fds_license"],
+ default_team: "trendy_team_android_rust",
}
license {
@@ -17,7 +18,7 @@
host_supported: true,
crate_name: "command_fds",
cargo_env_compat: true,
- cargo_pkg_version: "0.2.2",
+ cargo_pkg_version: "0.3.0",
crate_root: "src/lib.rs",
edition: "2018",
features: ["default"],
diff --git a/Cargo.toml b/Cargo.toml
index bb015ea..b64d52c 100644
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -12,21 +12,30 @@
[package]
edition = "2018"
name = "command-fds"
-version = "0.2.2"
+version = "0.3.0"
authors = ["Andrew Walbran <[email protected]>"]
description = "A library for passing arbitrary file descriptors when spawning child processes."
-keywords = ["command", "process", "child", "subprocess", "fd"]
+readme = "README.md"
+keywords = [
+ "command",
+ "process",
+ "child",
+ "subprocess",
+ "fd",
+]
categories = ["os::unix-apis"]
license = "Apache-2.0"
repository = "https://github.com/google/command-fds/"
+
[dependencies.nix]
-version = "0.22.0"
+version = "0.27.0"
+features = ["fs"]
[dependencies.thiserror]
version = "1.0.24"
[dependencies.tokio-crate]
-version = "^1.0"
+version = "1.32.0"
features = ["process"]
optional = true
default-features = false
diff --git a/Cargo.toml.orig b/Cargo.toml.orig
index ef218ac..a3e6c81 100644
--- a/Cargo.toml.orig
+++ b/Cargo.toml.orig
@@ -1,6 +1,6 @@
[package]
name = "command-fds"
-version = "0.2.2"
+version = "0.3.0"
edition = "2018"
authors = ["Andrew Walbran <[email protected]>"]
license = "Apache-2.0"
@@ -10,15 +10,11 @@
categories = ["os::unix-apis"]
[dependencies]
-nix = "0.22.0"
+nix = { version = "0.27.0", features = ["fs"] }
thiserror = "1.0.24"
-
-[dependencies.tokio-crate]
-package = "tokio"
-version = "^1.0"
-optional = true
-default-features = false
-features = ["process"]
+tokio-crate = { package = "tokio", version = "1.32.0", optional = true, default-features = false, features = [
+ "process",
+] }
[features]
default = []
diff --git a/METADATA b/METADATA
index 7b98618..9187d47 100644
--- a/METADATA
+++ b/METADATA
@@ -1,19 +1,20 @@
+# This project was upgraded with external_updater.
+# Usage: tools/external_updater/updater.sh update external/rust/crates/command-fds
+# For more info, check https://cs.android.com/android/platform/superproject/main/+/main:tools/external_updater/README.md
+
name: "command-fds"
description: "A library for passing arbitrary file descriptors when spawning child processes."
third_party {
- url {
- type: HOMEPAGE
- value: "https://crates.io/crates/command-fds"
- }
- url {
- type: ARCHIVE
- value: "https://static.crates.io/crates/command-fds/command-fds-0.2.2.crate"
- }
- version: "0.2.2"
license_type: NOTICE
last_upgrade_date {
- year: 2022
- month: 6
- day: 28
+ year: 2024
+ month: 8
+ day: 16
+ }
+ homepage: "https://crates.io/crates/command-fds"
+ identifier {
+ type: "Archive"
+ value: "https://static.crates.io/crates/command-fds/command-fds-0.3.0.crate"
+ version: "0.3.0"
}
}
diff --git a/examples/spawn.rs b/examples/spawn.rs
index b399dc7..c65fb1d 100644
--- a/examples/spawn.rs
+++ b/examples/spawn.rs
@@ -14,7 +14,8 @@
use command_fds::{CommandFdExt, FdMapping};
use std::fs::{read_dir, read_link, File};
-use std::os::unix::io::AsRawFd;
+use std::io::stdin;
+use std::os::fd::AsFd;
use std::os::unix::process::CommandExt;
use std::process::Command;
use std::thread::sleep;
@@ -40,17 +41,18 @@
// Prepare to run `ls -l /proc/self/fd` with some FDs mapped.
let mut command = Command::new("ls");
+ let stdin = stdin().as_fd().try_clone_to_owned().unwrap();
command.arg("-l").arg("/proc/self/fd");
command
.fd_mappings(vec![
// Map `file` as FD 3 in the child process.
FdMapping {
- parent_fd: file.as_raw_fd(),
+ parent_fd: file.into(),
child_fd: 3,
},
// Map this process's stdin as FD 5 in the child process.
FdMapping {
- parent_fd: 0,
+ parent_fd: stdin,
child_fd: 5,
},
])
diff --git a/src/lib.rs b/src/lib.rs
index fde680a..8b7f227 100644
--- a/src/lib.rs
+++ b/src/lib.rs
@@ -19,6 +19,8 @@
//! ```rust
//! use command_fds::{CommandFdExt, FdMapping};
//! use std::fs::File;
+//! use std::io::stdin;
+//! use std::os::fd::AsFd;
//! use std::os::unix::io::AsRawFd;
//! use std::process::Command;
//!
@@ -32,12 +34,12 @@
//! .fd_mappings(vec![
//! // Map `file` as FD 3 in the child process.
//! FdMapping {
-//! parent_fd: file.as_raw_fd(),
+//! parent_fd: file.into(),
//! child_fd: 3,
//! },
//! // Map this process's stdin as FD 5 in the child process.
//! FdMapping {
-//! parent_fd: 0,
+//! parent_fd: stdin().as_fd().try_clone_to_owned().unwrap(),
//! child_fd: 5,
//! },
//! ])
@@ -55,6 +57,7 @@
use nix::unistd::dup2;
use std::cmp::max;
use std::io;
+use std::os::fd::{AsRawFd, FromRawFd, OwnedFd};
use std::os::unix::io::RawFd;
use std::os::unix::process::CommandExt;
use std::process::Command;
@@ -63,10 +66,11 @@
/// A mapping from a file descriptor in the parent to a file descriptor in the child, to be applied
/// when spawning a child process.
///
-/// The parent_fd must be kept open until after the child is spawned.
-#[derive(Clone, Debug, Eq, PartialEq)]
+/// This takes ownership of the `parent_fd` to ensure that it is kept open until after the child is
+/// spawned.
+#[derive(Debug)]
pub struct FdMapping {
- pub parent_fd: RawFd,
+ pub parent_fd: OwnedFd,
pub child_fd: RawFd,
}
@@ -79,13 +83,21 @@
pub trait CommandFdExt {
/// Adds the given set of file descriptors to the command.
///
- /// Warning: Calling this more than once on the same command, or attempting to run the same
- /// command more than once after calling this, may result in unexpected behaviour.
+ /// Warning: Calling this more than once on the same command may result in unexpected behaviour.
+ /// In particular, it is not possible to check that two mappings applied separately don't use
+ /// the same `child_fd`. If there is such a collision then one will apply and the other will be
+ /// lost.
+ ///
+ /// Note that the `Command` takes ownership of the file descriptors, which means that they won't
+ /// be closed in the parent process until the `Command` is dropped.
fn fd_mappings(&mut self, mappings: Vec<FdMapping>) -> Result<&mut Self, FdMappingCollision>;
/// Adds the given set of file descriptors to be passed on to the child process when the command
/// is run.
- fn preserved_fds(&mut self, fds: Vec<RawFd>) -> &mut Self;
+ ///
+ /// Note that the `Command` takes ownership of the file descriptors, which means that they won't
+ /// be closed in the parent process until the `Command` is dropped.
+ fn preserved_fds(&mut self, fds: Vec<OwnedFd>) -> &mut Self;
}
impl CommandFdExt for Command {
@@ -98,17 +110,16 @@
// Register the callback to apply the mappings after forking but before execing.
// Safety: `map_fds` will not allocate, so it is safe to call from this hook.
unsafe {
- // If the command is run more than once, and hence this closure is called multiple
- // times, then `mappings` may be in an incorrect state. It would be good if we could
- // reset it to the initial state somehow, or use something else for saving the temporary
- // mappings.
+ // If the command is run more than once, the closure will be called multiple times but
+ // in different forked processes, which will have different copies of `mappings`. So
+ // their changes to it shouldn't be visible to each other.
self.pre_exec(move || map_fds(&mut mappings, &child_fds));
}
Ok(self)
}
- fn preserved_fds(&mut self, fds: Vec<RawFd>) -> &mut Self {
+ fn preserved_fds(&mut self, fds: Vec<OwnedFd>) -> &mut Self {
unsafe {
self.pre_exec(move || preserve_fds(&fds));
}
@@ -140,7 +151,7 @@
// so we still need to ensure we don't conflict with them.
let first_safe_fd = mappings
.iter()
- .map(|mapping| max(mapping.parent_fd, mapping.child_fd))
+ .map(|mapping| max(mapping.parent_fd.as_raw_fd(), mapping.child_fd))
.max()
.unwrap()
+ 1;
@@ -149,32 +160,44 @@
// is clear of either range. Mappings to the same FD are fine though, we can handle them by just
// removing the FD_CLOEXEC flag from the existing (parent) FD.
for mapping in mappings.iter_mut() {
- if child_fds.contains(&mapping.parent_fd) && mapping.parent_fd != mapping.child_fd {
- mapping.parent_fd = fcntl(mapping.parent_fd, FcntlArg::F_DUPFD_CLOEXEC(first_safe_fd))?;
+ if child_fds.contains(&mapping.parent_fd.as_raw_fd())
+ && mapping.parent_fd.as_raw_fd() != mapping.child_fd
+ {
+ let parent_fd = fcntl(
+ mapping.parent_fd.as_raw_fd(),
+ FcntlArg::F_DUPFD_CLOEXEC(first_safe_fd),
+ )?;
+ // SAFETY: We just created `parent_fd` so we can take ownership of it.
+ unsafe {
+ mapping.parent_fd = OwnedFd::from_raw_fd(parent_fd);
+ }
}
}
// Now we can actually duplicate FDs to the desired child FDs.
for mapping in mappings {
- if mapping.child_fd == mapping.parent_fd {
+ if mapping.child_fd == mapping.parent_fd.as_raw_fd() {
// Remove the FD_CLOEXEC flag, so the FD will be kept open when exec is called for the
// child.
- fcntl(mapping.parent_fd, FcntlArg::F_SETFD(FdFlag::empty()))?;
+ fcntl(
+ mapping.parent_fd.as_raw_fd(),
+ FcntlArg::F_SETFD(FdFlag::empty()),
+ )?;
} else {
// This closes child_fd if it is already open as something else, and clears the
// FD_CLOEXEC flag on child_fd.
- dup2(mapping.parent_fd, mapping.child_fd)?;
+ dup2(mapping.parent_fd.as_raw_fd(), mapping.child_fd)?;
}
}
Ok(())
}
-fn preserve_fds(fds: &[RawFd]) -> io::Result<()> {
+fn preserve_fds(fds: &[OwnedFd]) -> io::Result<()> {
for fd in fds {
// Remove the FD_CLOEXEC flag, so the FD will be kept open when exec is called for the
// child.
- fcntl(*fd, FcntlArg::F_SETFD(FdFlag::empty()))?;
+ fcntl(fd.as_raw_fd(), FcntlArg::F_SETFD(FdFlag::empty()))?;
}
Ok(())
@@ -199,30 +222,19 @@
let mut command = Command::new("ls");
- // The same mapping can't be included twice.
- assert!(command
- .fd_mappings(vec![
- FdMapping {
- child_fd: 4,
- parent_fd: 5,
- },
- FdMapping {
- child_fd: 4,
- parent_fd: 5,
- },
- ])
- .is_err());
+ let file1 = File::open("testdata/file1.txt").unwrap();
+ let file2 = File::open("testdata/file2.txt").unwrap();
- // Mapping two different FDs to the same FD isn't allowed either.
+ // Mapping two different FDs to the same FD isn't allowed.
assert!(command
.fd_mappings(vec![
FdMapping {
child_fd: 4,
- parent_fd: 5,
+ parent_fd: file1.into(),
},
FdMapping {
child_fd: 4,
- parent_fd: 6,
+ parent_fd: file2.into(),
},
])
.is_err());
@@ -265,7 +277,7 @@
// Map the file an otherwise unused FD.
assert!(command
.fd_mappings(vec![FdMapping {
- parent_fd: file.as_raw_fd(),
+ parent_fd: file.into(),
child_fd: 5,
},])
.is_ok());
@@ -275,6 +287,7 @@
}
#[test]
+ #[ignore = "flaky on GitHub"]
fn one_preserved() {
setup();
@@ -282,12 +295,13 @@
command.arg("/proc/self/fd");
let file = File::open("testdata/file1.txt").unwrap();
- let file_fd = file.as_raw_fd();
+ let file_fd: OwnedFd = file.into();
+ let raw_file_fd = file_fd.as_raw_fd();
+ assert!(raw_file_fd > 3);
command.preserved_fds(vec![file_fd]);
- assert!(file_fd > 3);
let output = command.output().unwrap();
- expect_fds(&output, &[0, 1, 2, 3, file_fd], 0);
+ expect_fds(&output, &[0, 1, 2, 3, raw_file_fd], 0);
}
#[test]
@@ -299,18 +313,20 @@
let file1 = File::open("testdata/file1.txt").unwrap();
let file2 = File::open("testdata/file2.txt").unwrap();
- let fd1 = file1.as_raw_fd();
- let fd2 = file2.as_raw_fd();
+ let fd1: OwnedFd = file1.into();
+ let fd2: OwnedFd = file2.into();
+ let fd1_raw = fd1.as_raw_fd();
+ let fd2_raw = fd2.as_raw_fd();
// Map files to each other's FDs, to ensure that the temporary FD logic works.
assert!(command
.fd_mappings(vec![
FdMapping {
parent_fd: fd1,
- child_fd: fd2,
+ child_fd: fd2_raw,
},
FdMapping {
parent_fd: fd2,
- child_fd: fd1,
+ child_fd: fd1_raw,
},
])
.is_ok(),);
@@ -318,7 +334,7 @@
let output = command.output().unwrap();
// Expect one more Fd for the /proc/self/fd directory. We can't predict what number it will
// be assigned, because 3 might or might not be taken already by fd1 or fd2.
- expect_fds(&output, &[0, 1, 2, fd1, fd2], 1);
+ expect_fds(&output, &[0, 1, 2, fd1_raw, fd2_raw], 1);
}
#[test]
@@ -330,19 +346,20 @@
let file1 = File::open("testdata/file1.txt").unwrap();
let file2 = File::open("testdata/file2.txt").unwrap();
- let fd1 = file1.as_raw_fd();
+ let fd1: OwnedFd = file1.into();
+ let fd1_raw = fd1.as_raw_fd();
// Map file1 to the same FD it currently has, to ensure the special case for that works.
assert!(command
.fd_mappings(vec![FdMapping {
parent_fd: fd1,
- child_fd: fd1,
+ child_fd: fd1_raw,
}])
.is_ok());
let output = command.output().unwrap();
// Expect one more Fd for the /proc/self/fd directory. We can't predict what number it will
// be assigned, because 3 might or might not be taken already by fd1 or fd2.
- expect_fds(&output, &[0, 1, 2, fd1], 1);
+ expect_fds(&output, &[0, 1, 2, fd1_raw], 1);
// Keep file2 open until the end, to ensure that it's not passed to the child.
drop(file2);
@@ -358,7 +375,7 @@
// Map the file to stdin.
assert!(command
.fd_mappings(vec![FdMapping {
- parent_fd: file.as_raw_fd(),
+ parent_fd: file.into(),
child_fd: 0,
},])
.is_ok());
diff --git a/src/tokio.rs b/src/tokio.rs
index 78979bf..57fe081 100644
--- a/src/tokio.rs
+++ b/src/tokio.rs
@@ -1,24 +1,12 @@
-use std::os::unix::prelude::RawFd;
-
+use std::os::fd::OwnedFd;
use tokio::process::Command;
use tokio_crate as tokio;
-use crate::{map_fds, preserve_fds, validate_child_fds, FdMapping, FdMappingCollision};
+use crate::{
+ map_fds, preserve_fds, validate_child_fds, CommandFdExt, FdMapping, FdMappingCollision,
+};
-/// Extension to add file descriptor mappings to a [`Command`].
-pub trait CommandFdAsyncExt {
- /// Adds the given set of file descriptors to the command.
- ///
- /// Warning: Calling this more than once on the same command, or attempting to run the same
- /// command more than once after calling this, may result in unexpected behaviour.
- fn fd_mappings(&mut self, mappings: Vec<FdMapping>) -> Result<&mut Self, FdMappingCollision>;
-
- /// Adds the given set of file descriptors to be passed on to the child process when the command
- /// is run.
- fn preserved_fds(&mut self, fds: Vec<RawFd>) -> &mut Self;
-}
-
-impl CommandFdAsyncExt for Command {
+impl CommandFdExt for Command {
fn fd_mappings(
&mut self,
mut mappings: Vec<FdMapping>,
@@ -32,7 +20,7 @@
Ok(self)
}
- fn preserved_fds(&mut self, fds: Vec<RawFd>) -> &mut Self {
+ fn preserved_fds(&mut self, fds: Vec<OwnedFd>) -> &mut Self {
unsafe {
self.pre_exec(move || preserve_fds(&fds));
}
