BUG - Fix i18n files and compilation for distribution #2042

trallard · 2024-11-18T18:19:47Z

While investigating #2040 I noticed that our current release missed the updated .mo files.
This was a bug I introduced in #1959 when reworking the localisation workflows.

TLD;R—Since we use stb to build the theme, I did not realise that compiling the i18n files had to be done within the stb package (I removed it from the webpack file as this was causing the infinite reload while working on our docs).

Since this was an easy miss, I added our build and inspect job to the pre-release workflow that runs on a chron job to check our build process periodically.

Once we merge this, we can make a small release to patch the current issue.

trallard · 2024-11-18T18:20:10Z

.github/workflows/CI.yml

@@ -75,7 +75,7 @@ jobs:
          # if not we use the default version
          # example substitution: tox run -e compile-assets,i18n-compile,py39-tests
          else
-            python -Im tox run -e compile,i18n-compile,py$(echo ${{ matrix.python-version }} | tr -d .)-tests
+            python -Im tox run -e compile-assets,i18n-compile,py$(echo ${{ matrix.python-version }} | tr -d .)-tests


For whatever reason I had missed this when renaming to compile-assets

trallard · 2024-11-18T18:21:00Z

docs/community/topics/i18n.rst

@@ -82,7 +82,7 @@ file, and visible to localizers. For example:

   {# L10n: Navigation button at the bottom of the page #}
   <button type="button">
-      {{- _("Next page") -}}
+      {{- _('Next page') -}}


Nit as we use single quotes in a lot of our localisable strings

trallard · 2024-11-18T18:23:57Z

tox.ini

@@ -156,7 +156,8 @@ allowlist_externals = bash
 commands =
    # explicitly pass this as a bash command to set PST_VERSION
    bash -c "PST_VERSION=$(pip show pydata-sphinx-theme | grep Version | awk -F': ' '{print $2}') && \
-    pybabel extract . -F babel.cfg -o src/pydata_sphinx_theme/locale/sphinx.pot --project=pydata-sphinx-theme --copyright-holder='PyData developers' --version=$PST_VERSION"
+    pybabel extract . -F babel.cfg -o src/pydata_sphinx_theme/locale/sphinx.pot --keywords='_ __ l_ lazy_gettext' \


this just makes the keywords explicit (added them while debugging, but there is no harm in leaving them)

github-actions · 2024-11-18T18:39:21Z

Coverage report

This PR does not seem to contain any modification to coverable code.

gabalafou

Nice detective work. There are some things I don't understand, so I left some questions inline.

gabalafou · 2024-11-19T15:05:29Z

webpack.config.js

@@ -170,3 +172,21 @@ module.exports = {
    topLevelAwait: true,
  },
 };
+
+module.exports = (env, argv) => {
+  // since STB  isolates the build, we need to compile the translations here for releases


Could you provide a link in this code comment to more info? What does it mean that STB isolates the build?

Under the hood stb uses build for perform stb package (which in turn calls python -m build).
build builds the package in an isolated env to create the source distribution (https://build.pypa.io/en/stable/)

That means, when compiling assets from tox (or even doing npm run build during development or in our CI) it would result in something like

# note I added a simple console print to demonstrate Webpack is building the theme in my-local-dir/pydata-sphinx-theme/src/pydata_sphinx_theme/theme/pydata_sphinx_theme/static Locale will be located in my-local-dir/pydata-sphinx-theme/src/pydata_sphinx_theme/locale

When calling via stb package or python -m build (which is what we do to build the dist) this is instead

Webpack is building the theme in /private/var/folders/6p/062gm41556s5p63x1555cjzw0000gn/T/build-via-sdist-rmc_7ti7/pydata_sphinx_theme-0.16.1.dev0/src/pydata_sphinx_theme/theme/pydata_sphinx_theme/static Locale will be located in /private/var/folders/6p/062gm41556s5p63x1555cjzw0000gn/T/build-via-sdist-rmc_7ti7/pydata_sphinx_theme-0.16.1.dev0/src/pydata_sphinx_theme/locale

so if you compile the translation files first with tox run -e i18n-compile then the compiled .mo will not be included in the distribution files by default (this isolation helps to avoid unwanted files and what not to be added to the distribution packages afterall)

MANIFEST.in

.github/workflows/prerelease.yml

gabalafou · 2024-11-19T16:29:20Z

webpack.config.js


 /*******************************************************************************
 * Paths for various assets (sources and destinations)
 */

 const scriptPath = resolve(__dirname, "src/pydata_sphinx_theme/assets/scripts");
 const staticPath = resolve(__dirname, "src/pydata_sphinx_theme/theme/pydata_sphinx_theme/static");
+const localePath = resolve(__dirname, "src/pydata_sphinx_theme/locale");


I do not understand why this path wasn't always resolve(staticPath, "/locale")

I do not know

My comment wasn't clear. There is a mismatch between the locale path here and the locale path specified in pyproject.toml. It specifies "locale/" within the additional-compiled-static-assets property for sphinx-theme-builder. If you work backwards from the line self.theme_static_path / asset_name in the sphinx-theme-builder source code, then you will see that the path part specified in pyproject.toml ultimately resolves to src/pydata_sphinx_theme/theme/pydata_sphinx_theme/static/locale.

These are completely different locations in the source code directory, so I'm wondering why there is this discrepancy.

Well yes, files in src/pydata_sphinx_theme/assets are considered static assets that need to be compiled (source scss, JS, etc.) while src/pydata_sphinx_theme/theme/pydata_sphinx_theme/static/ are the compiled assets (see for example the structure that stb expects https://sphinx-theme-builder.readthedocs.io/en/latest/filesystem-layout/)

It seems that we are following a similar pattern here where src/pydata_sphinx_theme/locale are the source (.po, .pot ) catalogue files and the compiled .mo files are in src/pydata_sphinx_theme/theme/pydata_sphinx_theme/static/locale (which is what confused me a bit).
Other themes seem to have locales in src/theme/locale (both source and compiled), so we could change that to align with other themes, but for now, I think we should just get the expected / default behaviour working again (which seems to match what stb expects for themes distribution anyway).

gabalafou

I checked that this brought in new translations on a test Sphinx site of my own, so this seems to be working.

However, I do have one concern about pybabel sending output to stderr (see inline comment).

gabalafou · 2024-11-21T08:14:14Z

webpack.config.js

+        return;
+      }
+      if (stderr) {
+        console.error(`stderr: ${stderr}`);


I noticed when I ran stb package that this command is outputting to stderr rather than stdout.

You can check this yourself with:

pybabel compile -d src/pydata_sphinx_theme/locale -D sphinx > stdout.log 2> stderr.log cat stderr.log

However, it seems to compile the .mo files, so I'm not sure what's going on.

Yeah pybabel marks those as INFO logs (they are not real errors as compilation is working as expected and they have a successful exit code) and these redirect to stderr by default (stderr is the default output for internal or debugging messages or a program, like logs and errors vs. the actual output of it). There is no real output for the compile command (compiled files are generated but there is not a single return object from the compile method like a list of files or checksums or what not) so nothing is sent to stdout.
It's not an issue per se and I added these bits so we could at least capture the output, otherwise this gets just obscured by the subprocess calling pybabel.

trallard added 8 commits November 18, 2024 12:35

Fix tox command in CI

42d479a

Compile translations before JS assets

594a040

Update translation strings

4cce361

Add comment to theme-version.html

e3c503e

🌐 Update catalogues

6b95431

Add locale compilation to webpack - production only

a911121

Add comments to webpack

ed1decb

Add build and inspect package to pre-release

5866568

trallard added kind: bug Something isn't working tag: i18N Items related to internationalization labels Nov 18, 2024

Revert unnecessary change

a686722

trallard commented Nov 18, 2024

View reviewed changes

trallard and others added 2 commits November 18, 2024 18:29

🚨 Lint files

0c28478

[pre-commit.ci] Automatic linting and formatting fixes

773239f

gabalafou reviewed Nov 19, 2024

View reviewed changes

trallard and others added 2 commits November 20, 2024 16:33

🔥 Remove unnecessary file

9494167

[pre-commit.ci] Automatic linting and formatting fixes

0f3aa5c

gabalafou approved these changes Nov 21, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

BUG - Fix i18n files and compilation for distribution #2042

BUG - Fix i18n files and compilation for distribution #2042

trallard commented Nov 18, 2024

trallard Nov 18, 2024

trallard Nov 18, 2024

trallard Nov 18, 2024

github-actions bot commented Nov 18, 2024

gabalafou left a comment

gabalafou Nov 19, 2024

trallard Nov 20, 2024

gabalafou Nov 19, 2024

trallard Nov 19, 2024

gabalafou Nov 19, 2024

trallard Nov 20, 2024 •

edited

Loading

gabalafou left a comment •

edited

Loading

gabalafou Nov 21, 2024

trallard Nov 21, 2024 •

edited

Loading

BUG - Fix i18n files and compilation for distribution #2042

Are you sure you want to change the base?

BUG - Fix i18n files and compilation for distribution #2042

Conversation

trallard commented Nov 18, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

github-actions bot commented Nov 18, 2024

Coverage report

gabalafou left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

trallard Nov 20, 2024 • edited Loading

Choose a reason for hiding this comment

gabalafou left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

trallard Nov 21, 2024 • edited Loading

Choose a reason for hiding this comment

trallard Nov 20, 2024 •

edited

Loading

gabalafou left a comment •

edited

Loading

trallard Nov 21, 2024 •

edited

Loading