Skip to content

fix(pkg): make autolock solver env same as pkg lock#12719

Merged
Alizter merged 1 commit intoocaml:mainfrom
Alizter:push-mnmqpqzysnkr
Nov 17, 2025
Merged

fix(pkg): make autolock solver env same as pkg lock#12719
Alizter merged 1 commit intoocaml:mainfrom
Alizter:push-mnmqpqzysnkr

Conversation

@Alizter
Copy link
Copy Markdown
Collaborator

@Alizter Alizter commented Nov 13, 2025

Follow up to #12694. I've reverted the changes to autolocking by adding some more copy and pasted code and keeping the two implementations in sync.

This means we are solving for multiple platforms for autolocking, but lock dirs here are an implementation detail and this inefficiency can be removed in a future change.

We are still not settled on the question of default target platforms yet so no point making any decisions here.

A full cleanup of all dupes will follow this PR soon.

@Alizter Alizter requested a review from gridbugs November 13, 2025 20:25
@gridbugs
Copy link
Copy Markdown
Collaborator

I thought it would be better if the solve done by autolocking would only solve for the current platform since that would be faster, and ensure the build plan would work on the current platform. Does this PR change it to solve for the default platforms as well as the current platform or just the default platforms?

@Alizter
Copy link
Copy Markdown
Collaborator Author

Alizter commented Nov 14, 2025

@gridbugs It's the same as dune pkg lock which means autolock will not work outside of the supported platforms. The choice of platforms will have to be done at some point, but my goal is to consolidate the two locking functionalities we have at the moment.

@Leonidas-from-XIV
Copy link
Copy Markdown
Collaborator

Maybe before doing this or any other change in this regard we should figure out what we want autolocking to do. @Alizter mentioned some options in #11868 (comment) and I think some of the proposals (including my current favorite, so I am a bit biased) suggest that autolocking should indeed just lock for the current system.

@Alizter
Copy link
Copy Markdown
Collaborator Author

Alizter commented Nov 14, 2025

I would argue that this change should have been made in #12694. I'm not arguing that the behaviour introduced here is correct for autolocking, but it unbreaks some invariants I was hoping to preserve until I refactored the autolocking code.

Copy link
Copy Markdown
Collaborator

@gridbugs gridbugs left a comment

Choose a reason for hiding this comment

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

Ok, happy for you to merge this if it simplifies things @Alizter. We can always change the behaviour later once we have a consensus on #11868.

Signed-off-by: Ali Caglayan <alizter@gmail.com>
@Alizter Alizter merged commit 1538b06 into ocaml:main Nov 17, 2025
26 checks passed
@Alizter Alizter deleted the push-mnmqpqzysnkr branch November 17, 2025 21:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants