dub icon indicating copy to clipboard operation
dub copied to clipboard

dub.internal.vibecompat.core.log: Always log to stderr

Open CyberShadow opened this issue 4 years ago • 12 comments
trafficstars

Program output should generally go to stdout, and messages intended for humans should go to stderr. For example, a program that consumes and emits JSON data could produce JSON on stdout, and any informational messages pertaining it (such as parsing warnings or errors) to stderr.

Dub selects the output stream depending on log level. However, this is not particularly useful. This also corrupts the output of programs executed via dub.

Fix this by always sending log messages to stderr.

CyberShadow avatar Sep 27 '21 07:09 CyberShadow

The problem is that logInfo is actually used for regular output, see for example dub.commandline.ListCommand. We'll probably have to review all logInfo calls to avoid breakage for external tools or scripts.

s-ludwig avatar Sep 27 '21 07:09 s-ludwig

I'll have a look.

How to run the test suite in the test directory (without parallelism)? I can't find the reason why the tests are failing in GH Actions.

CyberShadow avatar Sep 27 '21 07:09 CyberShadow

ListCommand starts its output with:

https://github.com/dlang/dub/blob/d99df8c83a81ecc1dde1a092191b1fac6b0bd887/source/dub/commandline.d#L2093

That looks like the entire output is intended for humans, not programs. Are there programs which consume this output, despite it being formatted with plain English?

CyberShadow avatar Sep 27 '21 07:09 CyberShadow

DUB=$PWD/bin/dub test/run-unittest.sh passes locally. I don't know what's wrong with CI.

CyberShadow avatar Sep 27 '21 07:09 CyberShadow

That looks like the entire output is intended for humans, not programs. Are there programs which consume this output, despite it being formatted with plain English?

Since that is the only output format available, I'd assume so, but maybe nobody was interested in that particular information so far (i.e. I don't really know).

I'm not sure whether we should make a hard distinction between "machine readable" and "intended for human consumption" - most command line tools (e.g. ls) output human readable information to stdout and are still frequently used in scripts. IMO, the important part is that this is the regular program output, not error messages or optional notices/warnings.

s-ludwig avatar Sep 27 '21 07:09 s-ludwig

I'm not sure whether we should make a hard distinction between "machine readable" and "intended for human consumption" - most command line tools (e.g. ls) output human readable information to stdout and are still frequently used in scripts.

This is not true. GNU ls produces stable, machine-readable output (as standardized by POSIX) if stdout is not attached to a terminal. If it is a terminal, it produces pretty colors and wraps the output to the terminal width. :)

CyberShadow avatar Sep 27 '21 07:09 CyberShadow

This is not true. GNU ls produces stable, machine-readable output (as standardized by POSIX) if stdout is not attached to a terminal. If it is a terminal, it produces pretty colors and wraps the output to the terminal width. :)

Standardized or not is yet another matter, but this is clearly intended for human consumption (ls -lh | cat):

total 15M
drwxrwxrwx 1 root root 4.0K Sep  8 10:44 bin
-rw-r--r-- 1 root root  15M Mar 11  2021 dmd_2.096.0-0_amd64.deb
drwxr-xr-x 1 root root 4.0K Jun 16  2020 ldc2-1.22.0-linux-x86_64
drwxr-xr-x 1 root root 4.0K Oct 24  2020 ldc2-1.24.0-linux-x86_64
drwxr-xr-x 1 root root 4.0K Apr 28 16:56 ldc2-1.26.0-linux-x86_64
drwxr-xr-x 1 root root 4.0K Aug 14 21:59 ldc2-1.27.1-linux-x86_64

This output pretty much the same in terms of parsing as dub list.

s-ludwig avatar Sep 27 '21 08:09 s-ludwig

Yeah, I think it's not actually standardized, just stable.

I added ListCommand, with a note that the first line is part of the ossified protocol that we've pledged to support. :) (If Dub is to be localized in the future, then I guess we need to leave that line alone, at least when the output is not a terminal.)

CyberShadow avatar Sep 27 '21 08:09 CyberShadow

Maybe we should just take the time to go through all commands one by one to check both, whether stdout is used correctly, and whether the content sent to stdout is layed out in a way that is friendly for script parsing. Any changes to the output format could be made optional for the time being (until 2.0.0 probably), if they feel like they might actually break something.

Honestly, this is an area that didn't receive any attention during the 1.0.0 preparations.

s-ludwig avatar Sep 27 '21 08:09 s-ludwig

I keep running into this when I try running dub > output.xml. How can we move this forward?

CyberShadow avatar Feb 13 '22 19:02 CyberShadow

A quick possibility to avoid breaking things could be to have a runtime switch to route logInfo to stderr, but only enable it for "run" actions. It's unlikely that anyone depends on dub's output in that case.

s-ludwig avatar Feb 15 '22 15:02 s-ludwig

I really think this PR has merit and we should log to stderr.

Regarding the output of dub list, it currently is machine friendly-ish, but isn't consumed by any program I'm aware of. However, it's very human unfriendly, and most likely to be consumed by more humans than machine. We have started to make dub describe the go to place for machine-readable data, so probably we should consider providing dub list data via describe, and allow ourselves to change to format to be more human friendly.

Geod24 avatar Sep 12 '22 07:09 Geod24