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 check" or similar to run all phases prior to trans #1313

Closed
tomjakubowski opened this issue Feb 15, 2015 · 26 comments
Closed

"cargo check" or similar to run all phases prior to trans #1313

tomjakubowski opened this issue Feb 15, 2015 · 26 comments
Labels
A-new-subcommand Area: new subcommand

Comments

@tomjakubowski
Copy link
Contributor

During the edit-compile cycle it's often annoying to have to wait for trans to finish before moving on to the next edits. I propose a new subcommand, cargo check, that runs only the parsing, expansion, and checking/analysis phases where possible when building the project.

The implementation would be just like cargo build, but passing --no-trans to rustc when building the leaves of the crate dependency graph. A --lib option, like on cargo build, would only build the project's lib target if it has one.

@Gankra
Copy link
Contributor

Gankra commented Feb 15, 2015

👍 This would be super great.

@iopq
Copy link

iopq commented Feb 16, 2015

I compile mostly to check program correctness. I only run the tests when I am about to push my changes.

Now, my stuff is toy programs, but if it ever got to the point that I wrote a larger example that took longer to compile this would be great.

@alexcrichton alexcrichton added the A-new-subcommand Area: new subcommand label Feb 16, 2015
@alexcrichton
Copy link
Member

cc #595, this may just be cargo rustc --no-trans

@Valloric
Copy link

Valloric commented Jun 2, 2015

@alexcrichton cargo rustc --no-trans is neither very user-friendly nor easily discoverable. cargo check sounds like a really solid idea.

@norcalli
Copy link

norcalli commented Jun 5, 2015

rustc --no-trans main.rs doesn't work for me, is it supposed to?

@DanielKeep
Copy link

It's cargo rustc -- -Z no-trans, and apparently, even if the build succeeds, the cargo rustc command itself fails and reports "An unknown error occurred", with an exit status of 101.

So it's not quite the same thing. :P

@rsolomo
Copy link

rsolomo commented Jul 9, 2015

I've taken a pass at creating a cargo check as an external subcommand here: https://github.com/rsolomo/cargo-check

@munael
Copy link

munael commented Jul 10, 2015

Would an integrated cargo check be just an alias for -Zno-trans? I don't know either way, genuinely asking.

@MarkSwanson
Copy link

$ cargo rustc -- -Z no-trans
extra arguments to rustc can only be passed to one target, consider filtering
the package by passing e.g. --lib or --bin NAME to specify a single target

(cargo-check has the same problem)

In my Cargo.toml I have:
[[bin]]
name = "runme"
path = "src/bin/runme.rs"

[lib]
name = "my_lib"
path = "src/lib.rs"

I'd like them both to be compiled with -Zno-trans (remove the restriction of a single target).

Reason: I do not want to manually type different commands to compile everything with -Zno-trans; it's just too much work. In vim I'd just like to ":mak" - which works fine until I add -Zno-trans :-(

Thanks!

@alexcrichton
Copy link
Member

This now exists! It should also be quite easy to install via cargo install.

@Valloric
Copy link

Why does the user have to install it separately? It's neither easily discoverable nor friendly to newbies.

@arthurprs
Copy link

@Valloric I share the opinion.

Even when incremental compilation lands llvm will probably always take a big chunk of compilation time, so I see no reason not to incorporate this feature in cargo itself.

@Valloric
Copy link

@arthurprs Exactly! This is a trivial command to add and cargo should have it by default so that it shows up in cargo help, which every single newbie will run. It should then be recommended in the Rust guide to further increase the chances that newbies pick up on it.

It would lead to a massive quality-of-life improvement for everyday Rust development. It's such an easy, low-cost win that I can't fathom why the core devs aren't falling over themselves to implement it.

@alexcrichton
Copy link
Member

@Valloric I've opened #2187 on the discovery aspect of subcommands, but I agree that there's a general sentiment that a subcommand such as this should be in cargo itself as well.

Unfortunately there's no stabilization route for Cargo itself like there is for Rust, so all new additions are instantly stable. This means that we need to be extra careful before adding new features, and it's intended that subcommands can serve as both a breeding and testing ground for movement into Cargo itself.

@Valloric
Copy link

Valloric commented Dec 1, 2015

Unfortunately there's no stabilization route for Cargo itself like there is for Rust, so all new additions are instantly stable. This means that we need to be extra careful before adding new features, and it's intended that subcommands can serve as both a breeding and testing ground for movement into Cargo itself.

Excellent point. I agree that the subcommands should first exist as separate binaries like cargo check exists today. In this specific case, cargo check has been around since July and is quite literally just a wrapper around cargo rustc -- -Zno-trans. It's about as low risk as we could hope a new subcommand to be. It doesn't add new functionality that might come with bugs, it takes existing functionality and presents it in a user-friendly way.

Though I'd argue that a built-in cargo check shouldn't be documented as being merely a wrapper around cargo rustc -- -Zno-trans; that's an implementation detail. The contract should be something like the following:

Presents most diagnostic errors without producing an output binary. Some checks might be removed or added in the future.

This way there's room for future extension and the subcommand is not bound by the (sub) set of checks currently performed.

@alexcrichton
Copy link
Member

Yeah that was basically our thinking as well. I suspect that it will be one of the first subcommands to be moved into Cargo. Also yes, it won't be documented as using cargo rustc.

@Valloric
Copy link

Valloric commented Dec 5, 2015

Sounds great. Thanks for being open about this!

@Valloric
Copy link

To provide some data on the need for a built-in cargo check, the very top comment on the Rust 1.5 reddit thread in both the r/rust subreddit and in r/programming is talking about how great an idea that command is.

@calebmer
Copy link

Having cargo check as a command shipped by default would help developer experience. The visual studio code plugin, for example, depends on it.

@bruno-medeiros
Copy link

The check command as in https://github.com/rsolomo/cargo-check has severe limitations: you have to build a specific Cargo target each time (rsolomo/cargo-check#4), which is very problematic, especially with regards to the tests targets. I want a Cargo command that checks everything in my crate, this is what you'd expect to be the default behavior. (see also: #2495)

@bruno-medeiros
Copy link

Having cargo check as a command shipped by default would help developer experience

I agree. I want to add this functionality to https://github.com/RustDT/RustDT, and it's the same story, more convenient if it's bundled with Rust. Also https://github.com/rsolomo/cargo-check has the limitations I mentioned above, which means changes to Cargo itself might be required if those limitations are to be addressed.

Unfortunately there's no stabilization route for Cargo itself like there is for Rust, so all new additions are instantly stable

Shouldn't there be a stabilization route for Cargo then, if it's causing so much trouble? In terms of making suggestions to improve IDE interaction, I've barely got started doing Cargo feedback, but already the stabilization issue of the Cargo interface is making progress very difficult (the deterministic filenames for test targets is another example of that - a change that otherwise would be trivial).

@bruno-medeiros
Copy link

Yeah that was basically our thinking as well. I suspect that it will be one of the first subcommands to be moved into Cargo. Also yes, it won't be documented as using cargo rustc.

@alexcrichton If you agree with that, can a new issue be created so that we track it? (Or re-open this one if you prefer)

@alexcrichton
Copy link
Member

Ok

@nrc
Copy link
Member

nrc commented Oct 17, 2016

Another vote for built-in cargo check support from me - would greatly improve the RLS/IDE experience.

A problem we currently have (and I'm not sure if the external check solves this or not) is that if a dependent crate is a macro crate, then -Zno-trans will break the build. We need Cargo to build macro crates to completion, and skip translation for all other crates. I think there may also be build script issues, although this might be a separate issue.

@nrc
Copy link
Member

nrc commented Dec 15, 2016

Closed by #3296

@alexcrichton
Copy link
Member

Yay!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-new-subcommand Area: new subcommand
Projects
None yet
Development

No branches or pull requests