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

fix: don't take build lock for coq top --no-build #10547

Merged
merged 1 commit into from
May 29, 2024
Merged

Conversation

lzy0505
Copy link
Contributor

@lzy0505 lzy0505 commented May 17, 2024

This fixes #7671 where dune coq top tries to take the global build lock even if --no-build is enabled.

The problem was that Common.Builder.term had allow_builds set to true by default and didn't get updated when --no-build was specified. This led to the creation of an RPC server at initialization, which triggers the lock.

The fix was to update allow_builds to false with Common.Builder.forbid_builds.

@Alizter
Copy link
Collaborator

Alizter commented May 23, 2024

@rlepigre could you test this?

@rlepigre
Copy link
Contributor

@rlepigre could you test this?

@Alizter I gave this PR a quick try, and it seems to do the trick. Thanks for the fix @lzy0505!

@Alizter
Copy link
Collaborator

Alizter commented May 28, 2024

@lzy0505 Could you add a changelog entry? Have a look at doc/changes to see how it is done. Once that is done, we can probably merge.

@lzy0505
Copy link
Contributor Author

lzy0505 commented May 28, 2024

@Alizter Done.

@Alizter
Copy link
Collaborator

Alizter commented May 28, 2024

Thanks this looks good. @rgrinberg @emillon this is ready for merging.

@rgrinberg rgrinberg added this to the 3.16.0 milestone May 29, 2024
@rgrinberg rgrinberg merged commit 86b8a6d into ocaml:main May 29, 2024
27 of 28 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Running "dune coq top --no-build ..." tries to take the lock
4 participants