Skip to content

dune_file_watcher: do not exclude _opam and _esy systematically#7086

Closed
nojb wants to merge 2 commits intoocaml:mainfrom
nojb:_opam
Closed

dune_file_watcher: do not exclude _opam and _esy systematically#7086
nojb wants to merge 2 commits intoocaml:mainfrom
nojb:_opam

Conversation

@nojb
Copy link
Copy Markdown
Collaborator

@nojb nojb commented Feb 15, 2023

As discussed in the thread starting at #7010 (comment), it seems that it is not right to systematically exclude _opam (and _esy) from the file notification watchers, since these directories may be added explicitly to the Dune build (eg by using (vendored_dirs ...)).

cc @jonahbeckford @rgrinberg

nojb added 2 commits February 15, 2023 14:31
Signed-off-by: Nicolás Ojeda Bär <n.oje.bar@gmail.com>
Signed-off-by: Nicolás Ojeda Bär <n.oje.bar@gmail.com>
@jonahbeckford
Copy link
Copy Markdown
Collaborator

Could we just have a command line option / dune-project config item / environment variable for “don’t use default exclusion rules”? I think that Boolean would be simplest.

But perhaps we can let the user specify the exact exclusion rules as a list of strings (ex. semicolon separated list of rules) instead. Maybe I’m the only one in the world, but I have huge node_modules/ directories inside my source tree when I get Dune to generate JS. And large Conda (Python) environments when I mix OCaml with Sphinx.

@rgrinberg
Copy link
Copy Markdown
Member

I also think that removing _opam and _esy from the default list should be opt-in. Many users have a local switch in _opam so it doesn't make sense to inconvenience the majority. A command line option or a workspace option to remove the default paths would make more sense.

@jonahbeckford
Copy link
Copy Markdown
Collaborator

I don't know my way around the dune codebase. This could be something I can take over though if someone could point me to an example in the code of using a workspace option.

@rgrinberg
Copy link
Copy Markdown
Member

To start, you can just make the patterns configurable. The fact that they're hard coded now means that there's now way to customize them from the code at all. Once you do that, you can add the appropriate option to dune_rules/workspace.ml

@nojb
Copy link
Copy Markdown
Collaborator Author

nojb commented Feb 16, 2023

Being able to customize the set of exclusion patterns seems the better way to go, so I'll close this one.

@nojb nojb closed this Feb 16, 2023
@nojb nojb deleted the _opam branch February 16, 2023 17:20
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.

3 participants