The Wayback Machine - http://web.archive.org/web/20200911101046/https://github.com/ocaml/dune/issues/3271
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

Ability to control the meaning of :standard dirs at workspace level. #3271

Open
jordwalke opened this issue Mar 16, 2020 · 11 comments
Open

Ability to control the meaning of :standard dirs at workspace level. #3271

jordwalke opened this issue Mar 16, 2020 · 11 comments

Comments

@jordwalke
Copy link
Contributor

@jordwalke jordwalke commented Mar 16, 2020

Currently directories with leading underscores are ignored by dune by default, even in cases when specifying (include_subdirs unqualified). There is a pattern in the JS community with the most popular testing library (Jest), where developers put a directory called __tests__ right next to the code that it tests. That pattern doesn't translate to Dune because it ignores all directories with a leading underscore. There are ways around it, but there are some downsides to those workarounds.

  • Each dune file that has tests has to remember to include __tests__ in its (dirs).
  • (dirs) isn't respected when it is generated using the "include" or Tuareg mode features.

I might suggest it would be better if Dune was not so opinionated to ignore directories starting with underscore, but that's probably a more invasive breaking change.

Instead, could we allow projects to specify at the dune-project or dune-workspace level, the meaning of :standard dirs? For example, you could override the default definition of :standard. Right now standard is a regex [^_\.].*, but this could be overridden so be .* for this Jest style pattern.

@rgrinberg
Copy link
Member

@rgrinberg rgrinberg commented Mar 16, 2020

I had a look at this now, and it would actually be possible to set this at the project level (inside dune-project files). However, it seems somewhat confusing to me to have multiple defaults in a single workspace. Would setting it in workspace file be enough?

@ejgallego
Copy link
Collaborator

@ejgallego ejgallego commented Mar 16, 2020

A workspace file would be lovely, we could have a workspace include some more dirs, for example for extended testing under __test__ by actually using that workspace [I think there is already some issue about that]

@jordwalke
Copy link
Contributor Author

@jordwalke jordwalke commented Mar 25, 2020

Honestly, any possible way to allow Dune to see directories underscore directories by default (and have them be seen by things like (cp# and (copy_files#, without having to have them added to every single dune file that might have a __tests__ dir.

@jeremiedimino
Copy link
Member

@jeremiedimino jeremiedimino commented Mar 25, 2020

Workspace files are for settings that are dependant on the user configurations. Such as opam switch names. That's why they are not meant to be committed and they are not composable.

In the case described here, the __tests__ directories are part of the repository. So the fact that dune should see them is a universal property and should be written down in either dune-project or dune files.

@ejgallego
Copy link
Collaborator

@ejgallego ejgallego commented Mar 25, 2020

@diml in some sense I was understanding the set of considered directories as a kind of user configurable setup; in the same way I can tell dune "build this 3 context" I'd like to tell dune "consider this set of extended tests" which in this case come by composition.

But indeed maybe you are right that the best way to handle this is using some symlinks to place into scope the desired extended setup.

[The use case for us in Coq is actually multi-repos composition. When we break Coq API, we first work in Coq. Once we are happy, we want @check to consider the _coq_ci directory that contains a checkout of the projects in the CI]

@jeremiedimino
Copy link
Member

@jeremiedimino jeremiedimino commented Mar 26, 2020

@ejgallego your use case seems valid to me, and maybe we should also support it in Dune. But @jordwalke's case seems orthogonal to me. It's really just: dune thinks these directories contain ancillary data, I'm putting real stuff in there, how do I convince Dune that they are normal directories once and for all?

@jordwalke
Copy link
Contributor Author

@jordwalke jordwalke commented Mar 27, 2020

@diml You described my case correctly. The thing is they can't go in dune files because:

  1. It's a big burden on developers in a monorepo when we have a single dune file to manage all recursive directories.
  2. The (dirs directive is not observed when using dune's generation of dune files via ocaml syntax (I'm aware there are talks of making a better replacement, but the same issue might persist even in that replacement).

It's very nice for monorepos to be able to declare it once per large monorepo (in one dune file or dune-project, or workspace config)

@rgrinberg
Copy link
Member

@rgrinberg rgrinberg commented Mar 27, 2020

It's somewhat annoying that we already have env for setting "defaults" in a way that is inherited by sub directories. I'd hate to introduce a parallel mechanism, but at the same time it seems like env is the stanza that is slightly mis-designed by being tied to profiles.

I'm also not sure if modifying the meaning of :standard is the right way of solving this problem. One alternative is to have something like this for example:

(all_subdirs
 (dirs :standard __tests__))

Where all_subdirs evaluates its argument as a stanza for all sub-directories.

@jordwalke
Copy link
Contributor Author

@jordwalke jordwalke commented Mar 31, 2020

@rgrinberg That would work if it only requires having to specify it once, but I'm not sure if that would work with the OCaml syntax for Dune or whatever replaces it.

@jeremiedimino
Copy link
Member

@jeremiedimino jeremiedimino commented Mar 31, 2020

That would work with dune files in OCaml syntax, however what happens if a sub-directory also has a dirs stanza?

@jeremiedimino
Copy link
Member

@jeremiedimino jeremiedimino commented May 19, 2020

@jordwalke, is this still an issue? If yes, do you want write a PR for it? We can also take it on our stack, but it will be low-priority.

Regarding the design, let's not generalise prematurely in a given direction. All we need here is the ability to change the meaning of :standard via the dune-project file, so that's what we should do.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
4 participants
You can’t perform that action at this time.