Stop treating toolchains as packages in pkg rules #10534
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Previously toolchains were treated as special packages by the package rules logic. This change removes toolchains from the package dependency graph used by package rules (but not from any other part of dune such as the solver or lockfiles). This separation lets us remove a bunch of special treatment for packages that contain toolchains. Signed-off-by: Stephen Sherratt <[email protected]>
gridbugs
force-pushed
the
stop-treating-toolchains-as-packages-in-pkg-rules
branch
from
a061aa4
to
97f0934
Compare
|
@Leonidas-from-XIV can you take a second look at this please |
| | Error e -> Fiber.return @@ Error e | ||
| | Ok `Success -> Fiber.return (Ok `Success) | ||
| | Ok `Failure -> | ||
| let sleep_duration_s = 0.1 in |
There was a problem hiding this comment.
This could possibly be pulled out of the function. I don't know, even in the original code it is an admittedly ugly bit.
There was a problem hiding this comment.
I've split this into two mutually recursive closures to avoid needing to pass as much info from one recursive call to the next.
src/dune_pkg/flock.ml
Outdated
| let fd = | ||
| Unix.openfile | ||
| (Path.to_string lock_path) | ||
| [ Unix.O_CREAT; O_WRONLY; O_SHARE_DELETE; Unix.O_CLOEXEC ] |
There was a problem hiding this comment.
That second Unix there can be removed.
Signed-off-by: Stephen Sherratt <[email protected]>
gridbugs
force-pushed
the
stop-treating-toolchains-as-packages-in-pkg-rules
branch
from
97f0934
to
020b2ec
Compare
Signed-off-by: Stephen Sherratt <[email protected]>
This prevents multiple concurrent instances of dune interfering with one another while installing toolchains. Signed-off-by: Stephen Sherratt <[email protected]>
gridbugs
force-pushed
the
stop-treating-toolchains-as-packages-in-pkg-rules
branch
from
5712586
to
c740f6a
Compare
Leonidas-from-XIV
approved these changes
May 22, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This grew out of the work I'm doing to read toolchain information from the opam repo rather than hardcoding it (https://github.com/tarides/team-build-system/issues/11), but the main benefit of this change is that it removes a bunch of special cases I needed to add so that packages containing toolchains were ignored by most of the package rules.
It's also going to make it easier to thread the extra information needed to represent the
ocaml-variantstoolchain to the parts of the code where it's needed in a later change.