Skip to content

refactor(pkg): remove pkg digests - #16292

Draft
Alizter wants to merge 3 commits into
ocaml:mainfrom
Alizter:remove-pkg-digest
Draft

refactor(pkg): remove pkg digests#16292
Alizter wants to merge 3 commits into
ocaml:mainfrom
Alizter:remove-pkg-digest

Conversation

@Alizter

@Alizter Alizter commented Sep 1, 2026 •
edited
Loading

Copy link
Copy Markdown
Collaborator

Description

This PR removes Pkg_digest as the identity used by package databases and package build paths. Packages are instead keyed by name within an isolated package universe:

  • project dependencies use .pkg/<package>;
  • dev-tool packages use .dev-tool/<tool>/<package>.

On main, Pkg_digest hashes a package's lock entry and transitive dependency closure. It currently serves three roles:

  1. distinguishing package solutions in physical build paths;
  2. conservatively invalidating package builds when lock-derived identity changes;
  3. sharing identical package closures between project and dev-tool universes.

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-digest debugging 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.

@Alizter Alizter added no changelog package management Dune's package management — `(pkg)` stanza, lockdirs, `dune pkg` commands labels Sep 1, 2026
@Alizter
Alizter marked this pull request as ready for review September 2, 2026 15:33
@Alizter
Alizter requested a balanced review from Copilot September 2, 2026 18:51
Copilot AI reviewed Sep 2, 2026
View reviewed changes

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟡 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_digest and dune 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.

Comment thread src/dune_rules/pkg_rules.ml
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

@Alizter Alizter Sep 2, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

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.

Comment thread src/dune_rules/pkg_rules.ml
Comment thread src/dune_rules/pkg_rules.ml
@Alizter
Alizter marked this pull request as draft September 3, 2026 11:40
@Alizter
Alizter force-pushed the remove-pkg-digest branch 2 times, most recently from a3d1eb7 to 26aa8ac Compare September 7, 2026 15:21
@Alizter Alizter mentioned this pull request Sep 7, 2026
Merged
Alizter added a commit that referenced this pull request Sep 7, 2026
@Alizter
test(pkg): cover stale dev-tool compiler dependencies (#16350)
f4e8a99 
Demonstrate that the compiler isn't currently shared between the main
build and a dev tool. This needs to be fixed before #16292.
@Alizter Alizter mentioned this pull request Sep 7, 2026
Closed
@Alizter
Alizter force-pushed the remove-pkg-digest branch 3 times, most recently from 7d47cde to 74ee0aa Compare September 8, 2026 15:20
@Alizter
test(pkg): cover changes to dependency-exported environments
291c48a 
Record stale expanded exports in both the workspace and shared caches.
Also check same-version lockfile edits and unchanged builds in preparation
for removing package digests.

Signed-off-by: Ali Caglayan <alizter@gmail.com>
@Alizter
fix(pkg): track dependency-exported environment variables
18d6ed1 
Add environment dependencies for the variables exported by the transitive
package dependency closure. This invalidates package rules and cached
artifacts when the effective exported values change, without depending on
unrelated ambient variables or transitive package digests.

Signed-off-by: Ali Caglayan <alizter@gmail.com>
@Alizter
refactor(pkg): remove package digests
c3453ec 
Signed-off-by: Ali Caglayan <alizter@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Copilot code review Copilot
Copilot review effort, defaults to Balanced
Applies to this pull request for everyone.Learn more about Copilot code review.
Copilot left review comments

Assignees

No one assigned

Labels

no changelog package management Dune's package management — `(pkg)` stanza, lockdirs, `dune pkg` commands

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@Alizter