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

warning/errors reporting breaks at nightly #4998

Closed
davemilter opened this issue Feb 1, 2018 · 11 comments
Closed

warning/errors reporting breaks at nightly #4998

davemilter opened this issue Feb 1, 2018 · 11 comments

Comments

@davemilter
Copy link

davemilter commented Feb 1, 2018

With nightly cargo if I run cargo cmd inside of crate that part of workspace,
it reports errors like this:

$ cargo +nightly build
   Compiling foo v0.1.0 (file:///tmp/1/foo)
error: expected one of `!` or `::`, found `#`
 --> foo/src/lib.rs:2:1

instead of

$ cargo +stable build
   Compiling foo v0.1.0 (file:///tmp/1/foo)
error: expected one of `!` or `::`, found `#`
 --> src/lib.rs:2:1

See the difference - foo/src/lib.rs vs src/lib.rs.

This breaks many editor/IDE I suppose. In particular it breaks functionality
press on error/warning in compilation log and jump to suitable location in file.

For example with intellij-rust/intellij-rust jump from error/warning doesn't work with nightly: https://users.rust-lang.org/t/intellij-rust-and-compile-errors/15103

It breaks cargo.el project kwrooijen/cargo.el#51 (emacs wrapper around cargo commands),
also it breaks standard emacs compilation-mode, because of compilation-mode doesn't handle situation when cwd of compile comand is not equal to root of relative path.

I suppose this is related to #4788

Tested with

rustc 1.25.0-nightly (8ccab7eed 2018-01-31)
cargo 0.26.0-nightly (1d6dfea44 2018-01-26)

Also I attach simple workspace project to make problem clear.

1.zip

@alexcrichton
Copy link
Member

Yes this was caused by #4788 and currently (if we don't revert) IDEs will need to update to call cargo metadata and learn the root of a workspace

@davemilter
Copy link
Author

@alexcrichton

Yes this was caused by #4788 and currently (if we don't revert) IDEs will need to update to call cargo metadata and learn the root of a workspace

I am expecting that would be rather complex for at least GNU Emacs, long release cycle,
too wide range of users and particularly compile-mode.
But why would internal caching (as I understand #4788 ) should affect visible program behaviour,
why not save cwd at start and then use it during output?

@alexcrichton
Copy link
Member

@davemilter there was a feature request on Cargo to not recompile artifacts when the cwd changed or when the project was renamed, and fixing that also necessitated #4788. I think rust-lang/rust#47939 would help with this issue as well

@fmdkdd
Copy link
Contributor

fmdkdd commented Feb 3, 2018

Flycheck reporting in. #4788 surprised us as well, but in our case we are using the JSON output, and there, until #4788, we had absolute file paths. Is the change to relative paths in JSON output intended?

I've seen #4938, and while we can use that, I'd rather have absolute paths than call cargo metadata to figure out the workspace root.

@alexcrichton
Copy link
Member

@fmdkdd another good point! Mind filing an issue against rustc for that? (I think those paths in error messages come from rustc)

@fmdkdd
Copy link
Contributor

fmdkdd commented Feb 4, 2018

I was mistaken: before, #4788, cargo outputted paths relative to the CWD. Now they are relative to the workspace root. They were never absolute (as far as I can tell).

I'd still like absolute paths, but I guess that with rust-lang/rust#47939, we could at least restore the previous behavior right?

@matklad
Copy link
Member

matklad commented Feb 13, 2018

@fmdkdd there's now workspace_root key in cargo metadata which can be used to make relative paths absolute. See kwrooijen/cargo.el#55 for an example of usage.

@fmdkdd
Copy link
Contributor

fmdkdd commented Feb 13, 2018

@matklad I've seen #4938. I was just hoping for a solution that would not require us to call cargo metadata beforehand. Absolute paths in the JSON output still make more sense to me, as they cannot be wrong. But it looks like rust-lang/rust#47939 is not leaning in that direction.

@fmdkdd
Copy link
Contributor

fmdkdd commented Mar 13, 2018

@matklad Since rust-lang/rust#47839 has been closed, we have resorted to using cargo metadata and workspace_root after all. Consider this issue closed for Flycheck.

@stale
Copy link

stale bot commented Sep 15, 2018

As there hasn't been any activity here in over 6 months I've marked this as stale and if no further activity happens for 7 days I will close it.

I'm a bot so this may be in error! If this issue should remain open, could someone (the author, a team member, or any interested party) please comment to that effect?

The team would be especially grateful if such a comment included details such as:

  • Is this still relevant?
  • If so, what is blocking it?
  • Is it known what could be done to help move this forward?

Thank you for contributing!

If you're reading this comment from the distant future, fear not if this was closed automatically. If you believe it's still an issue please leave a comment and a team member can reopen this issue. Opening a new issue is also acceptable!

@stale stale bot added the stale label Sep 15, 2018
@stale
Copy link

stale bot commented Oct 15, 2018

As I didn't see any updates in 30 days I'm going to close this. Please see the previous comment for more information!

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

4 participants