suppress colorization if stdout is not a tty

[denys-duchier]

suppressing colorization makes the output easier to process by other scripts.

[inclement]

Why use a NullHandler if the stdout isn't a tty? Don't we still want to send the output?

[denys-duchier]

if stdout is not a tty, it is not connected to a terminal. which (presumably) means it is actually piped into another script. logging interspersed with actual data is ok for humans, it is very inconvenient for scripts. If the logger actually wrote to a file, then I would have kept it, but since it only writes to stdout that seems neither wise nor convenient. If logging to a file is subsequently added, then I'd be happy to revisit the logic.

[inclement]

So the real intention is to remove all stdout output when not going to a tty, and this just catches most of it (the rest should go through logging too, of course, but just checking).

Does this also mean that e.g. buildozer wouldn't get all the log output when running it via subprocess?

[denys-duchier]

for the remaining cases of explicit calls to distribute.sh that I have seen, buildozer not only doesn't want the log, but it's actually detrimental to its functionning. Maybe the log should be preserved, but it has to go somewhere else than stdout. I am not certain what the best design should be (I am discovering the new code base).

[inclement]

I think that buildozer actually should be able to receive and display the log, even if it can't right now. I'm not worried about the log being preserved if nothing is using it, but we're always asking for buildozer logs to work out what problems people have, and seeing the full verbose log is important for this. That's really why I'm asking, I wasn't sure if buildozer already does this, but it did seem like it should be able to.

I'll merge this change (it's good anyway), but there probably does need to be some way to achieve this. Probably there should be an additional command line argument to toggle it anyway, so buildozer could toggle it on.

[inclement]

Also, just to be clear, a buildozer target for this toolchain would call the p4a executable (or maybe manually call the python), the distribute.sh is only there to give a clear error if people try the old-toolchain method.

[denys-duchier]

yes, I am leaning towards calling the python so p4a doesn't have to be "installed".

[inclement]

You can also run toolchain.py file directly, it's the same.

[inclement]

Thanks.