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

Add an alias from cargo x to cargo run -- in .cargo/config.toml. #95858

Closed
jyn514 opened this issue Apr 9, 2022 · 6 comments
Closed

Add an alias from cargo x to cargo run -- in .cargo/config.toml. #95858

jyn514 opened this issue Apr 9, 2022 · 6 comments
Labels
S-tracking-design-concerns Status: There are blocking ❌ design concerns. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap)

Comments

@jyn514
Copy link
Member

jyn514 commented Apr 9, 2022

This helps with #94829 by making it more visually similar to the existing src/tools/x command.

Originally posted by @bjorn3 in #95854 (comment)

@rustbot label +S-tracking-design-concerns

@bjorn3 bjorn3 added the T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) label Apr 9, 2022
@jyn514
Copy link
Member Author

jyn514 commented Apr 10, 2022

@rustbot label +E-easy +E-help-wanted

@rustbot rustbot added E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue. E-help-wanted Call for participation: Help is requested to fix this issue. labels Apr 10, 2022
@hi-rustin
Copy link
Member

@rustbot claim

@jyn514
Copy link
Member Author

jyn514 commented May 22, 2022

@Mark-Simulacrum: #95980 (comment)

I am not sure this is really worth it -- do we think that the long-term expectation for user interface is cargo x build?

Hmm, that's fair - I would like to eventually have the entrypoint be x and deprecate other entrypoints, which allows us to do things like cargo +beta-20YY-MM-DD without requiring rust-toolchain in the top-level directory. But that said, I think it's unusual to have a custom entrypoint like that? Maybe we should be using existing standard tools, like cargo xtask build? cc @Lokathor @thomcc

I am also happy to run a mini user survey in Zulip / the community discord; I think it will be good to learn what entrypoints people expect, even if the decision is not actually a democratic process. (update: done and done)

I think it might be worth holding off on a change like this PR until we have a clearer story around cargo run -- working and an expectation that users should migrate to it. I'm not sure we're there yet, though there's definitely a relatively happy path to getting there I expect.

👍

Maybe there's a general integration point -- similar to https://github.com/matklad/cargo-xtask/, but without .cargo/config.toml modifications which feel to me like somewhat of an anti-pattern (it's sort of a user, not upstream file, it seems to me).

That makes sense to me, but if we're avoiding changes to .cargo/config in rustbuild, we should also avoid them in bootstrap.py - it's very strange to me that bootstrap.py tries to guess whether you'd like vendoring or not and set that it up itself, IMO the user should have to configure that, which is closer to a normal cargo workflow. If we really want to support autoconfiguring vendoring, we should have that as part of x.py setup, rather than redoing it on each run.

It might be that we push for ability to declare aliased commands from Cargo.toml, pointing to subcrates in the workspace explicitly, as an unstable feature. That could mean a natural path for xbuild/xdoc/xcheck and so on to work out of the box, for example, if you already have a local rustc sysroot. Features like that have stalled out historically, but I think given a strong use case and interested hands, it's not impossible we could add something quite limited and unstable, though likely hard.

I don't really want to pursue that, especially since the cargo team is short on review time lately. I'd rather get cargo xtask or aliases to work.

@thomcc
Copy link
Member

thomcc commented May 22, 2022

cc ... @thomcc

I actually think the current situation is largely fine, FWIW. But I may be too used to the status quo -- I did have to explain how to do the build to someone in a PR recently, I assume they were trying cargo build to no avail.

@jyn514
Copy link
Member Author

jyn514 commented May 22, 2022

Some good points brought up by various people:

  • @5225225 would expect the entrypoint to work from anywhere in tree, not just at top-level like x.py does currently
  • @digama0 thinks x anywhere is confusing and we should have more explicit names
  • cargo run run expand-yaml-anchors is ... strange. cargo run in general is confusing, since it's not clear that "run" is referring to bootstrap itself.
  • there is surprisingly (at least to me) strong support for "x.py is fine as is"

@rustbot rustbot added S-tracking-design-concerns Status: There are blocking ❌ design concerns. and removed E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue. E-help-wanted Call for participation: Help is requested to fix this issue. labels May 22, 2022
@jyn514
Copy link
Member Author

jyn514 commented May 25, 2022

I am also happy to run a mini user survey in Zulip / the community discord; I think it will be good to learn what entrypoints people expect, even if the decision is not actually a democratic process. (update: done and done)

The conclusion from that survey is we're going to use x.sh and x.ps1 scripts rather than cargo aliases. https://rust-lang.zulipchat.com/#narrow/stream/122651-general/topic/bootstrap.20user.20survey/near/283897117

@jyn514 jyn514 closed this as not planned Won't fix, can't repro, duplicate, stale May 25, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-tracking-design-concerns Status: There are blocking ❌ design concerns. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap)
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants