Conversation
Alizter
force-pushed
the
remove-pkg-digest
branch
from
September 1, 2026 21:25
5a05e73 to
65134db
Compare
Alizter
marked this pull request as ready for review
September 2, 2026 15:33
Alizter
force-pushed
the
remove-pkg-digest
branch
from
September 2, 2026 17:00
65134db to
fa48aa5
Compare
There was a problem hiding this comment.
🟡 Changes recommended
Stable paths currently permit stale environment-dependent builds and case-insensitive package-root collisions.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Removes digest-qualified package identities, using package names within isolated project and dev-tool universes instead.
Changes:
- Reworks package databases, resolution, and build paths.
- Removes
Pkg_digestanddune pkg print-digest. - Updates package cram tests for the new layout.
File summaries
| File | Description |
|---|---|
test/blackbox-tests/test-cases/pkg/withenv-path.t |
Updates package paths. |
test/blackbox-tests/test-cases/pkg/variables.t |
Updates paths and diagnostics. |
test/blackbox-tests/test-cases/pkg/unknown-package.t |
Updates unknown-package error. |
test/blackbox-tests/test-cases/pkg/unavailable-package-source.t |
Updates source failure paths. |
test/blackbox-tests/test-cases/pkg/toolchain-variables.t |
Updates toolchain path. |
test/blackbox-tests/test-cases/pkg/substitute.t |
Updates dependency substitutions. |
test/blackbox-tests/test-cases/pkg/source-with-directory-symlink.t |
Updates source paths. |
test/blackbox-tests/test-cases/pkg/set-ocamlfind-destdir.t |
Updates install destination paths. |
test/blackbox-tests/test-cases/pkg/portable-lockdirs/portable-lockdirs-platform-dependant-dependencies.t |
Updates portable package roots. |
test/blackbox-tests/test-cases/pkg/portable-lockdirs/portable-lockdirs-platform-alternative-selection.t |
Updates selected package roots. |
test/blackbox-tests/test-cases/pkg/portable-lockdirs/portable-lockdirs-older-common-version.t |
Removes digest lookup. |
test/blackbox-tests/test-cases/pkg/portable-lockdirs/portable-lockdirs-custom-solver-env.t |
Uses name-based package path. |
test/blackbox-tests/test-cases/pkg/portable-lockdirs/portable-lockdirs-custom-platforms.t |
Uses name-based package path. |
test/blackbox-tests/test-cases/pkg/portable-lockdirs/portable-lockdirs-build-context-solver-env.t |
Uses name-based package path. |
test/blackbox-tests/test-cases/pkg/portable-lockdirs/portable-lockdirs-basic.t |
Updates portable build paths. |
test/blackbox-tests/test-cases/pkg/pkg-deps.t |
Updates package dependency paths. |
test/blackbox-tests/test-cases/pkg/pkg-deps-lock.t |
Updates rule dependencies. |
test/blackbox-tests/test-cases/pkg/patch.t |
Updates patch diagnostics. |
test/blackbox-tests/test-cases/pkg/opam-var/opam-var-switch.t |
Updates opam variable paths. |
test/blackbox-tests/test-cases/pkg/odoc/dev-tool-odoc-dune-tools-install.t |
Updates isolated odoc path. |
test/blackbox-tests/test-cases/pkg/ocamllsp/dev-tool-ocamllsp-env-path-var.t |
Updates dev-tool environments. |
test/blackbox-tests/test-cases/pkg/ocamllsp/dev-tool-ocamllsp-basic.t |
Updates ocamllsp path. |
test/blackbox-tests/test-cases/pkg/ocamlformat/ocamlformat-which.t |
Updates ocamlformat lookup. |
test/blackbox-tests/test-cases/pkg/ocamlformat/ocamlformat-install.t |
Updates ocamlformat install path. |
test/blackbox-tests/test-cases/pkg/ocamlformat/dune |
Adds curl dependencies. |
test/blackbox-tests/test-cases/pkg/ocaml-compiler.t |
Updates compiler package path. |
test/blackbox-tests/test-cases/pkg/merlin/dev-tool-merlin-env.t |
Updates Merlin environment path. |
test/blackbox-tests/test-cases/pkg/lock-directory-selection.t |
Updates package errors. |
test/blackbox-tests/test-cases/pkg/installed-binary.t |
Updates install-cookie paths. |
test/blackbox-tests/test-cases/pkg/install-symlinks/link-permission-error.t |
Updates symlink failure paths. |
test/blackbox-tests/test-cases/pkg/install-symlinks/dir-symlink.t |
Updates directory-symlink paths. |
test/blackbox-tests/test-cases/pkg/install-symlinks/broken-symlink.t |
Updates broken-symlink paths. |
test/blackbox-tests/test-cases/pkg/install-missing-entry.t |
Updates missing-entry paths. |
test/blackbox-tests/test-cases/pkg/install-action.t |
Updates install action output. |
test/blackbox-tests/test-cases/pkg/ignored-dune-lock.t |
Updates inactive-lock error. |
test/blackbox-tests/test-cases/pkg/helpers.sh |
Simplifies package path helper. |
test/blackbox-tests/test-cases/pkg/findlib-meta-optional-directory.t |
Updates findlib diagnostic. |
test/blackbox-tests/test-cases/pkg/fetch-symlinks/valid-links.t |
Updates fetched source paths. |
test/blackbox-tests/test-cases/pkg/fetch-symlinks/link-to-parent.t |
Updates cycle diagnostics. |
test/blackbox-tests/test-cases/pkg/fetch-symlinks/escaping-link.t |
Updates escaping-link diagnostics. |
test/blackbox-tests/test-cases/pkg/fetch-symlinks/basic-cycle.t |
Updates cycle path. |
test/blackbox-tests/test-cases/pkg/fetch-local-source.t |
Removes digest lookup. |
test/blackbox-tests/test-cases/pkg/fetch-cache-symlink.t |
Updates cache expectations. |
test/blackbox-tests/test-cases/pkg/extra-source-overlap-with-source.t |
Uses name-based source path. |
test/blackbox-tests/test-cases/pkg/dune |
Updates cram dependencies. |
test/blackbox-tests/test-cases/pkg/different-dune-in-path.t |
Uses direct package paths. |
test/blackbox-tests/test-cases/pkg/dep-section-variables.t |
Updates section paths. |
test/blackbox-tests/test-cases/pkg/default-exported-env.t |
Updates exported environment paths. |
test/blackbox-tests/test-cases/pkg/build-with-executable-script-on-windows.t |
Updates Windows diagnostics. |
test/blackbox-tests/test-cases/pkg/broken-symlink-in-dependency.t |
Updates dependency source paths. |
test/blackbox-tests/test-cases/pkg/bin-narrowing/workspace-shadows-lockdir.t |
Updates narrowed PATH entries. |
test/blackbox-tests/test-cases/pkg/bin-narrowing/transitive-deps.t |
Updates transitive PATH entries. |
test/blackbox-tests/test-cases/pkg/bin-narrowing/resolve-program.t |
Updates resolved program paths. |
test/blackbox-tests/test-cases/pkg/bin-narrowing/multi-package.t |
Updates multi-package paths. |
test/blackbox-tests/test-cases/pkg/bin-narrowing/lockdir-deps.t |
Updates lock-directory paths. |
test/blackbox-tests/test-cases/pkg/bin-narrowing/ambient-path-fallback.t |
Updates fallback PATH. |
test/blackbox-tests/test-cases/pkg/absolute-paths-in-sections.t |
Updates absolute package paths. |
test/blackbox-tests/test-cases/describe/describe_location.t |
Updates described executable path. |
src/dune_rules/pkg_rules.mli |
Removes digest API. |
src/dune_rules/pkg_rules.ml |
Implements name-keyed package universes. |
src/dune_rules/pkg_dev_tool.ml |
Adds per-package dev-tool root. |
src/dune_rules/lock_dir.mli |
Removes optional dev-tool loader. |
src/dune_rules/lock_dir.ml |
Removes optional dev-tool loading. |
src/dune_rules/dune_rules.ml |
Removes exported digest API. |
bin/pkg/print_digest.mli |
Deletes command interface. |
bin/pkg/print_digest.ml |
Deletes digest command. |
bin/pkg/group.ml |
Unregisters digest command. |
Review details
- Files reviewed: 64/67 changed files
- Comments generated: 3
- Review effort level: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| let id = Pkg.Id.gen () in | ||
| let write_paths = | ||
| Paths.make pkg_digest package_universe ~relative:Path.Build.relative | ||
| Paths.make package_universe package ~relative:Path.Build.relative |
Collaborator
Author
There was a problem hiding this comment.
This is true, but was likely a pre-existing bug that happened to be ignroed due to the directory changing. We'll have to fix it before this change.
Alizter
force-pushed
the
remove-pkg-digest
branch
from
September 3, 2026 11:16
fa48aa5 to
e2f29ac
Compare
Alizter
marked this pull request as draft
September 3, 2026 11:40
Alizter
force-pushed
the
remove-pkg-digest
branch
2 times, most recently
from
September 7, 2026 15:21
a3d1eb7 to
26aa8ac
Compare
Alizter
force-pushed
the
remove-pkg-digest
branch
from
September 7, 2026 16:26
26aa8ac to
26a3592
Compare
Alizter
added a commit
that referenced
this pull request
Sep 7, 2026
Demonstrate that the compiler isn't currently shared between the main build and a dev tool. This needs to be fixed before #16292.
Alizter
force-pushed
the
remove-pkg-digest
branch
2 times, most recently
from
September 8, 2026 12:25
aa31446 to
7d47cde
Compare
Signed-off-by: Ali Caglayan <alizter@gmail.com>
Alizter
force-pushed
the
remove-pkg-digest
branch
from
September 8, 2026 15:20
7d47cde to
74ee0aa
Compare
This file contains hidden or 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
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.
Description
This PR removes
Pkg_digestas the identity used by package databases and package build paths. Packages are instead keyed by name within an isolated package universe:.pkg/<package>;.dev-tool/<tool>/<package>.On
main,Pkg_digesthashes a package's lock entry and transitive dependency closure. It currently serves three roles:Portable lock-directory selection now chooses at most one version of each package name per universe. Different contexts and dev tools use isolated roots, so digest-qualified paths are no longer needed to distinguish package versions within a universe.
This change intentionally removes the existing opportunistic sharing of identical dev-tool dependencies. That sharing will need to be reconsidered as part of the dev-tool redesign.
The internal
dune pkg print-digestdebugging command is also removed.Correctness requirements
Because stable name-based paths no longer encode lock identity, ordinary Dune dependencies must provide equivalent invalidation for every lock-derived build input. Before this draft is ready, it must cover dependency-exported environment changes and avoid package-root collisions on case-insensitive filesystems.
Existing package tests have been updated for the new layout. Regression coverage for these stable-path properties is still required.
Related Issue and Motivation
This is part of #8652. Removing the separate package-digest representation simplifies the package-management code and enables subsequent refactoring.