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.

Sign up for GitHub

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

Jump to bottom

Control+C handling #1912

Merged
merged 13 commits into from
Mar 7, 2019
Merged

Control+C handling #1912

merged 13 commits into from
Mar 7, 2019

Conversation

aalekseyev
Copy link
Collaborator

This PR deals with problematic behavior of dune when it's interrupted with Ctrl+C while running a build in polling mode.
Before this PR, dune would just fail with "assert false" and keep watching, among other weird behaviors.

After this PR, killing with Ctrl+C should just terminate dune quickly.

@aalekseyev
wait for all the forks to finish before [Fiber.run] returns
3bd1633 
Signed-off-by: Arseniy Alekseyev <aalekseyev@janestreet.com>
@aalekseyev
Improve signal handling in polling mode
07c2269 
Signed-off-by: Arseniy Alekseyev <aalekseyev@janestreet.com>
@aalekseyev
Convert polymorphic variants to normal variants
f044257 
Signed-off-by: Arseniy Alekseyev <aalekseyev@janestreet.com>
@aalekseyev aalekseyev requested a review from a user March 6, 2019 15:44
@aalekseyev
simplification#1
72d1539 
Signed-off-by: Arseniy Alekseyev <aalekseyev@janestreet.com>
@aalekseyev
move cleanup from continue_on_error to [run_and_cleanup], disassemble…
c86b4da 
… [continue_on_error]

Signed-off-by: Arseniy Alekseyev <aalekseyev@janestreet.com>
@aalekseyev
simplify polling scheduler: merge the two main loops and move event p…
fd0efad 
…rocessing out of a fiber

Signed-off-by: Arseniy Alekseyev <aalekseyev@janestreet.com>
@aalekseyev
Copy link
Collaborator Author

@diml, this should be ready for review now. The first two commits implement the actual fix, and the rest are a cleanup with no significant intended change of behavior.

ghost
ghost suggested changes Mar 7, 2019
View reviewed changes
src/scheduler.ml Outdated Show resolved Hide resolved
src/scheduler.ml Outdated Show resolved Hide resolved
@ghost
Copy link

ghost commented Mar 7, 2019

Thanks, the result does look easier to follow. BTW, looking again at this file, the function names are not great: we have go_rec that is called by run, itself called by run_and_cleanup which is then called by both go and poll. I suggest to put the first three functions into a sub-module Run_once or Main_loop that only exposes run_and_cleanup and maybe rename go_rec to something else, such as main_loop.

@aalekseyev
renames and small cleanup
b71f87a 
Signed-off-by: Arseniy Alekseyev <aalekseyev@janestreet.com>
@aalekseyev
cleaner exn handling
7fb255c 
Signed-off-by: Arseniy Alekseyev <aalekseyev@janestreet.com>
@aalekseyev
Copy link
Collaborator Author

Pushed the changes.
I renamed go_rec to pump_events because that seems like what it's doing.

@aalekseyev aalekseyev requested a review from a user March 7, 2019 10:38
ghost
ghost approved these changes Mar 7, 2019
View reviewed changes
src/scheduler.ml Outdated
Fiber.return Files_changed
| Signal signal ->
got_signal t signal;
Fiber.return (Got_signal : pump_events_result)
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should be able to get rid of this type annotation now:

Suggested change
Fiber.return (Got_signal : pump_events_result)
Fiber.return Got_signal

rgrinberg added a commit to rgrinberg/jbuilder that referenced this pull request Mar 7, 2019
@rgrinberg
Fix ocaml#1912
67bc7b9 
Error code of dune format should be non zero when it fails
rgrinberg added a commit to rgrinberg/jbuilder that referenced this pull request Mar 7, 2019
@rgrinberg
Fix ocaml#1912
9ab7660 
Error code of dune format should be non zero when it fails

Signed-off-by: Rudi Grinberg <rudi.grinberg@gmail.com>
rgrinberg added a commit to rgrinberg/jbuilder that referenced this pull request Mar 7, 2019
@rgrinberg
Fix ocaml#1912
dc3eddf 
Error code of dune format should be non zero when it fails

Signed-off-by: Rudi Grinberg <rudi.grinberg@gmail.com>
@aalekseyev
add changelog entries
b591429 
Signed-off-by: Arseniy Alekseyev <aalekseyev@janestreet.com>
@aalekseyev
Merge branch 'control-c-handling' of github.com:aalekseyev/dune into …
2007fa1 
…control-c-handling
@aalekseyev
cleanup
5023031 
Signed-off-by: Arseniy Alekseyev <aalekseyev@janestreet.com>
@aalekseyev
Merge branch 'master' into control-c-handling
c669c7a
@aalekseyev aalekseyev merged commit f595dcb into ocaml:master Mar 7, 2019
rgrinberg added a commit to rgrinberg/opam-repository that referenced this pull request Mar 7, 2019
@rgrinberg
[new release] dune (1.8.0)
234e8d2 
CHANGES:

- Clean up watch mode polling loop: improves signal handling and error handling
  during polling (ocaml/dune#1912, fix ocaml/dune#1907, fix ocaml/dune#1671, @aalekseyev)

- Change status messages during polling to be one-line, so that the messages are
  correctly erased by ^K. (ocaml/dune#1912, @aalekseyev)

- Add support for `.cxx` extension for C++ stubs (ocaml/dune#1831, @rgrinberg)

- Add `DUNE_WORKSPACE` variable. This variable is equivalent to setting
  `--workspace` in the command line. (ocaml/dune#1711, fix ocaml/dune#1503, @rgrinberg)

- Add `c_flags` and `cxx_flags` to env profile settings (ocaml/dune#1700 and ocaml/dune#1800,
  @gretay-js)

- Format `dune printenv` output (ocaml/dune#1867, fix ocaml/dune#1862, @emillon)

- Add the `(promote-into <dir>)` and `(promote-until-clean-into
  <dir>)` modes for `(rule ...)` stanzas, so that files can be
  promoted in another directory than the current one. For instance,
  this is used in merlin to promote menhir generated files in a
  directory that depends on the version of the compiler (ocaml/dune#1890, @diml)

- Improve error message when `dune subst` fails (ocaml/dune#1898, fix ocaml/dune#1897, @rgrinberg)

- Add more GC counters to catapult traces (fix908, @rgrinberg)

- Add a preprocessor shim for the `let+` syntax of OCaml 4.08 (ocaml/dune#1899,
  implements ocaml/dune#1891, @diml)

- Fix generation of `.merlin` files on Windows. `\` characters needed
  to be escaped (ocaml/dune#1869, @mlasson)

- Fix 0 error code when `$ dune format-dune-file` fails. (ocaml/dune#1915, fix ocaml/dune#1914,
  @rgrinberg)

- Configurator: deprecated `query_expr` and introduced `query_expr_err` which is
  the same but with a better error in case it fails. (ocaml/dune#1886, @ejgallego)
@rgrinberg rgrinberg mentioned this pull request Mar 7, 2019
Closed
rgrinberg added a commit to rgrinberg/opam-repository that referenced this pull request Mar 7, 2019
@rgrinberg
[new release] dune (1.8.0)
e0b2348 
CHANGES:

- Clean up watch mode polling loop: improves signal handling and error handling
  during polling (ocaml/dune#1912, fix ocaml/dune#1907, fix ocaml/dune#1671, @aalekseyev)

- Change status messages during polling to be one-line, so that the messages are
  correctly erased by ^K. (ocaml/dune#1912, @aalekseyev)

- Add support for `.cxx` extension for C++ stubs (ocaml/dune#1831, @rgrinberg)

- Add `DUNE_WORKSPACE` variable. This variable is equivalent to setting
  `--workspace` in the command line. (ocaml/dune#1711, fix ocaml/dune#1503, @rgrinberg)

- Add `c_flags` and `cxx_flags` to env profile settings (ocaml/dune#1700 and ocaml/dune#1800,
  @gretay-js)

- Format `dune printenv` output (ocaml/dune#1867, fix ocaml/dune#1862, @emillon)

- Add the `(promote-into <dir>)` and `(promote-until-clean-into
  <dir>)` modes for `(rule ...)` stanzas, so that files can be
  promoted in another directory than the current one. For instance,
  this is used in merlin to promote menhir generated files in a
  directory that depends on the version of the compiler (ocaml/dune#1890, @diml)

- Improve error message when `dune subst` fails (ocaml/dune#1898, fix ocaml/dune#1897, @rgrinberg)

- Add more GC counters to catapult traces (fix908, @rgrinberg)

- Add a preprocessor shim for the `let+` syntax of OCaml 4.08 (ocaml/dune#1899,
  implements ocaml/dune#1891, @diml)

- Fix generation of `.merlin` files on Windows. `\` characters needed
  to be escaped (ocaml/dune#1869, @mlasson)

- Fix 0 error code when `$ dune format-dune-file` fails. (ocaml/dune#1915, fix ocaml/dune#1914,
  @rgrinberg)

- Configurator: deprecated `query_expr` and introduced `query_expr_err` which is
  the same but with a better error in case it fails. (ocaml/dune#1886, @ejgallego)

- Make sure `(menhir (mode promote) ...)` stanzas are ignored when
  using `--ignore-promoted-rules` or `-p` (ocaml/dune#1917, @diml)
@rgrinberg rgrinberg mentioned this pull request Mar 7, 2019
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
1 participant
@aalekseyev