Bazel backdating#3223
Conversation
This moves the remaining two tools into Bazel: `i18n-report` and
`mdbook` itself. They are still installed with
cargo xtask install-tools
to make this less disruptive. This will go away as we start using the
`mdbook` rule in Bazel to build the many books.
This ensures that future use of the `mdbook` rule will use `mdbook` version 5.0.3 to match #3207.
bazel: add repository rule to generate per-renderer book.toml When `mdbook` is running, it renders output for each of the `output` keys. This happens sequentially and we need different output for different cases: | Renderer | English | Translations | Use Case | | ------------ | ------- | ------------ | --------------------------- | | `html` | ✓ | ✓ | Website HTML output | | `pandoc` | ✓ | ✓ | PDF generation | | `exerciser` | ✓ | ✗ | Exercise starter code | | `xgettext` | ✓ | ✗ | `.pot` file for translators | | `linkcheck2` | ✓ | ✗ | Link validation in CI | This little repository rule is a step towards this: it lets us trim the `book.toml` file down to the renderers we want to each case. We can thus keep maintaining the overall set of renderers in the checked in `book.toml` file, and then select a subset of the settings at build time.
04d879b to
a5bd5ae
Compare
|
From this infrastructure, it's a small step to make a The complexity is still there... but it's now suddenly in a form that allows us to reproduce things locally that we could not really do easily today. |
| # Patch language-specific settings. | ||
| book["book"]["language"] = lang | ||
| book["output"]["html"]["site-url"] = "/comprehensive-rust/{}/".format(lang) | ||
| book["output"]["html"].pop("redirect", None) |
There was a problem hiding this comment.
is that removing the redirects completely? In my understanding this is necessary to be kept at the state at which it was backdated.
There was a problem hiding this comment.
Hey @michael-kerscher, it removes them just like we do today in build.sh:
comprehensive-rust/.github/workflows/build.sh
Lines 36 to 39 in 565f4c0
But as the comment says, we might not need to do this?
My gut feeling would be that we should not expect translators to know about or maintain redirects. But I guess they get them from free via updates to main, so yeah, we could stop clearing them.
That would be a change in functionality and so far I've only aimed at re-creating our current workflow, but in Bazel.
| book["output"]["html"].pop("redirect", None) | ||
|
|
||
| book["output"].pop("linkcheck", None) | ||
| book["output"].pop("linkcheck2", None) |
There was a problem hiding this comment.
When we are modifying the entire file alrready, should we care about the linkcheck2 results? #2996 - this would have been avoidable or at least we could have fixed the translations if linkcheck2 would have been enabled
| load("@toml.bzl", "toml") | ||
| load("@yaml.bzl", "yaml") | ||
|
|
||
| def _book_sources_repo_impl(repository_ctx): |
There was a problem hiding this comment.
this function has a high mental load. I would prefer to split that into multiple rules. Maybe we could even separate some build rules and depend on them here. e.g. the archive.tar.gz creation, the config (book.toml) modification and reassembly
There was a problem hiding this comment.
Yeah, I agree that there is a lot going on! I have oscillated between having things more split and more centralized... the version here is more centralized.
I found that the split versions were complicated because of another reason: they would have implicit dependencies between different files and rules. So one rule would register a @backdated_sources//:da_srcs and another rule in another file would use this... that's (also) not nice.
|
Hey folks, just a note that this PR was only for demonstration so you could get a feeling for what I've been experimenting with over the last few months. |
Hey @qwandor and @djmitche, this is a WIP of where I'm taking the project. On this branch,
produces a Danish book!
I've been spinning off small bits and pieces of this WIP commit over the last few weeks in the PRs you've reviewed.