Skip to content

Use Fpath.traverse in pkg_rules#14291

Merged
Alizter merged 1 commit intoocaml:mainfrom
ElectreAAS:push-xxyzrmlroppn
Apr 23, 2026
Merged

Use Fpath.traverse in pkg_rules#14291
Alizter merged 1 commit intoocaml:mainfrom
ElectreAAS:push-xxyzrmlroppn

Conversation

@ElectreAAS
Copy link
Copy Markdown
Collaborator

@ElectreAAS ElectreAAS commented Apr 22, 2026

Just a little change extracted from #13792, nothing changing dune behaviour.
Figured I'd use Fpath.traverse where it's appropriate

@Alizter
Copy link
Copy Markdown
Collaborator

Alizter commented Apr 22, 2026

Could you split the traversal change into a separate PR, that should be simpler to review.

@ElectreAAS ElectreAAS changed the title Simplify Pkg_rules.source_files a little Use Fpath.traverse in pkg_rules Apr 23, 2026
@Alizter Alizter requested review from Alizter and Copilot April 23, 2026 12:44
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Refactors pkg_rules symlink resolution logic to use Stdune.Fpath.traverse instead of manual recursive directory walking.

Changes:

  • Replaced manual Readdir.read_directory_with_kinds recursion with Fpath.traverse for scanning target_dir.
  • Kept behavior of replacing file symlinks (that ultimately resolve to regular files) with hardlinks for cache compatibility.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/dune_rules/pkg_rules.ml Outdated
@ElectreAAS
Copy link
Copy Markdown
Collaborator Author

Could you split the traversal change into a separate PR, that should be simpler to review.

Done -> #14301

Signed-off-by: Ambre Austen Suhamy <ambre@tarides.com>

Signed-off-by: Ambre Austen Suhamy <ambre@tarides.com>
@Alizter Alizter merged commit d7f46a7 into ocaml:main Apr 23, 2026
82 of 87 checks passed
@ElectreAAS ElectreAAS deleted the push-xxyzrmlroppn branch April 24, 2026 11:38
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