Ninja Builder

[simoncozens]

An experimental version of gftools.builder which uses ninja to orchestrate font builds instead of fontmake's top-level methods.

Currently only does the variable font bit; the real speedup is with the statics.

[simoncozens]

Building all Rubik targets with gftools-builder: 1m 52s.
Building all Rubik targets with gftools-builder-ninja: 23s.

[felipesanches]

I think you'll need to update requirements.txt and setup.py to include the new ninja dependency.

[m4rc1e]

I don't mind adding these at some point if you want.

[simoncozens]

Fine. It's just a matter of moving the code we have inside the buider into distinct gftools-* command line utilities. Which arguably they should be already.

[madig]

You maybe want to put this in a https://docs.python.org/3/library/tempfile.html?highlight=tempdir#tempfile.TemporaryDirectory so parallel invocations of gfbuild don't overwrite each other. The dir can then be deleted after asuccessful run (del it) or stay in place otherwise for diagnostics.

[simoncozens]

I don't want to do that, precisely because I want a second successive run to be fast and only generate what needs to be generated.

[simoncozens]

There is still some problem here with instance UFO filenames. Sometimes fontmake puts them into instance_ufo/ regardless of what's in the designspace file and sometimes it goes with the path in the designspace file, and I can't work out what happens when.

[arrowtype]

Is this function where we could add the additional argument to --expand-features-to-instances?

See #581 for more background on this.

[simoncozens]

Yes, that's where it should go.

[arrowtype]

Sorry, I only intended to highlight Line 131 here.

I tried hacking this in the download within my local venv, but at least in initial tests, it wasn't working as I had hoped.

[arrowtype]

Hmm, yeah, I’ve edited this in my venv, but I can’t even get it to print debugging statements (even using a config.yaml file with logLevel set to DEBUG. I’m not entirely sure if the Ninja builder is even running, or even how I would determine that.

    def fontmake_args(self, args):
        my_args = []
        my_args.append("--expand-features-to-instances") # TESTING;
        my_args.append("--filter ...")
        if self.config["flattenComponents"]:
            my_args.append("--filter FlattenComponentsFilter")
        if self.config["decomposeTransformedComponents"]:
            my_args.append("--filter DecomposeTransformedComponentsFilter")
        if "output_dir" in args:
            my_args.append("--output-dir " + args["output_dir"])
        if "output_path" in args:
            my_args.append("--output-path " + args["output_path"])

        print(" ".join(my_args)) # DEBUGGING;
        
        return " ".join(my_args)

[simoncozens]

It looks very much like you’re not running the ninja builder. Try python -m gftools.builder.ninja (configfile).

[arrowtype]

Thank you for the tip! I’ll give that a try.

[arrowtype]

Awesome, that definitely ran the Ninja builder, including my hacks!

However (this may be stuff you already know), it seems that:

• it will only run on a config file, not a designspace
• add the arg as suggested yields the following part way through the build: fontmake: error: "--expand-features-to-instances" option invalid for UFO source
• It expects/requires stat table info in the config

ninja: build stopped: subcommand failed.
Traceback (most recent call last):
  File "/Library/Frameworks/Python.framework/Versions/3.10/lib/python3.10/runpy.py", line 196, in _run_module_as_main
    return _run_code(code, main_globals, None,
  File "/Library/Frameworks/Python.framework/Versions/3.10/lib/python3.10/runpy.py", line 86, in _run_code
    exec(code, run_globals)
  File "/Users/stephennixon/type-repos/shantell-sans/venv-ninja/lib/python3.10/site-packages/gftools/builder/ninja.py", line 289, in <module>
    NinjaBuilder(sys.argv[1]).build()
  File "/Users/stephennixon/type-repos/shantell-sans/venv-ninja/lib/python3.10/site-packages/gftools/builder/ninja.py", line 47, in build
    os.remove(temporary)
FileNotFoundError: [Errno 2] No such file or directory: './fonts/builder/variable/shantell_sans-ital_wght_BNCE_IRGL_TRAK--static-ninja[wght].ttf.statstamp'

[simoncozens]

1. Yeah, that's true when you run it as python3 -m gftools.builder.ninja, but it's meant to work as gftools-builder as well.
2. Urgh. We need to add that only when creating UFOs.
3. No, that's a problem removing temporary files, which I think I fixed.

[arrowtype]

Got it. Thanks for the insights here! I might try to hack away at this on my end, but don't let my issues block an otherwise solid PR.

[simoncozens]

Just an update on this: I've used it to build hundreds of Noto fonts, and for that it works well. I haven't tracked down the issue with instance UFOs ending up in the wrong place, because I'm scrambling to get lots of fonts built and released and don't really have any bandwidth for anything else. If anyone wants to play with it, clone the googlefonts-project-template and try building that.

[arrowtype]

Hey, sorry to have complicated this PR. I really appreciate you taking a bit of time to respond to my questions! I definitely don't want to block it when it's already proving useful for a bunch of projects. Mine is a somewhat weird use case (it is also for Google Fonts, FWIW), so I can try to hack on it on my end if what I need isn't applicable to your projects. Thanks for all your great work here!

[m4rc1e]

Just had a quick look and it looks decent. Since it will use the original builder if it encounters an unsupported feature, I'm happy to merge this.

We can always improve it in 2023 Q1.

[m4rc1e]

Scratch the above. I've just started playing with some files I have locally and it crashed.

Traceback (most recent call last):
  File "/Users/marcfoley/Type/tools/venv/bin/gftools-gen-stat.py", line 7, in <module>
    exec(compile(f.read(), __file__, 'exec'))
  File "/Users/marcfoley/Type/tools/bin/gftools-gen-stat.py", line 92, in <module>
    main()
  File "/Users/marcfoley/Type/tools/bin/gftools-gen-stat.py", line 66, in main
    fonts = [TTFont(f) for f in args.fonts]
  File "/Users/marcfoley/Type/tools/bin/gftools-gen-stat.py", line 66, in <listcomp>
    fonts = [TTFont(f) for f in args.fonts]
  File "/Users/marcfoley/Type/tools/venv/lib/python3.10/site-packages/fontTools/ttLib/ttFont.py", line 130, in __init__
    file = open(file, "rb")
FileNotFoundError: [Errno 2] No such file or directory: '[{name:'

When I was adding ninja to diffenator2, it didn't like relative paths so it could be this. I'll inspect it now.

[simoncozens]

Start with looking at the build.ninja. Looks like an argument to genstat has become a JSON string instead of a filename.

[simoncozens]

OK, I've fixed the stat table issue, but there's another problem, which is that sometimes glyphs2ufo produces a designspace file with instance filenames like this:

  <instances>
    <instance name="Texturina Thin" familyname="Texturina" stylename="Thin" filename="instance_ufos/Texturina-Thin.ufo">

(Note "instance_ufos".)

When you run fontmake -o ttf -i -m on that designspace, all the generated instances end up getting placed in instance_ufo/.

I don't know if that's a glyphsLib bug or a fontmake bug or both. (@anthrotype?) I suppose we could probably work around it by passing -n instance_ufo to glyphs2ufo, but it's a bit ugly.

(Edit: Actually, we don't use glyphs2ufo directly, we use fontmake -o ufo -g, but it comes to the same thing: it writes instance_ufos and fontmake doesn't honour that.)

[simoncozens]

Boggle

$ grep "ExtraLight" master_ufo/Texturina-Italic.designspace | grep 72pt
    <instance name="Texturina 72pt ExtraLight Italic" familyname="Texturina 72pt" stylename="ExtraLight Italic" filename="instance_ufo/Texturina72pt-ExtraLightItalic.ufo" stylemapfamilyname="Texturina 72pt ExtraLight" stylemapstylename="italic">

$ fontmake -o ufo -i -m master_ufo/Texturina-Italic.designspace
...
INFO:fontmake.font_project:Generating instance UFO for "Texturina 72pt ExtraLight Italic"
...

$ ls -al instance_ufo/Texturina72pt-ExtraLightItalic.ufo
ls: cannot access 'instance_ufo/Texturina72pt-ExtraLightItalic.ufo': No such file or directory

[simoncozens]

And another issue: if we have a .designspace file where there are instances but no filenames defined (e.g. the Roboto Serif designspace), I don't know how to work out what the eventual UFO file names will be. (We need those as targets, so we can then add rules to convert them to TTF.)

[anthrotype]

if there's an instance filename attribute, it is meant to be interpreted as relative to the directory of the .designspace file itself. So in your example above (where you grep'ed for "ExtraLight" in Texturina), the instance has filename="instance_ufo/Texturina72pt-ExtraLightItalic.ufo" but the designspace file is inside master_ufo/Texturina-Italic.designspace, so the generated instances are going to be in.. master_ufo/instance_ufo/... fontmake is following what it's told, some other tool might have set those instance filename attributes incorrectly (glyphsLib?).

if we have a .designspace file where there are instances but no filenames defined (e.g. the Roboto Serif designspace), I don't know how to work out what the eventual UFO file names will be.

I'm fixing that in googlefonts/fontmake#102. If an instance does not have any filename attribute defined, we make up its name from the instance ufo's family/style name (similarly to how we make up the OTF/TTF output path from a UFO input). In addition to this, you will be able to pass --output-path if you are building one instance UFO (you know you can already filter which instance(s) to build with -i option which takes a regex matching instance.name attr). I hope that helps..

[anthrotype]

and you will also be able to pass --output-dir option to override in which directory the instance UFOs gets saved when building -i -o ufo

[simoncozens]

Well this is frustrating; coming back to try to fix it, I now can't make it fail in the way that I'm sure it used to. It correctly builds:

• Albert Sans
• Texturina
• League Spartan
• Nunito
• Questrial
• Inconsolata (with fallback to old builder)
• Silkscreen

So... it works?

[anthrotype]

frustrating but in a good way 😉

[m4rc1e]

Seems to be failing the builder tests. I put these in place so we don't cause regressions for The Type founders, #670

[simoncozens]

Thanks. It seems like it works for .glyphs and .designspace files but not for individual .ufos.

[simoncozens]

OK, apart from Windows which I'll disable because, well, Windows users shouldn't expect nice things shell syntax.

[madig]

FWIW, when fiddling with filename attributes with designspaceLib, set the filename on .filename and set .path = None. This will stop dsLib from trying to be clever.

[miloush]

Is there any notes about switching to ninja-builder? How can I build now with what used to be --no-autohint argument?

[simoncozens]

The switch to ninja builder happened over a year ago, and we've moved to builder2 since then - sorry to leave you behind! Arguments should be moved into the config.yaml file, so you should add

autohintTTF: false
autohintOTF: false

to your config file.

[miloush]

Ah ok no worries, thank you!