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

dune init appears to ignore its name argument #3088

Closed
emengd opened this issue Feb 5, 2020 · 3 comments · Fixed by #3103 or ocaml/opam-repository#15845
Closed

dune init appears to ignore its name argument #3088

emengd opened this issue Feb 5, 2020 · 3 comments · Fixed by #3103 or ocaml/opam-repository#15845
Assignees

Comments

@emengd
Copy link

emengd commented Feb 5, 2020

Dune version: 2.1.3

When I do dune init exe myexe I get the following dune file:

(executable
 (name main))

plus a file main.ml.

I expect name myexe and a myexe.ml file. Did I misunderstand what dune init is supposed to do?

@ghost ghost assigned shonfeder Feb 5, 2020
@shonfeder
Copy link
Collaborator

Thanks for the report, @emengd!

So, under the current behavior, the name argument for executable components only applies to the public_name field. E.g., (note the addition of the --public flag):

[sf@comp dune_test]$ dune init exe myexe --public
Success: initialized executable component named myexe
[sf@comp dune_test]$ cat dune 
(executable
 (public_name myexe)
 (name main))

This entails that all initialized .ml files for executable components (whether public or private) will be called main.ml. The (admittedly confusing) result is that the name is simply ignored when the executable component is not public! I can't recall whether this was a design error or an implementation error, but it is confusing behavior regardless.

  • re: private components, it seems like the right thing to do is to name the .ml files the way you were expecting.
  • re: public components, I am not sure whether or not we want to change the behavior, since it might be viewed as a positive that we enforce the (imo, reasonable) convention that the main module of public executables is named main.

@emengd
Copy link
Author

emengd commented Feb 5, 2020

Thanks for the answer! One thing I can say is that I would definitely expect to get (name myexe) based on the current documentation (by which I mean dune init --help). The meta-variable is called NAME, and there is a --public option that sets the public name and “defaults to the name”, so you expect this argument to become the name field.

I would also find it confusing if the positional argument had a different meaning depending on whether --public is present.

Here are two states that I find not confusing:

  1. The argument is called PUBLIC_NAME. Components are public when it is present, and it determines the value of public_name. Otherwise they are private. There could be an option to set the name that defaults to main (with perhaps no argument meaning “same as public”).

  2. The argument is NAME. It sets name in all cases. The public name can only come from --public (the behaviour I was expecting).

@shonfeder
Copy link
Collaborator

I agree with your reasoning. Indeed, the current inline help (which I wrote) is incorrect, as it reads:

       Define an executable component named 'myexe' in a dune file in the
       current directory:

                 dune init exe myexe

I prefer option 2, as it keeps the CLI more uniform across components. I'll plan to fix things in that way, unless I hear opinions otherwise.

shonfeder added a commit to shonfeder/dune that referenced this issue Feb 8, 2020
- Make dune init exec NAME use the value of NAME for private
  modules (closes ocaml#3088)
- Uses Cmdliner Arg.conv converters to guard against invalid inputs
  (closes ocaml#3046)
- Adds library name validation checks to relevant inputs
- Adds an ADT to better represent the logic of the public_name option
- The Dune_init module now expects the value of relevant options to be
  of type Dune_lang.Atom.t
- Minor improvements to the inline documentation
- Move 2 CLI only functions into the init.ml CLI module

Signed-off-by: Shon Feder <[email protected]>
shonfeder added a commit that referenced this issue Feb 9, 2020
* [#3046] Fix dune init validation and error handling

- Make dune init exec NAME use the value of NAME for private
  modules (closes #3088)
- Uses Cmdliner Arg.conv converters to guard against invalid inputs
  (closes #3046)
- Adds library name validation checks to relevant inputs
- Adds an ADT to better represent the logic of the public_name option
- The Dune_init module now expects the value of relevant options to be
  of type Dune_lang.Atom.t
- Minor improvements to the inline documentation
- Move 2 CLI only functions into the init.ml CLI module
- Add test cases against reported bug
- Add Dune_lang.Atom.parse

Signed-off-by: Shon Feder <[email protected]>
rgrinberg added a commit to rgrinberg/opam-repository that referenced this issue Feb 15, 2020
…lugin, dune-private-libs and dune-glob (2.3.0)

CHANGES:

- Improve validation and error handling of arguments to `dune init` (ocaml/dune#3103, fixes
  ocaml/dune#3046, @shonfeder)

- `dune init exec NAME` now uses the `NAME` argument for private modules (ocaml/dune#3103,
  fixes ocaml/dune#3088, @shonfeder)

- Avoid linear walk to detect children, this should greatly improve
  performance when a target has a large number of dependencies (ocaml/dune#2959,
  @ejgallego, @aalekseyev, @Armael)

- [coq] Add `(boot)` option to `(coq.theories)` to enable bootstrap of
  Coq's stdlib (ocaml/dune#3096, @ejgallego)

- [coq] Deprecate `public_name` field in favour of `package` (ocaml/dune#2087, @ejgallego)

- Better error reporting for "data only" and "vendored" dirs. Using these with
  anything else than a strict subdirectory or `*` will raise an error. The
  previous behavior was to just do nothing  (ocaml/dune#3056, fixes ocaml/dune#3019, @voodoos)

- Fix bootstrap on bytecode only switches on windows or where `-j1` is set.
  (ocaml/dune#3112, @xclerc, @rgrinberg)

- Allow `enabled_if` fields in `executable(s)` stanzas (ocaml/dune#3137, fixes ocaml/dune#1690
  @voodoos)

- Do not fail if `ocamldep`, `ocamlmklib`, or `ocaml` are absent. Wait for them
  to be used to fail (ocaml/dune#3138, @rgrinberg)

- Introduce a `strict_package_deps` mode that verifies that dependencies between
  packages in the workspace are specified correctly. (@rgrinberg, ocaml/dune#3117)
rgrinberg added a commit to rgrinberg/opam-repository that referenced this issue Feb 15, 2020
…lugin, dune-private-libs and dune-glob (2.3.0)

CHANGES:

- Improve validation and error handling of arguments to `dune init` (ocaml/dune#3103, fixes
  ocaml/dune#3046, @shonfeder)

- `dune init exec NAME` now uses the `NAME` argument for private modules (ocaml/dune#3103,
  fixes ocaml/dune#3088, @shonfeder)

- Avoid linear walk to detect children, this should greatly improve
  performance when a target has a large number of dependencies (ocaml/dune#2959,
  @ejgallego, @aalekseyev, @Armael)

- [coq] Add `(boot)` option to `(coq.theories)` to enable bootstrap of
  Coq's stdlib (ocaml/dune#3096, @ejgallego)

- [coq] Deprecate `public_name` field in favour of `package` (ocaml/dune#2087, @ejgallego)

- Better error reporting for "data only" and "vendored" dirs. Using these with
  anything else than a strict subdirectory or `*` will raise an error. The
  previous behavior was to just do nothing  (ocaml/dune#3056, fixes ocaml/dune#3019, @voodoos)

- Fix bootstrap on bytecode only switches on windows or where `-j1` is set.
  (ocaml/dune#3112, @xclerc, @rgrinberg)

- Allow `enabled_if` fields in `executable(s)` stanzas (ocaml/dune#3137, fixes ocaml/dune#1690
  @voodoos)

- Do not fail if `ocamldep`, `ocamlmklib`, or `ocaml` are absent. Wait for them
  to be used to fail (ocaml/dune#3138, @rgrinberg)

- Introduce a `strict_package_deps` mode that verifies that dependencies between
  packages in the workspace are specified correctly. (@rgrinberg, ocaml/dune#3117)

- Make sure the `@all` alias is defined when no `dune` file is present
  in a directory (ocaml/dune#2946, fix ocaml/dune#2927, @diml)
mgree pushed a commit to mgree/opam-repository that referenced this issue Feb 19, 2020
…lugin, dune-private-libs and dune-glob (2.3.0)

CHANGES:

- Improve validation and error handling of arguments to `dune init` (ocaml/dune#3103, fixes
  ocaml/dune#3046, @shonfeder)

- `dune init exec NAME` now uses the `NAME` argument for private modules (ocaml/dune#3103,
  fixes ocaml/dune#3088, @shonfeder)

- Avoid linear walk to detect children, this should greatly improve
  performance when a target has a large number of dependencies (ocaml/dune#2959,
  @ejgallego, @aalekseyev, @Armael)

- [coq] Add `(boot)` option to `(coq.theories)` to enable bootstrap of
  Coq's stdlib (ocaml/dune#3096, @ejgallego)

- [coq] Deprecate `public_name` field in favour of `package` (ocaml/dune#2087, @ejgallego)

- Better error reporting for "data only" and "vendored" dirs. Using these with
  anything else than a strict subdirectory or `*` will raise an error. The
  previous behavior was to just do nothing  (ocaml/dune#3056, fixes ocaml/dune#3019, @voodoos)

- Fix bootstrap on bytecode only switches on windows or where `-j1` is set.
  (ocaml/dune#3112, @xclerc, @rgrinberg)

- Allow `enabled_if` fields in `executable(s)` stanzas (ocaml/dune#3137, fixes ocaml/dune#1690
  @voodoos)

- Do not fail if `ocamldep`, `ocamlmklib`, or `ocaml` are absent. Wait for them
  to be used to fail (ocaml/dune#3138, @rgrinberg)

- Introduce a `strict_package_deps` mode that verifies that dependencies between
  packages in the workspace are specified correctly. (@rgrinberg, ocaml/dune#3117)

- Make sure the `@all` alias is defined when no `dune` file is present
  in a directory (ocaml/dune#2946, fix ocaml/dune#2927, @diml)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment