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

Option to make "dune coq top" more permissive #7355

Closed
rlepigre opened this issue Mar 20, 2023 · 8 comments · Fixed by #7380 or ocaml/opam-repository#23814
Closed

Option to make "dune coq top" more permissive #7355

rlepigre opened this issue Mar 20, 2023 · 8 comments · Fixed by #7380 or ocaml/opam-repository#23814
Labels

Comments

@rlepigre
Copy link
Contributor

Desired Behavior

Currently, running dune coq top on a Coq file starts by ensuring that all dependencies of that file are built and up-to-date, which in turn require that the file itself is part of a stanza (so, saved to disk) and that it can be parsed by coqdep.

This disturbs some people's workflow when writing Coq, as they often want to edit files more quickly without constantly recompiling deps even if they are not completely consistent, work on dirty files only a prefix of which is syntactically correct, or work on new files that have not been saved yet.

I feel like we could have an opt-in option to dune coq top to support such use-cases. In particular, it would do no building of dependencies and rely on the user having called, e.g., dune build to get the dependencies in a reasonable state. Of course, to achieve this, one needs to figure out what options to pass to coqtop. One possibility (that was suggested by a colleague of mine) would be to compute a "most general configuration" from the workspace, that would put all theories and all plugins in scope.

CC @Alizter @ejgallego

Example

@Alizter
Copy link
Collaborator

Alizter commented Mar 20, 2023

There is an analogous option for dune exec:

       --no-build
           don't rebuild target before executing

Since we will be doing no building, we could also make sure that it doesn't possess the workspace lock allowing dune build and dune coq top to be run in tandem.

@Alizter Alizter moved this to Todo in Coq + Dune Mar 20, 2023
@rgrinberg rgrinberg added the coq label Mar 21, 2023
@rlepigre
Copy link
Contributor Author

@Alizter I think this issue is a real blocker for people fully moving to using dune for Coq project, so perhaps we should try to implement this sooner rather than later?

I'm happy to do some hacking on this, but it would be good to try an come up with some kind of plan first.

@Alizter
Copy link
Collaborator

Alizter commented Mar 22, 2023

@rlepigre I'll have a look today.

@ejgallego
Copy link
Collaborator

ejgallego commented Mar 22, 2023

I guess this is usually implemented by replacing Dep.path with Dep.goal (which ensures that the file is present, but doesn't depend on their checksums.

I think tho in general this is a misfeature. While I'm happy if you folks want to add this, I'm convinced that having to use this flag is just a way to mask deeper issues in the toolchain.

This disturbs some people's workflow when writing Coq, as they often want to edit files more quickly without constantly recompiling deps even if they are not completely consistent

When a dep of a Coq file changes, there are two cases: the change is relevant for your edit, or it is not. In the first case you would be wasting your time by skipping to build the dependency, in the second, a proper incremental compiler like coq-lsp will be one day would just not block your edit while it rebuilds the dep.

I'm willing to bet that the cognitive overhead of the dev having to think about --no-rebuild is already a greater burden than just having to recompile sometimes.

Tho I agree that editors should allow you to edit while the stuff is compiling.

coq-lsp has the right infra to delay stuff, so for example you can work on your file and not check proofs, I've just realized that this also works for Require.

What do I mean:

From Foo Require Bar.

Lemma too : Type. .... Qed.

As of coq-lsp 0.2.x , coq-lsp will delay the checking of too until Bar is ready. But actually we should indeed handle the Require effect different, which is to actually go ahead and check too, while we launch the build of Bar in the background.

Once Bar is built, we compare with the previous version and we decide if we need to re-run too.

work on dirty files only a prefix of which is syntactically correct

What does this mean? Do you mean that you would like to have a .vo even if the file is not fully compilable?

coq-lsp already generates .vo like this, hopefully coqc too soon.

or work on new files that have not been saved yet.

That's needed yes.

@rlepigre
Copy link
Contributor Author

rlepigre commented Mar 22, 2023

I think tho in general this is a misfeature. While I'm happy if you folks want to add this, I'm convinced that having to use this flag is just a way to mask deeper issues in the toolchain.

That's kind of true, but the fact is that today one still needs to maintain a _CoqProject file by hand for standard editors to properly work with dune projects, and that's a real pain. This becomes especially true when you have plugins that are loaded with the findlib method, in which case it even becomes not possible to make it work just with a _CoqProject file since -I options for plugin directories under _build/default are not enough (you can still pull it of by calling dune build @install for the plugins, and exporting an OCAMLPATH with the _build/install/default/lib directory in it, but that's terrible since you need to remember to call dune build @install again whenever you modify plugins, and if you forget you're suddenly using an older version of the plugins).

Being more flexible with dune coq top would give us a not-so-terrible way to completely get rid of _CoqProject files in dune projects (provided support for dune coq top in editors, which we have hacks for), and that's already a big win.

work on dirty files only a prefix of which is syntactically correct

What does this mean? Do you mean that you would like to have a .vo even if the file is not fully compilable?

No, I mean that when you (maybe by accident) saved your working file to disk, and re-run dune coq top, then coqdep on the file will fail due to syntax errors, and you'll never get your toplevel.

@ejgallego
Copy link
Collaborator

ejgallego commented Mar 22, 2023

That's kind of true, but the fact is that today one still needs to maintain a _CoqProject file by hand for standard editors to properly work with dune projects, and that's a real pain. This becomes especially true when you have plugins that are loaded with the findlib method, in which case it even becomes not possible to make it work just with a _CoqProject file since -I options for plugin directories under _build/default are not enough (you can still pull it of by calling dune build @install for the plugins, and exporting an OCAMLPATH with the _build/install/default/lib directory in it, but that's terrible since you need to remember to call dune build @install again whenever you modify plugins, and if you forget you're suddenly using an older version of the plugins).

Umm, once we fix the coqdep bug that should work tho right? Because coqdep will correctly emit the dependency on the plugin in _build/install, or am I missing something?

Being more flexible with dune coq top would give us a not-so-terrible way to completely get rid of _CoqProject files in dune projects (provided support for dune coq top in editors, which we have hacks for), and that's already a big win.

Yes, that's a big win, however the above concern seems unrelated to the --no-build flag, right?

work on dirty files only a prefix of which is syntactically correct

What does this mean? Do you mean that you would like to have a .vo even if the file is not fully compilable?

No, I mean that when you (maybe by accident) saved your working file to disk, and re-run dune coq top, then coqdep on the file will fail due to syntax errors, and you'll never get your toplevel.

Ah, very interesting thanks! Good to know for coq-lsp (where running coqdep is optional)

@Alizter
Copy link
Collaborator

Alizter commented Mar 22, 2023

I've made a draft implementing a --no-build option. Please test it. I will write some cram tests later.

@Alizter Alizter linked a pull request Mar 22, 2023 that will close this issue
2 tasks
@rlepigre
Copy link
Contributor Author

Umm, once we fix the coqdep bug that should work tho right? Because coqdep will correctly emit the dependency on the plugin in _build/install, or am I missing something?

The coqdep bug is part of the issue, but I actually circumvent it by using a wrapper script that fixes the paths. The actual issue is that I don't really know what to write in a _CoqProject file to make editors happy. It seems that I need one META file per plugin, while dune generates a single META file for all plugins in the package (not sure if that is the actual problem though). Also, since the META file is in _build/install and the other files are in _build/default, I need to force the generation of the whole "installed" version of plugins under _build/install to make it so that everything is in one place and I can extend OCAMLPATH.

@github-project-automation github-project-automation bot moved this from Todo to Done in Coq + Dune Apr 3, 2023
emillon added a commit to emillon/opam-repository that referenced this issue Apr 18, 2023
CHANGES:

- When a rule's action is interrupted, delete any leftover directory targets.
  This is consistent with how we treat file targets. (@rgrinberg, 7564)

- Fix plugin loading with findlib. The functionality was broken in 3.7.0.
  (ocaml/dune#7556, @anmonteiro)

- Introduce a `public_headers` field on libraries. This field is like
  `install_c_headers`, but it allows to choose the extension and choose the
  paths for the installed headers. (ocaml/dune#7512, @rgrinberg)

- Load the host context `findlib.conf` when cross-compiling (ocaml/dune#7428, fixes
  ocaml/dune#1701, @rgrinberg, @anmonteiro)

- Resolve `ppx_runtime_libraries` in the target context when cross compiling
  (ocaml/dune#7450, fixes ocaml/dune#2794, @anmonteiro)

- Use `$PKG_CONFIG`, when set, to find the `pkg-config` binary  (ocaml/dune#7469, fixes
  ocaml/dune#2572, @anmonteiro)

- Preliminary support for Coq compiled intefaces (`.vos` files) enabled via
  `(mode vos)` in `coq.theory` stanzas. This can be used in combination with
  `dune coq top` to obtain fast re-building of dependencies (with no checking
  of proofs) prior to stepping into a file. (ocaml/dune#7406, @rlepigre)

- Fix dune crashing on MacOS in watch mode whenever `$PATH` contains `$PWD`
  (ocaml/dune#7441, fixes ocaml/dune#6907, @rgrinberg)

- Fix `dune install` when cross compiling (ocaml/dune#7410, fixes ocaml/dune#6191, @anmonteiro,
  @rizo)

- Find `pps` dependencies in the host context when cross-compiling,  (ocaml/dune#7410,
  fixes ocaml/dune#4156, @anmonteiro)

- Dune in watch mode no longer builds concurrent rules in serial (ocaml/dune#7395
  @rgrinberg, @jchavarri)

- `dune coq top` now correctly respects the project root when called from a
  subdirectory. However, absolute filenames passed to `dune coq top` are no
  longer supported (due to being buggy) (ocaml/dune#7357, fixes ocaml/dune#7344, @rlepigre and
  @Alizter)

- Added a `--no-build` option to `dune coq top` for avoiding rebuilds (ocaml/dune#7380,
  fixes ocaml/dune#7355, @Alizter)

- RPC: Ignore SIGPIPE when clients suddenly disconnect (ocaml/dune#7299, ocaml/dune#7319, fixes
  ocaml/dune#6879, @rgrinberg)

- Always clean up the UI on exit. (ocaml/dune#7271, fixes ocaml/dune#7142 @rgrinberg)

- Bootstrap: remove reliance on shell. Previously, we'd use the shell to get
  the number of processors. (ocaml/dune#7274, @rgrinberg)

- Bootstrap: correctly detect the number of processors by allowing `nproc` to be
  looked up in `$PATH` (ocaml/dune#7272, @Alizter)

- Speed up file copying on macos by using `clonefile` when available
  (@rgrinberg, ocaml/dune#7210)

- Adds support for loading plugins in toplevels (ocaml/dune#6082, fixes ocaml/dune#6081,
  @ivg, @richardlford)

- Support commands that output 8-bit and 24-bit colors in the terminal (ocaml/dune#7188,
  @Alizter)

- Speed up rule generation for libraries and executables with many modules
  (ocaml/dune#7187, @jchavarri)

- Add `--watch-exclusions` to Dune build options (ocaml/dune#7216, @jonahbeckford)

- Do not re-render UI on every frame if the UI doesn't change (ocaml/dune#7186, fix
  ocaml/dune#7184, @rgrinberg)

- Make coq_db creation in scope lazy (@ejgallego, ocaml/dune#7133)

- Non-user proccesses such as version control or config checking are now run
  silently. (ocaml/dune#6994, fixes ocaml/dune#4066, @Alizter)

- Add the `--display-separate-messages` flag to separate the error messages
  produced by commands with a blank line. (ocaml/dune#6823, fixes ocaml/dune#6158, @esope)

- Accept the Ordered Set Language for the `modes` field in `library` stanzas
  (ocaml/dune#6611, @anmonteiro).

- dune install now respects --display quiet mode (ocaml/dune#7116, fixes ocaml/dune#4573, fixes
  ocaml/dune#7106, @Alizter)

- Stub shared libraries (dllXXX_stubs.so) in Dune-installed libraries could not
  be used as dependencies of libraries in the workspace (eg when compiling to
  bytecode and/or Javascript).  This is now fixed. (ocaml/dune#7151, @nojb)

- Allow the main module of a library with `(stdlib ...)` to depend on other
  libraries (ocaml/dune#7154, @anmonteiro).

- Bytecode executables built for JSOO are linked with `-noautolink` and no
  longer depend on the shared stubs of their dependent libraries (ocaml/dune#7156, @nojb)

- Added a new user action `(concurrent )` which is like `(progn )` but runs the
  actions concurrently. (ocaml/dune#6933, @Alizter)

- Allow `(stdlib ...)` to be used with `(wrapped false)` in library stanzas
  (ocaml/dune#7139, @anmonteiro).

- Allow parallel execution of inline tests partitions (ocaml/dune#7012, @hhugo)

- Support `(link_flags ...)` in `(cinaps ...)` stanza. (ocaml/dune#7423, fixes ocaml/dune#7416,
  @nojb)

- Allow `(package ...)` in any position within `(rule ...)` stanza (ocaml/dune#7445,
  @Leonidas-from-XIV)

- Always include `opam` files in the generated `.install` file. Previously, it
  would not be included whenever `(generate_opam_files true)` was set and the
  `.install` file wasn't yet generated. (ocaml/dune#7547, @rgrinberg)
emillon added a commit to emillon/opam-repository that referenced this issue May 23, 2023
CHANGES:

- Fix string quoting in the json file written by `--trace-file` (ocaml/dune#7773,
  @rleshchinskiy)

- Read `pkg-config` arguments from the `PKG_CONFIG_ARGN` environment variable
  (ocaml/dune#1492, ocaml/dune#7734, @anmonteiro)

- Correctly set `MANPATH` in `dune exec`. Previously, we would use the `bin/`
  directory of the context. (ocaml/dune#7655, @rgrinberg)

- Allow overriding the `ocaml` binary with findlib configuration (ocaml/dune#7648,
  @rgrinberg)

- merlin: ignore instrumentation settings for preprocessing. (ocaml/dune#7606, fixes
  ocaml/dune#7465, @Alizter)

- When a rule's action is interrupted, delete any leftover directory targets.
  This is consistent with how we treat file targets. (ocaml/dune#7564, @rgrinberg)

- Fix plugin loading with findlib. The functionality was broken in 3.7.0.
  (ocaml/dune#7556, @anmonteiro)

- Introduce a `public_headers` field on libraries. This field is like
  `install_c_headers`, but it allows to choose the extension and choose the
  paths for the installed headers. (ocaml/dune#7512, @rgrinberg)

- Load the host context `findlib.conf` when cross-compiling (ocaml/dune#7428, fixes
  ocaml/dune#1701, @rgrinberg, @anmonteiro)

- Add a `coqdoc_flags` field to the `coq.theory` stanza allowing the user to
  pass extra arguments to `coqdoc`. (ocaml/dune#7676, fixes ocaml/dune#7954 @Alizter)

- Resolve `ppx_runtime_libraries` in the target context when cross compiling
  (ocaml/dune#7450, fixes ocaml/dune#2794, @anmonteiro)

- Use `$PKG_CONFIG`, when set, to find the `pkg-config` binary  (ocaml/dune#7469, fixes
  ocaml/dune#2572, @anmonteiro)

- Modules that were declared in `(modules_without_implementation)`,
  `(private_modules)` or `(virtual_modules)` but not declared in `(modules)`
  will cause Dune to emit a warning which will become an error in 3.9. (ocaml/dune#7608,
  fixes ocaml/dune#7026, @Alizter)

- Preliminary support for Coq compiled intefaces (`.vos` files) enabled via
  `(mode vos)` in `coq.theory` stanzas. This can be used in combination with
  `dune coq top` to obtain fast re-building of dependencies (with no checking
  of proofs) prior to stepping into a file. (ocaml/dune#7406, @rlepigre)

- Fix dune crashing on MacOS in watch mode whenever `$PATH` contains `$PWD`
  (ocaml/dune#7441, fixes ocaml/dune#6907, @rgrinberg)

- Fix `dune install` when cross compiling (ocaml/dune#7410, fixes ocaml/dune#6191, @anmonteiro,
  @rizo)

- Find `pps` dependencies in the host context when cross-compiling,  (ocaml/dune#7410,
  fixes ocaml/dune#4156, @anmonteiro)

- Dune in watch mode no longer builds concurrent rules in serial (ocaml/dune#7395
  @rgrinberg, @jchavarri)

- Dune can now detect Coq theories from outside the workspace. This allows for
  composition with installed theories (not necessarily installed with Dune).
  (ocaml/dune#7047, @Alizter, @ejgallego)

- `dune coq top` now correctly respects the project root when called from a
  subdirectory. However, absolute filenames passed to `dune coq top` are no
  longer supported (due to being buggy) (ocaml/dune#7357, fixes ocaml/dune#7344, @rlepigre and
  @Alizter)

- Added a `--no-build` option to `dune coq top` for avoiding rebuilds (ocaml/dune#7380,
  fixes ocaml/dune#7355, @Alizter)

- RPC: Ignore SIGPIPE when clients suddenly disconnect (ocaml/dune#7299, ocaml/dune#7319, fixes
  ocaml/dune#6879, @rgrinberg)

- Always clean up the UI on exit. (ocaml/dune#7271, fixes ocaml/dune#7142 @rgrinberg)

- Bootstrap: remove reliance on shell. Previously, we'd use the shell to get
  the number of processors. (ocaml/dune#7274, @rgrinberg)

- Bootstrap: correctly detect the number of processors by allowing `nproc` to be
  looked up in `$PATH` (ocaml/dune#7272, @Alizter)

- Speed up file copying on macos by using `clonefile` when available
  (@rgrinberg, ocaml/dune#7210)

- Adds support for loading plugins in toplevels (ocaml/dune#6082, fixes ocaml/dune#6081,
  @ivg, @richardlford)

- Support commands that output 8-bit and 24-bit colors in the terminal (ocaml/dune#7188,
  @Alizter)

- Speed up rule generation for libraries and executables with many modules
  (ocaml/dune#7187, @jchavarri)

- Add `--watch-exclusions` to Dune build options (ocaml/dune#7216, @jonahbeckford)

- Do not re-render UI on every frame if the UI doesn't change (ocaml/dune#7186, fix
  ocaml/dune#7184, @rgrinberg)

- Make coq_db creation in scope lazy (@ejgallego, ocaml/dune#7133)

- Non-user proccesses such as version control or config checking are now run
  silently. (ocaml/dune#6994, fixes ocaml/dune#4066, @Alizter)

- Add the `--display-separate-messages` flag to separate the error messages
  produced by commands with a blank line. (ocaml/dune#6823, fixes ocaml/dune#6158, @esope)

- Accept the Ordered Set Language for the `modes` field in `library` stanzas
  (ocaml/dune#6611, @anmonteiro).

- dune install now respects --display quiet mode (ocaml/dune#7116, fixes ocaml/dune#4573, fixes
  ocaml/dune#7106, @Alizter)

- Stub shared libraries (dllXXX_stubs.so) in Dune-installed libraries could not
  be used as dependencies of libraries in the workspace (eg when compiling to
  bytecode and/or Javascript).  This is now fixed. (ocaml/dune#7151, @nojb)

- Allow the main module of a library with `(stdlib ...)` to depend on other
  libraries (ocaml/dune#7154, @anmonteiro).

- Bytecode executables built for JSOO are linked with `-noautolink` and no
  longer depend on the shared stubs of their dependent libraries (ocaml/dune#7156, @nojb)

- Added a new user action `(concurrent )` which is like `(progn )` but runs the
  actions concurrently. (ocaml/dune#6933, @Alizter)

- Allow `(stdlib ...)` to be used with `(wrapped false)` in library stanzas
  (ocaml/dune#7139, @anmonteiro).

- Allow parallel execution of inline tests partitions (ocaml/dune#7012, @hhugo)

- Support `(link_flags ...)` in `(cinaps ...)` stanza. (ocaml/dune#7423, fixes ocaml/dune#7416,
  @nojb)

- Allow `(package ...)` in any position within `(rule ...)` stanza (ocaml/dune#7445,
  @Leonidas-from-XIV)

- Always include `opam` files in the generated `.install` file. Previously, it
  would not be included whenever `(generate_opam_files true)` was set and the
  `.install` file wasn't yet generated. (ocaml/dune#7547, @rgrinberg)

- Fix regression where Merlin was unable to handle filenames with uppercase
  letters under Windows. (ocaml/dune#7577, @nojb)

- On nix+macos, pass `-f` to the codesign hook to avoid errors when the binary
  is already signed (ocaml/dune#7183, fixes ocaml/dune#6265, @greedy)

- Fix bug where RPC clients built with dune-rpc-lwt would crash when closing
  their connection to the server (ocaml/dune#7581, @gridbugs)

- Introduce mdx stanza 0.4 requiring mdx >= 2.3.0 which updates the default
  list of files to include `*.mld` files (ocaml/dune#7582, @Leonidas-from-XIV)

- Fix RPC server on Windows (used for OCaml-LSP). (ocaml/dune#7666, @nojb)

- Coq language versions less 0.8 are deprecated, and will be removed
  in an upcoming Dune version. All users are required to migrate to
  `(coq lang 0.8)` which provides the right semantics for theories
  that have been globally installed, such as those coming from opam
  (@ejgallego, @Alizter)

- Bump minimum version of the dune language for the melange syntax extension
  from 3.7 to 3.8 (ocaml/dune#7665, @jchavarri)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment