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

7 files changed, 266 insertions, 73 deletions

diff --git a/Makefile b/Makefile
index cbaa46f79..695678998 100644
--- a/Makefile
+++ b/Makefile
@@ -96,28 +96,72 @@ all_ufo_masters = $(Thin_ufo_d) \
 $(FONTDIR)/var/%.var.ttf: src/%.designspace $(all_ufo_masters)
 	misc/fontbuild compile-var -o $@ $<
 
-$(FONTDIR)/const/Inter-UI-Thin.%: src/Inter-UI.designspace $(Thin_ufo_d)
+
+# ---------- begin workaround for issue #110 -----------
+
+# $(FONTDIR)/const/Inter-UI-Thin.%: src/Inter-UI.designspace $(Thin_ufo_d)
+# 	misc/fontbuild compile -o $@ src/Inter-UI-Thin.ufo
+
+# $(FONTDIR)/const/Inter-UI-ThinItalic.%: src/Inter-UI.designspace $(ThinItalic_ufo_d)
+# 	misc/fontbuild compile -o $@ src/Inter-UI-ThinItalic.ufo
+
+# $(FONTDIR)/const/Inter-UI-Regular.%: src/Inter-UI.designspace $(Regular_ufo_d)
+# 	misc/fontbuild compile -o $@ src/Inter-UI-Regular.ufo
+
+# $(FONTDIR)/const/Inter-UI-Italic.%: src/Inter-UI.designspace $(Italic_ufo_d)
+# 	misc/fontbuild compile -o $@ src/Inter-UI-Italic.ufo
+
+# $(FONTDIR)/const/Inter-UI-Black.%: src/Inter-UI.designspace $(Black_ufo_d)
+# 	misc/fontbuild compile -o $@ src/Inter-UI-Black.ufo
+
+# $(FONTDIR)/const/Inter-UI-BlackItalic.%: src/Inter-UI.designspace $(BlackItalic_ufo_d)
+# 	misc/fontbuild compile -o $@ src/Inter-UI-BlackItalic.ufo
+
+
+$(FONTDIR)/const/Inter-UI-Thin.ttf: src/Inter-UI.designspace $(Thin_ufo_d)
 	misc/fontbuild compile -o $@ src/Inter-UI-Thin.ufo
 
-$(FONTDIR)/const/Inter-UI-ThinItalic.%: src/Inter-UI.designspace $(ThinItalic_ufo_d)
+$(FONTDIR)/const/Inter-UI-ThinItalic.ttf: src/Inter-UI.designspace $(ThinItalic_ufo_d)
 	misc/fontbuild compile -o $@ src/Inter-UI-ThinItalic.ufo
 
-$(FONTDIR)/const/Inter-UI-Regular.%: src/Inter-UI.designspace $(Regular_ufo_d)
+$(FONTDIR)/const/Inter-UI-Regular.ttf: src/Inter-UI.designspace $(Regular_ufo_d)
 	misc/fontbuild compile -o $@ src/Inter-UI-Regular.ufo
 
-$(FONTDIR)/const/Inter-UI-Italic.%: src/Inter-UI.designspace $(Italic_ufo_d)
+$(FONTDIR)/const/Inter-UI-Italic.ttf: src/Inter-UI.designspace $(Italic_ufo_d)
 	misc/fontbuild compile -o $@ src/Inter-UI-Italic.ufo
 
-$(FONTDIR)/const/Inter-UI-Black.%: src/Inter-UI.designspace $(Black_ufo_d)
+$(FONTDIR)/const/Inter-UI-Black.ttf: src/Inter-UI.designspace $(Black_ufo_d)
 	misc/fontbuild compile -o $@ src/Inter-UI-Black.ufo
 
-$(FONTDIR)/const/Inter-UI-BlackItalic.%: src/Inter-UI.designspace $(BlackItalic_ufo_d)
+$(FONTDIR)/const/Inter-UI-BlackItalic.ttf: src/Inter-UI.designspace $(BlackItalic_ufo_d)
 	misc/fontbuild compile -o $@ src/Inter-UI-BlackItalic.ufo
 
+
+$(FONTDIR)/const/Inter-UI-Thin.otf: src/Inter-UI.designspace $(Thin_ufo_d)
+	misc/fontbuild2 compile -o $@ src/Inter-UI-Thin.ufo
+
+$(FONTDIR)/const/Inter-UI-ThinItalic.otf: src/Inter-UI.designspace $(ThinItalic_ufo_d)
+	misc/fontbuild2 compile -o $@ src/Inter-UI-ThinItalic.ufo
+
+$(FONTDIR)/const/Inter-UI-Regular.otf: src/Inter-UI.designspace $(Regular_ufo_d)
+	misc/fontbuild2 compile -o $@ src/Inter-UI-Regular.ufo
+
+$(FONTDIR)/const/Inter-UI-Italic.otf: src/Inter-UI.designspace $(Italic_ufo_d)
+	misc/fontbuild2 compile -o $@ src/Inter-UI-Italic.ufo
+
+$(FONTDIR)/const/Inter-UI-Black.otf: src/Inter-UI.designspace $(Black_ufo_d)
+	misc/fontbuild2 compile -o $@ src/Inter-UI-Black.ufo
+
+$(FONTDIR)/const/Inter-UI-BlackItalic.otf: src/Inter-UI.designspace $(BlackItalic_ufo_d)
+	misc/fontbuild2 compile -o $@ src/Inter-UI-BlackItalic.ufo
+
+# ---------- end workaround for issue #110 -----------
+
+
 # Instance UFO -> OTF, TTF
 
 $(FONTDIR)/const/Inter-UI-%.otf: build/ufo/Inter-UI-%.ufo
-	misc/fontbuild compile -o $@ $<
+	misc/fontbuild2 compile -o $@ $<
 
 $(FONTDIR)/const/Inter-UI-%.ttf: build/ufo/Inter-UI-%.ufo
 	misc/fontbuild compile -o $@ $<
@@ -152,23 +196,53 @@ build/ufo/Inter-UI-%.ufo: src/Inter-UI.designspace $(all_ufo_masters)
 # hinted TTF files via autohint
 $(FONTDIR)/const-hinted/%.ttf: $(FONTDIR)/const/%.ttf
 	mkdir -p "$(dir $@)"
-	ttfautohint --fallback-stem-width=256 --no-info --composites "$<" "$@"
+	ttfautohint --fallback-stem-width=256 --no-info "$<" "$@"
 
 # $(FONTDIR)/var-hinted/%.ttf: $(FONTDIR)/var/%.ttf
 # 	mkdir -p "$(dir $@)"
-# 	ttfautohint --fallback-stem-width=256 --no-info --composites "$<" "$@"
+# 	ttfautohint --fallback-stem-width=256 --no-info "$<" "$@"
 
 # make sure intermediate TTFs are not gc'd by make
 .PRECIOUS: $(FONTDIR)/const/%.ttf $(FONTDIR)/const-hinted/%.ttf $(FONTDIR)/var/%.var.ttf
 
+
+
+
 # check var
 all_check_var: $(FONTDIR)/var/Inter-UI.var.ttf
-	misc/fontbuild checkfont $^
+	misc/fontbuild checkfont $(FONTDIR)/var/*.*
 
 # test runs all tests
 # Note: all_check_const is generated by init.sh and runs "fontbuild checkfont"
 # on all otf and ttf files.
 test: all_check_const  all_check_var
+	@echo "test: all ok"
+
+# check does the same thing as test, but without any dependency checks, meaning
+# it will check whatever font files are already built.
+check:
+	misc/fontbuild checkfont $(FONTDIR)/const/*.* $(FONTDIR)/var/*.*
+	@echo "check: all ok"
+
+.PHONY: test check
+
+
+
+
+# samples renders PDF and PNG samples
+samples: $(FONTDIR)/samples all_samples_pdf all_samples_png
+
+$(FONTDIR)/samples/%.pdf: $(FONTDIR)/const/%.otf
+	misc/tools/fontsample/fontsample -o "$@" "$<"
+
+$(FONTDIR)/samples/%.png: $(FONTDIR)/const/%.otf
+	misc/tools/fontsample/fontsample -o "$@" "$<"
+
+$(FONTDIR)/samples:
+	mkdir -p $@
+
+
+
 
 # load version, used by zip and dist
 VERSION := $(shell cat version.txt)
@@ -223,8 +297,14 @@ build/release/Inter-UI-%.zip: build/tmp/a.zip
 	@echo write "$@"
 	@sh -c "if [ -f /usr/bin/open ]; then /usr/bin/open --reveal '$@'; fi"
 
-zip: ${ZIP_FILE_DEV}
-zip_dist: pre_dist test ${ZIP_FILE_DIST}
+zip: all
+	$(MAKE) check
+	$(MAKE) ${ZIP_FILE_DEV}
+
+zip_dist: pre_dist all
+	$(MAKE) check
+	$(MAKE) ${ZIP_FILE_DIST}
+
 .PHONY: zip zip_dist
 
 # distribution
@@ -235,7 +315,8 @@ pre_dist:
 		exit 1; \
   fi
 
-dist: zip_dist  docs
+dist: zip_dist
+	$(MAKE) -j docs
 	misc/tools/versionize-css.py
 	@echo "——————————————————————————————————————————————————————————————————"
 	@echo ""
@@ -309,4 +390,4 @@ install: install_otf
 clean:
 	rm -rvf build/tmp build/fonts
 
-.PHONY: all web clean install install_otf install_ttf deploy pre_dist dist geninfo test glyphsync
+.PHONY: all web clean install install_otf install_ttf deploy pre_dist dist geninfo glyphsync
diff --git a/init.sh b/init.sh
index 2083dd9a7..5e702c676 100755
--- a/init.sh
+++ b/init.sh
@@ -41,7 +41,8 @@ else
     clean=true
   fi
 
-  # ————————————————————————————————————————————————————————————————————————————————————————————————
+
+  # ——————————————————————————————————————————————————————————————————
   # virtualenv
 
   mkdir -p "$VENV_DIR"
@@ -57,7 +58,7 @@ else
   fi
 
   require_virtualenv() {
-    # find pip
+    # find pip3 (Python 3)
     export pip=$(which pip3)
     if [ "$pip" = "" ]; then
       export pip=$(which pip)
@@ -83,7 +84,7 @@ else
   # TODO: allow setting a flag to recreate venv
   if $clean; then
     rm -rf "$VENV_DIR"
-  fi
+  fi 
 
   if [[ ! -d "$VENV_DIR/bin" ]]; then
     echo "Setting up virtualenv in '$VENV_DIR'"
@@ -107,20 +108,53 @@ else
     done
   fi
 
-  source "$VENV_DIR/bin/activate"
+  # --------------------
+  # ISSUE #110 (bug in ufo2ft) requires Python 2 for OTF CFF compilation
+  # See also: https://github.com/googlei18n/ufo2ft/issues/306
+  PYTHON2=
+  if (which python2 >/dev/null); then
+    PYTHON2=$(which python2)
+  elif (which python >/dev/null) && (python --version | grep -q 'ython 2'); then
+    PYTHON2=$(which python)
+  else
+    echo "Unable to find Python 2 (tried python2 and python)." >&2
+    echo "Python 2 is required to compile OTF files because of a Python-3 bug in a third-party library." >&2
+    exit 1
+  fi
+  # ln -svf "$PYTHON2" "$VENV_DIR/bin/python2"
+  VENV2_DIR=$BUILD_DIR/venv2
+  if [[ ! -d "$VENV2_DIR/bin" ]]; then
+    require_virtualenv
+    $virtualenv "--python=$PYTHON2" "$VENV2_DIR"
+  fi
+
+  # ——————————————————————————————————————————————————————————————————
+  # check pip requirements
 
-  UPDATE_TIMESTAMP_FILE="$VENV_DIR/last-pip-run.mark"
-  REQUIREMENTS_FILE=$SRCDIR/requirements.txt
-  PY_REQUIREMENTS_CHANGED=false
 
+  # primary env
+  REQUIREMENTS_FILE=$SRCDIR/requirements.txt
+  UPDATE_TIMESTAMP_FILE="$VENV_DIR/last-pip-run.mark"
   if [ "$REQUIREMENTS_FILE" -nt "$UPDATE_TIMESTAMP_FILE" ]; then
-    echo "pip install -r $REQUIREMENTS_FILE"
-    pip install -r "$REQUIREMENTS_FILE"
+    echo "$VENV_DIR/bin/pip install -r $REQUIREMENTS_FILE"
+    "$VENV_DIR/bin/pip" install -r "$REQUIREMENTS_FILE"
     date '+%s' > "$UPDATE_TIMESTAMP_FILE"
-    PY_REQUIREMENTS_CHANGED=true
   fi
 
-  # ————————————————————————————————————————————————————————————————————————————————————————————————
+  # secondary env (Python 2)
+  REQUIREMENTS2_FILE=$SRCDIR/requirements2.txt
+  UPDATE_TIMESTAMP2_FILE="$VENV2_DIR/last-pip-run.mark"
+  if [ "$REQUIREMENTS2_FILE" -nt "$UPDATE_TIMESTAMP2_FILE" ]; then
+    echo "$VENV2_DIR/bin/pip install -r $REQUIREMENTS2_FILE"
+    "$VENV2_DIR/bin/pip" install -r "$REQUIREMENTS2_FILE"
+    date '+%s' > "$UPDATE_TIMESTAMP2_FILE"
+  fi
+
+  # ——————————————————————————————————————————————————————————————————
+  # active primary env (Python 3)
+  source "$VENV_DIR/bin/activate"
+
+  # ——————————————————————————————————————————————————————————————————
   # deps
   DEPS_DIR=$BUILD_DIR/deps
   PATCH_DIR=$(pwd)/misc/patches
@@ -448,6 +482,20 @@ else
     done
     echo "" >> "$GEN_MAKE_FILE"
 
+    # all_samples_pdf target
+    echo -n "all_samples_pdf:" >> "$GEN_MAKE_FILE"
+    for style in "${all_styles[@]}"; do
+      echo -n " \$(FONTDIR)/samples/Inter-UI-${style}.pdf" >> "$GEN_MAKE_FILE"
+    done
+    echo "" >> "$GEN_MAKE_FILE"
+
+    # all_samples_png target
+    echo -n "all_samples_png:" >> "$GEN_MAKE_FILE"
+    for style in "${all_styles[@]}"; do
+      echo -n " \$(FONTDIR)/samples/Inter-UI-${style}.png" >> "$GEN_MAKE_FILE"
+    done
+    echo "" >> "$GEN_MAKE_FILE"
+
     # all_const_fonts target
     # echo -n "all_const_fonts:" >> "$GEN_MAKE_FILE"
     # for style in "${all_styles[@]}"; do
@@ -463,15 +511,15 @@ else
     echo "" >> "$GEN_MAKE_FILE"
   fi
 
-  # ————————————————————————————————————————————————————————————————————————————————————————————————
-  # summary
-  if ! $VENV_ACTIVE; then
-    echo -n "You can activate virtualenv by running "
-    if [ "$0" == "./init.sh" ]; then
-      # pretty format for common case
-      echo '`source init.sh`'
-    else
-      echo "\`source \"$0\"\`"
-    fi
-  fi
+  # # ————————————————————————————————————————————————————————————————
+  # # summary
+  # if ! $VENV_ACTIVE; then
+  #   echo -n "You can activate virtualenv by running "
+  #   if [ "$0" == "./init.sh" ]; then
+  #     # pretty format for common case
+  #     echo '`source init.sh`'
+  #   else
+  #     echo "\`source \"$0\"\`"
+  #   fi
+  # fi
 fi
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
 
 
diff --git a/requirements.txt b/requirements.txt
index 4aae4fb61..8c8580bbd 100644
--- a/requirements.txt
+++ b/requirements.txt
@@ -1,7 +1,9 @@
+glyphsLib==3.1.4
 skia-pathops==0.2.0.post2
 fontmake==1.8.0
+fs==2.2.0
 
 # for fontTools/varLib/interpolatable.py
 numpy==1.15.4
-scipy==1.1.0
+scipy==1.2.0
 munkres==1.0.12
diff --git a/requirements2.txt b/requirements2.txt
new file mode 100644
index 000000000..44db5e3fd
--- /dev/null
+++ b/requirements2.txt
@@ -0,0 +1,17 @@
+# for secondry env (Python 2)
+# should match requirements.txt except from the "Python 2 specific" section.
+
+glyphsLib==3.1.4
+skia-pathops==0.2.0.post2
+fontmake==1.8.0
+fs==2.2.0
+
+# for fontTools/varLib/interpolatable.py
+numpy==1.15.4
+scipy==1.2.0
+munkres==1.0.12
+
+# Python 2 specific
+enum34==1.1.6
+typing==3.6.6
+backports.os==0.1.1