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.

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

fix stdout assumptions #12806

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Conversation

Jan200101
Copy link
Contributor

stdout, just like stderr, may be None which would cause a heap of problems
see 02733bc

Testing by setting up lite-xl with meson setup build >&- in zsh but should also work in bash

stdout, just like stderr, may be None which would cause a heap of problems
see 02733bc
@eli-schwartz
Copy link
Member

stdout, just like stderr, may be None which would cause a heap of problems
see 02733bc

I objected to that PR for the same reason I object to this one: using >&- is not correct, and thus doesn't need to be handled. I would like to understand what the actual reasonable use case is for this.

When is stdout None in expected use cases? Why is the correct solution to avoid re-encoding highly specific call sites instead of fixing the underlying problem?

The changes in mesonbuild/utils/universal.py may end up causing important data to be incorrectly decoded, so it's not obvious why silently ignoring the problem is correct (especially given that that function is used in a whole bunch of places).

@Jan200101
Copy link
Contributor Author

I objected to that PR for the same reason I object to this one: using >&- is not correct, and thus doesn't need to be handled. I would like to understand what the actual reasonable use case is for this.

I'm aware >&- is not something thats expected, it was an example of how to run into the behavior I saw.

I ran into problems with a proprietary CI system which closes stdout instead of redirecting it to devnull before running a build script.

When is stdout None in expected use cases? Why is the correct solution to avoid re-encoding highly specific call sites instead of fixing the underlying problem?

The reality is that stdout and stderr can be None and this is supported behavior by at least Linux and Python.
By luck that system I mentioned prior keeps stderr open where I only saw an ugly Python exception instead of any useful.

What would a better solution look like?
Write an error to stdout before quitting?
Reassign stdout/stderr to the other when its not available?
Writing to a file?

@eli-schwartz
Copy link
Member

I guess my biggest concern here is knowing what encoding to assume for Popen_safe. It's not just a matter of "clean", and the current logic seems to assume utf-8? How do we know this is true?

Thanks for clarifying the use case. :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants