Skip to content

fix(pkg): fix extra files with autolocking #13595

Open
Alizter wants to merge 2 commits intoocaml:mainfrom
Alizter:push-vruwomrnrnxn
Open

fix(pkg): fix extra files with autolocking #13595
Alizter wants to merge 2 commits intoocaml:mainfrom
Alizter:push-vruwomrnrnxn

Conversation

@Alizter
Copy link
Collaborator

@Alizter Alizter commented Feb 9, 2026

This patch makes sure we are using the internal lock directory rather than the source one. The previous code was traversing the lock directory source for the extra files and therefore autolocking, which directly produces the internal lock, would fail.

@Alizter Alizter force-pushed the push-vruwomrnrnxn branch 2 times, most recently from 073a829 to d8db822 Compare February 9, 2026 12:31
@Alizter Alizter marked this pull request as ready for review February 9, 2026 12:40
@ElectreAAS ElectreAAS requested a review from punchagan February 10, 2026 13:10
punchagan
punchagan previously approved these changes Feb 19, 2026
Copy link
Collaborator

@punchagan punchagan left a comment

Choose a reason for hiding this comment

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

Thanks for the clean up, alongside the bug fix!

@shonfeder shonfeder self-requested a review February 19, 2026 13:12
@Alizter
Copy link
Collaborator Author

Alizter commented Feb 20, 2026

This change fixes a race condition with dev tools that triggers some other hidden bugs. I will factor those fixes out first and fix those issues, so marking this as draft in the meantime.

@Alizter Alizter marked this pull request as draft February 20, 2026 09:09
@Alizter Alizter force-pushed the push-vruwomrnrnxn branch from 999a575 to fb07ba2 Compare March 1, 2026 13:19
@Alizter Alizter marked this pull request as ready for review March 1, 2026 13:19
@Alizter
Copy link
Collaborator Author

Alizter commented Mar 2, 2026

Important technical notes about this patch:

When we copy a source lock directory to an internal one we are at the mercy of the interpreter of the pkg rules to read this lock directory correctly. At no point do we record the fact that there are extra files, we instead leave this to be inferred by the directory structure.

The previous implementation was inferring it directly from the source location which didn't work in every case, and left out autolocking.

@shonfeder shonfeder requested review from punchagan and removed request for punchagan March 3, 2026 16:54
@Alizter Alizter force-pushed the push-vruwomrnrnxn branch from fb07ba2 to 171aeed Compare March 4, 2026 07:57
@Alizter Alizter requested review from rgrinberg and shonfeder March 4, 2026 07:59
@Alizter Alizter marked this pull request as draft March 4, 2026 08:10
@Alizter

This comment was marked as outdated.

Alizter added 2 commits March 4, 2026 11:25
Signed-off-by: Ali Caglayan <alizter@gmail.com>
Signed-off-by: Ali Caglayan <alizter@gmail.com>
@Alizter Alizter force-pushed the push-vruwomrnrnxn branch from 171aeed to 1c3b41d Compare March 4, 2026 10:25
@Alizter Alizter marked this pull request as ready for review March 4, 2026 10:26
@shonfeder shonfeder dismissed punchagan’s stale review March 4, 2026 12:35

Reviewed an earlier iteration of the code and is busy at the moment.

Copy link
Member

@shonfeder shonfeder left a comment

Choose a reason for hiding this comment

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

This looks great to me. Super clear. Thanks!

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.

pkg: dune with pkg enabled fails to build an oxcaml project without a lock

4 participants