conan-io / conan

Conan - The open-source C and C++ package manager
https://conan.io
MIT License
8.11k stars 960 forks source link

[bug] Documentation for 'Packages lists' has misleading/incorrect instructions. #15826

Open RochaStratovan opened 6 months ago

RochaStratovan commented 6 months ago

Environment details

Steps to reproduce

The Packages lists documentation at https://docs.conan.io/2/tutorial/other_features.html#packages-lists provides a command sequence that doesn't work.

The listed commands are as follows:

conan create . --format=json > build.json
conan list --graph=build.json --graph-binaries=build --format=json > pkglist.json
conan upload --list=pkglist.json -r=myremote -c

The problem is with the very first command.

conan create . displays a lot of information, adding the --format=json option merely appends the build information to the end in a json format.

The complete command takes all the normal output from conan create combined with the extra json output and places that into a build.json file.

This is not a valid json file. It's prefixed by all the general output from conan create ..

The normal conan create output could be suppressed by using the -vquite option, but that assumes the underlying build (in our case CMake) doesn't display any messages along the way when it's compiling.

Additionally I found that the file generated from the '>' output had a bunch of extra entries at the end of each line such as ^[[0k. This was inserted by the git-bash shell I have running. A total shocker to me. I don't see the strange line endings on ubuntu linux, I see different line endings when I run the command from Windows CMD terminal

git-lab line endings

image

windows CMD line endings

image

It might be more portable for this command to write the build list information to a file rather than have it captured from standard output. This way it's in the format you want and only includes the list information.

Logs

No response

memsharded commented 6 months ago

It seems that there is something there that is broken in the setup. Conan outputs the json to stdout and all other output to stderr. So this shouldn't be an issue and it has been working correctly so far by us and many users.

The possible causes:

The normal conan create output could be suppressed by using the -vquite option, but that assumes the underlying build (in our case CMake) doesn't display any messages along the way when it's compiling.

This definitely shouldn't be necessary.

It would be great to know how to reproduce it locally. If I understand correctly, this only happens locally in Windows + git bash? Any specific git-bash version/installation?

RochaStratovan commented 6 months ago

Thank you for the additional information. I will work on creating a small reproduction repository in github/gitlab for you. I originally thought this might be related to #15824, because I was essentially calling cmake via a script using the subprocess.call() method, and you mentioned that self.run() takes care of output control.

However, my initial test with this was the same result. I will work on this further tomorrow and update when I learn more.

Thank you.

RochaStratovan commented 5 months ago

Problem Restatement

  1. conan build foo --format=json > build.json is causing CMake output to appear in the json file too. Moreover, Windows has strange line endings.

  2. @memsharded explained that conan should be redirecting all output to stderr and the json data would come from stdout. So this shouldn't be happening.

  3. This links #15824 into this issue.

    My failing code was using subprocess.call instead of conan's self.run().

    The syntax was changed to self.run.

    1. Linux now works

    2. Windows CMD terminal now works. No line ending issues either.

    3. git-bash shell on Windows still fails with all the original symptoms.


Further Investigation

I've had other problems with git-bash. Such as "The conan auth command locks up the CLI on Windows git-bash mingw64" #15722 which is ultimately caused by a defect in python "getpass hangs on Windows in Ming64 git-bash shell" (python/cpython#115798)

I have a work around in place to avoid the lockup aliasing the conan command

alias conan='winpty -Xallow-non-tty conan.exe'

THIS IS NOT A PROBLEM IF I REMOVE THIS ALIAS!


Root Cause

This was a combination of two issue.

  1. I was using subproces.call() instead of self.run()

  2. I had the conan command aliases to work around a python defect relating to password entry.


Suggestion

Could you please add a Limitations section to the documentation that identifies git-bash as not compatible due to: python/cpython#115798.

As a user I can either add the alias so interactive password entry works, or remove the alias which allows the conan redirection logic to work correctly.

P.S. I'm not really happy with the fact that conan uses stderr for it's normal output. As a user, I expect conan to follow standard guidelines for stdout and stderr, i.e. stderr is used for error output, not normal output. There are some applications/tools that will check stderr and report a command failure based on that output, not just the return value.

As always, thank you for your patience and help.

memsharded commented 5 months ago

P.S. I'm not really happy with the fact that conan uses stderr for it's normal output. As a user, I expect conan to follow standard guidelines for stdout and stderr, i.e. stderr is used for error output, not normal output.

The truth is that we didn't invent this pattern, and it is at the very least controversial, see for example https://stackoverflow.com/questions/4919093/should-i-log-messages-to-stderr-or-stdout/4919110, or https://www.reddit.com/r/golang/comments/jqvaft/why_does_the_log_package_always_output_to_stderr/ many users assume that "logging" output should go to stderr and the effective "result" or "payload" of the command should go to stdout.

In Conan this is the case for example, there are many operations like conan download that will do logging of all the operations, connection to the server, download transfers, but the actual payload of the command is the final package list which can be printed to text, json (and sometimes other formats). Not having this differentiation between stderr for Conan logging messages and the final command payload would force usages like conan download .... --format=json --output-file=myfile.json, which is also against standard guidelines, which recommends to use the >myfile.json redirection instead of inventing arguments to send some output to a file. The idea is that some commands like conan cache path ... result could be piped too into other commands, but for that it means that Conan could not log absolutely anything, and there are many times that Conan commands need to log and report what is happening, still the piping of a clean "stdout" should be possible.

Could you please add a Limitations section to the documentation that identifies git-bash as not compatible due to: https://github.com/python/cpython/issues/115798.

Yes, this is a good idea to add to the knowledge base.

As a user I can either add the alias so interactive password entry works, or remove the alias which allows the conan redirection logic to work correctly.

I am checking that some of the errors in the redirection in git base in the images above look like colorama and control characters issues. Maybe you can try to define the environment variable NO_COLOR=1 and see what happens?

RochaStratovan commented 5 months ago

Which conan commands support piped operations?

What you're describe is appropriate for command sequences like:

conan command1 args ... | conan command2 args

But if conan doesn't read from stdin, and always uses a parameter to identify the file for input such as conan list --graph=<input file> then you are correct the standard expected pattern for the information producer would be conan cmd --output-file=<file-name> --format=json

Regardless, this is a conan implementation detail that greatly surprised me and some colleagues I discussed this with. (Although, to be fair, most of us graduated from the same University)

I am checking that some of the errors in the redirection in git base in the images above look like colorama and control characters issues. Maybe you can try to define the environment variable NO_COLOR=1 and see what happens?

NO_COLOR doesn't appear to be a supported option from winpty

$ winpty --help
Usage: winpty [options] [--] program [args]

Options:
  -h, --help  Show this help message
  --mouse     Enable terminal mouse input
  --showkey   Dump STDIN escape sequences
  --version   Show the winpty version number

This is obviously not a complete listing because the alias I use has other parameters such as -Xallow-no-tty.

alias conan='winpty -Xallow-non-tty conan.exe'

The github repository for winpty doesn't have any documentation for usage or command line parameters that I could easily find, and a google search isn't helping with this :anger:


I tried variations

$ winpty NO_COLOR=1 -Xallow-non-tty conan.exe
winpty: error: cannot start 'NO_COLOR=1': Not found in PATH

The following executed, but it gives the same results as before, specifically stderr is being combined with stdout. Moreover, it still had the trailing escape codes ^[[0K

NO_COLOR=1 winpty -Xallow-non-tty conan.exe


I found it!

I took a look at the winpty source code from github, and found additional parameters that don't appear in help:

-Xallow-non-tty
-Xconerr
-Xplain
-Xcolor

I also found a winpty issue that provides additional information "Document the undocumented -X switches rprichard/winpty#103

If I use -Xplain it no longer adds the escape codes to the output. However it still has the cmake output, even thought self.run is being used. I'm pretty sure this is because winpty might be combining stderr and stdout into stdout

winpty -Xallow-non-tty -Xplain conan.exe create appA --format=json > jrr.json


A little lower in the thread I referenced above seems to explain additional aspects of this problem

while using -Xallow-non-tty disables the tty check, allowing redirecting winpty output to a pipe or file, I don't think the behavior is usually what people want. i.e. The child program will still think it's writing to a console screen buffer rather than to a pipe or file.

However, dropping -Xallow-non-tty fails

$ winpty conan.exe create appA --format=json > jrr2.json
stdout is not a tty

So -Xallow-non-tty and -Xplain would be needed together except for the fact that stdout and stderr are clubed into winpty's stdout.

Summary:

  1. winpty conan.exe is needed in order for the interactive mode of python's getpass method to work, which is used by conan's remote authentication logic.

    This Stackoverflow entry that you found indicates that using winpty conan.exe would resolve this problem. And that is confirmed. However, further comments indicate that this won't work for redirection or piping so we need to use winpty -Xallow-non-tty

  2. winpty -Xallow-non-tty conan.exe > file.txt creates file.txt where:

    1. stdout and stderr output are combined and written to file.txt
    2. color screen characters are also written (because it thinks it's writting to a tty, not a file.)
  3. winpty -Xallow-non-tty -Xplain conan.exe > file.txt creats file.txt where:

    1. stdout and stderr output are combined and written to file.txt
memsharded commented 5 months ago

But if conan doesn't read from stdin, and always uses a parameter to identify the file

Not necessarily for piping to Conan commands, but to other commands, for example I do this in my WSL

ls $(conan cache path zlib/1.3)
Migration: Successfully updated settings.yml
conandata.yml  conanfile.py  conanmanifest.txt

Note how the Migration: Successfully updated settings.yml message sent to stderr didn't break the ls command

or something like

conan list "*" | code -

There are some users that the conan ... --format=json command run in CI they are not even redirecting it to a file. They directly capture stdout in memory and parse it from there, to avoid generating extra files in disk (I don't see a big advantage here, I prefer to generate the files, but they might have their reasons).

We are aware that some tools might check stderr and if there is something there it will consider it an error, even if the application didn't return an exit error code (not really agree with those tools, these are what the exit codes are for), and there are other popular applications like git itself that is sending non-error information, like progress of git-clone to stderr.

NO_COLOR doesn't appear to be a supported option from winpty

NO_COLOR is an env-var that Conan uses to disable the color generation in terminal, that generates those special control characters, this isn't a winpty thing

memsharded commented 4 months ago

Following up on this @RochaStratovan

Did the above clarified the uses that explained the piping?

I'd also like to understand when else could be done from the Conan side. It seems that using git-bash accumulates some unfortunate issues like the Python getpass one, but still not sure if there is something that we could do to help with this.

RochaStratovan commented 4 months ago

Hello @memsharded,

I understand they reason your team used piping as they did and how you use stderr. I and my team disagree with the that direction, but we also understand that this is the eco system that has evolved for conan, and we understand the reasoning, we just don't agree with it. But, meh, it is what it is.

The NO_COLOR option didn't resolve the issue. The root cause is the Winpty workaround that's needed in order for git-bash to work correctly with gitpass. So this is another symptom/side effect of the gitpass/git-bash/python issue.

FYI, python issue https://github.com/python/cpython/issues/115798 exists for this, and they've politely explained that git-bash isn't high on their support list.

What can conan do? I think we've discussed adding a section that explains known issues with git-bash. Maybe this documentation set can add an explanation that conan commands print logs as stderr and the dependency output as stdout which allows piping to work, and if the user's environment does any sort of output redirection this can cause issues?

Thank you for your patience.

Cheers!