Various fixes #364
Various fixes #364
Conversation
The test cases were disabled due to a bug, which was fixed in commit 3662079.
When a trap action is interrupted by a shell error, the exit status of the trap action should be propagated to the calling context. This is necessary to ensure that the exit status of the shell reflects the error.
This commit applies the same fix as in commit 5e82a55 to the EXIT trap. If the EXIT trap is interrupted by a shell error, the exit status of the shell error is propagated to the calling context, which means that the exit status of the shell error overrides the exit status of the shell. The `run_trap` function is refactored to be used by both the EXIT and signal traps. The `run_trap` function now takes a `Condition` argument to specify the type of the trap, which is no longer limited to signals.
This commit refactors the closure parameter of Subshell::new to return a Future that resolves to () instead of Result. The Subshell implementation no longer calls Env::apply_result on the result of the closure. Instead, Env::apply_result must be called explicitly in the closure, if necessary. This commit is a preparation for the upcoming fix for the issue that the EXIT trap is not run in subshells. The EXIT trap should be run after Env::apply_result is called in the closure, which cannot be implemented in the yash-env crate as trap execution depends on the yash-semantics crate.
Every subshell that may run the trap built-in should run the EXIT trap on termination. This includes: - Subshells created by command substitution - Subshells created by asynchronous and-or lists - Subshells created by job-controlled pipelines This commit adds a call to `run_exit_trap` in the appropriate places.
WalkthroughThis comprehensive update streamlines and refines the Yash project's codebase across multiple modules. Changes focus on simplifying asynchronous operations, refining error handling, updating test scripts, and enhancing functionality related to file system permissions and signal trapping. The modifications aim to boost the system's robustness and prepare it for a feature-complete beta release. Changes
Possibly related issues
Poem
Recent Review DetailsConfiguration used: CodeRabbit UI Files selected for processing (23)
Files skipped from review due to trivial changes (2)
Additional Context UsedLanguageTool (30)
AST-based Instructions (2)
ShellCheck (184)
Learnings (2)
Additional comments not posted (23)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
Out of diff range and nitpick comments (6)
yash-cli/tests/scripted_test/path-p.sh (2)
Line range hint
1-1: Add a shebang line to specify the shell interpreter.+ #!/bin/sh
Line range hint
6-6: Multiple redirections compete for stdout. Consider using 'cat', 'tee', or specifying filenames directly.- >foo/dir/file >foo/no_read_dir/file >foo/no_search_dir/file + echo "" | tee foo/dir/file foo/no_read_dir/file foo/no_search_dir/file > /dev/nullyash-cli/tests/scripted_test/trap-p.sh (1)
Line range hint
1-1: Add a shebang line to specify the shell interpreter.+ #!/bin/shyash-cli/tests/scripted_test/quote-p.sh (1)
Line range hint
13-13: Syntax error in test expression. Ensure proper formatting to allow further checks.- test_oE 'backslash (not preceding newline)' + test_oE 'backslash (not preceding newline)' {yash-cli/tests/scripted_test/redir-p.sh (2)
Line range hint
435-435: Fix the syntax error in the here-document.- cat <<E'N'D && + cat <<'END' &&
Line range hint
566-566: Ensure the here-document is correctly terminated.- cat << -END + cat <<-'END'
Addresses the remaining failing scripted tests tracked in #336
Summary by CodeRabbit
New Features
Bug Fixes
Refactor
Documentation