Workaround for Python 3 bug in ufo2ft which caused invalid OTF files to be generated. Closes #110

3 files changed, 81 insertions, 36 deletions

diff --git a/misc/fontbuild b/misc/fontbuild
index 34888cc83..cf5d21f68 100755
--- a/misc/fontbuild
+++ b/misc/fontbuild
@@ -4,7 +4,7 @@ from __future__ import print_function, absolute_import
 import sys, os
 from os.path import dirname, basename, abspath, relpath, join as pjoin
 sys.path.append(abspath(pjoin(dirname(__file__), 'tools')))
-from common import BASEDIR, VENVDIR, getGitHash, getVersion
+from common import BASEDIR, VENVDIR, getGitHash, getVersion, execproc
 
 import argparse
 import datetime
@@ -26,6 +26,7 @@ from glyphsLib.interpolation import apply_instance_data
 from mutatorMath.ufo.document import DesignSpaceDocumentReader
 from multiprocessing import Process, Queue
 from ufo2ft.filters.removeOverlaps import RemoveOverlapsFilter
+from ufo2ft import CFFOptimization
 
 log = logging.getLogger(__name__)
 stripItalic_re = re.compile(r'(?:^|\b)italic(?:\b|$)', re.I | re.U)
@@ -55,6 +56,7 @@ def fatal(msg):
   sys.exit(1)
 
 
+
 def composedGlyphIsNonTrivial(g, yAxisIsNonTrivial=False):
   # A non-trivial glyph is one that is composed from either multiple
   # components or that uses component transformations.
@@ -361,10 +363,10 @@ class Main(object):
       args.srcfile,
       interpolate=False,
       masters_as_instances=False,
+      use_production_names=True,
       round_instances=True,
       output_path=outfilename,
       output=['variable'],
-      optimize_cff=True,
       overlaps_backend='pathops', # use Skia's pathops
     )
 
@@ -426,21 +428,27 @@ class Main(object):
     ufo = Font(args.srcfile)
     updateFontVersion(ufo)
 
+    # if outfile is a ufo, simply move it to outfilename instead
+    # of running ots-sanitize.
+    output_path = tmpfilename
+    if formats[0] == 'ufo':
+      output_path = outfilename
+
     # run fontmake to produce OTF/TTF file at tmpfilename
     project.run_from_ufos(
       [ ufo ],
-      output_path=tmpfilename,
+      output_path=output_path,
       output=formats,
-      optimize_cff=True,
+      use_production_names=True,
+      optimize_cff=CFFOptimization.SUBROUTINIZE, # NONE
       overlaps_backend='pathops', # use Skia's pathops
+      remove_overlaps=True,
     )
 
-    # TODO: if outfile is a ufo, simply move it to outfilename instead
-    # of running ots-sanitize
-
-    # Run ots-sanitize on produced OTF/TTF file and write sanitized version
-    # to outfilename
-    self._ots_sanitize(tmpfilename, outfilename)
+    if output_path == tmpfilename:
+      # Run ots-sanitize on produced OTF/TTF file and write sanitized version
+      # to outfilename
+      self._ots_sanitize(output_path, outfilename)
 
 
   def _ots_sanitize(self, tmpfilename, outfilename):
@@ -592,7 +600,6 @@ class Main(object):
     # patch and write UFO files
     # TODO: Only write out-of-date UFOs
     procs = []
-    q = Queue()
     for source in designspace.sources:
       # source : fontTools.designspaceLib.SourceDescriptor
       # source.font : defcon.objects.font.Font
@@ -737,23 +744,24 @@ class Main(object):
       font.save()
 
 
+  def checkfont(self, fontfile, q):
+    res, ok = execproc('ots-idempotent', fontfile)
+    # Note: ots-idempotent does not exit with an error in many cases where
+    # it fails to sanitize the font, so we look at its output to determine
+    # success or failure.
+    if not ok or res.find('Failed') != -1:
+      log.error('%s: checkfont ots-idempotent: %s' % (fontfile, res))
+      q.put(False)
+      return
 
-  def checkfont(self, fontfile):
-    try:
-      res = subprocess.check_output(
-        ['ots-idempotent', fontfile],
-        shell=False
-      ).strip()
-      # Note: ots-idempotent does not exit with an error in many cases where
-      # it fails to sanitize the font.
-      if str(res).find('Failed') != -1:
-        log.error('[checkfont] ots-idempotent failed for %r: %s' % (
-          fontfile, res))
-        return False
-    except:
-      log.error('[checkfont] ots-idempotent failed for %r' % fontfile)
-      return False
-    return True
+    res, ok = execproc('ots-validator-checker', fontfile)
+    if not ok or res.find("didn't crash") == -1:
+      log.error('%s: checkfont ots-validator-checker: %s' % (fontfile, res))
+      q.put(False)
+      return
+
+    # ok
+    q.put(True)
 
 
   def cmd_checkfont(self, argv):
@@ -765,16 +773,23 @@ class Main(object):
       help='Font files')
     
     args = argparser.parse_args(argv)
+    q = Queue()
+
+    if len(args.files) < 2:
+      self.checkfont(args.files[0], q)
+    else:
+      procs = []
+      for fontfile in args.files:
+        p = Process(target=self.checkfont, args=(fontfile, q))
+        p.start()
+        procs.append(p)
+      for p in procs:
+        p.join()
 
     for fontfile in args.files:
-      if not self.checkfont(fontfile):
+      if not q.get():
         sys.exit(1)
 
-    # could use from multiprocessing import Pool
-    # p = Pool(8)
-    # p.map(self.checkfont, args.files)
-    # p.terminate()
-
 
 if __name__ == '__main__':
   Main().main(sys.argv)
diff --git a/misc/fontbuild2 b/misc/fontbuild2
new file mode 100755
index 000000000..bece1063c
--- /dev/null
+++ b/misc/fontbuild2
@@ -0,0 +1,6 @@
+#!/bin/bash
+pushd "$(dirname "$0")/.." >/dev/null
+SRCDIR=$(PWD)
+popd >/dev/null
+
+"$SRCDIR/build/venv2/bin/python" misc/fontbuild "$@"
diff --git a/misc/tools/common.py b/misc/tools/common.py
index d53c8e755..132558c2b 100644
--- a/misc/tools/common.py
+++ b/misc/tools/common.py
@@ -10,6 +10,30 @@ BASEDIR = abspath(pjoin(dirname(__file__), os.pardir, os.pardir))
 VENVDIR = pjoin(BASEDIR, 'build', 'venv')
 sys.path.append(pjoin(VENVDIR, 'lib', 'python', 'site-packages'))
 
+PYVER = sys.version_info[0]
+
+
+_enc_kwargs = {}
+if PYVER >= 3:
+  _enc_kwargs = {'encoding': 'utf-8'}
+
+
+# Returns (output :str, success :bool)
+def execproc(*args):
+  p = subprocess.run(
+    args,
+    shell=False,
+    stdout=subprocess.PIPE,
+    stderr=subprocess.STDOUT,
+    **_enc_kwargs
+  )
+  return (p.stdout.strip(), p.returncode == 0)
+
+
+def readTextFile(filename):
+  with open(filename, 'r', **_enc_kwargs) as f:
+    return f.read()
+
 
 _gitHash = None
 def getGitHash():
@@ -19,7 +43,8 @@ def getGitHash():
     try:
       _gitHash = subprocess.check_output(
         ['git', '-C', BASEDIR, 'rev-parse', '--short', 'HEAD'],
-        shell=False
+        shell=False,
+        **_enc_kwargs
       ).strip()
     except:
       pass
@@ -30,8 +55,7 @@ _version = None
 def getVersion():
   global _version
   if _version is None:
-    with open(pjoin(BASEDIR, 'version.txt'), 'r') as f:
-      _version = f.read().strip()
+    _version = readTextFile(pjoin(BASEDIR, 'version.txt')).strip()
   return _version