Repository: reubeno/brush
Pull request #136 (2024-07-23 10:09:44)

Fixes for relative paths, action failures


Patch:
diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml
index b82ceae..ca69d97 100644
--- a/.github/workflows/ci.yaml
+++ b/.github/workflows/ci.yaml
@@ -48,9 +48,6 @@ jobs:
       - name: Enable cargo cache
         uses: Swatinem/rust-cache@v2
 
-      - name: Install cargo-binstall
-        uses: cargo-bins/cargo-binstall@v1.7.4
-
       - name: Build
         run: cargo build --release
 
@@ -70,12 +67,15 @@ jobs:
           - target: "aarch64-unknown-linux-gnu"
             arch: "aarch64"
             os: "Linux"
+            binary_name: "brush"
           - target: "wasm32-wasip1"
             arch: "wasm32"
             os: "WASI-0.1"
+            binary_name: "brush.wasm"
           - target: "x86_64-pc-windows-gnu"
             arch: "x86_64"
             os: "Windows"
+            binary_name: "brush.exe"
 
     steps:
       - name: Checkout
@@ -110,7 +110,7 @@ jobs:
         uses: actions/upload-artifact@v4
         with:
           name: binaries-${{ matrix.target }}
-          path: target/${{ matrix.target }}/release/brush
+          path: target/${{ matrix.target }}/release/${{ matrix.binary_name }}
 
   test:
     name: "Test"
diff --git a/brush-core/src/builtins/command.rs b/brush-core/src/builtins/command.rs
index adbe0ab..ee0e6b7 100644
--- a/brush-core/src/builtins/command.rs
+++ b/brush-core/src/builtins/command.rs
@@ -1,5 +1,5 @@
 use clap::Parser;
-use std::{fmt::Display, io::Write};
+use std::{fmt::Display, io::Write, path::Path};
 
 use crate::{builtins, commands, error, shell, sys::fs::PathExt};
 
@@ -86,7 +86,7 @@ impl CommandCommand {
     fn try_find_command(&self, shell: &shell::Shell) -> Option<FoundCommand> {
         // Look in path.
         if self.command_name.contains(std::path::MAIN_SEPARATOR) {
-            let candidate_path = std::path::Path::new(&self.command_name);
+            let candidate_path = shell.get_absolute_path(Path::new(&self.command_name));
             if candidate_path.executable() {
                 Some(FoundCommand::External(
                     candidate_path
diff --git a/brush-core/src/builtins/type_.rs b/brush-core/src/builtins/type_.rs
index f20f74b..fded6e2 100644
--- a/brush-core/src/builtins/type_.rs
+++ b/brush-core/src/builtins/type_.rs
@@ -1,4 +1,4 @@
-use std::path::PathBuf;
+use std::path::{Path, PathBuf};
 use std::{io::Write, sync::Arc};
 
 use brush_parser::ast;
@@ -160,7 +160,7 @@ impl TypeCommand {
 
         // Look in path.
         if name.contains(std::path::MAIN_SEPARATOR) {
-            if std::path::Path::new(name).executable() {
+            if shell.get_absolute_path(Path::new(name)).executable() {
                 types.push(ResolvedType::File(PathBuf::from(name)));
             }
         } else {
diff --git a/brush-core/src/interp.rs b/brush-core/src/interp.rs
index 6817f79..9389c54 100644
--- a/brush-core/src/interp.rs
+++ b/brush-core/src/interp.rs
@@ -6,6 +6,7 @@ use std::io::Write;
 use std::os::fd::{AsFd, AsRawFd};
 #[cfg(unix)]
 use std::os::unix::process::ExitStatusExt;
+use std::path::{Path, PathBuf};
 use std::sync::Arc;
 
 use crate::arithmetic::ExpandAndEvaluate;
@@ -1272,20 +1273,26 @@ pub(crate) async fn setup_redirect<'a>(
 ) -> Result<Option<u32>, error::Error> {
     match redirect {
         ast::IoRedirect::OutputAndError(f, append) => {
-            let mut expanded_file_path = expansion::full_expand_and_split_word(shell, f).await?;
-            if expanded_file_path.len() != 1 {
+            let mut expanded_fields = expansion::full_expand_and_split_word(shell, f).await?;
+            if expanded_fields.len() != 1 {
                 return Err(error::Error::InvalidRedirection);
             }
 
-            let expanded_file_path = expanded_file_path.remove(0);
+            let expanded_file_path: PathBuf =
+                shell.get_absolute_path(Path::new(expanded_fields.remove(0).as_str()));
 
             let opened_file = std::fs::File::options()
                 .create(true)
                 .write(true)
                 .truncate(!*append)
                 .append(*append)
-                .open(expanded_file_path.as_str())
-                .map_err(|err| error::Error::RedirectionFailure(expanded_file_path, err))?;
+                .open(expanded_file_path.as_path())
+                .map_err(|err| {
+                    error::Error::RedirectionFailure(
+                        expanded_file_path.to_string_lossy().to_string(),
+                        err,
+                    )
+                })?;
 
             let stdout_file = OpenFile::File(opened_file);
             let stderr_file = stdout_file.try_dup()?;
@@ -1345,18 +1352,23 @@ pub(crate) async fn setup_redirect<'a>(
 
                     fd_num = specified_fd_num.unwrap_or(default_fd_if_unspecified);
 
-                    let mut expanded_file_path =
+                    let mut expanded_fields =
                         expansion::full_expand_and_split_word(shell, f).await?;
 
-                    if expanded_file_path.len() != 1 {
+                    if expanded_fields.len() != 1 {
                         return Err(error::Error::InvalidRedirection);
                     }
 
-                    let expanded_file_path = expanded_file_path.remove(0);
+                    let expanded_file_path: PathBuf =
+                        shell.get_absolute_path(Path::new(expanded_fields.remove(0).as_str()));
 
-                    let opened_file = options
-                        .open(expanded_file_path.as_str())
-                        .map_err(|err| error::Error::RedirectionFailure(expanded_file_path, err))?;
+                    let opened_file =
+                        options.open(expanded_file_path.as_path()).map_err(|err| {
+                            error::Error::RedirectionFailure(
+                                expanded_file_path.to_string_lossy().to_string(),
+                                err,
+                            )
+                        })?;
                     target_file = OpenFile::File(opened_file);
                 }
                 ast::IoFileRedirectTarget::Fd(fd) => {
diff --git a/brush-core/src/shell.rs b/brush-core/src/shell.rs
index 0094c12..fa7c04a 100644
--- a/brush-core/src/shell.rs
+++ b/brush-core/src/shell.rs
@@ -373,7 +373,9 @@ impl Shell {
     ) -> Result<ExecutionResult, error::Error> {
         tracing::debug!("sourcing: {}", path.display());
 
-        let opened_file = std::fs::File::open(path)
+        let path_to_open = self.get_absolute_path(path);
+
+        let opened_file = std::fs::File::open(path_to_open)
             .map_err(|e| error::Error::FailedSourcingFile(path.to_owned(), e))?;
 
         let file_metadata = opened_file
@@ -879,17 +881,26 @@ impl Shell {
         executables
     }
 
+    /// Gets the absolute form of the given path.
+    ///
+    /// # Arguments
+    ///
+    /// * `path` - The path to get the absolute form of.
+    pub(crate) fn get_absolute_path(&self, path: &Path) -> PathBuf {
+        if path.is_absolute() {
+            path.to_owned()
+        } else {
+            self.working_dir.join(path)
+        }
+    }
+
     /// Sets the shell's current working directory to the given path.
     ///
     /// # Arguments
     ///
     /// * `target_dir` - The path to set as the working directory.
     pub(crate) fn set_working_dir(&mut self, target_dir: &Path) -> Result<(), error::Error> {
-        let abs_path = if target_dir.is_absolute() {
-            PathBuf::from(target_dir)
-        } else {
-            self.working_dir.join(target_dir)
-        };
+        let abs_path = self.get_absolute_path(target_dir);
 
         match std::fs::metadata(&abs_path) {
             Ok(m) => {


Test patch:
diff --git a/brush-core/src/extendedtests.rs b/brush-core/src/extendedtests.rs
index 2a25eac..4fab511 100644
--- a/brush-core/src/extendedtests.rs
+++ b/brush-core/src/extendedtests.rs
@@ -64,47 +64,47 @@ pub(crate) fn apply_unary_predicate_to_str(
         ast::UnaryPredicate::StringHasNonZeroLength => Ok(!operand.is_empty()),
         ast::UnaryPredicate::StringHasZeroLength => Ok(operand.is_empty()),
         ast::UnaryPredicate::FileExists => {
-            let path = Path::new(operand);
+            let path = shell.get_absolute_path(Path::new(operand));
             Ok(path.exists())
         }
         ast::UnaryPredicate::FileExistsAndIsBlockSpecialFile => {
-            let path = Path::new(operand);
+            let path = shell.get_absolute_path(Path::new(operand));
             Ok(path.exists_and_is_block_device())
         }
         ast::UnaryPredicate::FileExistsAndIsCharSpecialFile => {
-            let path = Path::new(operand);
+            let path = shell.get_absolute_path(Path::new(operand));
             Ok(path.exists_and_is_char_device())
         }
         ast::UnaryPredicate::FileExistsAndIsDir => {
-            let path = Path::new(operand);
+            let path = shell.get_absolute_path(Path::new(operand));
             Ok(path.is_dir())
         }
         ast::UnaryPredicate::FileExistsAndIsRegularFile => {
-            let path = Path::new(operand);
+            let path = shell.get_absolute_path(Path::new(operand));
             Ok(path.is_file())
         }
         ast::UnaryPredicate::FileExistsAndIsSetgid => {
-            let path = Path::new(operand);
+            let path = shell.get_absolute_path(Path::new(operand));
             Ok(path.exists_and_is_setgid())
         }
         ast::UnaryPredicate::FileExistsAndIsSymlink => {
-            let path = Path::new(operand);
+            let path = shell.get_absolute_path(Path::new(operand));
             Ok(path.is_symlink())
         }
         ast::UnaryPredicate::FileExistsAndHasStickyBit => {
-            let path = Path::new(operand);
+            let path = shell.get_absolute_path(Path::new(operand));
             Ok(path.exists_and_is_sticky_bit())
         }
         ast::UnaryPredicate::FileExistsAndIsFifo => {
-            let path = Path::new(operand);
+            let path = shell.get_absolute_path(Path::new(operand));
             Ok(path.exists_and_is_fifo())
         }
         ast::UnaryPredicate::FileExistsAndIsReadable => {
-            let path = Path::new(operand);
+            let path = shell.get_absolute_path(Path::new(operand));
             Ok(path.readable())
         }
         ast::UnaryPredicate::FileExistsAndIsNotZeroLength => {
-            let path = Path::new(operand);
+            let path = shell.get_absolute_path(Path::new(operand));
             if let Ok(metadata) = path.metadata() {
                 Ok(metadata.len() > 0)
             } else {
@@ -123,19 +123,19 @@ pub(crate) fn apply_unary_predicate_to_str(
             }
         }
         ast::UnaryPredicate::FileExistsAndIsSetuid => {
-            let path = Path::new(operand);
+            let path = shell.get_absolute_path(Path::new(operand));
             Ok(path.exists_and_is_setuid())
         }
         ast::UnaryPredicate::FileExistsAndIsWritable => {
-            let path = Path::new(operand);
+            let path = shell.get_absolute_path(Path::new(operand));
             Ok(path.writable())
         }
         ast::UnaryPredicate::FileExistsAndIsExecutable => {
-            let path = Path::new(operand);
+            let path = shell.get_absolute_path(Path::new(operand));
             Ok(path.executable())
         }
         ast::UnaryPredicate::FileExistsAndOwnedByEffectiveGroupId => {
-            let path = Path::new(operand);
+            let path = shell.get_absolute_path(Path::new(operand));
             if !path.exists() {
                 return Ok(false);
             }
@@ -147,7 +147,7 @@ pub(crate) fn apply_unary_predicate_to_str(
             error::unimp("unary extended test predicate: FileExistsAndModifiedSinceLastRead")
         }
         ast::UnaryPredicate::FileExistsAndOwnedByEffectiveUserId => {
-            let path = Path::new(operand);
+            let path = shell.get_absolute_path(Path::new(operand));
             if !path.exists() {
                 return Ok(false);
             }
@@ -156,7 +156,7 @@ pub(crate) fn apply_unary_predicate_to_str(
             Ok(md.uid() == users::get_effective_uid()?)
         }
         ast::UnaryPredicate::FileExistsAndIsSocket => {
-            let path = Path::new(operand);
+            let path = shell.get_absolute_path(Path::new(operand));
             Ok(path.exists_and_is_socket())
         }
         ast::UnaryPredicate::ShellOptionEnabled => {