FIX: fall back to kpsewhich when luatex starts but is unusable#31984
FIX: fall back to kpsewhich when luatex starts but is unusable#31984kikeenator wants to merge 1 commit into
Conversation
| if lk: | ||
| path = lk.search(filename) | ||
| else: | ||
| if not path: # luatex unavailable or unusable; fall back to kpsewhich. |
There was a problem hiding this comment.
I'm not sure we want to fall back if lk succeeds but returns None due to a missing path (it's just extra cost for little gain); maybe the two cases need to be distinguished?
There was a problem hiding this comment.
My thought process behind that was that a missing path leads to an error in any way, so its really time-complexity versus code complexity/readability question. The two assumptions I made are the following: 1) The time-save is only ever constant, since not finding a file leads to error and abort 2) Calling kpsewhich (the fallback), is rather quick anyway.
Both assumptions arent fact checked, and even if they hold, the question is still a philosohpical one. I decided against the more efficient solution since I believe its chasing an efficiency dead-end in a robust piece of code.
I am not in the position to judge, I dont know coding philosophy for this project. If youd like I can change it so the cases are distinguished and the fallback isnt called for a really missing part.
There was a problem hiding this comment.
If missing fonts always led to hard crashes then I agree it would be OK to have a slower path if the implementation is much simpler, but I believe this is not the case, essentially due to the case described at #28212 (comment) (although admittedly I didn't check very carefully): we need to support vf fonts with partially missing tfm subfonts. (It's a fairly technical issue, feel free to ask if it's not clear after reading that thread.) As noted in that issue there may be cleaner designs that involver lazier vf loading, but that's not what we have now.
There was a problem hiding this comment.
I was missing the context, thanks for pointing it out. Great learning case for me.
I now changed the code so that search returns "" if communication with luatex was successfull but the path was not found (instead of None like before). I cascaded this change to the find_tex_file function so it now only falls backs if the luatex search genuienly didnt work.
I also added a test that explicitly checks that the fallback isnt called in that specific case.
Thanks for your patience and explanation, really appreciated.
|
Looks good to me, can you squash the commits? |
find_tex_file only fell back to kpsewhich when the luatex process failed to start. If luatex started but then died or became unusable at runtime, its search() returned an empty result and find_tex_file raised FileNotFoundError without ever trying kpsewhich. Make _LuatexKpsewhich.search() return None when luatex is unusable (a broken pipe or an empty read) and "" when luatex works but the file is missing, and fall back to kpsewhich only when search() returns None, so a genuinely missing file does not trigger a needless kpsewhich call. Closes matplotlib#31983
kikeenator
force-pushed
the
fix-31983-luatex-kpsewhich-fallback
branch
from
c19e8fb to
9e9e91f
Compare
2 participants
find_tex_file spawns a luatex instance via which it can locate files in the path of a latex installation. If this doesnt work, we fallback to calling kpsewhich directly. Currently this only happens when the luatex process failed to start. If luatex started but then died or became unusable at runtime, its search() returned an empty result and find_tex_file raised FileNotFoundError without ever trying kpsewhich. In practice this means a user whose luatex is broken cant use usetex at all, even though kpsewhich works fine on their machine
I fixed this by making _LuatexKpsewhich.search() return None on a broken pipe or empty read, and restructuring find_tex_file to fall back to kpsewhich whenever the luatex route yields no usable path.
AI disclosure:
I used AI to get feedback on my proposal of how to fix, with the idea and diagnosis being fully mine (obviously inspired by the issue creators hotfix to give credit where its due). I also used it to draft test cases. I understand the code fully and can explain and defend all decisions undertaken.
Closes #31983