mupdf: fix fallback fonts handling

[benoit-pierre]

This is the correct MuPDF 1.24.2 update to the "external fonts" patch, preventing both koreader/koreader#11959 and koreader/koreader#11983.

And our little guy ◂ is now correctly rendered too!

---

This change is 

[poire-z]

I git pull base, and ran kodev build, but it did not recompile mupdf.
What's the procedure to be sure anything new is accounted in the build?

[benoit-pierre]

There's no sure way beside rebuilding: CMake cached variables, make based systems that don't account for update to external libraries (or headers), patches that may have been changed… The old build system would just nuke and reconfigure some thirdparty projects, but I don't think that's a great behavior.

[benoit-pierre]

If you know what changed, you can make mupdf-re all for example.

[benoit-pierre]

I don't know if outdated projects could be detected. Like meson: each subproject (CMake external project equivalent, more or less) checkout has a .meson-subproject-wrap-hash.txt file at the root, that can be used to check if there's a mismatch with the corresponding wrap file (equivalent to our ~thirdparty/PROJECT/CMakeLists.txt).

[poire-z]

If you know what changed, you can make mupdf-re all for example

That worked.
But what does -re mean, and why all ? And why wasn't it listed in your make help output ?

 $ make help|grep mupdf
... koreader-build-wrap-mupdf
... mupdf
... mupdf-build
... mupdf-clean
... mupdf-configure
... mupdf-deps
... mupdf-download
... mupdf-install
... mupdf-patch
... wrap-mupdf
... wrap-mupdf-deps

Would be nice if these make tricks would be referenced in a single place, in a file koreader or koreader-base.

The old build system would just nuke and reconfigure some thirdparty projects, but I don't think that's a great behavior.

Well, at least it was practical, and I always had my emulator build up to date, without needing to follow what has changed.
I hope all you build masters will find something to restore a bit of that "auto do the right thing" behaviour :)

[Frenzie]

Like meson

Though to be clear, the way it works in meson is extremely inconvenient and it's a pita to get it behave anywhere close to sane, requiring all kinds of weird commands and flags that should not be necessary. What we had is in fact not terrible and significantly better than anything meson does imho. Because it just works.

I don't mean to say much with that other than that I do not wish to see anything remotely meson-like happening because in this regard it just plain sucks.

[mergen3107]

@benoit-pierre
I did git pull, then make feththirdparty, then make mupdf-re all (this one took a while, no errors though). Then did a build koreader-kindlepw2-v2024.04-154-gc8f4008e9_2024-06-08, but I still see empty boxes instead of the speaker icon (ref. #11983). Here is crash.log of me restarting KOReader, then looking up word "learn" and then exiting.
koreader-kindlepw2-v2024.04-154-gc8f4008e9_2024-06-08_mupdf_re_all.crash.txt

[benoit-pierre]

Must… resist… urge… to rant about submodules…

@mergen3107: make fetchthirdparty update submodules: they are reset to the last recorded commit. The base submodule is usually lagging behind koreader-base master branch: as of koreader/koreader@c8f4008, this PR is not part of it.

Currently missing commits are:

▸ git -C base fetch https://github.com/koreader/koreader-base.git && git -C base log --reverse --oneline $(git ls-tree --object-only master base)..FETCH_HEAD
3f00257f mupdf: fix fallback fonts handling (#1815)
b369ed30 cmake: fix `download-all` target (#1817)
f43f1830 (origin/master, origin/HEAD, github/master, master) Update Lua-RapidJSON (#1816)

You can checkout another version of base, if you know it's compatible with your koreader commit, or wait for the next bump containing the fix.

Also note: make mupdf-re all by default will build for the release version of the emulator.

[mergen3107]

Thanks! I shall wait then, don't really want to mess with it :D

[benoit-pierre]

If you know what changed, you can make mupdf-re all for example

That worked. But what does -re mean,

re as in re(build). (make re is equivalent to make clean all)

and why all ?

• make mupdf-re rebuild only mupdf
• make all ensures that any downstream targets (like wrap-mupdf) are updated as needed

And why wasn't it listed in your make help output ?

make help is a cmake build system target, it does not know about the top-level / base makefile targets, of which the re rules are a part:

%-re:
	$(MAKE) $*-clean
	$(MAKE) $*

The old build system would just nuke and reconfigure some thirdparty projects, but I don't think that's a great behavior.

Well, at least it was practical, and I always had my emulator build up to date, without needing to follow what has changed. I hope all you build masters will find something to restore a bit of that "auto do the right thing" behaviour :)

Is nuking the external project you may be working on the right behavior?

I suspect it's another instance where we're going to have wildly different views on the subject.

[benoit-pierre]

Like meson

Though to be clear, the way it works in meson is extremely inconvenient and it's a pita to get it behave anywhere close to sane, requiring all kinds of weird commands and flags that should not be necessary. What we had is in fact not terrible and significantly better than anything meson does imho. Because it just works.

I don't mean to say much with that other than that I do not wish to see anything remotely meson-like happening because in this regard it just plain sucks.

You'll have to elaborate.

[poire-z]

Thanks for the explanations. I again think these tips need to be documented in a file we can have at hands.

Is nuking the external project you may be working on the right behavior?

I never lost anything (except once, where I blindly did kodev fetchthirdparty or something and it recloned from scratch crengine and I lost all my archived and current commits).
It was sometimes hard to work/play with thirdparty stuff, ie. hacking original code to try things (or even just working on LunaSVG) and getting it to rebuild: had to enter the dir and try various "make" commands. But it was out of the regular flow, and I somehow always managed to reach my ends.

But the current behaviour of not picking pulled changes is neither great nor right :)

Anyway, even a shell script outside of the make/cmake infrastructure, that would walk the dirs and compare timestamps to run the needed make xyz would be ok to me. Something we can trust (better than our memory and following what has been commited to base).
Pulling and having to clean all and rebuild the world, when the changes were only affecting Lua files in base/ffi/, is a waste of time, power and trees.

[benoit-pierre]

Thanks for the explanations. I again think these tips need to be documented in a file we can have at hands.

Is nuking the external project you may be working on the right behavior?

I never lost anything (except once, where I blindly did kodev fetchthirdparty or something and it recloned from scratch crengine and I lost all my archived and current commits).

I don't see how that's possible. One thing that was changed recently, is to not hard-reset the checkout: 1c24dc1.

You should not loose uncommitted changes:

▸ make fetchthirdparty
git submodule init
git submodule sync
Synchronizing submodule url for 'base'
Synchronizing submodule url for 'l10n'
Synchronizing submodule url for 'platform/android/luajit-launcher'
Synchronizing submodule url for 'platform/ubuntu-touch/ubuntu-touch-sdl'
Synchronizing submodule url for 'resources/fonts'
Synchronizing submodule url for 'test'
# Force shallow clones of submodules configured as such.
git submodule update --jobs 3 --depth 1 test resources/fonts l10n
# Update the rest.
git submodule update --jobs 3
error: Your local changes to the following files would be overwritten by checkout:
        thirdparty/mupdf/CMakeLists.txt
Please commit your changes or stash them before you switch branches.
Aborting
fatal: Unable to checkout '4518f064473c46d875239c0d352cf7e3ffd1ee34' in submodule path 'base'
make: *** [Makefile:155: fetchthirdparty] Error 1

Of course, if you've have made a number of commits without checking out a branch first, an update would leave those unconnected, and you'll have to use the reflog to rescue them.

It was sometimes hard to work/play with thirdparty stuff, ie. hacking original code to try things (or even just working on LunaSVG) and getting it to rebuild: had to enter the dir and try various "make" commands.

You don't have to do that anymore.

[Frenzie]

You'll have to elaborate.

Best as I can tell meson subprojects update never does anything useful whatsoever, nor does it seem to be run automatically for a meson compile invocation (not that it'd do you any good). All it does is fail for just about every changed subproject because it changed, aka the very reason you need to run the command in the first place.

Iff [sic] it only ever failed when you had manually changed something the behavior would arguably make sense. The desired behavior requires convoluted invocations like meson subprojects update --reset. See issues like mesonbuild/meson#7526

Taking that back to the discussion at hand, it means our desired behavior is to checkout the desired commit, unlike meson which doesn't. Any potential forcing is orthogonal to that primary desire.

[poire-z]

But the current behaviour of not picking pulled changes is neither great nor right :)

Any update on that?
I just updated base, and kodev build, and got a bunch of stuff downloaded and/or reconfigured and/or rebuilt:
luajit libk2pdfopt lpeg lua-rapidjson luajson freetype2 luasec luasocket tesseract djvulibre harfbuzz.

Can I be sure everything was picked ? Or there are still risks stuff was not detected as needing rebuild?

Oh, just got:

    CC /space/kobo/koreader/base/build/x86_64-linux-gnu-debug/thirdparty/mupdf/build/source/fitz/color-lcms.o
source/fitz/color-lcms.c:38:10: fatal error: lcms2mt.h: No such file or directory
   38 | #include "lcms2mt.h"
      |          ^~~~~~~~~~~
compilation terminated.
make[5]: *** [Makefile:144: /space/kobo/koreader/base/build/x86_64-linux-gnu-debug/thirdparty/mupdf/build/source/fitz/color-lcms.o] Error 1

I thought this was somehow handled better? Previously, when that happened, I needed to rm thirdparty/mupdf/build.
make mupdf-re solved that.
But still not smooth:

[  1%] Performing build step for 'crengine'
make[5]: warning: -j2 forced in submake: resetting jobserver mode.
[  2%] Performing install step for 'crengine'
[  2%] Completed 'crengine'
[  2%] Performing install step for 'libk2pdfopt'
-- libk2pdfopt install command succeeded.  See also /space/kobo/koreader/base/build/x86_64-linux-gnu-debug/thirdparty/libk2pdfopt/log/libk2pdfopt-install-*.log
[  2%] Completed 'libk2pdfopt'
[  2%] Completed build dependencies for 'koreader'
[  2%] Performing build step for 'koreader'
make[5]: warning: -j2 forced in submake: resetting jobserver mode.
-- Configuring done
-- Generating done
-- Build files have been written to: /space/kobo/koreader/base/build/x86_64-linux-gnu-debug/cmake/koreader/build
[  5%] Building CXX object CMakeFiles/koreader-cre.dir/cre.cpp.o
make[7]: *** No rule to make target '/space/kobo/koreader/base/build/x86_64-linux-gnu-debug/staging/lib/libdjvulibre.a', needed by 'libkoreader-djvu.so'.  Stop.
make[7]: *** Waiting for unfinished jobs....
[ 11%] Building C object CMakeFiles/koreader-djvu.dir/djvu.c.o
make[6]: *** [CMakeFiles/Makefile2:152: CMakeFiles/koreader-djvu.dir/all] Error 2
make[6]: *** Waiting for unfinished jobs....
[ 17%] Linking CXX shared library libkoreader-cre.so
make[5]: *** [Makefile:91: all] Error 2
make[4]: *** [koreader/CMakeFiles/koreader-build.dir/build.make:77: koreader/stamp/koreader-build] Error 2
make[3]: *** [CMakeFiles/Makefile2:13033: koreader/CMakeFiles/koreader-build.dir/all] Error 2
make[2]: *** [Makefile:91: all] Error 2
make[1]: *** [Makefile:71: all] Error 2
make[1]: Leaving directory '/space/kobo/koreader/base'
make: *** [Makefile:121: base] Error 2

The interleaving of many stuff does not help understanding what causes what :/

[benoit-pierre]

The behavior is the same as the old build system: external projects steps don't depend on files, but are re-run when the step command itself changes.

So when you change the version in the URL of a project, or change the revision in a call to ko_write_gitclone_script (which changes the resulting DOWNLOAD_COMMAND), then the source/build directories are nuked and the project is re-built.

Adding a patch to the PATCH_COMMAND will trigger the patch step again (which may fail), but not changing a patch.

Changing CONFIGURE_COMMAND will trigger the configure step again, etc…

[benoit-pierre]

And that configure step can also fail, for example bumping the zlib version will re-build zlib from scratch, but fail in libpng because the build is not redone from scratch.

[benoit-pierre]

And prior to f11b599, some thirdparty projects would nuke their build directories on thirdparty/PROJECT/*.* change: lodepng, luasocket, lua-Spore, minizip, MuPDF, openssl.

[poire-z]

Mentionning that as it was not obvious in my latest output paste, it does not succesfully build because:

make[7]: *** No rule to make target '/space/kobo/koreader/base/build/x86_64-linux-gnu-debug/staging/lib/libdjvulibre.a', needed by 'libkoreader-djvu.so'.  Stop.
make[6]: *** [CMakeFiles/Makefile2:152: CMakeFiles/koreader-djvu.dir/all] Error 2

No idea what to do - I might suck at escape games :)

[benoit-pierre]

Anyway, provide me a spec, and I'll look into it.

[benoit-pierre]

Install ccache, make re.

[poire-z]

A "spec" ? Like my own description of how I'd like kodev build to work ?
Easy: it should just do the right thing :)
I'm not confortable with build tools/stuff to say more than that :/ I guess I'd like the few issues I mentionned above (or later will) to not happen, and be taken care automagically.

[Frenzie]

And prior to f11b599, some thirdparty projects would nuke their build directories on thirdparty/PROJECT/*.* change: lodepng, luasocket, lua-Spore, minizip, MuPDF, openssl.

That's the key factor causing more friction now: it's not doing enough of that. :-)

[benoit-pierre]

Ah! right… Even if all external projects were rebuild from scratch, that still does not account for possible old-leftovers in the output directory or the staging tree.

[poire-z]

make djvulibre-re allowed me to complete the build.

[benoit-pierre]

The earlier version of the build system using by-products (manually listing all installed files) was sort of self-healing: remove part of the output files, and they automatically get re-created.