Skip to content

feat(pkg): autolocking #12653

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
Merged
merged 1 commit into from
Nov 12, 2025
Merged

feat(pkg): autolocking #12653

merged 1 commit into from
Nov 12, 2025

Conversation

@Alizter
Copy link
Collaborator

@Alizter Alizter commented Oct 29, 2025 •
edited
Loading

We add support for autolocking. This works when (pkg enabled) is present in the dune-workspace and allows users to build package dependencies without having to go through dune pkg lock.

If a lock file is present, it will continue to build using that as is currently the case.

This started as a rough draft of the working parts of #11775 and my branch that built on that. The advantage of this approach is that we don't touch dev tools or dune pkg lock.

@Alizter Alizter force-pushed the push-skrkpmlkomsw branch 2 times, most recently from c8b556f to 901b00e Compare October 30, 2025 11:43
@shonfeder
Copy link
Member

shonfeder commented Oct 30, 2025 •
edited
Loading

I'm excited to see this landing :D I'm reporting on an attempt to use it:

I have tried using a build from this branch to build cohttp. I have set (pkg enabled) in the dune-workspace and deleting the lock dir I was previously working with. Then trying to run with dune build fails with

$ dune build
Error: Program ocamlc not found in the tree or in PATH
 (context: default)
-> required by loading the OCaml compiler for context "default"

is it an instance of #11038 ?

shonfeder
shonfeder reviewed Oct 30, 2025
View reviewed changes
@Alizter

This comment was marked as resolved.

Sign in to view
@Alizter

This comment was marked as resolved.

Sign in to view
gridbugs
gridbugs reviewed Nov 3, 2025
View reviewed changes
gridbugs
gridbugs reviewed Nov 3, 2025
View reviewed changes
@Alizter Alizter force-pushed the push-skrkpmlkomsw branch 9 times, most recently from 2084d26 to 093bdf9 Compare November 5, 2025 13:05
@Alizter Alizter marked this pull request as ready for review November 5, 2025 13:08
Leonidas-from-XIV
Leonidas-from-XIV requested changes Nov 6, 2025
View reviewed changes
Copy link
Collaborator

@Leonidas-from-XIV Leonidas-from-XIV left a comment

Choose a reason for hiding this comment

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

I am very excited to see this finally come to a close. This will unblock a lot of things that we have going forward.

@Alizter Alizter force-pushed the push-skrkpmlkomsw branch 2 times, most recently from 1410922 to 35d5716 Compare November 6, 2025 16:52
@Alizter Alizter mentioned this pull request Nov 6, 2025
Closed
Alizter
Alizter commented Nov 6, 2025
View reviewed changes
@shonfeder shonfeder mentioned this pull request Nov 7, 2025
Closed
@gridbugs gridbugs mentioned this pull request Nov 10, 2025
Merged
Leonidas-from-XIV
Leonidas-from-XIV reviewed Nov 10, 2025
View reviewed changes
@Alizter Alizter force-pushed the push-skrkpmlkomsw branch 3 times, most recently from 7a4c0e6 to 9f0cd93 Compare November 11, 2025 14:23
Leonidas-from-XIV
Leonidas-from-XIV reviewed Nov 11, 2025
View reviewed changes
Copy link
Collaborator

@Leonidas-from-XIV Leonidas-from-XIV left a comment

Choose a reason for hiding this comment

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

I think it's pretty much there, some minor comments about the code but overall seems fine.

shonfeder
shonfeder reviewed Nov 11, 2025
View reviewed changes
@Alizter Alizter force-pushed the push-skrkpmlkomsw branch 6 times, most recently from 19bf821 to 6ab3e62 Compare November 11, 2025 16:22
Leonidas-from-XIV
Leonidas-from-XIV approved these changes Nov 12, 2025
View reviewed changes
Copy link
Collaborator

@Leonidas-from-XIV Leonidas-from-XIV left a comment

Choose a reason for hiding this comment

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

In my opinion this is ready to merge.

@Alizter
feat(pkg): autolocking
80ffe25 
Signed-off-by: Ali Caglayan <[email protected]>
@Alizter Alizter enabled auto-merge November 12, 2025 09:32
@Alizter Alizter merged commit d87f4a7 into ocaml:main Nov 12, 2025
25 checks passed
@Alizter Alizter deleted the push-skrkpmlkomsw branch November 12, 2025 09:48
This was referenced Nov 12, 2025
Merged
Draft
rgrinberg
rgrinberg reviewed Nov 12, 2025
View reviewed changes
src/dune_rules/lock_rules.ml
in
let combined_pins = Pin.DB.combine_exn workspace_pins_db project_pins_db in
Memo.return combined_pins
>>| resolve_project_pins
Copy link
Member

@rgrinberg rgrinberg Nov 12, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

How is this code not shared with the non watch mode locking code? Why would we handle pins differently here?

Copy link
Collaborator Author

@Alizter Alizter Nov 12, 2025

Choose a reason for hiding this comment

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

Not shared on purpose so that we don't break existing code. Will be shared in the future under more scrutiny. Comments added in #12713.

rgrinberg
rgrinberg reviewed Nov 12, 2025
View reviewed changes
src/dune_rules/lock_action.mli
@@ -0,0 +1,14 @@
open Import
Copy link
Member

@rgrinberg rgrinberg Nov 12, 2025

Choose a reason for hiding this comment

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

Can you move this module into Lock_rules?

Copy link
Collaborator Author

@Alizter Alizter Nov 12, 2025

Choose a reason for hiding this comment

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

Done in #12714

@Alizter Alizter mentioned this pull request Nov 12, 2025
Merged
This was referenced Nov 14, 2025
Closed
Merged
@Alizter Alizter mentioned this pull request Nov 18, 2025
Merged
@Leonidas-from-XIV Leonidas-from-XIV mentioned this pull request Nov 26, 2025
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@rgrinberg rgrinberg rgrinberg left review comments

@Leonidas-from-XIV Leonidas-from-XIV Leonidas-from-XIV approved these changes

@gridbugs gridbugs gridbugs approved these changes

@shonfeder shonfeder Awaiting requested review from shonfeder

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

pkg: make lock files targets pkg: dune watch mode with autolocking

5 participants

@Alizter @shonfeder @Leonidas-from-XIV @rgrinberg @gridbugs