User-configured watch exclusions #7216

jonahbeckford · 2023-03-04T02:19:10Z

This PR allows the command line option --exclude-from-watch to exclude directories from dune build --watch. ~~It should exclude more directories if those are listed in dune-workspace: (context (default (watch_exclusions node_modules doc)))~~

~~However I got lost in the Dune code base and can't figure out a way to get the Dune workspace information into the Dune file watcher. Need help with that!~~

(This is a continuation of #7086 by @nojb that was reviewed by @rgrinberg )

nojb

Thanks for the PR! I left some comments/questions. But the main one is that implementing this at the level of the "workspace" is going to be complicated, because file watching occurs at the level of the "scheduler", which is a lower-level concept. I think the natural place for this setting is to live in the Dune config file, which should make the implementation straightforward as far as I can see.

CHANGES.md

bin/common.ml

doc/dune-files.rst

src/dune_rules/workspace.mli

src/dune_rules/workspace.ml

jonahbeckford · 2023-03-07T15:56:34Z

... the main one is that implementing this at the level of the "workspace" is going to be complicated, because file watching occurs at the level of the "scheduler", which is a lower-level concept. I think the natural place for this setting is to live in the Dune config file, which should make the implementation straightforward as far as I can see.

TIL: I don't remember seeing the config file in the documentation, but now I do

The config file won't work for me; whether I want to exclude/include .opam-switch, _opam, node_modules, etc. is a local decision. And I don't like the idea of adding in a feature I can't see myself using. So, I'm going to narrow this PR to just the command line option. Nothing precludes a second PR to add in the config file support.

nojb · 2023-03-07T16:00:04Z

So, I'm going to narrow this PR to just the command line option.

Sounds good!

src/dune_config/dune_config.ml

src/dune_config/dune_config.mli

test/expect-tests/dune_file_watcher/dune_file_watcher_tests_patterns.ml

bin/common.ml

src/dune_file_watcher/dune_file_watcher.mli

test/expect-tests/dune_file_watcher/dune

christinerose

Just one capitalization suggestion for Dune

CHANGES.md

rgrinberg · 2023-03-17T03:41:46Z

test/blackbox-tests/test-cases/watching/what-dune-watches.t

  watch _build/.sync = 1
-  watch src = 3
+  watch src = 2


do we know why this test changed?

(This was super painful; I had to setup Linux to test this. It of course doesn't work on Windows, but it doesn't work on macOS either).

I updated the code to include the full set of watched directories. For whatever reason Linux is re-using the watch number (the return code of inotify_add_watch) for . and _build/.sync in the new code, while the original code has them separate.

The original code resulted in:

+| watch _build/.sync = 1 +| watch . = 2 +| watch src = 3 +| watch DUNE_SOURCEROOT/_build/default/test/blackbox-tests/test-cases/.bin = 4 +| watch DUNE_SOURCEROOT/_build/install/default/bin = 5 +| watch OPAM_SWITCH_PREFIX/bin = 6 +| watch OPAM_SWITCH_PREFIX/lib/ocaml = 7

I still don't see why your code should change this. As far as I can tell, it should be a no-op unless the watch list is somehow modified.

Fixed. Was initializing the inotify watcher twice during the switch from singleton to a parameterized function. (Last commit).

rgrinberg · 2023-03-20T21:59:08Z

src/dune_file_watcher/dune_file_watcher.ml

-let exclude_regex =
-  Re.compile (Re.alt (List.map exclude_patterns ~f:Re.Posix.re))
+let exclude_regex watch_exclusions =
+  Re.compile (Re.alt (List.map watch_exclusions ~f:Re.Posix.re))


Compilation of the pattern should only occur once. Otherwise this is quite a bit slower.

Good eye! Will fix.

rgrinberg · 2023-03-20T22:04:03Z

There's some worrying failures in CI

jonahbeckford · 2023-03-21T23:10:41Z

There's some worrying failures in CI

Fixed, all due to a rebase or the last commit (fixing the performance bug of precompiling the regex). Weird though that the CI is that sensitive to timings.

This PR allows the command line option `--exclude-from-watch` to exclude directories from dune build --watch. Signed-off-by: Jonah Beckford <71855677+jonahbeckford@users.noreply.github.com>

Signed-off-by: Jonah Beckford <71855677+jonahbeckford@users.noreply.github.com>

Co-authored-by: Nicolás Ojeda Bär <n.oje.bar@gmail.com> Signed-off-by: Jonah Beckford <71855677+jonahbeckford@users.noreply.github.com>

Signed-off-by: Jonah Beckford <71855677+jonahbeckford@users.noreply.github.com>

Co-authored-by: Christine Rose <christinerose@users.noreply.github.com> Signed-off-by: jonahbeckford <71855677+jonahbeckford@users.noreply.github.com>

Signed-off-by: Jonah Beckford <71855677+jonahbeckford@users.noreply.github.com>

Signed-off-by: Rudi Grinberg <me@rgrinberg.com>

rgrinberg · 2023-03-23T18:31:41Z

Thanks for the PR. I read it over again and I'm quite confident that it's exactly the same behavior as before except we can configure the watch list. Merged with some minor tweaks.

@rgrinberg

CHANGES: - When a rule's action is interrupted, delete any leftover directory targets. This is consistent with how we treat file targets. (@rgrinberg, 7564) - Fix plugin loading with findlib. The functionality was broken in 3.7.0. (ocaml/dune#7556, @anmonteiro) - Introduce a `public_headers` field on libraries. This field is like `install_c_headers`, but it allows to choose the extension and choose the paths for the installed headers. (ocaml/dune#7512, @rgrinberg) - Load the host context `findlib.conf` when cross-compiling (ocaml/dune#7428, fixes ocaml/dune#1701, @rgrinberg, @anmonteiro) - Resolve `ppx_runtime_libraries` in the target context when cross compiling (ocaml/dune#7450, fixes ocaml/dune#2794, @anmonteiro) - Use `$PKG_CONFIG`, when set, to find the `pkg-config` binary (ocaml/dune#7469, fixes ocaml/dune#2572, @anmonteiro) - Preliminary support for Coq compiled intefaces (`.vos` files) enabled via `(mode vos)` in `coq.theory` stanzas. This can be used in combination with `dune coq top` to obtain fast re-building of dependencies (with no checking of proofs) prior to stepping into a file. (ocaml/dune#7406, @rlepigre) - Fix dune crashing on MacOS in watch mode whenever `$PATH` contains `$PWD` (ocaml/dune#7441, fixes ocaml/dune#6907, @rgrinberg) - Fix `dune install` when cross compiling (ocaml/dune#7410, fixes ocaml/dune#6191, @anmonteiro, @rizo) - Find `pps` dependencies in the host context when cross-compiling, (ocaml/dune#7410, fixes ocaml/dune#4156, @anmonteiro) - Dune in watch mode no longer builds concurrent rules in serial (ocaml/dune#7395 @rgrinberg, @jchavarri) - `dune coq top` now correctly respects the project root when called from a subdirectory. However, absolute filenames passed to `dune coq top` are no longer supported (due to being buggy) (ocaml/dune#7357, fixes ocaml/dune#7344, @rlepigre and @Alizter) - Added a `--no-build` option to `dune coq top` for avoiding rebuilds (ocaml/dune#7380, fixes ocaml/dune#7355, @Alizter) - RPC: Ignore SIGPIPE when clients suddenly disconnect (ocaml/dune#7299, ocaml/dune#7319, fixes ocaml/dune#6879, @rgrinberg) - Always clean up the UI on exit. (ocaml/dune#7271, fixes ocaml/dune#7142 @rgrinberg) - Bootstrap: remove reliance on shell. Previously, we'd use the shell to get the number of processors. (ocaml/dune#7274, @rgrinberg) - Bootstrap: correctly detect the number of processors by allowing `nproc` to be looked up in `$PATH` (ocaml/dune#7272, @Alizter) - Speed up file copying on macos by using `clonefile` when available (@rgrinberg, ocaml/dune#7210) - Adds support for loading plugins in toplevels (ocaml/dune#6082, fixes ocaml/dune#6081, @ivg, @richardlford) - Support commands that output 8-bit and 24-bit colors in the terminal (ocaml/dune#7188, @Alizter) - Speed up rule generation for libraries and executables with many modules (ocaml/dune#7187, @jchavarri) - Add `--watch-exclusions` to Dune build options (ocaml/dune#7216, @jonahbeckford) - Do not re-render UI on every frame if the UI doesn't change (ocaml/dune#7186, fix ocaml/dune#7184, @rgrinberg) - Make coq_db creation in scope lazy (@ejgallego, ocaml/dune#7133) - Non-user proccesses such as version control or config checking are now run silently. (ocaml/dune#6994, fixes ocaml/dune#4066, @Alizter) - Add the `--display-separate-messages` flag to separate the error messages produced by commands with a blank line. (ocaml/dune#6823, fixes ocaml/dune#6158, @esope) - Accept the Ordered Set Language for the `modes` field in `library` stanzas (ocaml/dune#6611, @anmonteiro). - dune install now respects --display quiet mode (ocaml/dune#7116, fixes ocaml/dune#4573, fixes ocaml/dune#7106, @Alizter) - Stub shared libraries (dllXXX_stubs.so) in Dune-installed libraries could not be used as dependencies of libraries in the workspace (eg when compiling to bytecode and/or Javascript). This is now fixed. (ocaml/dune#7151, @nojb) - Allow the main module of a library with `(stdlib ...)` to depend on other libraries (ocaml/dune#7154, @anmonteiro). - Bytecode executables built for JSOO are linked with `-noautolink` and no longer depend on the shared stubs of their dependent libraries (ocaml/dune#7156, @nojb) - Added a new user action `(concurrent )` which is like `(progn )` but runs the actions concurrently. (ocaml/dune#6933, @Alizter) - Allow `(stdlib ...)` to be used with `(wrapped false)` in library stanzas (ocaml/dune#7139, @anmonteiro). - Allow parallel execution of inline tests partitions (ocaml/dune#7012, @hhugo) - Support `(link_flags ...)` in `(cinaps ...)` stanza. (ocaml/dune#7423, fixes ocaml/dune#7416, @nojb) - Allow `(package ...)` in any position within `(rule ...)` stanza (ocaml/dune#7445, @Leonidas-from-XIV) - Always include `opam` files in the generated `.install` file. Previously, it would not be included whenever `(generate_opam_files true)` was set and the `.install` file wasn't yet generated. (ocaml/dune#7547, @rgrinberg)

@rleshchinskiy

CHANGES: - Fix string quoting in the json file written by `--trace-file` (ocaml/dune#7773, @rleshchinskiy) - Read `pkg-config` arguments from the `PKG_CONFIG_ARGN` environment variable (ocaml/dune#1492, ocaml/dune#7734, @anmonteiro) - Correctly set `MANPATH` in `dune exec`. Previously, we would use the `bin/` directory of the context. (ocaml/dune#7655, @rgrinberg) - Allow overriding the `ocaml` binary with findlib configuration (ocaml/dune#7648, @rgrinberg) - merlin: ignore instrumentation settings for preprocessing. (ocaml/dune#7606, fixes ocaml/dune#7465, @Alizter) - When a rule's action is interrupted, delete any leftover directory targets. This is consistent with how we treat file targets. (ocaml/dune#7564, @rgrinberg) - Fix plugin loading with findlib. The functionality was broken in 3.7.0. (ocaml/dune#7556, @anmonteiro) - Introduce a `public_headers` field on libraries. This field is like `install_c_headers`, but it allows to choose the extension and choose the paths for the installed headers. (ocaml/dune#7512, @rgrinberg) - Load the host context `findlib.conf` when cross-compiling (ocaml/dune#7428, fixes ocaml/dune#1701, @rgrinberg, @anmonteiro) - Add a `coqdoc_flags` field to the `coq.theory` stanza allowing the user to pass extra arguments to `coqdoc`. (ocaml/dune#7676, fixes ocaml/dune#7954 @Alizter) - Resolve `ppx_runtime_libraries` in the target context when cross compiling (ocaml/dune#7450, fixes ocaml/dune#2794, @anmonteiro) - Use `$PKG_CONFIG`, when set, to find the `pkg-config` binary (ocaml/dune#7469, fixes ocaml/dune#2572, @anmonteiro) - Modules that were declared in `(modules_without_implementation)`, `(private_modules)` or `(virtual_modules)` but not declared in `(modules)` will cause Dune to emit a warning which will become an error in 3.9. (ocaml/dune#7608, fixes ocaml/dune#7026, @Alizter) - Preliminary support for Coq compiled intefaces (`.vos` files) enabled via `(mode vos)` in `coq.theory` stanzas. This can be used in combination with `dune coq top` to obtain fast re-building of dependencies (with no checking of proofs) prior to stepping into a file. (ocaml/dune#7406, @rlepigre) - Fix dune crashing on MacOS in watch mode whenever `$PATH` contains `$PWD` (ocaml/dune#7441, fixes ocaml/dune#6907, @rgrinberg) - Fix `dune install` when cross compiling (ocaml/dune#7410, fixes ocaml/dune#6191, @anmonteiro, @rizo) - Find `pps` dependencies in the host context when cross-compiling, (ocaml/dune#7410, fixes ocaml/dune#4156, @anmonteiro) - Dune in watch mode no longer builds concurrent rules in serial (ocaml/dune#7395 @rgrinberg, @jchavarri) - Dune can now detect Coq theories from outside the workspace. This allows for composition with installed theories (not necessarily installed with Dune). (ocaml/dune#7047, @Alizter, @ejgallego) - `dune coq top` now correctly respects the project root when called from a subdirectory. However, absolute filenames passed to `dune coq top` are no longer supported (due to being buggy) (ocaml/dune#7357, fixes ocaml/dune#7344, @rlepigre and @Alizter) - Added a `--no-build` option to `dune coq top` for avoiding rebuilds (ocaml/dune#7380, fixes ocaml/dune#7355, @Alizter) - RPC: Ignore SIGPIPE when clients suddenly disconnect (ocaml/dune#7299, ocaml/dune#7319, fixes ocaml/dune#6879, @rgrinberg) - Always clean up the UI on exit. (ocaml/dune#7271, fixes ocaml/dune#7142 @rgrinberg) - Bootstrap: remove reliance on shell. Previously, we'd use the shell to get the number of processors. (ocaml/dune#7274, @rgrinberg) - Bootstrap: correctly detect the number of processors by allowing `nproc` to be looked up in `$PATH` (ocaml/dune#7272, @Alizter) - Speed up file copying on macos by using `clonefile` when available (@rgrinberg, ocaml/dune#7210) - Adds support for loading plugins in toplevels (ocaml/dune#6082, fixes ocaml/dune#6081, @ivg, @richardlford) - Support commands that output 8-bit and 24-bit colors in the terminal (ocaml/dune#7188, @Alizter) - Speed up rule generation for libraries and executables with many modules (ocaml/dune#7187, @jchavarri) - Add `--watch-exclusions` to Dune build options (ocaml/dune#7216, @jonahbeckford) - Do not re-render UI on every frame if the UI doesn't change (ocaml/dune#7186, fix ocaml/dune#7184, @rgrinberg) - Make coq_db creation in scope lazy (@ejgallego, ocaml/dune#7133) - Non-user proccesses such as version control or config checking are now run silently. (ocaml/dune#6994, fixes ocaml/dune#4066, @Alizter) - Add the `--display-separate-messages` flag to separate the error messages produced by commands with a blank line. (ocaml/dune#6823, fixes ocaml/dune#6158, @esope) - Accept the Ordered Set Language for the `modes` field in `library` stanzas (ocaml/dune#6611, @anmonteiro). - dune install now respects --display quiet mode (ocaml/dune#7116, fixes ocaml/dune#4573, fixes ocaml/dune#7106, @Alizter) - Stub shared libraries (dllXXX_stubs.so) in Dune-installed libraries could not be used as dependencies of libraries in the workspace (eg when compiling to bytecode and/or Javascript). This is now fixed. (ocaml/dune#7151, @nojb) - Allow the main module of a library with `(stdlib ...)` to depend on other libraries (ocaml/dune#7154, @anmonteiro). - Bytecode executables built for JSOO are linked with `-noautolink` and no longer depend on the shared stubs of their dependent libraries (ocaml/dune#7156, @nojb) - Added a new user action `(concurrent )` which is like `(progn )` but runs the actions concurrently. (ocaml/dune#6933, @Alizter) - Allow `(stdlib ...)` to be used with `(wrapped false)` in library stanzas (ocaml/dune#7139, @anmonteiro). - Allow parallel execution of inline tests partitions (ocaml/dune#7012, @hhugo) - Support `(link_flags ...)` in `(cinaps ...)` stanza. (ocaml/dune#7423, fixes ocaml/dune#7416, @nojb) - Allow `(package ...)` in any position within `(rule ...)` stanza (ocaml/dune#7445, @Leonidas-from-XIV) - Always include `opam` files in the generated `.install` file. Previously, it would not be included whenever `(generate_opam_files true)` was set and the `.install` file wasn't yet generated. (ocaml/dune#7547, @rgrinberg) - Fix regression where Merlin was unable to handle filenames with uppercase letters under Windows. (ocaml/dune#7577, @nojb) - On nix+macos, pass `-f` to the codesign hook to avoid errors when the binary is already signed (ocaml/dune#7183, fixes ocaml/dune#6265, @greedy) - Fix bug where RPC clients built with dune-rpc-lwt would crash when closing their connection to the server (ocaml/dune#7581, @gridbugs) - Introduce mdx stanza 0.4 requiring mdx >= 2.3.0 which updates the default list of files to include `*.mld` files (ocaml/dune#7582, @Leonidas-from-XIV) - Fix RPC server on Windows (used for OCaml-LSP). (ocaml/dune#7666, @nojb) - Coq language versions less 0.8 are deprecated, and will be removed in an upcoming Dune version. All users are required to migrate to `(coq lang 0.8)` which provides the right semantics for theories that have been globally installed, such as those coming from opam (@ejgallego, @Alizter) - Bump minimum version of the dune language for the melange syntax extension from 3.7 to 3.8 (ocaml/dune#7665, @jchavarri)

jonahbeckford requested a review from christinerose as a code owner March 4, 2023 02:19

nojb reviewed Mar 5, 2023

View reviewed changes

christinerose approved these changes Mar 6, 2023

View reviewed changes

jonahbeckford force-pushed the feature-user-exclusions branch 2 times, most recently from 4bc992c to 548bf0c Compare March 7, 2023 17:21

rgrinberg requested a review from Alizter March 11, 2023 00:58

Alizter approved these changes Mar 11, 2023

View reviewed changes

src/dune_config/dune_config.ml Outdated Show resolved Hide resolved

src/dune_config/dune_config.mli Outdated Show resolved Hide resolved

test/expect-tests/dune_file_watcher/dune_file_watcher_tests_patterns.ml Outdated Show resolved Hide resolved

nojb reviewed Mar 11, 2023

View reviewed changes

bin/common.ml Outdated Show resolved Hide resolved

bin/common.ml Outdated Show resolved Hide resolved

jonahbeckford force-pushed the feature-user-exclusions branch 3 times, most recently from 7b61702 to 0a9d489 Compare March 12, 2023 15:00

rgrinberg reviewed Mar 12, 2023

View reviewed changes

src/dune_file_watcher/dune_file_watcher.mli Outdated Show resolved Hide resolved

rgrinberg reviewed Mar 12, 2023

View reviewed changes

test/expect-tests/dune_file_watcher/dune Outdated Show resolved Hide resolved

jonahbeckford force-pushed the feature-user-exclusions branch from 0a9d489 to f6f921c Compare March 14, 2023 21:16

christinerose approved these changes Mar 15, 2023

View reviewed changes

CHANGES.md Outdated Show resolved Hide resolved

christinerose approved these changes Mar 16, 2023

View reviewed changes

rgrinberg reviewed Mar 17, 2023

View reviewed changes

jonahbeckford force-pushed the feature-user-exclusions branch from ee1a32b to 9a81633 Compare March 20, 2023 04:48

rgrinberg reviewed Mar 20, 2023

View reviewed changes

jonahbeckford force-pushed the feature-user-exclusions branch from d386b2f to bd7be7a Compare March 21, 2023 21:02

jonahbeckford and others added 6 commits March 21, 2023 16:45

User-configured watch exclusions

967381a

This PR allows the command line option `--exclude-from-watch` to exclude directories from dune build --watch. Signed-off-by: Jonah Beckford <71855677+jonahbeckford@users.noreply.github.com>

Add watch_exclusions to tests

8e10a06

Signed-off-by: Jonah Beckford <71855677+jonahbeckford@users.noreply.github.com>

Apply nojb suggestions from code review

c17aeb0

Co-authored-by: Nicolás Ojeda Bär <n.oje.bar@gmail.com> Signed-off-by: Jonah Beckford <71855677+jonahbeckford@users.noreply.github.com>

Apply alizter suggestions from PR

0df8616

Signed-off-by: Jonah Beckford <71855677+jonahbeckford@users.noreply.github.com>

Approve christinerose changes: Update CHANGES.md

7035d27

Co-authored-by: Christine Rose <christinerose@users.noreply.github.com> Signed-off-by: jonahbeckford <71855677+jonahbeckford@users.noreply.github.com>

Apply rgrinberg suggestions from PR

97cd703

Signed-off-by: Jonah Beckford <71855677+jonahbeckford@users.noreply.github.com>

perf: Precompile watch exclusions

bf741c4

Signed-off-by: Jonah Beckford <71855677+jonahbeckford@users.noreply.github.com>

jonahbeckford force-pushed the feature-user-exclusions branch 2 times, most recently from 5bd8eb1 to ce2902d Compare March 21, 2023 23:47

bugfix: init inotify once

559b85a

Signed-off-by: Jonah Beckford <71855677+jonahbeckford@users.noreply.github.com>

jonahbeckford force-pushed the feature-user-exclusions branch from ce2902d to 559b85a Compare March 22, 2023 00:08

_

092764b

Signed-off-by: Rudi Grinberg <me@rgrinberg.com>

rgrinberg added this to the 3.8.0 milestone Mar 23, 2023

rgrinberg added 2 commits March 23, 2023 11:48

_

a405860

Signed-off-by: Rudi Grinberg <me@rgrinberg.com>

_

817aac2

Signed-off-by: Rudi Grinberg <me@rgrinberg.com>

rgrinberg merged commit 8ec05b3 into ocaml:main Mar 23, 2023

emillon mentioned this pull request Apr 18, 2023

[new release] dune (15 packages) (3.8.0~alpha1) ocaml/opam-repository#23683

Closed

emillon mentioned this pull request May 23, 2023

[new release] dune (15 packages) (3.8.0) ocaml/opam-repository#23814

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

User-configured watch exclusions #7216

User-configured watch exclusions #7216

jonahbeckford commented Mar 4, 2023 •

edited

Loading

nojb left a comment

jonahbeckford commented Mar 7, 2023 •

edited

Loading

nojb commented Mar 7, 2023

christinerose left a comment

rgrinberg Mar 17, 2023

jonahbeckford Mar 20, 2023

rgrinberg Mar 20, 2023

jonahbeckford Mar 21, 2023

rgrinberg Mar 20, 2023

jonahbeckford Mar 21, 2023

rgrinberg commented Mar 20, 2023

jonahbeckford commented Mar 21, 2023

rgrinberg commented Mar 23, 2023

User-configured watch exclusions #7216

User-configured watch exclusions #7216

Conversation

jonahbeckford commented Mar 4, 2023 • edited Loading

nojb left a comment

Choose a reason for hiding this comment

jonahbeckford commented Mar 7, 2023 • edited Loading

nojb commented Mar 7, 2023

christinerose left a comment

Choose a reason for hiding this comment

rgrinberg Mar 17, 2023

Choose a reason for hiding this comment

jonahbeckford Mar 20, 2023

Choose a reason for hiding this comment

rgrinberg Mar 20, 2023

Choose a reason for hiding this comment

jonahbeckford Mar 21, 2023

Choose a reason for hiding this comment

rgrinberg Mar 20, 2023

Choose a reason for hiding this comment

jonahbeckford Mar 21, 2023

Choose a reason for hiding this comment

rgrinberg commented Mar 20, 2023

jonahbeckford commented Mar 21, 2023

rgrinberg commented Mar 23, 2023

jonahbeckford commented Mar 4, 2023 •

edited

Loading

jonahbeckford commented Mar 7, 2023 •

edited

Loading