Closed navaati closed 8 years ago
Can you retry with debug-on-error
enabled?
Yes sir, here it is:
Debugger entered--Lisp error: (wrong-type-argument stringp nil) call-process(nil nil t nil "read-manifest") flycheck-rust-find-target("/home/navaati/Projects/fuse-music/src/musicfs/mod.rs") flycheck-rust-setup() funcall-interactively(flycheck-rust-setup) call-interactively(flycheck-rust-setup record nil) command-execute(flycheck-rust-setup record) execute-extended-command(nil "flycheck-rust-setup" "flycheck-rust") funcall-interactively(execute-extended-command nil "flycheck-rust-setup" "flycheck-rust") call-interactively(execute-extended-command nil nil) command-execute(execute-extended-command)
Looks like (funcall flycheck-executable-find "cargo")
is returning nil
.
We should probably check for that case, but in the meantime, @navaati I suppose you have cargo
installed and on your PATH environment variable?
Ah, maybe the PATH is right in my shell but not in emacs, there is a pretty good chance the problem come from that. Thanks for pointing the right direction to me ! I'll confirm that tomorrow.
Yep, confirmed (and now it works). Thanks, and sorry for wasting your time.
@navaati Not at all, thanks for reporting this issue! We should definitely not fail like that if a program is missing! It's an issue that we need to fix, to present a helpful error message!
Ok, I'll let you close the issue when the case is handled then. Have a nice day !
@navaati Yes, thank you for reporting this, and happy hacking :)
@lunaryorn I guess we can emit a warning when ~cargo~ isn't found, since users adding ~flycheck-rust-setup~ would expect it to do something.
@navaati Take a look at our documentation; I think it's already got this issue covered.
@fmdkdd I think an error is fine—without cargo we can't reasonably continue to setup the buffer anyway.
But it should be a better error imho: We should check whether flycheck-executable-find
returns nil
and if so, signal a user-error
that "Cargo" is missing, maybe even including a link to our troubleshooting section.
So I tried to add a user-error
when flycheck-rust-find-target
cannot find an executable. It works, but a maybe a bit too well.
With global-flycheck-mode
on, the error appears when you try to C-x C-f
a rust file that's in a cargo project, and it fails to open the file with this error:
Error in post-command-hook (global-flycheck-mode-check-buffers): <user error message>
Worse, most commands also trigger the error after this failure, rendering Emacs unusable until exit. So let's say I'm on the Emacs welcome screen, I C-x C-f
into a rust file in a cargo project, and I get the error. I'm still on the welcome screen, because the error prevented the file to open. But now, further C-x C-f
triggers the error again, as does M-x
.
Here's a sample .emacs
to reproduce this behavior:
(package-initialize)
(require 'flycheck)
(require 'flycheck-rust)
(require 'rust-mode)
(add-hook 'flycheck-mode-hook 'flycheck-rust-setup)
(global-flycheck-mode)
You also get this behavior when cargo
simply cannot be found and global-flycheck-mode
is active. No need to add the user-error
: this is already the current behavior. This raises an error, aborts the file opening, and sticks you on the current buffer.
The cause seems to be that define-globalized-minor-mode
will try to enable flycheck-mode
in find-file-hook
through the function global-flycheck-mode-check-buffers
, and when this is done, it will remove itself from post-command-hook
. Here's the code in easy-mmode.el
:
(defun ,MODE-check-buffers ()
(,MODE-enable-in-buffers)
(setq ,MODE-buffers nil)
(remove-hook 'post-command-hook ',MODE-check-buffers))
(put ',MODE-check-buffers 'definition-name ',global-mode)
But enabling flycheck-mode
will run flycheck-rust-setup
(when using the recommended add-hook
configuration), and since this can raise an error, the remove-hook
line is never executed.
So it seems we should try to avoid raising any error in flycheck-rust-setup
if we recommend setting it as a hook, otherwise we will trigger this corner case and render Emacs unusable for users.
Emitting a simple message does the trick (but then we run into another usability issue, "suspicious checker" errors because we run the checkers even when the rust-related variables are nil).
Alternatively, we could run flycheck-rust-setup
in a separate hook, one that runs after flycheck is loaded but before the checkers run.
@flycheck/maintainers what's your opinion?
So, I see that user-error
isn't a good idea, but I'm not sure I understand why we can't just skip the setup when Cargo isn't found? Why would the rust syntax checkers still run in this case? The cargo-based one should be skipped just as well—the other one might run for sure, but how likely is it to have rustc
in path but not cargo
? I don't think you can even install them separately these days… rust always comes with cargo doesn't it?
Oh we could skip the setup when cargo isn't found, and I think it's the sane thing to do too.
But the rust-cargo checker will still run, because its predicate simply looks for a Cargo.toml
file, not for the presence of the cargo binary or any variable set by flycheck-rust-setup
. We would have to modify the predicate. The rustc
predicate for instance checks for not flycheck-rust-crate-root
which should be set by flycheck-rust-setup
; I assume this is a way to fallback on the rustc
when flycheck-rust-setup
failed to set any variable. We could use that variable, and that would make the predicates the opposite of each other.
I'm sure you can get by installing rustc
without cargo
, but yes, it's safe to assume they are either both present or both missing.
I'm sure you can get by installing rustc without cargo, but yes, it's safe to assume they are either both present or both missing.
But then it's unlikely that any rust checker would run if cargo is missing, right?
I'd keep things simple: Let's just make sure that we skip the setup when Cargo is missing, and look whether the issue appears again.
But then it's unlikely that any rust checker would run if cargo is missing, right?
Ah, I see what you mean, if their executable aren't found then they would be disabled by flycheck automatically, so there's no hurt on skipping the setup since there won't be any checking.
It seems coherent to make the same check in flycheck-rust-setup
. Agreed on the simple skip.
@lunaryorn In the end, I used an user-error
in conjunction with with-demoted-errors
to ensure we do not raise any error. See f8ae845.
That's nice 👍 Good idea
Hi.
When running flycheck-rust-setup I get this error everytime. Then flycheck checks nothing, nothing is underlined and running flycheck-buffer manually does nothing (no error).