source-foundry / Hack

A typeface designed for source code
http://sourcefoundry.org/hack/
Other
16.39k stars 613 forks source link

Update to Python 3.7 interpreter for build tooling #447

Closed chrissimpkins closed 5 years ago

chrissimpkins commented 6 years ago

This PR also updates all Python tooling in the toolchain to current release versions on PyPI.

chrissimpkins commented 6 years ago

This proposal merges to dev branch as part of the v4.000 release work in #427

chrissimpkins commented 6 years ago

Just received word from the Semaphore CI team that there is now support for Py 3.7 testing. Will attempt to fix these tests this week. This PR has been on hold as we waited for this CI testing support.

chrissimpkins commented 5 years ago

Initial attempt didn't fix this. Still working on it with the Semaphore CI team

chrissimpkins commented 5 years ago

Closing in favor of a new PR that only includes a Py37 interpreter upgrade. I bumped the Python build tooling dependencies here with pipenv and there is going to be some troubleshooting required to make that move. With the Python tooling bump, ttfautohint fails because it looks like there has been a conversion away from builds with glyph production names in at least some cases.

anthrotype commented 5 years ago

With the Python tooling bump, ttfautohint fails because it looks like there has been a conversion away from builds with glyph production names in at least some cases

See ufo2ft 2.4 release notes https://github.com/googlei18n/ufo2ft/releases/tag/v2.4.0 and https://github.com/googlei18n/fontmake/issues/465

anthrotype commented 5 years ago

if you relied on the previous behavior whereby all the glyphs where renamed to uniXXXX production names based on their unicode value (and your UFO lib.plist does not contain an explicit public.postscriptNames mapping), you can pass --production-names option to fontmake, or set com.github.googlei18n.ufo2ft.useProductionNames to True in UFO lib.plist. Or generate the public.postscriptNames mapping yourself and store it in the UFOs.

chrissimpkins commented 5 years ago

@anthrotype Thanks Cosimo. I think that the only place where this is required in the build process is at the ttfautohint control instructions files style assignments. We were using the previous default production names from the fontmake 1.5 compiles there. This will be simple to update. Probably simpler than figuring out why pipenv will not permit me to maintain a deterministic build tool set with a Python interpreter version bump. For some reason it is bumping versions of fontmake deps when I try this despite requests to leave them be. :)

chrissimpkins commented 5 years ago

@anthrotype is there a simple way to get the new glyph names in the binaries by unicode code point values (from the previous production names) with fonttools?

chrissimpkins commented 5 years ago

or do these come from the GlyphData.xml mapping?

anthrotype commented 5 years ago

The uniXXXX production names are generated by ufo2ft postProcessor based on the glyph's unicode values, they don't come from GlyphData.xml. The latter is used by glyphsLib to create the public.postcriptNames mapping, which is then applied by ufo2ft postProcessor, when it is present.

Anyway, as I said, if you were happy with the previous glyph production names, just call fontmake with the --production-names command line option, or set com.github.googlei18n.ufo2ft.useProductionNames to true in all the UFO lib.plist.

<key>com.github.googlei18n.ufo2ft.useProductionNames</key>
<true/>
anthrotype commented 5 years ago

sorry I misstyped the lib key:

https://github.com/googlei18n/ufo2ft/blob/010bc51f25060366f271a57a674931b6f69d6127/Lib/ufo2ft/constants.py#L4

the correct one is

com.github.googlei18n.ufo2ft.useProductionNames
chrissimpkins commented 5 years ago

@anthrotype I would prefer to convert to default behavior with fontmake and this will require modification of the ttfautohint control instruction files. I need to change the glyph names in those files to match what is in the binary after a build with fontmake. Is it safe to assume that this is the glyph name defined in *.glif files after this change in the compile tooling? If not, I need to know where the unicode : name map is or figure out a way to search for the glyph name by unicode hex value in the binary.

anthrotype commented 5 years ago

The default behaviour is no glyph renaming unless a public.postscriptNames mapping is present in lib.plist.

chrissimpkins commented 5 years ago

Thanks Cosimo. Appreciate all of your help!

chrissimpkins commented 5 years ago

Worked fine with the glyph name replacements in the control instructions files. It looks like there was a transition to ceil from floor round somewhere along the way between the versions of the tooling that we are using. Huge diffs. :)

anthrotype commented 5 years ago

It looks like there was a transition to ceil from floor somewhere along the way

Where do you see these diffs?

chrissimpkins commented 5 years ago

ttx dump of fonts built with previous Pipfile.lock and built with new Pipfile.lock.

anthrotype commented 5 years ago

can you be more specific? e.g. paste an example of such diffs?

chrissimpkins commented 5 years ago

and it looks like it is a change to ceil from round, not ceil from floor. Some X and Y vals in contour definitions are shifted up by 1 unit c/w previous build. Occurs infrequently relative to all definitions but there are a large number of the changes. Somewhere between 900-1000 per variant binary.

anthrotype commented 5 years ago

i believe it might be a change introduced in fonttools 3.28: https://github.com/fonttools/fonttools/releases/tag/3.28.0

[fixedTools] Added otRound to round floats to nearest integer towards positive Infinity. This is now used where we deal with visual data like X/Y coordinates, advance widths/heights, variation deltas, and similar (#1274, #1248).

https://github.com/fonttools/fonttools/issues/1248 https://github.com/fonttools/fonttools/pull/1274

The change is for the good, I recommend you go on with the update.

chrissimpkins commented 5 years ago

kldzq-image

anthrotype commented 5 years ago

yeah, looks good to me. read on in the linked issues above for more info

chrissimpkins commented 5 years ago

Will be reviewing the fonts visually before we release. This should not cause major problems. ttfautohint did pick up on the fact that things changed in rare cases across each of the font variants. There were < 10 instruction set changes in each file

chrissimpkins commented 5 years ago

lw6xn-image

chrissimpkins commented 5 years ago

yeah, looks good to me. read on in the linked issues above for more info

Thanks for taking a look!! Much appreciated