Skip to content
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

fix: correct multiple issues with process substitution + redirection #145

Merged
merged 1 commit into from
Aug 4, 2024

Conversation

reubeno
Copy link
Owner

@reubeno reubeno commented Aug 3, 2024

The prior implementation of process substitution (e.g., <(command args)) was a bit confused. This isn't perfect, but should be significantly more accurate. With these changes, brush:

  • Supports use of <(command ...) or >(command ...) for any argument to a simple command.
  • Supports use of <(command ...) or >(command ...) as the filename for input or output redirection anywhere those redirections are supported.
  • When executing a process substitution command in a subshell, does not synchronously wait for it. (This resolves a known issue with >(command ...).

Also adds more test cases to capture this issue, what's working now, and what is better understood to still differ in behavior. (Notably, bash appears to accept process substitutions in a wider variety of places in its grammar. Perhaps not usefully in some cases, but we added a test case for completeness nonetheless.)

Fixes #144 (thanks to @39555 for finding and reporting it!)

Copy link

github-actions bot commented Aug 3, 2024

Test Results

408 tests   394 ✅  24s ⏱️
 64 suites   14 💤
  8 files      0 ❌

Results for commit 96ba8a0.

♻️ This comment has been updated with latest results.

Copy link

github-actions bot commented Aug 3, 2024

Performance Benchmark Report

Benchmark name Baseline (ns) Test/PR (ns) Delta (ns) Delta %
expand_one_string 3607 ns 3605 ns -2 ns 🟢 -0.06%
instantiate_shell 60658 ns 60908 ns +250 ns 🟠 +0.41%
instantiate_shell_with_init_scripts 27516557 ns 27620529 ns +103972 ns 🟠 +0.38%
parse_bash_completion 5651162 ns 5618331 ns -32831 ns 🟢 -0.58%
parse_sample_script 9069 ns 8897 ns -172 ns 🟢 -1.90%
run_echo_builtin_command 93844 ns 92710 ns -1134 ns 🟢 -1.21%
run_one_builtin_command 112614 ns 111154 ns -1460 ns 🟢 -1.30%
run_one_external_command 1901185 ns 2018117 ns +116932 ns 🟠 +6.15%
run_one_external_command_directly 999023 ns 999415 ns +392 ns 🟠 +0.04%

Code Coverage Report

Package Base Coverage New Coverage Difference
brush-core/src/arithmetic.rs 🟢 98.6% 🟢 98.6% ⚪ 0%
brush-core/src/builtins.rs 🔴 31.21% 🔴 31.21% ⚪ 0%
brush-core/src/builtins/alias.rs 🟢 83.87% 🟢 83.87% ⚪ 0%
brush-core/src/builtins/bg.rs 🔴 0% 🔴 0% ⚪ 0%
brush-core/src/builtins/break_.rs 🟢 90% 🟢 90% ⚪ 0%
brush-core/src/builtins/builtin_.rs 🟢 100% 🟢 100% ⚪ 0%
brush-core/src/builtins/cd.rs 🟢 88.57% 🟢 88.57% ⚪ 0%
brush-core/src/builtins/colon.rs 🟢 100% 🟢 100% ⚪ 0%
brush-core/src/builtins/command.rs 🟢 93.26% 🟢 93.26% ⚪ 0%
brush-core/src/builtins/complete.rs 🔴 21.48% 🔴 21.48% ⚪ 0%
brush-core/src/builtins/continue_.rs 🟢 91.67% 🟢 91.67% ⚪ 0%
brush-core/src/builtins/declare.rs 🟢 82.83% 🟢 82.83% ⚪ 0%
brush-core/src/builtins/dirs.rs 🟢 90.48% 🟢 90.48% ⚪ 0%
brush-core/src/builtins/dot.rs 🟢 95.65% 🟢 95.65% ⚪ 0%
brush-core/src/builtins/echo.rs 🟢 86.84% 🟢 86.84% ⚪ 0%
brush-core/src/builtins/enable.rs 🟢 91.07% 🟢 91.07% ⚪ 0%
brush-core/src/builtins/eval.rs 🟢 100% 🟢 100% ⚪ 0%
brush-core/src/builtins/exec.rs 🟢 90.32% 🟢 90.32% ⚪ 0%
brush-core/src/builtins/exit.rs 🟢 100% 🟢 100% ⚪ 0%
brush-core/src/builtins/export.rs 🟢 88.73% 🟢 88.73% ⚪ 0%
brush-core/src/builtins/factory.rs 🟢 82.2% 🟢 82.2% ⚪ 0%
brush-core/src/builtins/false_.rs 🟢 100% 🟢 100% ⚪ 0%
brush-core/src/builtins/fg.rs 🔴 0% 🔴 0% ⚪ 0%
brush-core/src/builtins/getopts.rs 🟢 97.7% 🟢 97.7% ⚪ 0%
brush-core/src/builtins/help.rs 🟢 90.7% 🟢 90.7% ⚪ 0%
brush-core/src/builtins/jobs.rs 🔴 41.86% 🔴 41.86% ⚪ 0%
brush-core/src/builtins/kill.rs 🔴 0% 🔴 0% ⚪ 0%
brush-core/src/builtins/let_.rs 🟢 100% 🟢 100% ⚪ 0%
brush-core/src/builtins/popd.rs 🟢 94.12% 🟢 94.12% ⚪ 0%
brush-core/src/builtins/printf.rs 🟢 91.18% 🟢 91.18% ⚪ 0%
brush-core/src/builtins/pushd.rs 🟢 91.3% 🟢 91.3% ⚪ 0%
brush-core/src/builtins/pwd.rs 🟢 89.47% 🟢 89.47% ⚪ 0%
brush-core/src/builtins/read.rs 🟢 76.69% 🟢 76.69% ⚪ 0%
brush-core/src/builtins/return_.rs 🟢 83.33% 🟢 83.33% ⚪ 0%
brush-core/src/builtins/set.rs 🟢 76.64% 🟢 76.64% ⚪ 0%
brush-core/src/builtins/shift.rs 🟢 100% 🟢 100% ⚪ 0%
brush-core/src/builtins/shopt.rs 🟢 88.39% 🟢 88.39% ⚪ 0%
brush-core/src/builtins/test.rs 🟢 100% 🟢 100% ⚪ 0%
brush-core/src/builtins/trap.rs 🟢 82.95% 🟢 82.95% ⚪ 0%
brush-core/src/builtins/true_.rs 🟢 100% 🟢 100% ⚪ 0%
brush-core/src/builtins/type_.rs 🟢 90.27% 🟢 90.27% ⚪ 0%
brush-core/src/builtins/umask.rs 🔴 0% 🔴 0% ⚪ 0%
brush-core/src/builtins/unalias.rs 🔴 0% 🔴 0% ⚪ 0%
brush-core/src/builtins/unimp.rs 🔴 0% 🔴 0% ⚪ 0%
brush-core/src/builtins/unset.rs 🟢 94.64% 🟢 94.64% ⚪ 0%
brush-core/src/builtins/wait.rs 🟢 85.71% 🟢 85.71% ⚪ 0%
brush-core/src/commands.rs 🟢 92.82% 🟢 92.82% ⚪ 0%
brush-core/src/completion.rs 🔴 42.73% 🔴 42.73% ⚪ 0%
brush-core/src/env.rs 🟢 87.85% 🟢 87.85% ⚪ 0%
brush-core/src/error.rs 🔴 25% 🔴 25% ⚪ 0%
brush-core/src/escape.rs 🟢 84.52% 🟢 84.52% ⚪ 0%
brush-core/src/expansion.rs 🟢 95.96% 🟢 95.96% ⚪ 0%
brush-core/src/extendedtests.rs 🟠 69.2% 🟠 69.2% ⚪ 0%
brush-core/src/functions.rs 🟢 100% 🟢 100% ⚪ 0%
brush-core/src/interp.rs 🟢 92.5% 🟢 93.47% 🟢 0.97%
brush-core/src/jobs.rs 🔴 41.3% 🔴 43.48% 🟢 2.18%
brush-core/src/keywords.rs 🟢 96.88% 🟢 96.88% ⚪ 0%
brush-core/src/namedoptions.rs 🟠 51.92% 🟠 51.92% ⚪ 0%
brush-core/src/openfiles.rs 🔴 47.22% 🔴 47.92% 🟢 0.7%
brush-core/src/options.rs 🟢 79.17% 🟢 79.17% ⚪ 0%
brush-core/src/patterns.rs 🟢 96.98% 🟢 96.98% ⚪ 0%
brush-core/src/prompt.rs 🟢 78.48% 🟢 78.48% ⚪ 0%
brush-core/src/regex.rs 🟢 100% 🟢 100% ⚪ 0%
brush-core/src/shell.rs 🟢 77.95% 🟢 77.95% ⚪ 0%
brush-core/src/sys/hostname.rs 🟢 100% 🟢 100% ⚪ 0%
brush-core/src/sys/tokio_process.rs 🟢 100% 🟢 100% ⚪ 0%
brush-core/src/sys/unix/fs.rs 🟠 54.76% 🟠 54.76% ⚪ 0%
brush-core/src/sys/unix/network.rs 🟢 100% 🟢 100% ⚪ 0%
brush-core/src/sys/unix/signal.rs 🔴 40% 🔴 40% ⚪ 0%
brush-core/src/sys/unix/terminal.rs 🔴 0% 🔴 0% ⚪ 0%
brush-core/src/sys/unix/users.rs 🔴 16.13% 🔴 16.13% ⚪ 0%
brush-core/src/tests.rs 🟠 50% 🟠 50% ⚪ 0%
brush-core/src/traps.rs 🟠 50% 🟠 50% ⚪ 0%
brush-core/src/variables.rs 🟢 79.91% 🟢 79.91% ⚪ 0%
brush-interactive/src/interactive_shell.rs 🟢 78.37% 🟢 78.37% ⚪ 0%
brush-parser/src/arithmetic.rs 🟢 100% 🟢 100% ⚪ 0%
brush-parser/src/ast.rs 🔴 24.91% 🔴 24.56% 🔴 -0.35%
brush-parser/src/error.rs 🟢 76.92% 🟢 76.92% ⚪ 0%
brush-parser/src/parser.rs 🟢 99.6% 🟢 99.6% ⚪ 0%
brush-parser/src/pattern.rs 🟢 100% 🟢 100% ⚪ 0%
brush-parser/src/prompt.rs 🟢 100% 🟢 100% ⚪ 0%
brush-parser/src/test_command.rs 🟢 100% 🟢 100% ⚪ 0%
brush-parser/src/tokenizer.rs 🟢 96.01% 🟢 96.01% ⚪ 0%
brush-parser/src/word.rs 🟢 100% 🟢 100% ⚪ 0%
brush-shell/src/brushctl.rs 🔴 16.67% 🔴 16.67% ⚪ 0%
brush-shell/src/events.rs 🔴 37.23% 🔴 37.23% ⚪ 0%
brush-shell/src/main.rs 🟢 80.43% 🟢 80.43% ⚪ 0%
brush-shell/src/productinfo.rs 🟢 100% 🟢 100% ⚪ 0%
Overall Coverage 🟢 78.01% 🟢 78.13% 🟢 0.12%

Minimum allowed coverage is 0%, this run produced 78.13%

@reubeno reubeno merged commit 1b04a01 into main Aug 4, 2024
10 checks passed
@reubeno reubeno deleted the subst-redir branch August 4, 2024 00:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Syntax error with input redirection combined with process substitution
1 participant