Skip to content

Call Pkg.resolve() before hashing Manifest #519

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Sep 11, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 4 additions & 3 deletions .github/workflows/preview.yml
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,9 @@ jobs:
with:
version: '1.10'

# Note: needs resolve() to fix #518
- name: Instantiate Julia environment
run: julia --project=. -e 'using Pkg; Pkg.instantiate()'
run: julia --project=. -e 'using Pkg; Pkg.instantiate(); Pkg.resolve()'
Copy link
Member

@yebai yebai Sep 10, 2024

Choose a reason for hiding this comment

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

On the downside, updating the Manifest.toml file for every PR might cause issues for docs/tutorials that are not changed by PRs. On the positive side, such a policy would ensure tutorials and docs are more actively maintained for new Julia package versions.

Is this a desirable policy that we wish to enforce?

cc @devmotion @mhauru

Copy link
Member Author

Choose a reason for hiding this comment

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

Hmm, Pkg.resolve() doesn't update packages though; that would be Pkg.update().

I'm sure I'm missing some weird edge case of an edge case 😄 but the only situation I could think of where resolve() actually changes anything is the already edge case where CI runs with a Julia version different from that in the Manifest.

Copy link
Member Author

Choose a reason for hiding this comment

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

(Or if the Manifest is inconsistent with Project.toml, but we have a check for that 🙂)

Copy link
Member

@yebai yebai Sep 11, 2024

Choose a reason for hiding this comment

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

@penelopeysm, you might want to ask in the Julia language workspace to clarify the difference between resolve and update. The docstrings are not super clear in this case:

(@v1.10) pkg> ?resolve
  resolve

  Resolve the project i.e. run package resolution and update the Manifest. This is useful in case the dependencies of
  developed packages have changed causing the current Manifest to be out of sync.

(@v1.10) pkg> ?update
  [up|update] [-p|--project]  [opts] pkg[=uuid] [@version] ...
  [up|update] [-m|--manifest] [opts] pkg[=uuid] [@version] ...

  opts: --major | --minor | --patch | --fixed
        --preserve=<all/direct/none>

  Update pkg within the constraints of the indicated version specifications. These specifications are of the form @1,
  @1.2 or @1.2.3, allowing any version with a prefix that matches, or ranges thereof, such as @1.2-3.4.5. In --project
  mode, package specifications only match project packages, while in --manifest mode they match any manifest package.
  Bound level options force the following packages to be upgraded only within the current major, minor, patch version;
  if the --fixed upgrade level is given, then the following packages will not be upgraded at all.

  After any package updates the project will be precompiled. For more information see pkg> ?precompile.

Copy link
Member Author

@penelopeysm penelopeysm Sep 11, 2024

Choose a reason for hiding this comment

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

Hmm, I found https://discourse.julialang.org/t/what-is-the-difference-between-pkg-resolve-and-pkg-instantiate/77643/2:

resolve will first check that the Manifest.toml is consistent with the Project.tomls of the current project and all its dependencies, updating it if necessary, then instantiate.

update will use the newest possible dependencies (subject to compatibility constraints)

But actually even better, I tested it locally :)

  • If Project.toml has a compat entry of "Foo = 0.5" and Manifest has [email protected], resolve() will fail with an error. We shouldn't run into this case because in this repository there only has one compat entry for Turing itself, and we already check that the Turing version is consistent

  • If Project.toml has no compat entry and Manifest has [email protected], resolve() doesn't do anything, even if there's a newer version available. This means that for all non-Turing deps the version in the Manifest will always be the one used.


- name: Set up Quarto
uses: quarto-dev/quarto-actions/setup@v2
Expand All @@ -33,7 +34,7 @@ jobs:
uses: actions/cache/restore@v4
with:
path: |
_freeze/
./_freeze/
key: ${{ runner.os }}-${{ hashFiles('**/Manifest.toml') }}

- name: Render Quarto site
Expand All @@ -44,7 +45,7 @@ jobs:
uses: actions/cache/save@v4
with:
path: |
_freeze/
./_freeze/
key: ${{ runner.os }}-${{ hashFiles('**/Manifest.toml') }}

- name: Deploy to GitHub Pages
Expand Down
7 changes: 4 additions & 3 deletions .github/workflows/publish.yml
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,9 @@ jobs:
with:
version: '1.10'

# Note: needs resolve() to fix #518
- name: Instantiate Julia environment
run: julia --project=. -e 'using Pkg; Pkg.instantiate()'
run: julia --project=. -e 'using Pkg; Pkg.instantiate(); Pkg.resolve()'

- name: Set up Quarto
uses: quarto-dev/quarto-actions/setup@v2
Expand All @@ -34,7 +35,7 @@ jobs:
uses: actions/cache/restore@v4
with:
path: |
_freeze/
./_freeze/
key: ${{ runner.os }}-${{ hashFiles('**/Manifest.toml') }}

- name: Extract version from _quarto.yml
Expand Down Expand Up @@ -75,7 +76,7 @@ jobs:
uses: actions/cache/save@v4
with:
path: |
_freeze/
./_freeze/
key: ${{ runner.os }}-${{ hashFiles('**/Manifest.toml') }}

- name: Fetch search_original.json from main site
Expand Down
48 changes: 48 additions & 0 deletions .github/workflows/resolve_manifest.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
# This action runs Pkg.instantiate() and Pkg.resolve() every time the master
# branch is pushed to. If this leads to a change in the Manifest.toml file, it
# will open a PR to update the Manifest.toml file. This ensures that the
# contents of the Manifest in the repository are consistent with the contents
# of the Manifest used by the CI system (i.e. during the actual docs
# generation).
#
# See https://github.com/TuringLang/docs/issues/518 for motivation.

name: Resolve Manifest
on:
push:
branches:
- master
workflow_dispatch:

jobs:
check-version:
runs-on: ubuntu-latest

permissions:
contents: write
pull-requests: write

env:
# Disable precompilation as it takes a long time and is not needed for this workflow
JULIA_PKG_PRECOMPILE_AUTO: 0

steps:
- name: Checkout
uses: actions/checkout@v4

- name: Setup Julia
uses: julia-actions/setup-julia@v2

- name: Instantiate and resolve
run: |
julia -e 'using Pkg; Pkg.instantiate(); Pkg.resolve()'

- name: Open PR
id: create_pr
uses: peter-evans/create-pull-request@v6
with:
branch: resolve-manifest
add-paths: Manifest.toml
commit-message: "Update Manifest.toml"
body: "This PR is automatically generated by the `resolve_manifest.yml` GitHub Action."
title: "Update Manifest.toml to match CI environment"
4 changes: 2 additions & 2 deletions Manifest.toml
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
# This file is machine-generated - editing it directly is not advised

julia_version = "1.10.4"
julia_version = "1.10.5"
manifest_format = "2.0"
project_hash = "e0661388214f9e03749b3fccc86939dfd3853246"

Expand Down Expand Up @@ -3328,7 +3328,7 @@ version = "0.15.2+0"
[[deps.libblastrampoline_jll]]
deps = ["Artifacts", "Libdl"]
uuid = "8e850b90-86db-534c-a0d3-1478176c7d93"
version = "5.8.0+1"
version = "5.11.0+0"

[[deps.libdecor_jll]]
deps = ["Artifacts", "Dbus_jll", "JLLWrappers", "Libdl", "Libglvnd_jll", "Pango_jll", "Wayland_jll", "xkbcommon_jll"]
Expand Down
Loading