Skip to content
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

LANGUAGE environment variable inconsistently affects output of objects.inv #9778

Closed
lamby opened this issue Oct 26, 2021 · 19 comments · Fixed by #10949
Closed

LANGUAGE environment variable inconsistently affects output of objects.inv #9778

lamby opened this issue Oct 26, 2021 · 19 comments · Fixed by #10949

Comments

@lamby
Copy link
Contributor

lamby commented Oct 26, 2021

Describe the bug

Hi,

Not entirely sure where the bug is here, but it seems like there is something up with language handling and generating the objects.inv file. The context to all this is that I'm working on Reproducible Builds, and some update has suddenly rendered a lot of packages that use Sphinx unreproducible - that is, generating different output regardless of the surrounding environment.

In particular, I discovered this by comparing two builds: the first with LANGUAGE="en_GB:en" and the second with LANGUAGE="et_EE:et" environment variable. What happens is that all of the documentation is identical except that a single entry in the objects.inv file appears to be translated. This is despite the output including the following logging message in both builds:

dumping search index in English (code: en)... done
dumping object inventory... done

(NB. code: en here in both builds)

Decoding this zlib-encoded file, I can see that the difference is a translation one:

# Sphinx inventory version 2
# Project: OpenDrop
# Version: 
# The remainder of this file is compressed using zlib.

developers/index std:doc -1 developers/index.html Developer notes
-genindex std:label -1 genindex.html Index
+genindex std:label -1 genindex.html Indeks
getting_started/index std:doc -1 getting_started/index.html Getting Started
index std:doc -1 index.html Overview
modindex std:label -1 py-modindex.html Module Index
py-modindex std:label -1 py-modindex.html Python Module Index
search std:label -1 search.html Search Page
usage/conan std:doc -1 usage/conan.html Contact Angle
usage/ift std:doc -1 usage/ift.html Interfacial Tension

... and, indeed, "Indeks" is in the Estonian .po file:

#: sphinx/builders/latex/__init__.py:194 sphinx/domains/std.py:604
#: sphinx/templates/latex/latex.tex_t:97
#: sphinx/themes/basic/genindex-single.html:30
#: sphinx/themes/basic/genindex-single.html:55
#: sphinx/themes/basic/genindex-split.html:11
#: sphinx/themes/basic/genindex-split.html:14
#: sphinx/themes/basic/genindex.html:11 sphinx/themes/basic/genindex.html:34
#: sphix/themes/basic/genindex.html:67 sphinx/themes/basic/layout.html:147
#: sphinx/writers/texinfo.py:498
msgid "Index"
msgstr "Indeks"

This is just confusing though because why isn't "Module Index" translated as well? "Mooduli indeks" is also there in the Estonian .po:

#: sphinx/domains/std.py:605
msgid "Module Index"
msgstr "Mooduli indeks"

... so I suppose the bug here is either that "Index" gets translated whilst "Module Index" is not... or the other way around. This why I use "inconsistency" in the title of this issue.

Playing around with the code, I am pretty certain that the translated entry is in sphinx/domains/std.py — could it be that the data in initial_data is being prematurely translated? Either way, though, I was expecting that the documentation and entries are all identical, regardless of the LANGUAGE environment variable.

How to Reproduce

Compare the builds between exporting the LANGUAGE="en_GB:en and LANGUAGE="et_EE:et" environment variable, specifically the objects.inv file.

Expected behavior

No response

Your project

I'm using opendrop, but this will occur with any package

Screenshots

No response

OS

Linux

Python version

3.9

Sphinx version

4.2.0

Sphinx extensions

No response

Extra tools

No response

Additional context

No response

@lamby lamby added the type:bug label Oct 26, 2021
@lamby
Copy link
Contributor Author

lamby commented Oct 27, 2021

  • This is likely more than just the LANGUAGE environment variable. It will be the same as gettext, etc. (eg. LANGUAGE, LC_ALL, LC_MESSAGES and LANG)

  • Oh, and just to braindump a nasty local hack:

diff --git a/sphinx/locale/__init__.py b/sphinx/locale/__init__.py
index 8fc6c1519..8bd7d0314 100644
--- a/sphinx/locale/__init__.py
+++ b/sphinx/locale/__init__.py
@@ -10,6 +10,7 @@
 
 import gettext
 import locale
+import os
 from collections import UserString, defaultdict
 from gettext import NullTranslations
 from typing import Any, Callable, Dict, Iterable, List, Optional, Tuple, Union
@@ -129,6 +130,10 @@ def init(locale_dirs: List[Optional[str]], language: Optional[str],
     else:
         languages = None
 
+        # Don't fallback to consulting LANG, etc. if we want a reproducible build.
+        if 'SOURCE_DATE_EPOCH' in os.environ:
+            languages = []
+
     # loading
     for dir_ in locale_dirs:
         try:

@lamby lamby changed the title LANGUAGE environement variable inconsistently affects output of objects.inv LANGUAGE environment variable inconsistently affects output of objects.inv Oct 29, 2021
@jayaddison
Copy link
Contributor

Does the contents of the built objects.inv differ after temporarily disabling the use of lazy proxy-based message translation (for example, removing this if branch)?

From investigation here, it does seem like there is some kind of catalog load-timing issue going on, but I'm yet to figure out where or why it occurs.

@AA-Turner
Copy link
Member

Improvements to sphinx.locale in general are welcome, up to and including a full redesign.

A

@jayaddison
Copy link
Contributor

Thank you. Without understanding much of the localization process here yet, I'm optimistic/hopeful that there might be an opportunity for a light-touch fix.

I've opened draft pull request #10882 with code removal that resolves the issue when I build objects.inv locally -- but I'm not confident that it's free of negative side-effects, and I don't understand what/where the root cause with resource lazy-loading was occurring. Any help appreciated (including "that's not going to work", if it's not a workable approach for some reason).

@jayaddison
Copy link
Contributor

I think the approach in #10882 is probably basically incorrect: for whatever reason, it's not translating any of the dispname values in the objects.inv file -- in other words, it is "consistent" but only by skipping localization.

After instrumenting the _TranslationProxy a little, it's possible to identify what initiates the early-translation of the Index / Indeks names: it seems that the proxy is called to translate those names during a deepcopy operation called from the StandardDomain's constructor.

That results in numerous __getattr__ calls to the nodes in the initial_data datastructure, and for untranslated strings accessed by that process, the translation proxy calls through to gettext (the callthrough is implemented in the __str__ method).

@jayaddison
Copy link
Contributor

(some more internal monologue exposition: as I understand it, the objects.inv file is used to publish content for intersphinx cross-site linking purposes. each site would likely prefer to publish their objects in their own native translated language (although arguably in some cases it could be nice if the remote site were able to retrieve their own localized display names for objects in the remote site), and so allowing a default-language inventory doesn't seem sensible)

@jayaddison
Copy link
Contributor

jayaddison commented Sep 29, 2022

Ok, so: the search index dump and the object index dump retrieve their language settings from different places; the fact that those two log lines appear next to each other in the output doesn't imply a connection.

dumping search index in English (code: en)... done  # yep, this is language=en
dumping object inventory... done                    # however, this is language=et

It does appear to me that all elements with lang=et translations are being localized in the inventory (objects.inv).

So: is there a bug here?

@AA-Turner AA-Turner added this to the some future version milestone Sep 29, 2022
@jayaddison
Copy link
Contributor

@AA-Turner is the fact that you tagged #788 recently perhaps no coincidence? :)

My current best theory for what has happened here is that at some point -- perhaps between versions 4.3.0 and 4.4.0 of sphinx, based on my guesses regarding build failures in Debian's diffoscope, a fix was applied where the objects.inv build process correctly began detecting the environment LANGUAGE and producing partially-localized results (where translations are available).

That caused reproducible build check failures in Debian -- but I think that that's a symptom of a deeper problem, which is that Debian has been building a single copy of the documentation with each build (in a single language selected by the build-time environment).

Comparing with what happens for infopages/manpages, multiple languages are bundled into the same package at build-time. I think that makes sense, because it means that every system (regardless of the languages that their current staff understand) receives the same content, and there can't be any sleight-of-hand regarding differing build content as could happen if per-language builds were performed (in other words: it's inline with both good user/system internationalization practices and also the goals of the reproducible builds project, I think).

That does roll around to the question of how to perform multi-language builds in sphinx, and that's what led me to take a look at #788 (support for which would also need to filter down to each affected package - either in Debian patches and/or upstream).

cc @lamby

@AA-Turner
Copy link
Member

It is a coincidence! (I was cleaning up all issues without a milestone). I'll read through your comments here later in the evening, thank you for looking into this issue.

A

@lamby
Copy link
Contributor Author

lamby commented Sep 30, 2022

Perhaps between versions 4.3.0 and 4.4.0 of sphinx, based on my guesses regarding build failures in Debian's diffoscope, a fix was applied where the objects.inv build process correctly began detecting the environment LANGUAGE and producing partially-localized results […]

This sounds like an entirely plausible chain of events, although I can't recall the specifics or easily dig up any evidence from quickly going through the bugs filed against the sphinx package in Debian. That said, I am juggling 10+ quite similar long-running bugs in various toolchains, so I don't tend to retain details! I reply to this bit of your message specifically because: would pinning down the chronology and changeset that exposed this issue actually help in solving the issue at hand? Given what you go on to say, I'm not entirely sure that's true. :)

Anyway, I suppose my personal/reproducible view is that it would be a shame if implementing #788 (a good thing!) would block on resolving this issue, even if it was something of a stop-gap solution. Ironically, the success of Sphinx means that quite a lot of Debian packages are currently unreproducible because of this, so I would be quite minded to see a way of just generating English.

Thank you all for your recent activity on this issue.

ps. Regarding the suggestion to remove an if branch, would you still like me to try that?

@jayaddison
Copy link
Contributor

Thanks @lamby!

Regarding the if branch - no, I don't think it's worth exploring further. It disabled objects.inv localization, which produced repeatable results, but not for acceptable reasons.

And regarding #788: distributing multi-locale documentation seems ideal, but I agree it seems hard to predict when it might arrive, and so no, I don't think that should block reproducible build progress.

I believe I now understand your quick-and-dirty approach. However: I don't love the idea of code that branches based on the existence of SOURCE_DATE_EPOCH: it has a bit of a "system acts differently when running under test" smell. Maybe OK in a limited number of cases, but in a widely-used toolchain it feels risky.

Idea / question: would it be possible to configure the the SPHINXOPTS environment variable to define a single fixed language when building packages tagged sphinxdoc_translations?

@jayaddison
Copy link
Contributor

From a sample size of one test build using python-psutil with dpkg-buildpackage locally: it does seem that exporting SPHINXOPTS="-D LANGUAGE='en_US.UTF-8'" in debian/rules selectively overrode the documentation build language for sphinx (reproducible objects.inv) without affecting the rest of the build process.

I'm not sure about all the tradeoffs involved with that approach, or whether it's the correct place to put an environment variable like that, but it did appear to work around the issue. I'll cross-post this to the relevant bug.

@lamby
Copy link
Contributor Author

lamby commented Oct 3, 2022

I believe I now understand your #9778 (comment). However: I don't love the idea of code that branches based on the existence of SOURCE_DATE_EPOCH: it has a bit of a "system acts differently when running under test" smell. Maybe OK in a limited number of cases, but in a widely-used toolchain it feels risky.

I appreciate your perspective, and this tension is something we sometimes encounter when changing programs' behaviour based on the contents of SOURCE_DATE_EPOCH. However, this probably isn't the place to (badly!) repeat all the arguments in favour of doing so regardless of the tradeoffs you outline, so I'll park that for now.

would it be possible to configure the the SPHINXOPTS environment variable to define a single fixed language when building packages tagged sphinxdoc_translations?

This would work fine for individual packages, but when I consider how Debian might apply this as a default to all packages in one fell swoop, we quickly run into a number of difficulties. To start with there is no canonical place where a value of SPHINXOPTS might be exported (packages are not required to use the dh_sphinx utility, for example). And there are then many packages that export their own version of SPHINXOPTS for various reasons without regard to any pre-existing value.

@jayaddison
Copy link
Contributor

Potential fixup attempt available for review in #10949 - this does follow the approach of disabling localization when the SOURCE_DATE_EPOCH environment variable is set.

@jayaddison
Copy link
Contributor

@AA-Turner @lamby after re-reading #10949 post-merge, I'm starting to have second thoughts about that approach.

Deactivating all localisation (not only objects.inv) during reproducible builds seems like an approach that could cause significant annoyance for anyone happily using reproducible-and-localised software that contains sphinx-generated documentation.

Roughly speaking: I think I became too focused on fixing the immediate problem without considering all the implications (especially as the nature of the fix itself changed).

I don't know for certain whether to revert the change yet, but I have prepared a branch to do that while thinking about it.

It seems like support for multiple-locale display-names in the objects.inv file could be part of the requirements for a more-reliable, localisation-enabled resolution.

@jayaddison
Copy link
Contributor

jayaddison commented Apr 8, 2023

Describe the bug

...

In particular, I discovered this by comparing two builds: the first with LANGUAGE="en_GB:en" and the second with LANGUAGE="et_EE:et" environment variable. What happens is that all of the documentation is identical except that a single entry in the objects.inv file appears to be translated. This is despite the output including the following logging message in both builds:

dumping search index in English (code: en)... done
dumping object inventory... done

(NB. code: en here in both builds)

Could it be that the lazy-loading was the only problem, and that disabling localization itself isn't necessary to achieve reproducible results?

(that would seem ideal, if I understand correctly: no reduction-in-localisation of the objects.inv file -- it was intended to be produced using an en language-only configuration anyway -- and no reduction-in-localisation of the documentation project-under-build)

@jayaddison
Copy link
Contributor

I think I should go back and reconfirm the original opendrop-3.3.1 documentation build issue using both en_GB and et_EE locales before proceeding further with this.

@jayaddison
Copy link
Contributor

Three commits in particular to test:

  • 4659fc2
    • this is the commit before the original fix was merged, and the problem should be replicable here
    • so far, I haven't managed to replicate the problem from this commit, and that confuses me
  • f82c3c9
    • this is the commit containing the original fix
    • the problem should not be replicable from this commit -- but the solution (disabling all localisation) may have been too heavyweight
  • eded1ff
    • this commit is an 'improved' (really a reduced) version of the fix
    • the problem should not be replicable from this commit, either
    • this commit retains localisation support when SOURCE_DATE_EPOCH is configured; that should be preferable

Checking for the problem should be possible by building the Sphinx documentation for opendrop and then comparing the contents of the build directories. In particular, the objects.inv file is expected to contain differences when the problem occurs.

Note: I'm also finding that a file named html/.doctrees/environment.pickle differs between some of the builds. That is likely a separate, unrelated issue however.

@jayaddison
Copy link
Contributor

jayaddison commented Apr 9, 2023

🤦

After all that, I did what I probably should've done to begin with: confirm whether the issue still exists, and if not, bisect the place where it disappeared.

What I found as a result of doing that today is that the issue continued to appear up-to-and-including sphinx v4.5.0, and it was fixed in the v5.0.0 release.

From bisecting those commits and using opendrop-3.3.1 to rebuild documentation each time -- and confirming whether objects.inv varies between en and et language builds: it turns out that commit e4e58a4 contains the fix for this bug.

For reference, the approximate build process used during each git bisect step (this was applied manually) was:

sphinx $ git bisect <result:good|bad>  # reporting the result of each bisect iteration
sphinx $ pip install -e .  # (re)install sphinx for the current bisect iteraction
sphinx $ cd ../opendrop-3.3.1
opendrop-3.3.1 $ SOURCE_DATE_EPOCH=0 LANGUAGE="et_EE:et" PYTHONPATH=. sphinx-build docs build && mv build build.et
opendrop-3.3.1 $ SOURCE_DATE_EPOCH=0 LANGUAGE="en_GB:en" PYTHONPATH=. sphinx-build docs build && mv build build.en
opendrop-3.3.1 $ diff -r build.en build.et  # determining the result of the current bisect iteration 
opendrop-3.3.1 $ cd -  # return to the sphinx project directory to continue the bisect process

@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 11, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
3 participants