Replace wait_with_pipe() with wait_with_borrowed_pipe()

delan · delan · commit a0bb1230acde · 2025-08-03T17:58:59.000+08:00
diff --git a/src/child.rs b/src/child.rs
@@ -117,49 +117,12 @@ impl FunChildren {
         }
     }
 
-    /// Pipes stdout from the last child in the pipeline to the given function, which runs in
-    /// the current thread, then waits for all of the children to exit.
-    ///
-    /// <div class=warning>
-    ///
-    /// # Bugs
-    ///
-    /// The exit status of the last child is **ignored**. If the function returns early, without
-    /// reading from stdout until the last child exits, then the last child may be killed instead
-    /// of being waited for. To avoid these limitations, use [`Self::wait_with_stdout_thread`].
-    /// </div>
-    pub fn wait_with_pipe(&mut self, f: &mut dyn FnMut(Box<dyn Read>)) -> CmdResult {
-        let child = self.children.pop().unwrap();
-        let stderr_thread =
-            StderrThread::new(&child.cmd, &child.file, child.line, child.stderr, false);
-        match child.handle {
-            CmdChildHandle::Proc(mut proc) => {
-                if let Some(stdout) = child.stdout {
-                    f(Box::new(stdout));
-                    let _ = proc.kill();
-                }
-            }
-            CmdChildHandle::Thread(_) => {
-                if let Some(stdout) = child.stdout {
-                    f(Box::new(stdout));
-                }
-            }
-            CmdChildHandle::SyncFn => {
-                if let Some(stdout) = child.stdout {
-                    f(Box::new(stdout));
-                }
-            }
-        };
-        drop(stderr_thread);
-        CmdChildren::wait_children(&mut self.children)
-    }
-
     /// Pipes stdout from the last child in the pipeline to the given function, which runs in
     /// the current thread, then waits for all of the children to exit.
     ///
     /// If the function returns early, without reading from stdout until the last child exits,
     /// then the rest of stdout is automatically read and discarded to allow the child to finish.
-    pub fn wait_with_borrowed_pipe(&mut self, f: &mut dyn FnMut(&mut Box<dyn Read>)) -> CmdResult {
+    pub fn wait_with_pipe(&mut self, f: &mut dyn FnMut(&mut Box<dyn Read>)) -> CmdResult {
         let mut last_child = self.children.pop().unwrap();
         let mut stderr_thread = StderrThread::new(
             &last_child.cmd,
diff --git a/tests/test_macros.rs b/tests/test_macros.rs
@@ -134,7 +134,7 @@ fn test_tls_set() {
 }
 
 #[test]
-fn test_pipe() -> CmdResult {
+fn test_pipe() {
     assert!(run_cmd!(echo "xx").is_ok());
     assert_eq!(run_fun!(echo "xx").unwrap(), "xx");
     assert!(run_cmd!(echo xx | wc).is_ok());
@@ -246,7 +246,7 @@ fn test_pipe() -> CmdResult {
             .wait_with_raw_output(&mut vec![])),
         test_cases_for_entry_point!((spawn_with_output!(...))
             .unwrap()
-            .wait_with_borrowed_pipe(&mut |_stdout| {})),
+            .wait_with_pipe(&mut |_stdout| {})),
     ];
 
     macro_rules! check_eq {
@@ -281,64 +281,6 @@ fn test_pipe() -> CmdResult {
     }
 
     assert!(ok);
-
-    // test that illustrates the bugs in wait_with_pipe()
-    // FIXME: make set_pipefail() thread safe, then move this to a separate test function
-    assert!(spawn_with_output!(false)?.wait_with_all().0.is_err());
-    assert!(spawn_with_output!(false)?.wait_with_output().is_err());
-    assert!(spawn_with_output!(false)?
-        .wait_with_raw_output(&mut vec![])
-        .is_err());
-
-    // wait_with_pipe() can’t check the exit status of the last child
-    assert!(spawn_with_output!(false)?
-        .wait_with_pipe(&mut |_stdout| {})
-        .is_ok());
-
-    // wait_with_pipe() kills the last child when the provided function returns
-    assert!(spawn_with_output!(sh -c "while :; do :; done")?
-        .wait_with_pipe(&mut |_stdout| {})
-        .is_ok());
-
-    // wait_with_borrowed_pipe() checks the exit status of the last child, even if pipefail is disabled
-    set_pipefail(false);
-    assert!(spawn_with_output!(true | false)?
-        .wait_with_borrowed_pipe(&mut |_stdout| {})
-        .is_err());
-    assert!(spawn_with_output!(true | true)?
-        .wait_with_borrowed_pipe(&mut |_stdout| {})
-        .is_ok());
-    assert!(spawn_with_output!(false)?
-        .wait_with_borrowed_pipe(&mut |_stdout| {})
-        .is_err());
-    assert!(spawn_with_output!(true)?
-        .wait_with_borrowed_pipe(&mut |_stdout| {})
-        .is_ok());
-    set_pipefail(true);
-    // wait_with_borrowed_pipe() checks the exit status of the other children, unless pipefail is disabled
-    set_pipefail(false);
-    assert!(spawn_with_output!(false | true)?
-        .wait_with_borrowed_pipe(&mut |_stdout| {})
-        .is_ok());
-    set_pipefail(true);
-    assert!(spawn_with_output!(false | true)?
-        .wait_with_borrowed_pipe(&mut |_stdout| {})
-        .is_err());
-    assert!(spawn_with_output!(true | true)?
-        .wait_with_borrowed_pipe(&mut |_stdout| {})
-        .is_ok());
-    // wait_with_borrowed_pipe() handles `ignore`
-    assert!(spawn_with_output!(ignore false | true)?
-        .wait_with_borrowed_pipe(&mut |_stdout| {})
-        .is_ok());
-    assert!(spawn_with_output!(ignore true | false)?
-        .wait_with_borrowed_pipe(&mut |_stdout| {})
-        .is_ok());
-    assert!(spawn_with_output!(ignore false)?
-        .wait_with_borrowed_pipe(&mut |_stdout| {})
-        .is_ok());
-
-    Ok(())
 }
 
 #[test]
