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

fix(watch): track looked up binaries #5008

Merged
merged 1 commit into from
Oct 18, 2021
Merged

Conversation

rgrinberg
Copy link
Member

Now dune's watch mode will be responsive when binaries it uses changes.
Note: some basic binaries are exempt from this ($vcs, cygpath, bash, sh,
etc.)

@rgrinberg rgrinberg requested review from snowleopard and a user October 16, 2021 22:52
Copy link

@ghost ghost left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good once add_exe gets a doc comment.

Now dune's watch mode will be responsive when binaries it uses changes.
Note: some basic binaries are exempt from this ($vcs, cygpath, bash, sh,
etc.)

Signed-off-by: Rudi Grinberg <me@rgrinberg.com>
@rgrinberg rgrinberg merged commit 3add22b into ocaml:main Oct 18, 2021
@ghost
Copy link

ghost commented Oct 19, 2021

This seems to have broken things. Now I'm getting:

Error: inotify_add_watch: /usr/local/bin: No such file or directory
-> required by %{bin-available:hg} at test/blackbox-tests/test-cases/dune:153

@rgrinberg
Copy link
Member Author

That's weird, the directory clearly exists for you. Any idea what's happening @snowleopard

@ghost
Copy link

ghost commented Oct 19, 2021

It doesn't actually. But it is in my PATH. We have to assume that some of the directories in the PATH might not exist and cope with it.

@rgrinberg
Copy link
Member Author

I see. So you don't even have /usr/local?

We should definitely handle this. Will send a fix

@ghost
Copy link

ghost commented Oct 19, 2021

I have /usr/local but not /usr/local/bin

@ghost
Copy link

ghost commented Nov 1, 2021

@rgrinberg did you get a chance to look at this? I still have edit my PATH every time I start working on Dune

@rgrinberg
Copy link
Member Author

I fixed it for the fsevents backend in #4990. Can we get that PR merged and I'll do a fix for inotify. I've crammed enough into one PR already :)

@rgrinberg rgrinberg added this to the 3.0 milestone Nov 2, 2021
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

Successfully merging this pull request may close these issues.

1 participant