Repository navigation
Conversation
…hitespace at its ends `Expansion::post_subshell_expansion` is one scan. Each run of space, tab or newline in the output ends a field through `end_field`, also at the start and at the end of the output. So `x$(echo " a")` is `x` and `a`, and `$(printf 'a\tb')` is `a` and `b`, as in sh. `strip_trailing_line_ends` removes the newlines at the end of the output, and the CRs before them, before the split. In the output, the CRs directly before a newline are a part of that line end. Every other CR is data. `transition_to_glob_state` does not give a field with no bytes to the glob walk.
Collaborator
Author
Collaborator
Author
|
Status Reproduced on Bun 1.4.2 and on the head of #46691. bun exec 'args x$(echo " a")' # <xa>, one argument. bash and dash: <x><a>
bun exec 'args $(printf "a\tb")' # one argument with a tab. bash and dash: <a><b>With this PR both commands give the bash result. CI (build 136865): This PR stays a draft. It waits for three things:
PR: #47738 |
This branch has not been deployed
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.
Draft, stacked on #46691. It lands after #45968 and the per-field change (Notes). No user reported this.
Problem
$(...)output, or at a tab.args x$(echo " a")passes<xa>. sh passes<x><a>.Expansion::post_subshell_expansion(src/runtime/shell/states/Expansion.rs:866) trims both ends, then splits only at spaces.Fix
post_subshell_expansionis one scan. Each run of space, tab or newline ends a field throughend_field, also at both ends.transition_to_glob_stategives no empty field to the glob walk.test/js/bun/shell/bunshell.test.ts(18 of 23 new tests fail on the base), the rest oftest/js/bun/shell/(Notes). Self-reviewed: 38 concerns raised, 28 addressed.Background
end_field(from shell: expand${NAME}and the${NAME<op>word}forms #46691) ends an argument inside a word. It drops one with no bytes and no"".IFSas in shell: honor IFS when field-splitting command substitutions #33552 (an allocation per$(...)).Downsides
x$(echo ' a')is two arguments,> out$(echo ' x')isambiguous redirect. Notes lists each for a yes or no.${NAME}and the${NAME<op>word}forms #46691 alone, 5 of 77 probe words get worse ({a,b}$(echo 'c ')gives{a,b}c), and a[[ ]]operand loses a tab..text-1,280 bytes.$(echo abc)-51 instructions. Whitespace at an end: +5 to +61.Notes
Where this comes from
IFS, plus a customIFS. A stale sweep closed it with no review.Decisions for a maintainer. Each one changes the arguments of a script that runs today. Released 1.4.2 gives the "before" column.
x$(echo ' a')isxa,--n=$(echo ' 5')is--n=5x,aand--n=,5$(printf 'a\tb')is one argumenta,becho hi > x$(echo ' foo')writesxfooambiguous redirect, exit code 1$(printf 'a\r\nb\r\n')isa\r,ba,b(bash:a\r,b\r)x$(printf 'a\r')yisxayxa\ryIf the edge split is not wanted, the tab alone is two comparisons in the old loop. It needs #45968 first for the same reason as this PR.
The CR rule. No shell gives a precedent, because bash and dash keep every CR. Rows are every output of up to 5 bytes over a, space, tab, CR and LF, in 9 contexts. The last column is every output with a line end, printed with CR LF, compared with its LF form.
The rule is the same on every platform. The base has removed these CRs on every platform. CR LF output also reaches Linux and macOS with no Windows program: a CRLF file, HTTP headers, a pty.
Order of the stack
${NAME}and the${NAME<op>word}forms #46691. It addsend_field.[[ ]]operand to one word and match*on the right of == and != #45968. It makes a[[ ]]operand one word. TodayCondExpr::child_donejoins the fields of an operand, so the scan removes a tab or a CR LF from an operand:[[ $(printf 'a\tb') == "$(printf 'a\tb')" ]]is false here, and true on the base and in bash. 1,348 of 9,372 operand results move away from bash. Onetest.failingrow holds it.strip_trailing_line_ends. The copy here has the same name and body, so that the second one to land keeps one.robobun/954767e1/shell-field-glob). With it under the first cut of this scan, a local build gives the bash result for{a,b}$(echo " c"),{a,b}$(echo 'c '),*.txt$(echo ' '),{a,$(echo ' c')},{a,$(echo ' c')}{d,e},{$(echo 'c '),b}and{a$(echo ' '),b}.test.failingrows for a brace group and a glob then become plain tests.Worse than the base until step 4
*in it does not expand. On the base, the trim kept the word in one piece.{a,b}$(echo 'c ')gives{a,b}c(base:ac,bc).*.txt$(echo ' ')gives*.txt(base: the match).{a,$(echo ' c')}gives{a,andc}(base:a,c).{a,$(echo ' c')}{d,e}and{$(echo 'c '),b}are two more of the same kind.x*$(echo ' a')is the errorno matches found: a. On the base it isno matches found: x*a. The last field of a word with a*goes to the glob walk, as for*$(echo a b)on main.test.failingrows hold the bash results.Not changed here
~$(echo ' a')givesa(sh:~,a), and$(echo 'a ')~givesa,$HOME(sh:a,~). A~before a field with no bytes is no argument, as for~$UNSET. No open PR covers it. Twotest.failingrows hold the sh results.$(...)and an assignment value still lose their trailing spaces, tabs and CRs: shell: remove only trailing line ends from a quoted command substitution #45339.[[ ]]operand: shell: expand a[[ ]]operand to one word and match*on the right of == and != #45968.IFSis not read.exportoperand and the word of a${NAME:-$(...)}. Bun Shell has noforloop, so no loop takes a list.Rows inside
${NAME<op>word}. No release reads that form, sopre${U:-$(echo ' a ')}postchanges no released result. The scan is the same for every context.Arguments against bash 5.2.37 (debug builds, output from the
echobuiltin)xor""before it, and nothing,yor""after it): 49,149 rows. Equal to bash: 1.4.2 and the base 14,157, this PR 49,149.Reach
$(...)in an argument. Each prints one token: a tag, a hash, a version. None is in a redirect target, a[[ ]]operand or a word with a brace group or a glob.${NAME}and the${NAME<op>word}forms #46691 (332,632 scripts) was not available for this PR.Measurements (release builds of 3797331 and of this PR, Linux x64,
bun-profile)Instructions in shell code for one more unit. qemu-user 10.0.13, one instruction per block, scripts of 1,000 and 2,000 units. Each count was the same in two runs.
"$(echo a)"A=$(echo a)[[ -n ${U:-a} ]]$(true)$(echo abc)$(echo a b c)$(echo -n -e 'a\r\n')[[ -n $(echo a) ]]$(echo ' a')$(echo 'a ')$(echo ' ')x$(echo ' a ')y$(echo -e 'a\tb')$(echo -n -e 'a\r\nb\r\n')a\r,btoa,btrue $(cat f), 1 MiB and 2 MiB): 16.2 to 14.0 with no whitespace, 57.1 to 50.5 with lines of three words.$(echo abc): 28 and 24). The two units with more arguments allocate for them: 28 to 35 and 31 to 36.$(echo abc)and eachx$(echo ' a ')y: 2 on both builds (qemu -strace)..text110,661,145 to 110,659,865 bytes.post_subshell_expansion1,821 to 546 bytes.transition_to_glob_state4,840 to 4,862.end_fieldis one symbol of 826 bytes on both.Expansion(168) andNode(176) compile as they are.Tests
word splitting. On a debug build of the base, 18 fail.\rin 2 contexts (7,812 rows).end_fieldfor each whitespace byte of a run, which gives the same fields.transition_to_glob_state,*.txt$(echo ' ')is the errorno matches found:with an empty name. A test fails then.test/js/bun/shell/andtest/cli/run/run-shell.test.tspass on the debug build. On the release builds,leak.test.tspasses on both, andshell-load.test.tsfails on both.bun run rust:mordant(the tool is not on the build machine). The tests use only theechobuiltin for the output, so the same code runs on Windows CI.Self-review: what is not done, and why
${NAME}and the${NAME<op>word}forms #46691. The body names it as a prerequisite.[[ ]]operand rule here, as a one-line change or as a port of shell: expand a[[ ]]operand to one word and match*on the right of == and != #45968: shell: expand a[[ ]]operand to one word and match*on the right of == and != #45968 owns that rule.[[ ]]rule first, for the same reason.bun_shell_parser: the two other uses are in the lexer of shell: expand${NAME}and the${NAME<op>word}forms #46691. A test holds the set of${NAME:-word}equal to the set of$(...).breakinglabel and a release: for a maintainer.Bun.$code, and the rules of deno task, dax and Yarn: not verified for this PR, so not stated.no test proof · iteration 0 · platform-specific test(s) that do not run on this machine, deferring to CI, which covers all platforms: test/js/bun/shell/bunshell.test.ts