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

Restore parse-only to rustc #748

Closed
wants to merge 1 commit into from
Closed

Conversation

xcambar
Copy link

@xcambar xcambar commented Nov 1, 2016

It seems that #613 deleted the args passed to rustc, as introduced in #276, causing a regression as described in #275.

This PR restores the args (-Z parse-only).

It seems that neomake#613 deleted the args passed to rustc, as introduced in neomake#276.

This PR restores the args (`-Z parse-only`), so neomake#275 will not be reopened.
@blueyed
Copy link
Member

blueyed commented Nov 1, 2016

Ping @fuine

@fuine
Copy link
Contributor

fuine commented Nov 1, 2016

I decided to remove parse-only for two reasons:

  • it doesn't show whole class of errors (-Z no-trans does however)
  • all -Z flags are currently unstable (see this issue for more information)

Current solution is especially bad for cargo maker, as the cargo build command can take quite some time to recompile whole project. We could get around that by calling cargo/rustc with -Z flag and ignore the warning message in the errorfmt. For cargo maker we would also have to decide whether we want to call it with --lib or --bin, as there is no way of calling cargo with custom flags for rustc in the project with multiple targets without specifying the target (--lib or --bin) explicitly (maybe this should be left to user).

@fuine
Copy link
Contributor

fuine commented Nov 1, 2016

Also let me know if you have regression as described in #275, I use cargo maker for development of several crates and so far the new errorfmt works flawlessly.

@blueyed
Copy link
Member

blueyed commented Nov 10, 2016

@xcambar
What do you think?

@blueyed
Copy link
Member

blueyed commented Nov 14, 2016

Closing this for now, feel free to re-open.

@blueyed blueyed closed this Nov 14, 2016
@xcambar
Copy link
Author

xcambar commented Nov 14, 2016

Sorry I'm late to reply. I'm fine with closing the PR.

Note that I'm a super extra noob to Rust, still figuring out the environment, and as such I may not be the best person to discuss the choices of more seasoned Rust devs such as @fuine until I'm more comfortable with the ins and outs of Rust's tooling.

@blueyed
Copy link
Member

blueyed commented Nov 14, 2016

@xcambar
That is very reasonable - I'm even much more a noob in this regard, and happy that @fuine helps out.. :)

@jonhoo
Copy link

jonhoo commented Nov 19, 2016

@fuine what kind of Neomake configuration are you running with now then? I'm now getting a lot of "unresolved import" errors with make-on-save when working on files in a Cargo-managed project. Have you just disabled the rust maker altogether?

@jonhoo
Copy link

jonhoo commented Nov 19, 2016

The Cargo maker is also significantly slower than the Rust maker, so relying only on the former is a bit of a chore. For my current project, it takes 23 seconds from save until the errors are shown..

@fuine
Copy link
Contributor

fuine commented Nov 19, 2016

@jonhoo these are selected lines from my ftplugin/rust.vim

let g:neomake_enabled_makers = ['cargo']
autocmd! BufWritePost * Neomake! cargo

The time to show warnings is not an issue of neomake, but rather it's the time it takes cargo to perform the whole debug build of your project. That being said 23 seconds is still suspiciously high number - usually cargo will take much longer compile time if:

  • it has to recompile dependencies
  • project actually builds

the latter could be avoided with -Zno-trans, but as mentioned earlier it is currently unstable.
Usually when there are errors in the project cargo will stop the build almost immediately so you should see errors quite fast. As for warnings you need to wait for the full build, so it might take some time.

Rust community is currently working on implementing incremental compilation, alpha version of which is currently in the nightly compiler, so you might take a look at it, if the build time is really bugging you. Note that this feature is of course unstable. To learn more about it visit this link.

@jonhoo
Copy link

jonhoo commented Nov 19, 2016

Following this change, I'm currently using, which amounts to about the same

let g:neomake_rust_enabled_makers = []
autocmd! BufWritePost * Neomake
autocmd BufWritePost *.rs Neomake! cargo

I'm working on a decently sized project, and ~23 seconds is the build time if the previous build did not complete (but with all dependencies compiled). Note that the errors appear almost instantly, so I wonder if there's a flag we can pass to Cargo so that it will terminate early if any part of the compilation process fails with an error:

$ cargo build -v
       Fresh rustc-serialize v0.3.21
       Fresh libc v0.1.12
...
       Fresh thread_local v0.2.7
       Fresh regex v0.1.80
   Compiling foobar v0.1.0 (file:///home/jon/dev/projects/foobar)
     Running `rustc src/lib.rs --crate-name foobar --crate-type lib -g -C metadata=52b528dae9da63ec -C extra-filename=-52b528dae9da63ec --out-dir /home/jon/dev/projects/foobar/target/debug/deps --emit=dep-info,link -L dependency=/home/jon/dev/projects/foobar/target/debug/deps [more externs] -L /home/jon/dev/projects/foobar/target/debug/build/randomkit-c6d77419a385d677/out`
error[E0428]: a type named `Domain` has already been defined in this module
  --> src/flow/alt.rs:51:1
   |
8  | struct Domain(usize);
   | --------------------- previous definition of `Domain` here
...
51 | pub struct Domain(usize);
   | ^^^^^^^^^^^^^^^^^^^^^^^^^ already defined

[various other errors]

error: aborting due to previous error

[~20 seconds pause]

error: Could not compile `distributary`.

To learn more, run the command again with --verbose.
Command exited with non-zero status 101
20.99user 0.06system 0:21.08elapsed 99%CPU (0avgtext+0avgdata 153860maxresident)k
0inputs+352outputs (0major+21988minor)pagefaults 0swaps

I see the same behavior when I run the rustc command Cargo reports directly.

@jonhoo
Copy link

jonhoo commented Nov 19, 2016

Upon further inspection, this is probably related to rust-lang/rust#37477. Once that is fixed, this would hopefully be much more manageable.

@jonhoo
Copy link

jonhoo commented Nov 19, 2016

For others with the same issue, unsetting RUST_BACKTRACE (or setting it to 0) fixes this issue, and is a decent temporary workaround.

@fuine
Copy link
Contributor

fuine commented Nov 19, 2016

Nice, i wanted to note that i couldnt observe this, and now i know why. Sadly at the moment there isn't any way of getting errors/warnings faster without using unstable features. If compile times are that much of a problem, then i'd suggest you try them out - -Zno-trans and incremental compilation should do the trick. Please note that if you are going to use any of those things, then you most probably should append a line to errorfmt in order to ignore the warning about using unstable features. I hope that i cleared the situation out for you.

@jonhoo
Copy link

jonhoo commented Nov 20, 2016

For future record, we should probably also track rust-lang/cargo#1313, as that command sounds like exactly what we want here.

@romanlevin
Copy link

rust-lang/cargo#1313 has already landed in nightly Cargo.

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

Successfully merging this pull request may close these issues.

None yet

5 participants