feat: rework here doc files (#85)

Back here-documents and here-strings with pipes; this simplifies their implementation and makes them compatible with built-in commands.

brush-core/src/builtins/exec.rs

@@ -39,7 +39,7 @@ impl builtin::Command for ExecCommand {
             argv0 = Cow::Owned(std::format!("-{argv0}"));
         }
 
-        let (mut cmd, here_doc) = commands::compose_std_command(
+        let mut cmd = commands::compose_std_command(
             context.shell,
             &self.args[0],
             argv0.as_str(),
@@ -48,10 +48,6 @@ impl builtin::Command for ExecCommand {
             self.empty_environment,
         )?;
 
-        if here_doc.is_some() {
-            return error::unimp("exec with here doc");
-        }
-
         let exec_error = cmd.exec();
 
         if exec_error.kind() == std::io::ErrorKind::NotFound {

brush-core/src/commands.rs

@@ -3,7 +3,7 @@ use std::{ffi::OsStr, fmt::Display, os::unix::process::CommandExt, process::Stdi
 use brush_parser::ast;
 use command_fds::{CommandFdExt, FdMapping};
 use itertools::Itertools;
-use tokio::{io::AsyncWriteExt, process};
+use tokio::process;
 
 use crate::{
     builtin, error,
@@ -98,7 +98,7 @@ pub(crate) fn compose_std_command<S: AsRef<OsStr>>(
     args: &[S],
     mut open_files: OpenFiles,
     empty_env: bool,
-) -> Result<(std::process::Command, Option<String>), error::Error> {
+) -> Result<std::process::Command, error::Error> {
     let mut cmd = std::process::Command::new(command_name);
 
     // Override argv[0].
@@ -126,14 +126,12 @@ pub(crate) fn compose_std_command<S: AsRef<OsStr>>(
     }
 
     // Redirect stdin, if applicable.
-    let mut stdin_here_doc = None;
-    if let Some(stdin_file) = open_files.files.remove(&0) {
-        if let OpenFile::HereDocument(doc) = &stdin_file {
-            stdin_here_doc = Some(doc.clone());
+    match open_files.files.remove(&0) {
+        Some(OpenFile::Stdin) | None => (),
+        Some(stdin_file) => {
+            let as_stdio: Stdio = stdin_file.into();
+            cmd.stdin(as_stdio);
         }
-
-        let as_stdio: Stdio = stdin_file.into();
-        cmd.stdin(as_stdio);
     }
 
     // Redirect stdout, if applicable.
@@ -175,7 +173,7 @@ pub(crate) fn compose_std_command<S: AsRef<OsStr>>(
         }
     }
 
-    Ok((cmd, stdin_here_doc))
+    Ok(cmd)
 }
 
 pub(crate) async fn execute(
@@ -218,11 +216,11 @@ pub(crate) async fn execute(
     }
 
     // Strip the command name off args.
-    execute_external_command(cmd_context, &args[1..]).await
+    execute_external_command(cmd_context, &args[1..])
 }
 
 #[allow(clippy::too_many_lines)]
-pub(crate) async fn execute_external_command(
+pub(crate) fn execute_external_command(
     context: ExecutionContext<'_>,
     args: &[CommandArg],
 ) -> Result<SpawnResult, error::Error> {
@@ -235,7 +233,7 @@ pub(crate) async fn execute_external_command(
     }
 
     // Compose the std::process::Command that encapsulates what we want to launch.
-    let (cmd, stdin_here_doc) = compose_std_command(
+    let cmd = compose_std_command(
         context.shell,
         context.command_name.as_str(),
         context.command_name.as_str(),
@@ -258,16 +256,7 @@ pub(crate) async fn execute_external_command(
     let mut cmd = tokio::process::Command::from(cmd);
 
     match cmd.spawn() {
-        Ok(mut child) => {
-            // Special case: handle writing here document, if needed.
-            if let Some(doc) = stdin_here_doc {
-                if let Some(mut child_stdin) = child.stdin.take() {
-                    child_stdin.write_all(doc.as_bytes()).await?;
-                }
-            }
-
-            Ok(SpawnResult::SpawnedChild(child))
-        }
+        Ok(child) => Ok(SpawnResult::SpawnedChild(child)),
         Err(e) if e.kind() == std::io::ErrorKind::NotFound => {
             if context.shell.options.sh_mode {
                 tracing::error!(

brush-core/src/interp.rs

@@ -2,6 +2,7 @@ use brush_parser::ast::{self, CommandPrefixOrSuffixItem};
 use itertools::Itertools;
 use std::collections::VecDeque;
 use std::io::Write;
+use std::os::fd::{AsFd, AsRawFd};
 #[cfg(unix)]
 use std::os::unix::process::ExitStatusExt;
 use std::sync::Arc;
@@ -1433,9 +1434,9 @@ pub(crate) async fn setup_redirect<'a>(
             // TODO: figure out if we need to expand?
             let io_here_doc = io_here.doc.flatten();
 
-            open_files
-                .files
-                .insert(fd_num, OpenFile::HereDocument(io_here_doc));
+            let f = setup_open_file_with_contents(io_here_doc.as_str())?;
+
+            open_files.files.insert(fd_num, f);
             Ok(Some(fd_num))
         }
         ast::IoRedirect::HereString(fd_num, word) => {
@@ -1445,14 +1446,31 @@ pub(crate) async fn setup_redirect<'a>(
             let mut expanded_word = expansion::basic_expand_word(shell, word).await?;
             expanded_word.push('\n');
 
-            open_files
-                .files
-                .insert(fd_num, OpenFile::HereDocument(expanded_word));
+            let f = setup_open_file_with_contents(expanded_word.as_str())?;
+
+            open_files.files.insert(fd_num, f);
             Ok(Some(fd_num))
         }
     }
 }
 
+fn setup_open_file_with_contents(contents: &str) -> Result<OpenFile, error::Error> {
+    let (reader, mut writer) = os_pipe::pipe()?;
+
+    let bytes = contents.as_bytes();
+    let len = i32::try_from(bytes.len())?;
+
+    nix::fcntl::fcntl(
+        reader.as_fd().as_raw_fd(),
+        nix::fcntl::FcntlArg::F_SETPIPE_SZ(len),
+    )?;
+
+    writer.write_all(bytes)?;
+    drop(writer);
+
+    Ok(OpenFile::PipeReader(reader))
+}
+
 #[cfg(not(unix))]
 struct FakeSignal {}
 

brush-core/src/openfiles.rs

@@ -26,8 +26,6 @@ pub enum OpenFile {
     PipeReader(os_pipe::PipeReader),
     /// A write end of a pipe.
     PipeWriter(os_pipe::PipeWriter),
-    /// A here document.
-    HereDocument(String),
 }
 
 impl OpenFile {
@@ -41,7 +39,6 @@ impl OpenFile {
             OpenFile::File(f) => OpenFile::File(f.try_clone()?),
             OpenFile::PipeReader(f) => OpenFile::PipeReader(f.try_clone()?),
             OpenFile::PipeWriter(f) => OpenFile::PipeWriter(f.try_clone()?),
-            OpenFile::HereDocument(doc) => OpenFile::HereDocument(doc.clone()),
         };
 
         Ok(result)
@@ -58,7 +55,6 @@ impl OpenFile {
             OpenFile::File(f) => Ok(f.into()),
             OpenFile::PipeReader(r) => Ok(OwnedFd::from(r)),
             OpenFile::PipeWriter(w) => Ok(OwnedFd::from(w)),
-            OpenFile::HereDocument(_) => error::unimp("to_owned_fd for here doc"),
         }
     }
 
@@ -74,7 +70,6 @@ impl OpenFile {
             OpenFile::File(f) => Ok(f.as_raw_fd()),
             OpenFile::PipeReader(r) => Ok(r.as_raw_fd()),
             OpenFile::PipeWriter(w) => Ok(w.as_raw_fd()),
-            OpenFile::HereDocument(_) => error::unimp("as_raw_fd for here doc"),
         }
     }
 
@@ -87,7 +82,6 @@ impl OpenFile {
             OpenFile::File(f) => f.is_terminal(),
             OpenFile::PipeReader(_) => false,
             OpenFile::PipeWriter(_) => false,
-            OpenFile::HereDocument(_) => false,
         }
     }
 
@@ -104,7 +98,6 @@ impl OpenFile {
             OpenFile::File(f) => Some(nix::sys::termios::tcgetattr(f)?),
             OpenFile::PipeReader(_) => None,
             OpenFile::PipeWriter(_) => None,
-            OpenFile::HereDocument(_) => None,
         };
         Ok(result)
     }
@@ -122,7 +115,6 @@ impl OpenFile {
             OpenFile::File(f) => nix::sys::termios::tcsetattr(f, action, termios)?,
             OpenFile::PipeReader(_) => (),
             OpenFile::PipeWriter(_) => (),
-            OpenFile::HereDocument(_) => (),
         }
         Ok(())
     }
@@ -138,7 +130,6 @@ impl From<OpenFile> for Stdio {
             OpenFile::File(f) => f.into(),
             OpenFile::PipeReader(f) => f.into(),
             OpenFile::PipeWriter(f) => f.into(),
-            OpenFile::HereDocument(_) => Stdio::piped(),
         }
     }
 }
@@ -159,9 +150,6 @@ impl std::io::Read for OpenFile {
             OpenFile::PipeWriter(_) => Err(std::io::Error::other(
                 error::Error::OpenFileNotReadable("pipe writer"),
             )),
-            OpenFile::HereDocument(_) => Err(std::io::Error::other(
-                error::Error::OpenFileNotReadable("here document"),
-            )),
         }
     }
 }
@@ -180,9 +168,6 @@ impl std::io::Write for OpenFile {
                 error::Error::OpenFileNotWritable("pipe reader"),
             )),
             OpenFile::PipeWriter(writer) => writer.write(buf),
-            OpenFile::HereDocument(_) => Err(std::io::Error::other(
-                error::Error::OpenFileNotWritable("here document"),
-            )),
         }
     }
 
@@ -195,7 +180,6 @@ impl std::io::Write for OpenFile {
             OpenFile::File(f) => f.flush(),
             OpenFile::PipeReader(_) => Ok(()),
             OpenFile::PipeWriter(writer) => writer.flush(),
-            OpenFile::HereDocument(_) => Ok(()),
         }
     }
 }

brush-shell/tests/cases/builtins/read.yaml

@@ -21,7 +21,6 @@ cases:
       (echo "a b") | while read -a arr; do declare -p arr; done
 
   - name: "read from here string"
-    known_failure: true
     stdin: |
       read myvar <<< "hello"
       echo "myvar: ${myvar}"
@@ -31,9 +30,3 @@ cases:
     stdin: |
       read myvar < <(echo hello)
       echo "myvar: ${myvar}"
-
-  - name: "read from command substitution"
-    known_failure: true
-    stdin: |
-      read myvar <<< "$(echo hello)"
-      echo "myvar: ${myvar}"

brush-shell/tests/cases/here.yaml

@@ -11,6 +11,23 @@ cases:
           echo "This is after"
     args: ["./script.sh"]
 
+  - name: "Here doc with expansions"
+    known_failure: true
+    stdin: |
+      cat <<END-MARKER
+      Something here...
+      ...and here.
+      $(echo "This is after")
+      END-MARKER
+
+  - name: "Here doc with tab removal"
+    known_failure: true
+    stdin: |
+      cat <<-END-MARKER
+      	Something here...
+      	...and here.
+      	END-MARKER
+
   - name: "Basic here string"
     stdin: |
       shopt -ou posix