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

Cargo install and uninstall should use stdout #5057

Closed
moore3071 opened this issue Feb 19, 2018 · 6 comments
Closed

Cargo install and uninstall should use stdout #5057

moore3071 opened this issue Feb 19, 2018 · 6 comments

Comments

@moore3071
Copy link

Cargo install and uninstall currently solely utilize stderr for output. Certain tools, such as Vagrant when it's running provisioning scripts, colourize output based on whether it's stdout or stderr.

It should be noted that using stdout for installation and removal is common among package managers. I tested gem, pip, pacman, apt-get, and npm. Each of these managers used stdout for installation and removal, while only using stderr for warnings and errors.

@WiSaGaN
Copy link
Contributor

WiSaGaN commented Feb 22, 2018

There was some discussion in Cargo output could go to stderr #1473.

@moore3071
Copy link
Author

Ah, I was actually thinking about the piping problem mentioned in there when I posted this issue. The main problem seems to be that Cargo is such a multi-faceted tool. It's not just a package manager nor just a build tool.

I think @alexcrichton's question in #1473 is important here:

It does seem odd, though, to print informational output to stderr. Do other CLI tools have a habit of doing this?

So what is the habit for other tools? While most CLI tools leaves stdout for machine-readable output, package managers typically opt for using stdout by default and stderr is left for errors and warnings.

What about piping? Well Cargo as a package manager has very little to say that would need to be interpreted by another command. As such there would be no downside to changing from stderr to stdout for cargo install and cargo uninstall at this time.

We might want to think about what would happen if Cargo as a package manager wants to provide machine readable output in the future. This part is hypothetical. If this changed in the future, it would make sense to provide an option to filter out the human readable output. This might be a simple modification of -q/--quiet. Maybe a single -q flag for machine readable output and two for no output.

@nrc
Copy link
Member

nrc commented Feb 22, 2018

This would break the RLS (we could probably work around it, but it would be annoying) since stdout is reserved for communicating with the client (not that I think this is a great design decision, but it is an external constraint).

Also the reasoning in #1473 still applies.

Given those factors and that other tools probably rely on the current setup, I don't think it is possible to change, so closing.

@nrc nrc closed this as completed Feb 22, 2018
@moore3071
Copy link
Author

This would break the RLS (we could probably work around it, but it would be annoying) since stdout is reserved for communicating with the client (not that I think this is a great design decision, but it is an external constraint).

Could you explain what RLS stands for?

Also the reasoning in #1473 still applies.

#1473 was made to address cargo run originally. The arguments in that issue address cargo as a build tool. The convention for package managers for treatment of stdout is quite different from other cli tools.

...other tools probably rely on the current setup...

I honestly doubt this. Since cargo install and cargo uninstall have no output to stdout (correct me if I'm wrong about that), no tools would be relying on this for piping. If there are any tools reading stderr on these commands, this would be a breaking change, but it would be easy to address by them looking at stdout instead of stderr. If Cargo was past 1.0.0 you could argue that this would require a major version change, but since it's not, a minor bump will do.

The Rust owners seem to be trying to avoid needlessly breaking convention, and the current setup seems to be a break from the convention of package managers. It seems better to address this now while Cargo is young instead of having it come up when it's much harder to change.

@nrc
Copy link
Member

nrc commented Feb 22, 2018

RLS is Rust Language Server

It's also a breaking change if the tools assume there is nothing on stdout.

Although Cargo is pre-1.0, it is actually pretty stable and we do try pretty hard to avoid breaking changes.

I don't think we can abide by the convention of both build systems and package managers here (and Cargo is both).

@moore3071
Copy link
Author

Oh wow, I can't believe that I didn't know about RLS. That'll be helpful for future projects. You're quite right about tools expecting nothing on stdout breaking if we changed this. I was only thinking of piping, and let's be honest, it's foolish to pipe from something with no output on stdout.

I think it's actually reasonable to abide by both conventions. Most subcommands can be categorized as a build-tool or package-manager tool.

Build tools:

  • build
  • check
  • clean
  • doc
  • new
  • init
  • run
  • test
  • bench
  • update

package-manager tools:

  • search
  • install
  • uninstall

This leaves publish which I would categorize as a build-tool. It's also interesting to note that cargo search already uses stdout. The output would require a good little bit of manipulation to use too (with sed, grep, or whatever tool you want). If we wanted to strictly keep with the build-tools convention, then you could argue that other tools might want to read the dependencies and buildtime printed by cargo install 😉

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

No branches or pull requests

3 participants