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

Allow to mention the current directory in (ignored_subdirs ...) stanza #1047

Closed
andreypopp opened this issue Jul 24, 2018 · 10 comments
Closed

Comments

@andreypopp
Copy link
Member

In esy we fetch dependencies' sources into node_modules directory. That means a couple of dune files end up there (from dependencies) and confuse dune unless we put a dune file in a project root with

(ignored_subdirs (node_modules))

That extra dune file adds up to others (dune-project and dune files per compilation unit), I'd rather not impose another burden on esy+dune users to create it and maintain (the error due to non-ignored node_modules is the one beginners with esy+dune stumbl;e upon most often).

Therefore I'm wondering if there's a way to add a mechanism to ignore the current directory in (ignored_subdirs ...) stanza so that esy could put one at node_modules/dune automatically.

@aantron
Copy link
Collaborator

aantron commented Jul 26, 2018

Also ran into this today. Another option is to allow ignored_subdirs in dune-project, but I'm not sure what all the consequences of that would be, e.g. for esy.

@andreypopp
Copy link
Member Author

Another option is to allow ignored_subdirs in dune-project, but I'm not sure what all the consequences of that would be, e.g. for esy.

My idea was to make esy automatically add node_modules/dune which ignores all dirs inside node_modules. Editing dune-project (which is at the project root) automatically is something I don't want to do — really scary when tooling messes up with your files.

@aantron
Copy link
Collaborator

aantron commented Jul 26, 2018

That's reasonable.

My use case is a project that has both a BuckleScript/NPM build system, and a Dune one. Since NPM won't create a node_modules/dune, it seems reasonable to have ignored_subdirs in a root dune or dune-project, as part of the boilerplate.

@ghost
Copy link

ghost commented Jul 30, 2018

I suppose we could allow (ignored_subdirs all) to ignore all sub-directories. Would that be enough for this case?

@rgrinberg
Copy link
Member

Would supporting a glob be much harder? So the above would be equivalent to (ignored_subdirs *).

@andreypopp
Copy link
Member Author

andreypopp commented Jul 30, 2018

This is not high priority for esy anymore as we now generate

(ignored_subdirs <long list of all dirs inside node_modules>)

Having (ignored_subdirs *) or (ignored_subdirs all) would be better though.

@ghost
Copy link

ghost commented Jul 30, 2018

@rgrinberg nope. And I just realized that we forgot to remove the extra parentheses for this stanza. At least using globs isn't incompatible with deprecating the parentheses now. Plus since it is technically a breaking change, we could only allow globs if you don't put the parentheses.

@ELLIOTTCABLE
Copy link
Contributor

My use case is a project that has both a BuckleScript/NPM build system, and a Dune one.

Same here; and just ran into this problem today.

$ dune build @install
    ocamldep src/.excmd_parser.objs/uAX31.ml.d
Error: Conflict between the following libraries:
- "gen" in _build/default/node_modules/@elliottcable/bs-gen/src
- "gen" in /Users/ec/Sync/Code/excmd-parser/_opam/lib/gen
    -> required by library "sedlex" in /Users/ec/Sync/Code/excmd-parser/_opam/lib/sedlex
This is not allowed.

Not personally using esy, and the workaround for those of us not building a build-system is fairly easy (add a root-level dune file with no content but (ignored_subdirs (node_modules)) worked for me); but it'd be nice to have more flexible path declaration in the ignore-declaration.

Personally, although I don't have control of a consumer's whole node_modules, like esy does, I'd still like to ship a ./dune file with (ignored_subdirs *) to npm, so consumers of my BuckleScript libraries don't need to worry about this.

@rgrinberg
Copy link
Member

@andreypopp you can now do (dirs ()). does that address your issue?

@andreypopp
Copy link
Member Author

@rgrinberg yes thanks! closing this then.

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

No branches or pull requests

4 participants