Skip to content

Help for proxy programs should somehow grant a peek behind The Matrix #2285

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

Open
ssomers opened this issue Apr 13, 2020 · 7 comments
Open

Help for proxy programs should somehow grant a peek behind The Matrix #2285

ssomers opened this issue Apr 13, 2020 · 7 comments

Comments

@ssomers
Copy link
Contributor

ssomers commented Apr 13, 2020

When rustup acts as a proxy (to cargo, rustc, any others?), it does too good a job of hiding itself. cargo --help gives no hint to noobs like me that there is somewhere else to look at (like rustup --help) to understand examples featuring a cargo +toolchain command.

cargo --help, or at least cargo +stable --help, should either:

  • Say up front or down below that they're a wrapper for cargo with an additional option.
  • List the +toolchain option among cargo's options (but still make clear that is not part of cargo, not documented there).
  • Pass a secret option to the proxy, and/or rely on cargo and others to document the option itself? I sure don't like it.
  • Do something else?

Looking at the code, this looks like a non-trivial job for the clap crate to be exploited at src/cli/proxy_mode.rs. Which I'll look into myself, unless someone says it's a bad idea or knows how already.

@kinnison
Copy link
Contributor

I guess we could grab the first argument check if it's --help and if so, provide a little help about the proxy setup before chaining through to the proxied command. @rbtcollins what do you think about that as a UX?

$ cargo --help
rustup 1.xx.y (blahblah)
Proxy for `cargo`

USAGE:
    cargo [+toolchain] ...cargo arguments...

---
Rust's package manager

USAGE:
    cargo [OPTIONS] [SUBCOMMAND]

OPTIONS:
    -V, --version           Print version info and exit
        --list              List installed commands
......

@ssomers
Copy link
Contributor Author

ssomers commented Apr 26, 2020

I tried implementing that, but now I think that if you're new, and you don't care about toolchains and you just installed rustup because that's how you're supposed to start with Rust, this looks complicated and scary.

So I tried a short line after the normal page, and it looks better to me. But rustup help doesn't quite explain that it acts like a proxy. Is there or should there be a different help page for proxy use?

@kinnison
Copy link
Contributor

Most people won't, if they're new, be using the +toolchain stuff unless told to, and at that point they'll probably have it explained to them as to why. The fact that it's rustup and not cargo which is handling that won't bother them a lot. I think it's fine for the proxies to add their ha'pence to the help message but not for adding additional proxy help to the main rustup. I suppose you could add a rustup help proxies if clap supports that kind of thing (and then rustup proxies would probably list all the proxies it has installed and whether the components for them are installed in the current toolchain, hmm, that could be useful)

@ssomers
Copy link
Contributor Author

ssomers commented Apr 27, 2020

at that point they'll probably have it explained to them as to why

I definitely disagree with that, that's why I came knocking here. There's a lot of "do this" advice out there without a why (and without a searchable keyword). I guess brevity was the point of the short syntax. If you want to be clear, you can use rustup run.

And rustup help run is indeed the help page I had in mind.

I have hardly any idea what information is where and what a rustup proxies would be talking about. But from the other help, it seems pretty much limited to cargo and rustc, and I think I can already tell which one is installed: rustup component remove cargo && rustup which cargo says

error: not a file: 'C:\Users\stein\.rustup\toolchains\nightly-x86_64-pc-windows-msvc\bin\cargo.exe'

@kinnison
Copy link
Contributor

It strikes me that another approach is for cargo/rustc/rustfmt etc to detect if they're run under rustup and then display some extra help. I wonder if there's a sensible way we could make that happen?

@ssomers
Copy link
Contributor Author

ssomers commented Apr 28, 2020

The original idea I had, was to always pass some secret flag to the cargo/rustc command line, which would somehow contain or refer to the additional help text, without hardcoding that text into cargo/rustc themselves. But rustup might be proxying for an old version of cargo/rustc that complains about an option it doesn't understand, right? So it would have to be yet another environment variable. And it seems pretty hard to make that abstract, and rather easier to hardcode it all into (a module shared by) those two programs.

Maybe instead whenever you do "cargo +toolchain build", rustup could first print "performing rustup run toolchain cargo build" unconditionally. That probably wouldn't be appreciated.

@rbtcollins
Copy link
Contributor

rbtcollins commented Apr 28, 2020

I completely agree that there is a UX issue here. I had patches merged to rustup before I realised the details of it proxying things things.

Lets discuss it with the cargo etc teams to see if they have a preference before we do something unilaterally.

My proposal to them though, would be that we decorate from rustup with a one-liner before the proxied commands output, because:

  • It will be more visible to users
  • A single line should stop it being intimidated
  • Prefacing output will be a lot easier to implement robustly than detecting the child closing stdout/stderr and then and only then writing an suffix to their output (and we would not want to be buffering, since we can't assume that they don't e.g. trigger man, or $PAGER on --help).

If they do do such things and we've done a prefix, worst case, our output is lost or shown on the screen when the user exits, but we haven't prevented the invoked commands UI from working how it chose to work.

We may need to have proxied commands that do choose to use man or $PAGER also explicitly signal and co-operate with rustup - which is another reason to talk with those teams about how we solve this together.

$ cargo --help
*** rustup proxy - rustup help proxy for details ***
Rust's package manager

USAGE:
    cargo [OPTIONS] [SUBCOMMAND]

OPTIONS:
    -V, --version           Print version info and exit
        --list              List installed commands

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

No branches or pull requests

3 participants