If our /bin/sh is intended to support the SUSv3 User Portability (UP) option, it must do parameter expansion in PS[124]. If not, consider this PR null and void, even where prohibited. Quoting IEEE Std 1003.1-2001: PS1 Each time an interactive shell is ready to read a command, the value of this variable shall be subjected to parameter expansion and written to standard error. The default value shall be "$ " . For users who have specific additional implementation-defined privileges, the default may be another, implementation-defined value. The shell shall replace each instance of the character '!' in PS1 with the history file number of the next command to be typed. Escaping the '!' with another '!' (that is, "!!" ) shall place the literal character '!' in the prompt. This volume of IEEE Std 1003.1-2001 specifies the effects of the variable only for systems supporting the User Portability Utilities option. PS2 Each time the user enters a <newline> prior to completing a command line in an interactive shell, the value of this variable shall be subjected to parameter expansion and written to standard error. The default value is "> " . This volume of IEEE Std 1003.1-2001 specifies the effects of the variable only for systems supporting the User Portability Utilities option. PS4 When an execution trace ( set -x) is being performed in an interactive shell, before each line in the execution trace, the value of this variable shall be subjected to parameter expansion and written to standard error. The default value is "+ " . This volume of IEEE Std 1003.1-2001 specifies the effects of the variable only for systems supporting the User Portability Utilities option. PWD Set by the shell to be an absolute pathname of the current working directory, containing no components of type symbolic link, no components that are dot, and no components that are dot-dot when the shell is initialized. If an application sets or unsets the value of PWD, the behaviors of the cd and pwd utilities are unspecified. How-To-Repeat: $ /bin/sh $ PS1='$PWD >' $PWD >cd /tmp <- should expand $PWD $PWD >PS2='$PWD more:' $PWD >foo () $PWD more:{ $PWD more:} $PWD >PS4='$PWD -x : ' $PWD >set -x $PWD >foo + foo <- PS4 seems hard coded to "+ "
Responsible Changed From-To: freebsd-standards->tjr Over to tjr.
Responsible Changed From-To: tjr->freebsd-bugs Unassign due to lack of time and interest. Perhaps someone else will pick this up.
Responsible Changed From-To: freebsd-standards->stefanf Grab.
Technically the POSIX spec doesn't state that those must support parameter expansion. Reiterating more recent text... =============== ENV [UP XSI] [Option Start] The processing of the ENV shell variable shall be supported on all XSI-conformant systems or if the system supports the User Portability Utilities option. [Option End] This variable, when and only when an interactive shell is invoked, shall be subjected to parameter expansion (see Parameter Expansion ) by the shell and the resulting value shall be used as a pathname of a file containing shell commands to execute in the current environment. The file need not be executable. If the expanded value of ENV is not an absolute pathname, the results are unspecified. ENV shall be ignored if the user's real and effective user IDs or real and effective group IDs are different. ... PS1 Each time an interactive shell is ready to read a command, the value of this variable shall be subjected to parameter expansion and written to standard error. The default value shall be "$ " . For users who have specific additional implementation-defined privileges, the default may be another, implementation-defined value. The shell shall replace each instance of the character '!' in PS1 with the history file number of the next command to be typed. Escaping the '!' with another '!' (that is, "!!" ) shall place the literal character '!' in the prompt. This volume of POSIX.1-2008 specifies the effects of the variable only for systems supporting the User Portability Utilities option. PS2 Each time the user enters a <newline> prior to completing a command line in an interactive shell, the value of this variable shall be subjected to parameter expansion and written to standard error. The default value is "> " . This volume of POSIX.1-2008 specifies the effects of the variable only for systems supporting the User Portability Utilities option. PS4 When an execution trace ( set -x) is being performed in an interactive shell, before each line in the execution trace, the value of this variable shall be subjected to parameter expansion and written to standard error. The default value is "+ " . This volume of POSIX.1-2008 specifies the effects of the variable only for systems supporting the User Portability Utilities option. =============== Note that Parameter Expansion was explicitly mentioned in the definition of ENV, but not in PS1, PS2, or PS4. I'm pretty sure that bash expanding these variables is a `feature' of their shell and not of the spec. Thanks, -Garrett
On Sat, Jan 29, 2011 at 12:46:43PM -0800, Garrett Cooper wrote: # Technically the POSIX spec doesn't state that those must support # parameter expansion. Reiterating more recent text... # # =============== # # ENV # [UP XSI] [Option Start] The processing of the ENV shell variable # shall be supported on all XSI-conformant systems or if the system # supports the User Portability Utilities option. [Option End] # # This variable, when and only when an interactive shell is invoked, # shall be subjected to parameter expansion (see Parameter Expansion ) ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ # by the shell and the resulting value shall be used as a pathname of a # file containing shell commands to execute in the current environment. # The file need not be executable. If the expanded value of ENV is not # an absolute pathname, the results are unspecified. ENV shall be # ignored if the user's real and effective user IDs or real and # effective group IDs are different. # # ... # # PS1 # Each time an interactive shell is ready to read a command, the # value of this variable shall be subjected to parameter expansion and ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ # written to standard error. The default value shall be "$ " . For users # who have specific additional implementation-defined privileges, the # default may be another, implementation-defined value. The shell shall # replace each instance of the character '!' in PS1 with the history # file number of the next command to be typed. Escaping the '!' with # another '!' (that is, "!!" ) shall place the literal character '!' in # the prompt. This volume of POSIX.1-2008 specifies the effects of the # variable only for systems supporting the User Portability Utilities # option. # # PS2 # Each time the user enters a <newline> prior to completing a # command line in an interactive shell, the value of this variable shall # be subjected to parameter expansion and written to standard error. The ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ # default value is "> " . This volume of POSIX.1-2008 specifies the # effects of the variable only for systems supporting the User # Portability Utilities option. # # PS4 # When an execution trace ( set -x) is being performed in an # interactive shell, before each line in the execution trace, the value # of this variable shall be subjected to parameter expansion and written ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ # to standard error. The default value is "+ " . This volume of # POSIX.1-2008 specifies the effects of the variable only for systems # supporting the User Portability Utilities option. # # =============== # # Note that Parameter Expansion was explicitly mentioned in the # definition of ENV, but not in PS1, PS2, or PS4. I'm pretty sure that # bash expanding these variables is a `feature' of their shell and not # of the spec. My reading is that parameter expansion is mentioned for all of them. Am I missing something? Regards, Jens -- Jens Schweikhardt http://www.schweikhardt.net/ SIGSIG -- signature too long (core dumped)
On Sun, Jan 30, 2011 at 2:39 AM, Jens Schweikhardt <schweikh@schweikhardt.net> wrote: > On Sat, Jan 29, 2011 at 12:46:43PM -0800, Garrett Cooper wrote: > # =A0 =A0 Technically the POSIX spec doesn't state that those must suppor= t > # parameter expansion. Reiterating more recent text... > # > # =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D > # > # ENV > # =A0 =A0 [UP XSI] [Option Start] The processing of the ENV shell variabl= e > # shall be supported on all XSI-conformant systems or if the system > # supports the User Portability Utilities option. [Option End] > # > # =A0 =A0 This variable, when and only when an interactive shell is invok= ed, > # shall be subjected to parameter expansion (see Parameter Expansion ) > =A0^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ > # by the shell and the resulting value shall be used as a pathname of a > # file containing shell commands to execute in the current environment. > # The file need not be executable. If the expanded value of ENV is not > # an absolute pathname, the results are unspecified. ENV shall be > # ignored if the user's real and effective user IDs or real and > # effective group IDs are different. > # > # =A0 =A0 ... > # > # PS1 > # =A0 =A0 Each time an interactive shell is ready to read a command, the > # value of this variable shall be subjected to parameter expansion and > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 ^^^^^^^^^^^^^^^^^^^^^^^^^= ^^^^^^^^^^^^^^^^ > # written to standard error. The default value shall be "$ " . For users > # who have specific additional implementation-defined privileges, the > # default may be another, implementation-defined value. The shell shall > # replace each instance of the character '!' in PS1 with the history > # file number of the next command to be typed. Escaping the '!' with > # another '!' (that is, "!!" ) shall place the literal character '!' in > # the prompt. This volume of POSIX.1-2008 specifies the effects of the > # variable only for systems supporting the User Portability Utilities > # option. > # > # PS2 > # =A0 =A0 Each time the user enters a <newline> prior to completing a > # command line in an interactive shell, the value of this variable shall > # be subjected to parameter expansion and written to standard error. The > =A0^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ > # default value is "> " . This volume of POSIX.1-2008 specifies the > # effects of the variable only for systems supporting the User > # Portability Utilities option. > # > # PS4 > # =A0 =A0 When an execution trace ( set -x) is being performed in an > # interactive shell, before each line in the execution trace, the value > # of this variable shall be subjected to parameter expansion and written > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^= ^^^^ > # to standard error. The default value is "+ " . This volume of > # POSIX.1-2008 specifies the effects of the variable only for systems > # supporting the User Portability Utilities option. > # > # =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D > # > # =A0 =A0 Note that Parameter Expansion was explicitly mentioned in the > # definition of ENV, but not in PS1, PS2, or PS4. I'm pretty sure that > # bash expanding these variables is a `feature' of their shell and not > # of the spec. > > My reading is that parameter expansion is mentioned for all of them. > Am I missing something? Jilles noted I was wrong (I was being lazy when reading the text and was only looking for the Parameter Expansion link). There are several issues with parameter expansion comparing dash to FreeBSD= ash. Thanks for the correction, -Garrett
I'm not working on /bin/sh anymore.
batch change: For bugs that match the following - Status Is In progress AND - Untouched since 2018-01-01. AND - Affects Base System OR Documentation DO: Reset to open status. Note: I did a quick pass but if you are getting this email it might be worthwhile to double check to see if this bug ought to be closed.
^Triage: This still remains an issue in all current versions of FreeBSD, re-triage accordingly. @Jilles Ideas on any folks who might want to be looped in on this? [1] via: https://twitter.com/Noodlez1232/status/1496384035362385921
I am the person who made this be retriaged. I'm more then willing to *try* to figure out how to implement if anybody can guide me to the resources to go about that.
I took a look at the code super fast (not planning on any changes yet), and it seems that command substitution was not added on $PS1/PS2 because the parser relies on PS1 and PS2 not doing command substitution on those variables. From what it seems, the parser would need to be redone to become re-entrant. This is because it has some state stored in static global variables, which are cleared each parse. A possible solution, and one that frankly doesn't sound fun would be to figure out which variables are the ones that hold state local to parsing, and which are really required to be static and global, and move these state-keeping variables to a structure, then pass that structure to each parsing function.
For PS4, I implemented this in 2011. The problem with unbounded recursion from command substitutions is avoided by not allowing command substitutions; this error (like other parse and expansion errors in PS4) is handled by writing an error message and continuing with the unexpanded value. For PS1 and PS2, there are two additional problems: * how does $... expansion interact with the existing \u, \w, \$, etc. * because PS2 is needed during the middle of a parse, more work may be needed to prevent the two parse operations from interfering with each other
(In reply to Nathaniel Barragan from comment #11) I tried the naive approach to make the parser re-entrant by doing something like this static struct state { struct heredoc *heredoclist; /* list of here documents to read */ int doprompt; /* if set, prompt the user */ int needprompt; /* true if interactive and at start of line */ int lasttoken; /* last token read */ int tokpushback; /* last token pushed back */ char *wordtext; /* text of last word returned by readtoken */ int checkkwd; struct nodelist *backquotelist; union node *redirnode; struct heredoc *heredoc; int quoteflag; /* set if (part of) last token was quoted */ int startlinno; /* line # where last token started */ int funclinno; /* line # where the current function started */ struct parser_temp *parser_temp; } states[2]; static size_t current; then referring to the fields in states[current] in place of the formerly file scope identifiers, and also incrementing/decrementing current at the right times. But I'm not exactly sure when those are. Also, there's the problem of expdest invalidation on multiple calls to expandarg.
To expand a bit on what I was thinking, in the context of PS4 and > The problem with unbounded recursion from command substitutions is avoided by not allowing command substitutions my idea was to allow recursion bounded to the stack depth of 2 or perhaps some other low number that would allow simple PS4='`date`' or similar, but not anything more fancy.
(In reply to Piotr Pawel Stefaniak from comment #13) If you're creating a parser state struct, it may be better to pass a pointer to it to most functions in parser.c. The struct would be allocated on the stack in parsecmd(), parsewordexp() and expandstr(). This eliminates uncertainty about "which" parser state needs to be used. Due to pointers in the parser state that need to be freed, parsecmd() and parsewordexp() would need to use setjmp(). An alternative might be to keep the parser state for those statically allocated; I don't think they are re-entered (eval and . are not among the builtins that can execute in a subshell without a fork). To avoid unlimited recursion via PS4 while still allowing command substitution in expandstr(), it might work better to ignore xflag during an expandstr() invocation.
^Triage: clear stale flags.
^Triage: clear unneeded flags. Nothing has yet been committed to be merged.
I've implemented parameter expansion support for PS1/PS2/PS4 prompts. The patch adds support for: - Simple variables: $USER, $PWD, $HOSTNAME - Braced variables: ${USER}, ${HOME} - Special parameters: $$, $?, $! - Positional parameters: $0, $1, etc. Tested via some shell scripts. The tests in tests/ weren't interactive and PS1 requires it. Please advice if some test cases are needed and how to add them. Note: This implements parameter expansion only. Command substitution $(cmd) is not yet supported and would require additional work. Attachment below.
Created attachment 264417 [details] Parameter expansion implementation
(In reply to Matthew Phillips from comment #18) > I've implemented parameter expansion support for PS1/PS2/PS4 prompts. This is PS1 and PS2 only, since PS4 already supports expansions. > The patch adds support for: > - Simple variables: $USER, $PWD, $HOSTNAME > - Braced variables: ${USER}, ${HOME} > - Special parameters: $$, $?, $! > - Positional parameters: $0, $1, etc. It looks like this only supports simple and braced variables, since lookupvar() only looks up variables and not special or positional parameters. > Tested via some shell scripts. The tests in tests/ weren't interactive and PS1 > requires it. Please advice if some test cases are needed and how to add them. Some of the existing tests are for interactive features. Look for ${SH} -i +m and ${SH} +m -i in existing tests. Something like case $(testvar=abcdef PS1=\$testvar: ${SH} +m -i) in *abcdef*) ;; *) echo failed ;; esac could work as a test. The exact output can't be relied on; definitely not when it is desired to check and compare other shells as well. > Note: This implements parameter expansion only. Command substitution $(cmd) is > not yet supported and would require additional work. Interesting choice to re-implement expansion, but this keeps the changes local and ensures the interaction with the existing backslash sequences is correct (i.e. neither a backslash from parameter expansion nor a dollar sign from a prompt backslash sequence are special). Implementing the varieties like ${parameter#pattern} and ${parameter:-default} will not be tenable this way. If people are happy with this restricted form, it could be nice.
Created attachment 264503 [details] PS1/PS2 parameter expansion patch v2 Uploaded revised patch (v2) that addresses the review feedback. As mentioned, this patch maintains the localized approach within getprompt() and avoids full parser reentry. While this means we don't support advanced expansions like ${parameter#pattern}, it provides POSIX-compliant basic parameter expansion without the complexity of making the parser reentrant. This is sufficient for the vast majority of use cases and aligns with the existing PS4 implementation strategy. The original patch claimed to support special parameters ($$, $?, $!, $#) and positional parameters ($0-$9), but only called lookupvar() which cannot handle these parameter types. This has been fixed. Special parameters now properly implemented: - $$ → rootpid - $? → oexitstatus - $# → shellparam.nparam - $! → backgndpidval() Positional parameters now properly implemented: - $0 → arg0 (shell name) - $1-$9 → shellparam.p[num-1] Added tests as well: - ps1-expand1.0: Regular variable expansion ($testvar) - ps1-expand2.0: Braced variable expansion (${testvar}) - ps1-expand3.0: Special parameter $$ (PID) - ps1-expand4.0: Special parameter $? (exit status) - ps1-expand5.0: Positional parameter $0 (shell name) - ps2-expand1.0: PS2 continuation prompt expansion
Comment on attachment 264503 [details] PS1/PS2 parameter expansion patch v2 I like it. Please confirm the author name and email address to be written into Git. I have some nits but I can fix them myself. Nit: existing code does not check `is_in_name` on the same character `is_name` returned true for, but increments the pointer (`fmt` here) in between. Nit: line 2124 till 2132 (`/* Positional parameters - check digits FIRST */` and `} else if (namelen == 1 && is_special(*varname)) {`) are not indented correctly. Nit: `valbuf` on line 2134 is `static` but if `varname` above can live on the stack then so can `valbuf`. Some things like `$-` and `${10}` don't work but let's not add them to avoid adding even more duplicated code. I expect the implemented parts to be almost all of what people will actually use.
Fantastic Name: Matthew Phillips Email: matthew@matthewphillips.info
A commit in branch main references this bug: URL: https://cgit.FreeBSD.org/src/commit/?id=f9e79facf874567f25147b24863e5198164e8d04 commit f9e79facf874567f25147b24863e5198164e8d04 Author: Matthew Phillips <matthew@matthewphillips.info> AuthorDate: 2025-10-12 19:27:34 +0000 Commit: Jilles Tjoelker <jilles@FreeBSD.org> CommitDate: 2025-11-07 22:35:18 +0000 sh: Implement simple parameter expansion in PS1 and PS2 This change follows a localized approach within getprompt() and avoids full parser reentry. While this means we don't support advanced expansions like ${parameter#pattern}, it provides POSIX-compliant basic parameter expansion without the complexity of making the parser reentrant. This is sufficient for the vast majority of use cases. PR: 46441 bin/sh/parser.c | 126 +++++++++++++++++++++++++++++++- bin/sh/tests/parser/Makefile | 6 ++ bin/sh/tests/parser/ps1-expand1.0 (new) | 7 ++ bin/sh/tests/parser/ps1-expand2.0 (new) | 7 ++ bin/sh/tests/parser/ps1-expand3.0 (new) | 8 ++ bin/sh/tests/parser/ps1-expand4.0 (new) | 8 ++ bin/sh/tests/parser/ps1-expand5.0 (new) | 8 ++ bin/sh/tests/parser/ps2-expand1.0 (new) | 12 +++ 8 files changed, 181 insertions(+), 1 deletion(-)
I changed one more thing: oexitstatus to exitstatus so $? expands to the last command's exit status and not the one of the command before it. MFC is possible after 15.0.