Skip to content

Conversation

@Keno
Copy link
Member

@Keno Keno commented May 30, 2025

Our docsystem has the ability to install a local working copy of documenter, complete with manifest/project and local depot to keep things isolated from user packages. I recently made all of this work out of tree, and in #58559 added the same mechanism for the installation of revise in test-revise-*. However, then I realized that the docsystem also has the ability to install a copy of Reivse. Clearly a third copy of all of this is excessive. Instead, unify all the manifests into deps/jlutilities, so that the same Revise project/manifest is used for both doc and test. Similarly, only use one depot at buildroot/deps/jlutilities/depot that both systems share.

Our docsystem has the ability to install a local working copy of documenter,
complete with manifest/project and local depot to keep things isolated from
user packages. I recently made all of this work out of tree, and in #58559
added the same mechanism for the installation of revise in test-revise-*.
However, then I realized that the docsystem also has the ability to
install a copy of Reivse. Clearly a third copy of all of this is excessive.
Instead, unify all the manifests into `deps/jlutilities`, so that the same
Revise project/manifest is used for both doc and test. Similarly, only
use one depot at `buildroot/deps/jlutilities/depot` that both systems
share.
@Keno Keno requested a review from Copilot May 30, 2025 05:47
Copy link

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

This PR centralizes the out-of-tree utility installs for Documenter and Revise into a single deps/jlutilities directory, removing redundant local copies in both the doc build and test workflows.

  • Use a shared deps/jlutilities/depot instead of separate test/deps or doc/deps
  • Point Pkg.activate in tests and docs to the unified jlutilities/revise and jlutilities/documenter projects
  • Update the Makefile and Manifest to reference the new doc project path and bump dependency versions

Reviewed Changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated no comments.

File Description
test/runtests.jl Updated DEPOT_PATH and Pkg.activate calls to use the shared jlutilities Revise setup
doc/make.jl Switched documentation build to the shared jlutilities depot and project manifests
doc/Makefile Adjusted the update-documenter target to use the new jlutilities/documenter project
deps/jlutilities/documenter/Manifest.toml Added Zlib_jll to LibSSH2_jll deps and bumped MozillaCACerts_jll version
Comments suppressed due to low confidence (3)

test/runtests.jl:30

  • The comment about swizzling Revise to the end no longer matches the code (there’s no push or pop after loading). Consider removing or updating this comment to reflect the new workflow.
# Once it's loaded, we swizzle it to the end, to avoid confusing any tests.

doc/make.jl:27

  • This uses buildroot whereas the original doc build used buildrootdoc. Confirm that mixing the two variables is intentional, or switch back to buildrootdoc to isolate the docs depot.
push!(DEPOT_PATH, joinpath(buildroot, "deps", "jlutilities", "depot"))

doc/make.jl:317

  • After instantiating the Revise project you should using Revise (or explicitly import it) before relying on its functionality in the doc build.
Pkg.instantiate()

@Keno Keno merged commit f7d8a58 into master May 30, 2025
6 of 8 checks passed
@Keno Keno deleted the kf/consolidatedeps branch May 30, 2025 19:17
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.

2 participants