Github CI and Cargo.lock get out of sync

[michael-kerscher]

The Github CI workflows use specific versions for mdbook tools that are different from what developers are using. E.g. mdbook is currently using 0.4.37:

comprehensive-rust/.github/workflows/install-mdbook/action.yml

Line 11 in 4218c95

run: cargo install mdbook --locked --version 0.4.37

but Cargo.lock would use 0.4.44 - this is regularly bumped via dependabot like in 010bd29

comprehensive-rust/Cargo.lock

Lines 1640 to 1641 in 4218c95

	name = "mdbook"
	version = "0.4.44"

and the developers are even instructed to use a completely different version that might not be one of those as it is not even locked (other tools are locked and might not be compatible with the "random" (most up-to-date) version of mdbook

comprehensive-rust/README.md

Lines 74 to 80 in 4218c95

	cargo install mdbook
	cargo install --locked mdbook-svgbob
	cargo install --locked mdbook-i18n-helpers
	cargo install --locked i18n-report
	cargo install --locked mdbook-linkcheck
	cargo install --locked --path mdbook-exerciser
	cargo install --locked --path mdbook-course

We should use the same versions for developers and what the CI uses.

This is especially interesting for people developing javascript/theme code as mdbook generates/copies lots of files into the target folder if they are not overriden by us.

For example our book.js is copied but https://google.github.io/comprehensive-rust/css/general.css is not in our repository not existing general.css

Two options (second is my prefered proposal)

• Keep manual overrides for the Github CI: Dev and CI are not in sync and might cause issues when someone changes the javascript code or relies on its functionality (when using mdbook serve - as in Incorrect behavior with mdbook serve compared to published version. #2588
• Fix the instruction to install the --locked version of mdbook and use this version in the Github CI as well. Caveat: Dependabot updates need to be reviewed carefully and as we override some things (e.g. index.hbs) we need to carefully track updates to the official files with each update.
  • To make this update process reliable, write tests for the things we rely on in tests/src so things like missing theme buttons or search buttons are visible immediately.

[djmitche]

I think it's GitHub, not Gitlab :)

The second option seems good. We could also consider

• Disable dependabot updates of mdbook and friends
• Copy all files from the mdbook theme, so that nothing is supplied. This might at least break less often.

[michael-kerscher]

Yes, Github ^^ - I'm going to fix all occurrences above (and in the title)

Copying all files could lead to more reliable builds but could also fail in other spectacular ways and additionally comes with more update toil as we would need to regenerate everything on each update, otherwise we have an unknown combination of code.
I would like to try to keep updating toil at a minimum.

[michael-kerscher]

The approach I describe above does not work that way. https://doc.rust-lang.org/cargo/commands/cargo-install.html#dealing-with-the-lockfile:

The --locked flag can be used to force Cargo to use the packaged Cargo.lock file if it is available.

While this is partly what we want, we don't specify versions that need to be installed. The Cargo.lock file in this repository is irrelevant in choosing the installed versions of mdbook and the mdbook plugins.

Another way forward can be an install script that is used by both the CI and the developers to install exactly the versions that we want and have tested. This will replace the manual versions in action.yml and can also replace the developer instructions

[michael-kerscher]

@mgeisler: While looking at this issue I noticed these warnings from e.g. https://github.com/google/comprehensive-rust/actions/runs/13132471845/job/36641626915 - the step "Building bn translation as of 2024-03-03T20:37:57+05:30"

  2025-02-04 10:11:46 [INFO] (mdbook::book): Book building has started
  Warning: The gettext preprocessor was built against mdbook version 0.4.40, but we're being called from version 0.4.37
  2025-02-04 10:11:47 [INFO] (mdbook::book): Running the exerciser backend
  2025-02-04 10:11:47 [INFO] (mdbook::renderer): Invoking the "exerciser" renderer
  Warning: The gettext preprocessor was built against mdbook version 0.4.40, but we're being called from version 0.4.37
  2025-02-04 10:11:47 [INFO] (mdbook::book): Running the html backend
  Warning: The gettext preprocessor was built against mdbook version 0.4.40, but we're being called from version 0.4.37
  2025-02-04 10:11:49 [INFO] (mdbook::book): Running the linkcheck backend
  2025-02-04 10:11:49 [INFO] (mdbook::renderer): Invoking the "linkcheck" renderer
  2025-02-04 10:11:49 [WARN] (mdbook::renderer): The command `mdbook-linkcheck` for backend `linkcheck` was not found, but was marked as optional.
  Warning: The gettext preprocessor was built against mdbook version 0.4.40, but we're being called from version 0.4.37

We need to find a good and reliable way to sync the versions of all required versions.

[michael-kerscher]

Regarding the comment above this one this is coming from mdbook-i18n-helpers//i18n-helpers/mdbook-gettext that has its own lockfile and mdbook version that is locks to mdbook-i18n-helpers/Cargo.lock (tag: 0.3.3) that is different (if we don't take any precaution) from the mdbook version installed in this repo (via install-mdbook.sh - see PR above)

Syncing the versions is currently done manually and in different places

• Some versions are specified via https://github.com/google/comprehensive-rust/blob/main/Cargo.lock
  • mdbook-exerciser
  • mdbook-course
• Some versions are specified via the install-mdbook.sh script

[mgeisler]

Had this been improved with @egithinji's work on cargo xtask install-tools? This now uses the --locked flag and specifies precise versions of every tool.