Adapt the behavior of dune subst for dune projects#960
Adapt the behavior of dune subst for dune projects#9606 commits merged intomasterfrom unknown repository
Conversation
Signed-off-by: Jeremie Dimino <jeremie@dimino.org>
Signed-off-by: Jeremie Dimino <jeremie@dimino.org>
Signed-off-by: Jeremie Dimino <jeremie@dimino.org>
|
If there is no |
|
It's done in |
| You must pass a [--name] command line argument." | ||
| in | ||
| if not (List.mem name ~set:package_names) then | ||
| die "@{<error>Error@}: file %s.opam doesn't exist." name; |
There was a problem hiding this comment.
Could you harmonize this error message with the one on line 180? Also I wonder if it makes sense to include the package names.
There was a problem hiding this comment.
What do you have in mind for this error message?
There was a problem hiding this comment.
Looking at this again, the error on line 180 is slightly different. So ignore that comment. Everything looks fine. Though it might still be useful to include package_names in the error for convenience.
|
Btw, I notice that subst doesn't really have a test suite at the moment. @diml would you mind adding some general tests for it? Otherwise I can just create a ticket and we can do it at another time. |
Signed-off-by: Jeremie Dimino <jeremie@dimino.org>
|
Sure, I added a test |
| You must pass a [--name] command line argument." | ||
| in | ||
| if not (List.mem name ~set:package_names) then | ||
| die "@{<error>Error@}: file %s.opam doesn't exist." name; |
There was a problem hiding this comment.
Looking at this again, the error on line 180 is slightly different. So ignore that comment. Everything looks fine. Though it might still be useful to include package_names in the error for convenience.
|
PS, the tests still need to pass: |
|
Ah, it's because |
Yh, I'm not sure how to word the error message. The name comes from the
|
|
I'm merging for now, we can improve this message in another PR |
Projects are better defined with dune, so we don't need the automatic project name detection jbuilder was doing and can simply read the dune-project file. Additionally, since
dune substis only meant to be called from opam files, this PR simplify this command a bit so that it doesn't looks up the root of the workspace. This fix #954 which was due to users often forgetting to pass"-p" nameto thedune substinvocation.