yosys docs vs. downstream distro-package maintainability

[gsomlo]

Link to page

http://mirror.ini.cmu.edu/0003-fedora-yosys-doc-offline-patch.patch

Build number

a66e94c ("Docs: Switch to furo-ys")

Issue

Hi,

I maintain the yosys package in Fedora, and I'm running into a bit of trouble building the documentation portion of the package.

As you may be aware, distro-packaging requires the assumption of everything being available for an offline build at compile time (i.e., fetching things from a live internet server during build via e.g. wget or curl is forbidden). Also, bundling other existing packages is (heavily) frowned upon.

Side note: since yosys is the primary "consumer" of abc, we were able to justify packaging the yosys fork of abc in Fedora instead of the "true" upstream one, so that's one bullet dodged, but now I'm running into trouble with the documentation portion of the package:

First, I had to patch out the various badge.svg images which are fetched directly over the Internet during build time (this is the patch I'm applying for reference). So far, so good (the resulting pdf file may look a bit rough as a result, but the important content is still there).

However, with commit a66e94c5d ("Docs: Switch to furo-ys") I'm finally finding myself in a bit of a pickle.

Normally, python dependencies should be their own .rpm package, and funny enough, there's a python3-furo RPM already in the Fedora repositories. Unfortunately, use of the furo_ys fork presents a challenge for distro-packaging yosys:

• since the docs are shipped as part of the package, they should be built from source; side-loading a pdf doc file built elsewhere violates at least the spirit, if not the letter as well, of the Fedora packaging guidelines

• since building the docs now depends on furo-ys, that should come as its own RPM (and the "standard" furo actually does, but here we're using a fork, and this time I can't argue that the standard package should be replaced with furo-ys :(

• I could try to bundle furo-ys sources with the yosys source RPM, build it, install it and then use it during make docs portion of building yosys, but at this point things start to look rather contrived, and doing any more of this sort of thing would quickly become unsustainable

I'm wondering if there's another way that would ease downstream distro packaging without imposing too much additional hardship on maintaining the upstream documentation (e.g. limiting/eliminating "live" dependencies fetched over the network at build time, and also limiting/eliminating forked/bundled dependencies as much as possible) -- to the extent that downstream packages are considered useful by upstream maintainers.

Any ideas or advice much appreciated!

Thanks,
--Gabriel

Expected

No response

[KrystalDelusion]

Firstly, thanks for your work maintaining the Fedora package!

I didn't actually expect the pdf version of the docs to download the status badges at compile time, and should probably be html only. I will look into that and hopefully make your patch unnecessary :)

When we switched to using the Furo-ys fork there was a discussion about whether or not we want to maintain a pypi package but this was decided against since the fork is really only intended for YosysHQ use for consistent branding and reducing code duplication across our various projects. However most of that is focused on the html version of the docs used for the readthedocs. My suggestion would be to ignore furo-ys completely. I believe Furo only affects the html theming (html_theme = 'furo-ys' in conf.py), so for building the pdf that shouldn't make a difference. As a python package, furo_ys does currently provide the YoscryptLexer used to format yosys script code blocks; causing these to render without syntax highlighting is a regression but I think an acceptable one if it means saving on the hassle of figuring out how to include the forked package. Note that if you are using the existing makefiles to build the documentation it will fail on warnings, and missing the YoscryptLexer will raise a number of warnings; so you might want to override the SPHINXOPTS variable.

One other thing I will note is that docs/source/conf.py currently only checks for if os.getenv("READTHEDOCS"): to determine if it is a preview-style build or a release-style build. The former renders todo blocks which should be skipped for a release-style build (which I imagine the Fedora package as being), and can also affect the rendering of the version number (e.g. 0.47 vs 0.47-dev which we use to distinguish between latest and stable on read the docs). I would be more than happy to include another environment variable check or similar that can be used for packages like this if you have any suggestions of what that would be, rather than requiring you to patch it specifically for Fedora.

[KrystalDelusion]

If you want to modify the YoscryptLexer import to not error on a failed import instead of just patching it out I would also happily upstream that.

[gsomlo]

Hi @KrystalDelusion

Thanks for the quick reply! I am indeed only concerned with the ability to build the pdf manual in an isolated, off-line container (I was going to ask if picking some other format besides pdf would be a better option, but glad to hear pdf is still your recommendation :)

Right now, my command line is:

make ABCEXTERNAL=/bin/abc DOC_TARGET=latexpdf docs

(note: as mentioned earlier, /bin/abc is provided by the yosyshq-abc fedora package (built from the current yosyshq fork), and is required to match the version of the yosys being built).

The error I get is this:

[...]
make[1]: Entering directory '/builddir/build/BUILD/yosys-0.47-build/yosys-f20f913223c42fce6ecc4382b281cf67952e0a72/docs'
sphinx-build -b latex -d build/doctrees  -W --keep-going source build/latex
Running Sphinx v7.3.7

Exception occurred:
  File "/builddir/build/BUILD/yosys-0.47-build/yosys-f20f913223c42fce6ecc4382b281cf67952e0a72/docs/source/conf.py", line 90, in setup
    from furo_ys.lexers.YoscryptLexer import YoscryptLexer
ModuleNotFoundError: No module named 'furo_ys'
[...]

I'll have to study the sources and build scripts (and probably acquire some sort of superficial familiarity with sphinx) to get to the bottom of how I could express that "furo-ys shouldn't be required/expected when $DOC_TARGET is set to latexpdf". Once I figure it out, I'll post a PR and link it to this issue (Please feel free to pre-empt me if you know of a super-quick, obvious way to accomplish that :)

Also thanks in advance for looking into the badges (not) being pulled in while DOC_TARGET=latexpdf!

Best,
--Gabriel

[gsomlo]

oh, based on your earlier suggestions I'm experimenting with something along these lines:

diff --git a/docs/source/conf.py b/docs/source/conf.py
index 0de8cd445..c1f732901 100644
--- a/docs/source/conf.py
+++ b/docs/source/conf.py
@@ -87,5 +87,6 @@ def setup(app: Sphinx) -> None:
     from util.RtlilLexer import RtlilLexer
     app.add_lexer("RTLIL", RtlilLexer)

-    from furo_ys.lexers.YoscryptLexer import YoscryptLexer
-    app.add_lexer("yoscrypt", YoscryptLexer)
+    if os.getenv("DOC_TARGET") != "latexpdf":
+        from furo_ys.lexers.YoscryptLexer import YoscryptLexer
+        app.add_lexer("yoscrypt", YoscryptLexer)

I'll see if this takes care of things for me, then I'll ask you if it's something you might find palatable for the general case... :D

[KrystalDelusion]

I would probably suggest something like the following

    try:
        from furo_ys.lexers.YoscryptLexer import YoscryptLexer
        app.add_lexer("yoscrypt", YoscryptLexer)
    except ModuleNotFoundError:
        from pygments.lexers.special import TextLexer
        app.add_lexer("yoscrypt", TextLexer)

The except should also prevent any errors due to yoscrypt being left undefined without having to change SPHINXOPTS.

[gsomlo]

Thanks, that worked perfectly. I guess you don't need me to put in a PR anymore (but LMK if there's some workflow related reason why that might still be helpful)

[KrystalDelusion]

Do you know if there are any common environment variables that are used during package builds like this? Ideally something not Fedora specific. Or would a yosys specific flag be better? I am looking for something that would fit in the following code before the final else in order to prevent including todos for packaged documentation.

yosys/docs/source/conf.py

Lines 47 to 61 in 3afb772

	# set version
	if os.getenv("READTHEDOCS"):
	rtds_version = os.getenv("READTHEDOCS_VERSION")
	if rtds_version == "latest":
	release = yosys_ver + "-dev"
	todo_include_todos = False
	elif rtds_version.startswith("docs"):
	release = rtds_version
	todo_include_todos = True
	else:
	release = yosys_ver
	todo_include_todos = False
	else:
	release = yosys_ver
	todo_include_todos = True

[gsomlo]

I haven't stared at conf.py long enough to understand how adding a new branch to that outer if os.getenv(...) test would work in concert with the (working, at least for me) patch in #4725 (comment). But here (mock-build-env.log) is the output of running env just before the command building the (pdf) documentation as part of the RPM package via the following line:

make ABCEXTERNAL=%{_bindir}/abc DOC_TARGET=latexpdf docs

With any luck, one of those might make sense...

[KrystalDelusion]

First, I had to patch out the various badge.svg images which are fetched directly over the Internet during build time (this is the patch I'm applying for reference)

I'm rather confused by this. All of the places that reference the badge svg images are in .. only:: html blocks which prevent them from being included in the pdf output. And looking at it #3907 has a commit which says "Fixed pdf build, was previously breaking on trying to include the svg badges." If I recall correctly, it was just erroring and there was no attempt made to fetch them over the internet during build. I would guess that the patch may have been applied during the time when the pdf build was broken, although the exact reasoning given is wrong and is no longer necessary.

[gsomlo]

First, I had to patch out the various badge.svg images which are fetched directly over the Internet during build time (this is the patch I'm applying for reference)

I'm rather confused by this. All of the places that reference the badge svg images are in .. only:: html blocks which prevent them from being included in the pdf output. And looking at it #3907 has a commit which says "Fixed pdf build, was previously breaking on trying to include the svg badges." If I recall correctly, it was just erroring and there was no attempt made to fetch them over the internet during build. I would guess that the patch may have been applied during the time when the pdf build was broken, although the exact reasoning given is wrong and is no longer necessary.

When I don't patch out badge svg references, running make ABCEXTERNAL=/bin/abc DOC_TARGET=latexpdf docs in the rpm build container (mock) results in this error:badge_err.log

The main "highlight" is a bunch of entries similar to this:

WARNING: Could not fetch remote image: https://github.com/YosysHQ/oss-cad-suite-build/actions/workflows/darwin-arm64.yml/badge.svg
[HTTPSConnectionPool(host='github.com', port=443): Max retries exceeded with
url: /YosysHQ/oss-cad-suite-build/actions/workflows/darwin-arm64.yml/badge.svg  (Caused by
NameResolutionError("<urllib3.connection.HTTPSConnection object at   0x7f53a3dd42d0>:
Failed to resolve 'github.com' ([Errno -3] Temporary failure   in name resolution)"))]

I might be missing some environment setting that would bypass badge downloading (or otherwise be doing something silly on my end :) ) but as of right now (commit b89bd02) I can't build the pdf manual unless I explicitly remove badge references.

[gsomlo]

Hi @KrystalDelusion,

I'm rather confused by this.

I think I figured out why my pdf build is failing. The sphinx-build call here:
https://github.com/YosysHQ/yosys/blob/main/docs/Makefile#L143
returns 1 after issuing warnings on all the missing badges. If I then force it to run the next command:
https://github.com/YosysHQ/yosys/blob/main/docs/Makefile#L145
(e.g, manually), it succeeds.

So you're right in that it could build the pdf without first downloading the badges, but the makefile itself gives up when sphinx-build comes back with a non-zero return code...

EDIT: if I blank out SPHINXOPTS here: https://github.com/YosysHQ/yosys/blob/main/docs/Makefile#L5
(i.e., get rid of -W), the return code is 0 and I presume the makefile would succeed...

EDIT 2: so, in conclusion, my doc build command line should be:

make ABCEXTERNAL=%{_bindir}/abc DOC_TARGET=latexpdf SPHINXOPTS='' docs

which would allow the pdf build to succeed without needing to patch out the badge references. Mystery solved :D

Of course, if you think there's a more elegant way around this, please LMK.

[KrystalDelusion]

Are you sure the warning is for the SVG badges? I don't get any warnings locally when I run make latexpdf from the docs directory, but I'm not using ABCEXTERNAL and I noticed your patch also removes the inclusion of yosys-abc.

If it is the yosys-abc part giving the warning I would suggest reducing the patch to just that part, and then raise a separate issue that docs builds are failing when using external abc :)

[gsomlo]

On Thu, Nov 21, 2024 at 08:26:18AM -0800, KrystalDelusion wrote: Are you sure the warning is for the SVG badges? I don't get any warnings locally when I run make latexpdf from the docs directory, but I'm not using ABCEXTERNAL and I noticed your patch also removes the inclusion of yosys-abc

Any warning encountered by $(SPHINXBUILD) at https://github.com/YosysHQ/yosys/blob/main/docs/Makefile#L143 results in make quitting (due to sphinx-build returning non-zero, even if told to `--keep-going` in conjunction with `-W`, at least in my environment). If I patch out the include of `/generated/yosys-abc` in docs/source/appendix/auxprogs.rst, it still fails due to warnings about badges. So far, the only way to build latexpdf successfully from within the fedora "mock" rpm build container was to clear SPHINXOPTS (i.e., remove `-W`). I assume the result is identical to when I patch out inclusion of `generated/yosys-abc` and badges, but I haven't actually looked at the resulting pdf yet :)