Add dune init command

[shonfeder]

Towards completion of #159

This implements creation of new projects, as discussed in #159 (comment) and using the more simplistic, hard-coded strings approach encouraged here #159 (comment)

I think tests ensuring initialized projects can be build should be included in this PR, but I'd like to get the general shape of the implementation OKed before writing the test cases.

[shonfeder]

(Looks like I need to make several small changes for older OCaml versions. I'm looking in to that now.)

[rgrinberg]

I don't quite understand the necessity of functor or first class modules here. It seems like plain old records would work just as well here.

[shonfeder]

I opted for modules to avoid having to thread the name and path parameters through a bunch of functions. However, I can definitely understand that the end result seems quite heavy duty given the simplicity of the feature added! Moreover, if we're not generating any content for the files, there's less need to thread those parameters through. I'll re-structure for a more lightweight approach.

[rgrinberg]

I opted for modules to avoid having to thread the name and path parameters through a bunch of functions. However, I can definitely understand that the end result seems quite heavy duty given the simplicity of the feature added! Moreover, if we're not generating any content for the files, there's less need to thread those parameters through. I'll re-structure for a more lightweight approach.

Thanks, we have situations in dune where we have a little too much boilerplate and we go with functors but the situation here is nowhere near as acute. The only case where we use first class modules are plugins, so introducing it here to get rid of a boilerplate is certainly unprecedented. I think the code will look quite clean with records by the way so boilerplate won't even be an issue.

[shonfeder]

Notes on a few of the changes:

• Replaced the baroque first class modules with records (I think I'm overeager for 1ML 😋).
• Replaced the check for a preexisting project directory with checks for preexisting files during their creation. Now, if, e.g., a file exists at <project>/bin/main.ml, it will report this fact and skip creation. This protects against overwriting files (as the check for the root directory was meant to), but also allows initializing existing projects by fleshing out missing dune files that may be needed.
• Removed the <project>/src/lib.ml file, but sneakily kept a "Hello, World" line in <project>/bin/main.ml. I think this is innocuous: It doesn't introduce the same complications as the content in <project>/src/lib.ml but it still provides the benefit of being able to get a printing program with one command. However, I'll remove this line without complaint if @rgrinberg is opposed.

[rgrinberg]

Okay, looks really nice. Good work @shonfeder. Before I merge I'm planning the following UI changes:

• Change $ dune init foo --lib to $ dune init lib foo. The reason for this is that we'd like $ dune init lib --help to only show the relevant options for lib.

• For the executable template, we don't want an associated library. I think a single dune + source file is enough.

• Let's remove the generation of all .opam files. First of all, it will be done properly from dune files later and second of all it's not helpful for non public libs and binaries anyway. And the new @all alias makes building private libraries and executables much easier so there's even less incentive for this to be the default.

• Add the ability to put paths in the argument $ dune init <lib|exe> foo/bar should create a <lib|exe> named bar inside foo. There's no need directory nesting past this I believe.

If you see any problems or propose any changes to these suggestions, please feel free to discuss.

If you'd like some help in implementing some of this stuff, I'll gladly help out as well as I realize that some of these change aren't small.

We'll also need to add a test suite for this feature. I suggest that we add this test suite before we start any of the changes I've proposed. If you need help here, I'm happy to point you in the right direction or put up a few tests.

[emillon]

Thanks! This nex feature will probably open up a healthy amount of bikeshedding but this is probably necessary!

[rgrinberg]

By the way, this issue is relevant to make the opam files obsolete: #1112

There are no immediate plans to implement this (we are looking for volunteers), but it shouldn't be too hard and hopefully will be present in the next version or two

[shonfeder]

I have a question on one point, re: "Add the ability to put paths in the argument $ dune init <lib|exe> foo/bar". The current interface allows for dune init --lib my_project path/to/some/location. It seems to me preferable to keep this kind of behavior, rather than creating an idiosyncratic meaning for / and overloading the meaning of the first positional argument. So, it seems preferable to me to keep the current behavior.

Other than that, I don't have any problems with the proposed changes and I'm happy to make them. I will let know if I get stuck or bogged down, but I think I should be able to bring those changes about in the next several days.

Thanks for the guidance, corrections, and reviews so far, @rgrinberg and @emillon!

[rgrinberg]

have a question on one point, re: "Add the ability to put paths in the argument $ dune init <lib|exe> foo/bar". The current interface allows for dune init --lib my_project path/to/some/location. It seems to me preferable to keep this kind of behavior, rather than creating an idiosyncratic meaning for / and overloading the meaning of the first positional argument. So, it seems preferable to me to keep the current behavior.

Well the current behavior isn't ideal as it's forcing a particular directory structure. Dune doesn't really recommend any directory structure and many existing projects and users will have their own preference regarding this. Therefore it seems best to make the feature usable with any kind of directory structure.

My suggestion was just one idea on how to add the flexibility to create w/e directory structure the user wants. I'm happy to hear other suggestions. The use cases I have in mind are:

• How do I create a library in the current directory?
• How do I create a binary in an existing directory?

Other than that, I don't have any problems with the proposed changes and I'm happy to make them. I will let know if I get stuck or bogged down, but I think I should be able to bring those changes about in the next several days.

Thanks, we appreciate your work.

[shonfeder]

Ah, I think I understand what you are after now: we're not talking about the root directory of the project but the directory in which the dune file declaring the library is created. I can see the use.

@rgrinberg, according to the plan sketched out at #159 (comment), the two features you mention

• How do I create a library in the current directory?
• How do I create a binary in an existing directory?

would be supplied by the dune add command. The way discussed there (and what I understand many people to have been requesting), dune init should prime more holistic project development, and provide a quick and easy jumping off place. Thus, I think it makes sense to have a second command dedicated to small utility operations like creating a dune file of a certain sort sort in such and such a directory.

[rgrinberg]

It seems like $ dune init and $ dune add have a very overlapping feature set however. In fact, if $ dune init was idempotent, I can't think of any situation where I'd want to use $ dune add.

Also, I'm not very sure how $ dune init maps to a single project by the way. Most of the projects that I deal define more more than just 1 executables and/or a library. Having $ dune init be used once and $ dune add n times makes the latter the far more useful command to implement.

Here's another suggestion, if we want $ dune init to initialize a whole project skeleton for us, then we can have a $ dune init project subcommand. I agree that such a command should indeed create a directory.

By the way, I think it would help to have a type like:

type common =
  { name : string
  ; in_ : Path.t
  }
type public = bool
type inline_tests
type init = (* pick your own name here *)
  | Executable of common * public * libraries
  | Library of common * public * libraries * inline_tests
  | Test of common * Lib_name.t (* the library that we are testing *)
  | Project of common (* This constructor would include Library + Test for example *)

The above makes it very clear what arguments anything that we can init can accept. It's just a sketch though, I wouldn't necessarily use tuples for example.

[shonfeder]

I am OK with ditching the add command and just putting that functionality in init.

Rather than dune init project lib ... (pretty unwieldy) I suggest we follow the patterns of cargo and stack and use init for stuff that initializes things an existing project and use new for creating new projects. This will make it more concise, more accessible to new users, and more likely to be familiar to people coming from neighboring communities. (but this is all matter for another PR any how :) ).

I will build out the API for init along the lines you suggest: that makes good sense to me.

I'm hoping to have all the suggested changes in place and ready for review by the end of next week.

[edited to amend estimate on turnaround time for completion]

[rgrinberg]

Rather than dune init project lib ... (pretty unwieldy)

Sorry, my suggestion $ due init {project,lib,exe}. I did not suggest any kind of additional nesting. What I meant was that all the extra directories would only be created in the project init type.

I suppose the main difference between between our tool and cargo/stack is the once-only restriction of the templates that I'm not so fond of. It's true that a tool that quickly generates dune boilerplate vs a tool that starts a new project aren't the same thing. But it seems like at least a flexible $ dune init will handle the new with enough templates for it.

Btw, I have no experience with cargo but $ cargo init doesn't look very good. I've used stack a lot more and have found their custom templates quite useful, but their non composability is quite annoying. It's quite ugly to have hspec, servant, and servant-hspec templates for example. We can try and do better in this respect.

[shonfeder]

I thought I recalled this being an issue.... according to dbuenzli/cmdliner#24, cmdliner doesn't support nested subcommands. Does anyone know of a reasonable way around this?

[shonfeder]

Ah, in dbuenzli/cmdliner#95 there is mention of implementing sub commands manually using positional arguments. Before I dig in to that, I'd like to confirm that the increased complexity of manual implementation of subcommands here is worth the cost. We should perhaps also consider that, since dune init is no longer meant to create projects, it seems reasonable that someone might dune init --lib --bin --test some_name . to create test, executable, and library dune files with one command.... (or is the use of the single shared some_name for all a blocker?).

[shonfeder]

Just had the (obvious) realization that dune init {lib,test,exe} PROJECT_NAME [OPTIONS...] can be implemented as positional arguments, and there's no need for a nested sub-command. Please ignore the noise.

[rgrinberg]

If we intentionally choose a suboptimal CLI for dune just because it's more convenient we are going to regret it down the line.

[rgrinberg]

Just had the (obvious) realization that dune init {lib,test,exe} PROJECT_NAME [OPTIONS...] can be implemented as positional arguments, and there's no need for a nested sub-command. Please ignore the noise.

Yes, this is alright for now. Although note that it's still not optimal because the help shown will not be specialized by the type.

[shonfeder]

Ah, ok. So we do want this to be a nested subcommand -- for the focused help message (and added flexibility, etc.). So, just to be sure we're on the same page and that I understand correctly what's wanted here: completion of this PR (in it's ideal form) is now dependent on implementing a custom extension to cmdliner's CLI parsing that supports nested subcommands with their own help menus. Correct?

[rgrinberg]

Yup, that would be ideal. But the workaround you proposed is backwards compatible so I'm personally ok with delaying the fully featured CLI.

[rgrinberg]

@shonfeder I'm curious, why didn't you just use dune's own parsing code for parsing the stanzas? This way, I think we have some duplication in the code and it isn't very desirable.

[shonfeder]

@rgrinberg Can you please specify which part of the code you think introduces parsing duplication and what existing code you think I might use instead? Afaik, I do very little in the way of parsing here. The only exception that comes to mind is the temporary function https://github.com/ocaml/dune/pull/1448/files#diff-ec22fb5fe4ab585936d484f498556279R69, which has the very specific purpose of identifying conflicting stanzas from the already loaded Cst.t.

Edit: To clarify, I'm all for reducing code duplication if possible.

[shonfeder]

Also, note that we need to preserve comments, since we read in and then re-emit dune code in dune files that may be hand written.

[rgrinberg]

@shonfeder yeah, that's indeed the function I'm talking about. Why is there an issue here regarding comments btw? We are simply using the Cst.t to create Signature.t values.

My idea would be to get rid of Signature.t altogether and just use Dune_file.Stanzas.parse along with the Stanza.t type to detect exes/libs. Do you think that's reasonable?

Also note that we if we initialize a "super context", then we'll have all the parsed stanzas for free.

[shonfeder]

My idea would be to get rid of Signature.t altogether and just use Dune_file.Stanzas.parse along with the Stanza.t type to detect exes/libs. Do you think that's reasonable?

Yep. That's a better idea for identifying conflicts. I'll make the change this weekend.

Why is there an issue here regarding comments btw?

The issue with the comments in general is this: since we aim to be able to extend existing dune files and stanzas, we have to preserve the comments which users may have added. This means that in general, we can't just work with the Stanza.t type, or else we'd lose the comments when we write the stanzas back to the file. We can use Stanza.t for conflict detection, but we cannot simply use Stanza.t once we move to inserting dependencies, because we'll end up erasing user comments.

Also note that we if we initialize a "super context", then we'll have all the parsed stanzas for free.

I wasn't aware of super_context. That's quite cool. I think this would face the same problem of wiping out user comments tho, no?

[rgrinberg]

I wasn't aware of super_context. That's quite cool. I think this would face the same problem of wiping out user comments tho, no?

Indeed, but it it can be used for just identify conflicts. The dune file can then be re-parsed to cst's when necessary.

[shonfeder]

@rgrinberg In order for the planned code reuse to work in a sensible way, the new_stanza : Cst.t that we want to add to the file must also be converted to a Stanza.t. So far, I have not been able to find any existing function Cst.t -> Stanza.t (nor any function to provide a helpful segment of that transformation, such as Ast.t -> Stanza.t). Do you happen to know of a sensible way to make this conversation? (And if there is such a function, or straightforward composition, to be had, then I needn't bother with all the incidental complexity of super contexts or parsing in a new file, since I can just convert the already loaded stanzas : Cst.t list to a Stanza.t list directly.)

I would, of course, be happy to work out this plumbing myself, but if that is required, I think we should just mark this with a TODO and address it in a follow up PR. This PR has already suffered from enough gradual sprawl, feature creep, and friction from staleness, and it would really help with my momentum to land it without too much more ado.

[rgrinberg]

Yeah, that's fine with me as well. Let's do the following:

• Makes sure everything is signed-off/rebased on master
• Update the manual and add a little note that $ dune init is still in development and is subject to change without warning
• Merge this PR.

[kevinji]

Found a few typos.

[shonfeder]

Thanks for catching those, @kevinji -- fixed now.

@rgrinberg action items addressed. Unless something else comes up, I think this should be ready for merge once CI is finished.

[shonfeder]

After discussion on #1972, I've learned I was misunderstanding how Dune_file.Stanzas.parse worked. Witb my understanding corrected, I think I can now eliminate the ad-hoc parsing this PR.

I also need to correct a documentation comment is added to that function in this PR.

[rgrinberg]

@shonfeder Please make this improvement in a subsequent PR. Thank you for all the hard work on this. Let's track future improvements in other tickets.