Add user warning when waiting for build directory lock #12687
Conversation
shonfeder
left a comment
shonfeder
left a comment
There was a problem hiding this comment.
This looks like a nice fix to me!
From the CI, it looks like the tests to be updated.
src/dune_util/global_lock.ml
Outdated
| let emit_lock_warning lock_held_by = | ||
| let pid_suffix = | ||
| match lock_held_by with | ||
| | Lock_held_by.Unknown -> "" | ||
| | Pid_from_lockfile pid -> sprintf " (pid: %d)" pid | ||
| in | ||
| let condition = | ||
| match lock_held_by with | ||
| | Pid_from_lockfile _ -> "If this process is no longer running" | ||
| | Unknown -> "If no other dune process is running" | ||
| in | ||
| User_warning.emit | ||
| [ Pp.textf | ||
| "Build directory is locked by another dune process%s. Waiting for the lock to be \ | ||
| released..." | ||
| pid_suffix | ||
| ; Pp.textf "%s, delete the lock file: %s" condition (Path.Build.to_string lock_file) | ||
| ] | ||
| ;; | ||
|
|
There was a problem hiding this comment.
This looks like a really nicely reasoned out error message to me. 👌
src/dune_util/global_lock.ml
Outdated
| | `Success -> `Stop | ||
| | `Failure -> `Continue) | ||
| | `Failure -> | ||
| if not !warned |
There was a problem hiding this comment.
It was not obvious to me how warned could ever be true. I guess it is because with_timeout will try this operation repeatedly? If that's the reason, I think a short comment explaining this when the ref is created on line 129 could be helpful.
|
@sabine could you update the tests here, so we can land the PR? |
There was a problem hiding this comment.
Following the test failures will make this easier to understand, but the current behaviour is incorrect.
If one runs a server dune build -w and a client dune build @foo, they will receive the warning about "waiting for the lock" even where there is no waiting involved, and the build request is being served correctly.
When dune attempts to forward a build request to an existing dune process but cannot establish an RPC connection, it silently retries indefinitely. This adds a warning message to inform users what is happening and how to resolve the issue by deleting _build/.lock if no other dune process is actually running. Signed-off-by: Sabine Schmaltz <[email protected]>
sabine
force-pushed
the
no_more_silently_hanging_on_acquire_lock
branch
from
a4f217a to
3e24f22
Compare
Signed-off-by: Sabine Schmaltz <[email protected]>
3 participants
Resolves #12685
another process
Example output: