fix(interpreter): correct left-to-right redirect ordering for fd dup + file combos#863
Merged
fix(interpreter): correct left-to-right redirect ordering for fd dup + file combos#863
Conversation
…+ file combos Closes #853 — `result=$(cmd 2>&1 >file)` now correctly captures stderr in result and writes stdout to file. The old approach processed redirections by mutating stdout/stderr strings sequentially, which broke when DupOutput and file redirects were mixed. `2>&1` would merge stderr into stdout, then `>file` would write the merged string to the file — losing the fd identity. New approach: when DupOutput and file redirects coexist, build an fd table (fd1/fd2 targets) left-to-right — dup operations copy the current target — then route original stdout/stderr based on the final fd table. Also fixes dev_null tests that were asserting the old incorrect behavior for `echo error >&2 2>/dev/null` (which in real bash sends output to stderr, not /dev/null, because `>&2` copies fd2 before `2>/dev/null` redirects it).
console 0.15.11→0.16.3, insta 1.46.3→1.47.0, simd-adler32 0.3.8→0.3.9
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Closes #853
result=$(cmd 2>&1 >file)which incorrectly captured empty string instead of stderrDupOutputand file redirects are mixed>&2 2>/dev/nullpatternsWhat
Bash processes redirects left-to-right, building an fd table where each dup copies the current target of the source fd. For
2>&1 >file:2>&1— fd2 = copy(fd1) = capture pipe>file— fd1 = fileSo stderr goes to the capture pipe (result) and stdout goes to the file.
The old implementation mutated stdout/stderr strings sequentially —
2>&1merged stderr into stdout, then>filewrote the merged string to the file, losing the fd identity.How
DupOutputand file redirects, use a newapply_redirections_fd_tablemethod that builds an abstract fd table first, then routes original stdout/stderr based on the final mappingecho error >&2 2>/dev/nullshould output to stderr in real bash, not suppress it)Tests
redirect_2_to_1_then_file_in_cmdsub— core reproduction from issueredirect_2_to_1_then_file_outside_cmdsub— same pattern without command substitutionredirect_file_then_2_to_1— reverse order (>file 2>&1sends both to file)test_dev_null_stderr_redirect/test_dev_null_append_stderr— corrected to test proper fd semantics