Docker nerdfonts-patcher errors out on fontforge start

kjkent commented 1 month ago

🗹 Requirements

[X] I have searched the issues for my issue and found nothing related and/or helpful
[X] I have searched the FAQ for help
[X] I have searched the Wiki for help

🎯 Subject of the issue

Experienced behavior:

Docker container nerdfonts/patcher errors out when run. I've attempted both compose and run syntax:

 services:
   nerdfonts-patcher:
    command: |
      --mono
      --adjust-line-height
      --progressbars
      --complete
    container_name: nerdfonts-patcher
    image: nerdfonts/patcher
    #user: ${UID}:${GID}
    volumes:
    - ./nf-in:/in
    - ./nf-out:/out

or docker compose run --rm -v ./nf-in:/in -v ./nf-out/out nerdfonts/patcher --mono --adjust-line-height --progressbars --complete.

Both result in:

Running with options:
 --mono --adjust-line-height --progressbars --complete
Parallelism 0
fontforge -script /nerd/font-patcher -out /out --mono --adjust-line-height --progressbars --complete /in/berkeley-mono/OTF/BerkeleyMono-BoldItalic.otf
#### above line repeated for each font

Nerd Fonts Patcher v3.2.1 (4.14.3) (ff 20230101)
Done with Patch Sets, generating font...
Copyright (c) 2000-2023. See AUTHORS for Contributors.
 License GPLv3+: GNU GPL version 3 or later <http://gnu.org/licenses/gpl.html>
 with many parts BSD <http://fontforge.org/license.html>. Please read LICENSE.
 Version: 20230101
 Based on sources from 2023-10-26 13:09 UTC-D.
Core python package 'pkg_resources' not found: Cannot discover plugins
Traceback (most recent call last):
  File "/nerd/font-patcher", line 2155, in <module>
    main()
  File "/nerd/font-patcher", line 2147, in main
    patcher.generate(sourceFonts)
  File "/nerd/font-patcher", line 415, in generate
    sourceFont = sourceFonts[0]
                 ~~~~~~~~~~~^^^
IndexError: list index out of range
##### above block repeated multiple times

Expected behavior:

The fonts to patch without the errors Example symbols: N/A

🔧 Your Setup

Which font are you using (e.g. Anonymice Powerline Nerd Font Complete.ttf)? Nerdfonts/patcher docker image
Where did you get the file from (download link, self patched, source downloaded from link...) Docker Hub
Which terminal emulator are you using (e.g. iterm2, urxvt, gnome, konsole)? Alacritty
Are you using OS X, Linux or Windows? And which specific version or distribution? Arch Linux

kjkent commented 1 month ago

After some testing out of docker:

The errors above happened on variable fonts, and judging from some script output, ie., opening may fail, the results will most likely not be what you want, it appears compatibility with these fonts isn't fully supported.
The errors also occurred on .woff and .woff2 files. From the docker entrypoint script, I believed at least .woff would be supported; but, perhaps it's font-specific!

In any case it seems this isn't the container issue I thought, and instead is just incompatibility with variable and potentially web fonts.

If this is expected, I can close this issue? Sorry for the noise if so.

Finii commented 1 month ago

Thanks for reporting!

Indeed, variable fonts can not be opened (and then patched) with fontforge. This is a fontforge limitation and Nerd Fonts font-patcher needs to be rewritten for some more modern backend to support VF.

But that message "opening may fail" should also be visible in the docker runs 🤔

And this font file name does not sound very variableish /in/berkeley-mono/OTF/BerkeleyMono-BoldItalic.otf.

(On the other hand, I never had the idea to put subdirectories under /in, so that is completely untestet 😬 )

And woffs are also untested, but fontfore should be able to handle them just fine.

Maybe you can give me a link to the fonts that fail and I can investigate further.

The error message with the index 0 out of bounds is completely unexpected. It seems fontforge.fontsInFile() fails to detect any fonts in the file and does not not error out and WE do not check the result. I'd fix that, a file that causes this would be great for testing.

kjkent commented 1 month ago

Hey, thanks for the feedback! So I think the 'opening may fail' message perhaps was present in the Docker runs, although as the script now runs with parallel, the more noisy errors such as the index issue quickly drown out those lines! I can check :)

The BoldItalic font you mention didn't cause an error -- I truncated some of the logs where text was repeated. When I removed the WOFF/WOFF2/variable fonts, the rest sailed through without any issue (I ran --mono with fonts in subfolders and then without mono after realising I like the bigger glyphs, coincidentally without subfolders, both completed similarly problem-free).

I'd be happy to provide font files, but I'm concerned about running afoul of the licence as it's not a free font. There is a trial download of the font with some vowel glyphs swapped. Let me see if I can find that and what font files are in there. If all else fails I recently spoke with the author of the fontface and think I'll need to contact him again shortly; I can ask for permission if it that's helpful.

I'll also rerun the patcher and more specifically detail what fonts throw the index error, and what similarities they hold.

Finii commented 1 month ago

Thanks for the additional information!

Maybe it will be more helpful if you run the docker in sequential -j 1 mode, which is a bit harder to achieve. I would have liked the usual j option, but ... sigh. Here the example how to activate it by setting PN (which means process-number, presumably?):

docker run --rm -v /path/to/fonts:/in:Z -v /path/for/output:/out:Z -e "PN=1" nerdfonts/patcher [OPTIONS]

I will fix the empty array / index error. But it would be interesting which font triggers it. I expected it can never turn up empty, and it would be good to understand the situation where my assumptions fail. If you can identify the font file that does, maybe it is also the case with the same file as trial version.

Thanks again for helping :+1:

Finii commented 1 month ago

@allcontributors please add @kjkent for bug

allcontributors[bot] commented 1 month ago

@Finii

We had trouble processing your request. Please try again later.

Finii commented 1 month ago

Hmm, it's even documented ;-)

Ah, of course it triggers if the specified "font file" is no font at all :woman_facepalming:

Finii commented 1 month ago

Ok, thanks again, this will be fixed by the PR (see above).

Now I need to find out what's wrong with the allcontributors bot :grimacing:

Finii commented 1 month ago

@allcontributors please add @kjkent for bug

allcontributors[bot] commented 1 month ago

@Finii

I've put up a pull request to add @kjkent! :tada:

kjkent commented 2 weeks ago

Hey @Finii, I've been digging into this a little bit more. It appears to be an upstream issue with fontforge, as in, fontsInFile() is currently incompatible with woff fonts: https://github.com/fontforge/fontforge/issues/1964.

As a workaround, those with the woff2 library installed can run woff2_decompress prior to running the script, then woff2_compress after. The files I tested underwent this process without issue. It's woff2-specific though (sorry woff1)

If you'd find it beneficial, I could make a PR handling this in the script, either checking whether the user has https://github.com/google/woff2 installed and using it from PATH, or, as a fallback, cloning and building it. Admittedly, it's a bit of a ~~hack~~ workaround, so I understand if you'd rather leave it to the user :D

kjkent commented 2 weeks ago

Oh! And if it's useful to anyone reading this, I figured a way for users to grab the latest font-patcher and its deps without pulling the kitchen sink with it. It was useful for me for scripting as I was experimenting with the NerdFontsSymbolsOnly TTF. I found sparse-clone and cone mode very fickle to have it behave how I'd expect it, but the following does the job:

repo='https://github.com/ryanoasis/nerd-fonts'
branch='master'

# Accepts directories and single files
git_include=(
        '/font-patcher'
        '/src/glyphs/**'
        '/bin/scripts/name_parser/**'
)

git clone -n --depth=1 --filter=tree:0 "$repo" "${repo##*/}"
cd "$_"
git sparse-checkout set --no-cone ${git_include[@]}
git checkout "$branch"

Finii commented 2 weeks ago

Thanks for sharing the interesting find above regarding woff2!

I need to digest and let that simmer a bit ;-) But I want to comment on this right away:

Oh! And if it's useful to anyone reading this, I figured a way for users to grab the latest font-patcher and its deps without pulling the kitchen sink with it.

Why not just using the FontPatcher.zip; I believe that is far easier to grasp for usual users that have a very specific sparse checkout. Well, except you want to contribute.

https://github.com/ryanoasis/nerd-fonts/blob/master/FontPatcher.zip is updated automagically (via CI) whenever anything related to selfpatching changes in the default branch.

kjkent commented 1 week ago

@Finii oh yeah, no, you're absolutely right. What I wrote is probably more specific to my use case than others, I'm writing a python script to generate web fonts and strip unused glyphs out of the resultant file. My initial (convoluted) attempt used the icons-only font file as a reference, so, the sparse clone came in handy.

I think I was in too deep and didn't realise that it was very much a me problem haha.

Finii commented 1 week ago

After having read your report at the Fontforge repo I believe my fix could in fact be wrong (too limiting). Reopening to investigate. Thanks!

ryanoasis / nerd-fonts