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

Better error message on permission errors #228

Open
kpcyrd opened this issue Jan 2, 2023 · 7 comments
Open

Better error message on permission errors #228

kpcyrd opened this issue Jan 2, 2023 · 7 comments

Comments

@kpcyrd
Copy link
Contributor

kpcyrd commented Jan 2, 2023

I get the following output:

% cargo watch -- cargo run -- -h
Error: I/O error: Permission denied (os error 13)

there's indeed a folder I can't access:

% find > /dev/null 
find: ‘./svntogit-community/sh4d0wup/trunk/pkg’: Permission denied

I assume this error is from cargo-watch (but possibly in the watchexec codebase), I think this should either not be fatal, or the path should be in the error message.

@passcod
Copy link
Member

passcod commented Jan 3, 2023

That's fixed upstream (in watchexec library v2+) and will eventually make its way to cargo watch

@azzamsa

This comment was marked as off-topic.

@passcod

This comment was marked as off-topic.

@t3hmrman
Copy link

t3hmrman commented Oct 2, 2023

Hey @passcod I'm interested in contributing the upgrade for this -- after poking around I found a few things:

  • upgrading cargo-watch 2.x breaks everything (as one might expect), as a bunch of things have changed
  • cargo-watch 2.x now depends on tokio

It looks like the PR might be quite large in trying to upgrade these libraries and effectively make cargo-watch async...

If I could put a PR together would you be open to such large changes? I'd also probably try and pull in anyhow for error handling as well for convenience in reporting errors, and maybe env_logger or tracing for logging/errors.

@passcod
Copy link
Member

passcod commented Oct 2, 2023

There's already a branch to upgrade to watchexec 2.x. Ultimately the main issue is not cargo watch, it's that I don't consider watchexec 2 to be stable or performant enough yet for cargo watch.

(also miette instead of anyhow, tracing subscriber instead of env logger)

That said, if you want to help, then in watchexec what needs to happen is to fix the implementation of the ignore file loader/discovery so it correctly respects ignore files itself, and doesn't exhibit the behaviour of spinning for tens of seconds when started in a large project.

Alternatively there's a massive PR in watchexec where I'm refactoring a bunch of things with some breaking changes. Have a look there at the checklist to see if some chunk of it interests you.

Finally, there's a third area which is programmable filters. That's much more self contained: the upstream project changed a bunch since I prototyped it, so it needs adapting and polishing.

Once those first two areas are stabilised and merged, and possibly the last one released as well, then cargo watch can be upgraded. That step should take a day or so, not a big deal once all the actual work is done.

@t3hmrman
Copy link

t3hmrman commented Oct 2, 2023

There's already a branch to upgrade to watchexec 2.x. Ultimately the main issue is not cargo watch, it's that I don't consider watchexec 2 to be stable or performant enough yet for cargo watch.

Ah thanks for noting, this, was not aware of concern about watchexec 2.x

That said, if you want to help, then in watchexec what needs to happen is to fix the implementation of the ignore file loader/discovery so it correctly respects ignore files itself, and doesn't exhibit the behaviour of spinning for tens of seconds when started in a large project.

I see a bunch of issues under ignore loading, but the main one is this one, right?

I'll try and start there -- looks like ignore-filesand filterer need to be changed, based on this comment.

Alternatively there's a massive PR in watchexec where I'm refactoring a bunch of things with some breaking changes. Have a look there at the checklist to see if some chunk of it interests you.

I'm assuming you're referring to this PR. Assuming I'm right, I think I could tackle "Adapt the CLI" but I'm not 100% sure what it means -- I assume it means upgrading clap and refactoring the command structures and stuff like that?

We can continue the discussion in that PR since it's not related to cargo-watch directly.

@passcod
Copy link
Member

passcod commented Oct 2, 2023

Yep, that's right :)

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