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

Support for alias in subdirs #771

Closed
samoht opened this issue May 12, 2018 · 23 comments
Closed

Support for alias in subdirs #771

samoht opened this issue May 12, 2018 · 23 comments
Assignees

Comments

@samoht
Copy link
Member

samoht commented May 12, 2018

Would be great to support things like:

(alias
 ((name foo/bar/runtest)
  (action (chdir foo/bar (run echo hello)))))

This would allow to have a jbuild-ignore file ignoring the contents of the foo directory, while having jbuilder build @foo/bar/runtest still working. This is useful for instance when foo contains documentation code examples that you want to test but not to be scanned while compiling the project.

(This is related to #761)

@ghost
Copy link

ghost commented May 14, 2018

Yh, that makes sense.

I'm thinking of adding a pass in the compilation of jbuild files down to build rules, so that we can determine the dependencies between directories before generating the rules. For instance this would make it easy to have a single dune file cover several directories.

@samoht
Copy link
Member Author

samoht commented May 14, 2018

Also this is useful to run jbuilder runtest <path> when path is both ignored and a tree full of jbuildfiles (this happens in rwo).

@yminsky
Copy link
Contributor

yminsky commented May 26, 2018

Any sense of whether this is on the near-term roadmap? It would be quite useful for cleaning up some usability issues in the way the RWO build works right now.

@ghost
Copy link

ghost commented May 28, 2018

I think there is a simple to make it work, we can include it in Dune 1.0.0.

@samoht
Copy link
Member Author

samoht commented Jun 18, 2018

Just checking if this is still planned to be included in 1.0? It would be great to have this for RWO.

@yminsky
Copy link
Contributor

yminsky commented Jun 18, 2018

Agreed!

@ghost
Copy link

ghost commented Jun 19, 2018

How bad is it at the moment?

Note that the 1.0.0 release of Dune is taking longer than usual releases as we want to get the versioning story right and prepare the groundwork for future features we have in mind that require breaking changes. But once it's released we'll resume the usual release schedule.

@samoht
Copy link
Member Author

samoht commented Jun 19, 2018

How bad is it at the moment?

We have to generate a lot of custom rules like this one https://github.com/realworldocaml/book/blob/master/examples/jbuild#L4 which reduces the usability a lot (e.g. you have to replace / by -, you cannot run jbuilder runtest in a subdir, etc).

There's also an issue with the generation of .merlin files, not sure exactly what happened but they are not generated anymore for these sub-targets (and I am not sure which workaround to add for these).

@ghost
Copy link

ghost commented Jun 19, 2018

I see, that's quite annoying indeed. However, looking at this use case it seems to me that #622 would provide an even better solution since you wouldn't even need to generate these stanzas.

#622 is a bit ambitious, but we can make it much simpler by allowing the user to set the tool they want to use to run the test. We can even write this information only once in the dune-project file.

Technically the result is that you could simply delete the jbuild file you pointed, write one line in the dune-project file and everything would work as it should.

@samoht
Copy link
Member Author

samoht commented Jun 19, 2018

Note that it's not only cram tests, we also have ocaml-expect tests: https://github.com/realworldocaml/book/blob/master/examples/jbuild#L2708

But yes, having first-class support in dune for this kind of workflow would be even better :-)

Edit: there is also 2 modes of running the cram/expect tests, deterministic and non-deterministic ones so I am not sure how if could fit in #622 (with the current state of the proposal).

@ghost
Copy link

ghost commented Jun 19, 2018

The proposal could cover toplevel expect tests as well, in the end it's all the same idea. Currently how do you decide which which mode to use?

@samoht
Copy link
Member Author

samoht commented Jun 19, 2018

We generate runtest-* and runtest-all-* targets which pass different arguments to the underlying tools.

@ghost
Copy link

ghost commented Jun 19, 2018

Ok, so here is a proposal: you'd write this in the dune-project file:

(lang dune 1.0)

(using cram-style-tests 1.0
  (scheme (extension sh) (action (run cram %{<}))
  (scheme (extension sh) (alias runtest-all) (action (run cram -foo %{<}))
  (scheme (extension mlt) (action (run ocaml-topexpect %{<})))

and then tests would simply be put in .t directories.

@samoht
Copy link
Member Author

samoht commented Jun 19, 2018

That sounds good; I think I would make the (alias runtest) always explicit in the rules to be a bit less confusing.

Would be nice also if we could parametrise which directory has the tests: we plan to move the mlt tests in the book/ directory and interleave the book contents and the code examples somehow. Not sure renaming that to book.t would make sense, as the directory will also contains useful contents (not only tests).

@trefis
Copy link
Collaborator

trefis commented Jun 28, 2018

I am very much in favor of this!
At the moment the setup for tests in merlin look like this (duplicated across several directories).

I'd love to just have to write:

(using cram-style-tests 1.0
  (scheme (extension t) (action (setenv MERLIN ${bin:ocamlmerlin-server}
                                 (run cram %{<})))))

just once at the root (though I wouldn't mind to much the alternative of writing (cram_test (env MERLIN ${bin:ocamlmerlin-server})) in each directory if cram rules were added).

@samoht
Copy link
Member Author

samoht commented Oct 1, 2018

Any update on that issue? I am looking at RWO again and that would be nice to remove all my weird generated rules and use the default ones from dune instead :-)

@ghost
Copy link

ghost commented Oct 1, 2018

Nothing so far, but this is not forgotten. We'd like to have this for dune itself and the OCaml compiler as well.

@rgrinberg
Copy link
Member

I'm thinking of adding a pass in the compilation of jbuild files down to build rules, so that we can determine the dependencies between directories before generating the rules. For instance this would make it easy to have a single dune file cover several directories.

Is still planned? Or will the new computational model obsolete this?

@xclerc xclerc self-assigned this Oct 2, 2018
@ghost
Copy link

ghost commented Oct 2, 2018

@rgrinberg no, but in the end the new feature to add to dune is the new scheme for cram like tests, not aliases in sub-directories

@rgrinberg
Copy link
Member

So then we should close this issue. The new scheme for cram like tests exists in another ticket I think.

@ghost
Copy link

ghost commented Oct 2, 2018

Indeed, it's #622

@ghost ghost closed this as completed Oct 2, 2018
@samoht
Copy link
Member Author

samoht commented Oct 2, 2018

:-(

I am still very interested by the ability to create alias in subdirs. I still have the same use-case: #771 (comment)

@ghost
Copy link

ghost commented Oct 2, 2018

This will be covered by #622. The runtest alias will be defined in such test directories.

This issue was closed.
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

No branches or pull requests

5 participants