Attention is currently required from: Anton Samsonov, Anton Samsonov, Thomas Heijligen.
Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/77778?usp=email )
Change subject: Makefile,meson.build: Add support for Sphinx 3.x ......................................................................
Patch Set 5:
(5 comments)
Commit Message:
https://review.coreboot.org/c/flashrom/+/77778/comment/af38fdce_96f0466e : PS3, Line 7: 3
The patch adds support for older 3.x versions, since 4. […]
Oh I see! Maybe you can rename the title as `Makefile,meson: Add support for Sphinx versions prior to 4.0` so that it's more clear that it's adding backward compatibility
https://review.coreboot.org/c/flashrom/+/77778/comment/1c9505ba_03d7e996 : PS3, Line 13: but not on `sphinx-build` own level : upon which `meson` build relies
Alternatively, a symlink "8" may be created pointing at "man8" after the directory renaming occurs […]
Oh sorry I did not react on your previous message! but yes, I agree simlink makes sense, lets keep it (especially that you wrote the code already).
Patchset:
PS5: I discovered that at the moment jenkins skips sphinx-build :\ I will try to find out why, but meanwhile I assume you have tested on your environment (which is something prior 4.0), can you add test info into commit message (the sphinx version)? I will also test locally, and see maybe jenkins can have sphinx build again.
File doc/sphinx-wrapper.sh:
https://review.coreboot.org/c/flashrom/+/77778/comment/7b4b0eb2_b72d3c46 : PS5, Line 8: "$@" I have a question, just want to understand how it works. `$@` means all the command-line parameters, but I compare the old and new code, it seems to me that the parameters needed for this invocation of SPHINXBUILD (which is `sphinx` right?) are starting from $5 and until the end? do I miss something?
https://review.coreboot.org/c/flashrom/+/77778/comment/3fd00061_7b322755 : PS5, Line 14: Can you add a comment here, saying: for sphinx versions prior to 4.0 this block renames directory from 8 to man8, since man8 expected by makefile and meson builds.
I know this info is in commit message, and this is right, but for future people looking into the code, it is useful to have a comment right here too.
-
Anastasia Klimchuk (Code Review)