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.

Sign up for GitHub

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

Jump to bottom

Standard output tests as being on a terminal within a command substitution #1079

Closed
McDutchie opened this issue Dec 17, 2018 · 2 comments
Closed

Standard output tests as being on a terminal within a command substitution #1079

McDutchie opened this issue Dec 17, 2018 · 2 comments
Assignees
@krader1961
Labels
bug

Comments

@McDutchie
Copy link
Contributor

Description of problem:

Standard output (FD 1) tests as being on a terminal within a command substitution, which makes no sense as the command substitution is supposed to be catching standard output.

Ksh version:

Version A 2017.0.0-devel-2130-gd67bda0

Version AJM 93u+ 2012-08-01

How reproducible:

ksh -c 'v=$(echo begincomsub; [ -t 1 ] && echo oops; echo endcomsub); echo "$v"'

Actual results:

begincomsub
oops
endcomsub

Expected results:

begincomsub
endcomsub

Additional info:

As usual, forcing the subshell to fork using something like ulimit -t unlimited makes it work as expected:

ksh -c 'v=$(ulimit -t unlimited; echo begincomsub; [ -t 1 ] && echo oops; echo endcomsub); echo "$v"'

So this looks like another issue associated with non-forking subshells (ref.: #480).
@McDutchie
Copy link
Contributor Author

In case anyone wonders about a use case affected by this bug: it is entirely conceivable to have a utility shell function that colourises or otherwise pretty-prints its output if standard output is on a terminal, and otherwise produces plain text so it can be used with command substitutions, log files and the like.

@siteshwar siteshwar added the bug label Dec 17, 2018
@krader1961
Copy link
Contributor

As usual, forcing the subshell to fork using something like ulimit -t unlimited makes it work as expected.

Obligatory links to issue #480, #416, #73, and several others.

@krader1961 krader1961 self-assigned this Dec 18, 2018
@krader1961 krader1961 mentioned this issue Dec 18, 2018
Closed
@krader1961 krader1961 mentioned this issue Dec 22, 2018
Open
McDutchie added a commit to ksh93/ksh that referenced this issue Jun 11, 2020
@McDutchie
Fix 'test -t 1' in $(command substitutions)
a5a3f3e 
Standard output (FD 1) tested as being on a terminal within a
command substitution, which makes no sense as the command
substitution is supposed to be catching standard output.
    ksh -c 'v=$(echo begincomsub
                [ -t 1 ] && echo oops
                echo endcomsub)
            echo "$v"'
This should not output "oops".

This is one of the many bugs with ksh93 virtual (non-forked)
subshells. On the abandoned Vashist/Rader ksh2020 branch, this bug
was fixed by changing quite a lot of code, which introduced and/or
exposed another bug:
	att#1079
	att@8e1e405e
	att#1088
That issue was unresolved when the ksh2020 branch was abandoned.

The safer and more conservative fix is simply forcing the subshell
to fork if we're in a non-forked command substitution and testing
'-t 1'. It is hard to imagine a situation where this would cause a
noticable performance hit.

Note that this fix does not affect ksh93-specific "shared"
non-subshell ${ command substitutions; } which are executed in the
main shell environment, so that variables survive, etcetera.
'test -t 1' continues to wrongly return true there, but command
substitutions of that form cannot be forked because that would
defeat their purpose.

src/cmd/ksh93/bltins/test.c:
- Fix 'test -t 1', '[ -t 1 ]' and '[[ -t 1 ]]' by forking the
  current subshell if it is a virtual/non-forked subshell
  (shp->subshell), and a command substitution (shp->comsub), but
  NOT a "shared" ${ command substitution; } (!shp->subshare).

src/cmd/ksh93/tests/bracket.sh:
- Add two regression tests for this issue, which were adapted from
  the Vashist/Rader ksh2020 branch.

NEWS, src/cmd/ksh93/include/version.h:
- Update.

(cherry picked from commit b8ef05e457ead65b83417699b8dd8632f855e2fa)
McDutchie added a commit to ksh93/ksh that referenced this issue Jun 11, 2020
@McDutchie
Fix 'test -t 1' in $(command substitutions)
cafe33f 
Standard output (FD 1) tested as being on a terminal within a
command substitution, which makes no sense as the command
substitution is supposed to be catching standard output.
    ksh -c 'v=$(echo begincomsub
                [ -t 1 ] && echo oops
                echo endcomsub)
            echo "$v"'
This should not output "oops".

This is one of the many bugs with ksh93 virtual (non-forked)
subshells. On the abandoned Vashist/Rader ksh2020 branch, this bug
was fixed by changing quite a lot of code, which introduced and/or
exposed another bug:
	att#1079
	att@8e1e405e
	att#1088
That issue was unresolved when the ksh2020 branch was abandoned.

The safer and more conservative fix is simply forcing the subshell
to fork if we're in a non-forked command substitution and testing
'-t 1'. It is hard to imagine a situation where this would cause a
noticable performance hit.

Note that this fix does not affect ksh93-specific "shared"
non-subshell ${ command substitutions; } which are executed in the
main shell environment, so that variables survive, etcetera.
'test -t 1' continues to wrongly return true there, but command
substitutions of that form cannot be forked because that would
defeat their purpose.

src/cmd/ksh93/bltins/test.c:
- Fix 'test -t 1', '[ -t 1 ]' and '[[ -t 1 ]]' by forking the
  current subshell if it is a virtual/non-forked subshell
  (shp->subshell), and a command substitution (shp->comsub), but
  NOT a "shared" ${ command substitution; } (!shp->subshare).

src/cmd/ksh93/tests/bracket.sh:
- Add two regression tests for this issue, which were adapted from
  the Vashist/Rader ksh2020 branch.

NEWS, src/cmd/ksh93/include/version.h:
- Update.

(cherry picked from commit b8ef05e457ead65b83417699b8dd8632f855e2fa)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@krader1961 krader1961

Labels
bug
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@siteshwar @McDutchie @krader1961