/qubes-issues

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.

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

qvm-run does not always color stderr, even if it is a TTY #2190

Closed
rustybird opened this Issue Jul 21, 2016 · 7 comments

Comments

Assignees

@marmarek marmarek

Labels
Projects
None yet
Milestone
  Release 3.0 updates
3 participants
@rustybird @marmarek @andrewdavidwong
@rustybird

rustybird commented Jul 21, 2016

Qubes OS version:

R3.1, probably also R3.2

qvm-run --pass-io anyvm 'echo untrusted >&2' >/dev/null

is not colored red.
@marmarek

This comment has been minimized.

Show comment
Hide comment
@marmarek

marmarek Jul 21, 2016

Member

Indeed currently coloring enabled based on stdout being TTY or not. If you redirect it to /dev/null, it is no longer TTY...
But even if you force coloring, sequence enabling colors is outputed to stdout - so if redirected to /dev/null, it will be useless.

Do you think it both should receive color switch sequence? Probably yes...

Anyway, what is real use case for --pass-io, then redirecting stdout to /dev/null?
Member

marmarek commented Jul 21, 2016

Indeed currently coloring enabled based on stdout being TTY or not. If you redirect it to /dev/null, it is no longer TTY...
But even if you force coloring, sequence enabling colors is outputed to stdout - so if redirected to /dev/null, it will be useless.

Do you think it both should receive color switch sequence? Probably yes...

Anyway, what is real use case for --pass-io, then redirecting stdout to /dev/null?
@rustybird

This comment has been minimized.

Show comment
Hide comment
@rustybird

rustybird Jul 21, 2016

Do you think it both should receive color switch sequence?

Yes. Maybe it could even be simplified by letting qrexec-client take prologue string and epilogue string arguments (defaulting to \033[0;31m and \033[0m), which would be written to stdout and stderr if they are terminals. Because qrexec-client already handles something related with -t and -T.

Anyway, what is real use case for --pass-io, then redirecting stdout to /dev/null?

Heh, /dev/null was not the best example. My actual use use case is qvm-run ... | other_command.

rustybird commented Jul 21, 2016

Do you think it both should receive color switch sequence?

Yes. Maybe it could even be simplified by letting qrexec-client take prologue string and epilogue string arguments (defaulting to \033[0;31m and \033[0m), which would be written to stdout and stderr if they are terminals. Because qrexec-client already handles something related with -t and -T.

Anyway, what is real use case for --pass-io, then redirecting stdout to /dev/null?

Heh, /dev/null was not the best example. My actual use use case is qvm-run ... | other_command.
@rustybird

This comment has been minimized.

Show comment
Hide comment
@rustybird

rustybird Jul 21, 2016

Maybe it could even be simplified by letting qrexec-client take prologue string and epilogue string arguments

Scratch that: Dealing with signals in C to ensure that the epilogue string is written out would probably be too much trouble.

rustybird commented Jul 21, 2016

Maybe it could even be simplified by letting qrexec-client take prologue string and epilogue string arguments

Scratch that: Dealing with signals in C to ensure that the epilogue string is written out would probably be too much trouble.

@andrewdavidwong andrewdavidwong added enhancement C: core P: minor labels Jul 21, 2016

@marmarek marmarek added this to the Release 3.0 updates milestone Jul 24, 2016

@marmarek marmarek self-assigned this Jul 29, 2016

@marmarek marmarek closed this in marmarek/old-qubes-core-admin@86a14b5 Aug 7, 2016

@marmarek

This comment has been minimized.

Show comment
Hide comment
@marmarek

marmarek Aug 8, 2016

Member

Automated announcement from builder-github

The package qubes-core-dom0-3.2.8-1.fc23 has been pushed to the r3.2 testing repository for dom0.
To test this update, please install it with the following command:

sudo qubes-dom0-update --enablerepo=qubes-dom0-current-testing

Changes included in this update
Member

marmarek commented Aug 8, 2016

Automated announcement from builder-github

The package qubes-core-dom0-3.2.8-1.fc23 has been pushed to the r3.2 testing repository for dom0.
To test this update, please install it with the following command:

sudo qubes-dom0-update --enablerepo=qubes-dom0-current-testing

Changes included in this update

@marmarek marmarek added the r3.2-dom0-cur-test label Aug 8, 2016

@marmarek

This comment has been minimized.

Show comment
Hide comment
@marmarek

marmarek Aug 31, 2016

Member

Automated announcement from builder-github

The package qubes-core-dom0-3.2.8-1.fc23 has been pushed to the r3.2 stable repository for dom0.
To install this update, please use the standard update command:

sudo qubes-dom0-update

Or update dom0 via Qubes Manager.

Changes included in this update
Member

marmarek commented Aug 31, 2016

Automated announcement from builder-github

The package qubes-core-dom0-3.2.8-1.fc23 has been pushed to the r3.2 stable repository for dom0.
To install this update, please use the standard update command:

sudo qubes-dom0-update

Or update dom0 via Qubes Manager.

Changes included in this update

@marmarek marmarek added r3.2-dom0-stable and removed r3.2-dom0-cur-test labels Aug 31, 2016

marmarek added a commit to QubesOS/qubes-core-admin that referenced this issue Nov 20, 2016

@marmarek
qvm-run: color untrusted stderr even when stdout is redirected 
When stdout is redirected to some file or command two things will
happen:
 - qvm-run will not automatically color the output as stdout is not a
 TTY
 - even when coloring is forced, it will not work, as the control
 sequence (on stdout) will be redirected anyway

Fix this by handling stdout and stderr independently and output color
switching sequence to each of them.

Fixes QubesOS/qubes-issues#2190

(cherry picked from commit 86a14b5)
Verified
This commit was signed with a verified signature.
@marmarek marmarek Marek Marczykowski-Górecki
GPG key ID: 063938BA42CFA724 Learn about signing commits
7894394
@marmarek

This comment has been minimized.

Show comment
Hide comment
@marmarek

marmarek Nov 20, 2016

Member

Automated announcement from builder-github

The package qubes-core-dom0-3.1.18-1.fc20 has been pushed to the r3.1 testing repository for dom0.
To test this update, please install it with the following command:

sudo qubes-dom0-update --enablerepo=qubes-dom0-current-testing

Changes included in this update
Member

marmarek commented Nov 20, 2016

Automated announcement from builder-github

The package qubes-core-dom0-3.1.18-1.fc20 has been pushed to the r3.1 testing repository for dom0.
To test this update, please install it with the following command:

sudo qubes-dom0-update --enablerepo=qubes-dom0-current-testing

Changes included in this update

@marmarek marmarek added the r3.1-dom0-cur-test label Nov 20, 2016

@marmarek

This comment has been minimized.

Show comment
Hide comment
@marmarek

marmarek Dec 4, 2016

Member

Automated announcement from builder-github

The package qubes-core-dom0-3.1.18-1.fc20 has been pushed to the r3.1 stable repository for dom0.
To install this update, please use the standard update command:

sudo qubes-dom0-update

Or update dom0 via Qubes Manager.

Changes included in this update
Member

marmarek commented Dec 4, 2016

Automated announcement from builder-github

The package qubes-core-dom0-3.1.18-1.fc20 has been pushed to the r3.1 stable repository for dom0.
To install this update, please use the standard update command:

sudo qubes-dom0-update

Or update dom0 via Qubes Manager.

Changes included in this update

@marmarek marmarek added r3.1-dom0-stable and removed r3.1-dom0-cur-test labels Dec 4, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment