Repository: googlefonts/ufo2ft
Pull request #677 (2022-11-08 14:06:25)

Improve "no default source" error
Currently when I have designspace or axis mapping issues, fontmake dies like this:

```
fontmake: Error: In 'sources/sources-glyphs/Literata-Upright.glyphs' -> 'master_ufo/Literata.designspace':
Generating fonts from Designspace failed: No default source
```

This is normally followed by swearing and grovelling through `.designspace` XML files, which is not a pleasant experience. With this PR, fontmake dies like this:

```
fontmake: Error: In 'sources/sources-glyphs/Literata-Upright.glyphs' -> 'master_ufo/Literata.designspace':
Generating fonts from Designspace failed: No default source; expected default master at Weight=631.111,
Optical size=10. Found master locations:
Literata 12 pt Regular at Weight=400, Optical size=10
Literata 7pt Regular at Weight=400, Optical size=6
Literata 72pt Regular at Weight=400, Optical size=144
Literata M3 at Weight=200, Optical size=10
Literata M4 at Weight=900, Optical size=10
Literata 72pt ExtraLight at Weight=200, Optical size=144
Literata 72pt Black at Weight=900, Optical size=144
Literata M7 at Weight=900, Optical size=6
```

Which at least tells me immediately that there's something gone wrong with the weight axis map, and saves me looking up all the source locations.


Patch:
diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml
index f6c52e7..35291c3 100644
--- a/.github/workflows/ci.yml
+++ b/.github/workflows/ci.yml
@@ -27,7 +27,7 @@ jobs:
     if: "! contains(toJSON(github.event.commits.*.message), '[skip ci]')"
     strategy:
       matrix:
-        python-version: [3.7, 3.9]
+        python-version: [3.8, 3.11]
         platform: [ubuntu-latest, windows-latest]
     steps:
     - uses: actions/checkout@v2
diff --git a/Lib/ufo2ft/__init__.py b/Lib/ufo2ft/__init__.py
index d7c70ae..a08bb24 100644
--- a/Lib/ufo2ft/__init__.py
+++ b/Lib/ufo2ft/__init__.py
@@ -26,6 +26,7 @@ from ufo2ft.util import (
     _getDefaultNotdefGlyph,
     ensure_all_sources_have_names,
     init_kwargs,
+    location_to_string,
     prune_unknown_kwargs,
 )
 
@@ -765,7 +766,19 @@ def _compileNeededSources(
                 continue
             default_source = vfDoc.findDefault()
             if default_source is None:
-                raise InvalidDesignSpaceData("No default source.")
+                default_location = location_to_string(vfDoc.newDefaultLocation())
+                master_locations = []
+                for sourceDescriptor in vfDoc.sources:
+                    master_location = sourceDescriptor.name + " at "
+                    master_location += location_to_string(
+                        sourceDescriptor.getFullDesignLocation(vfDoc)
+                    )
+                    master_locations.append(master_location)
+                master_location_descriptions = "\n".join(master_locations)
+                raise InvalidDesignSpaceData(
+                    f"No default source; expected default master at {default_location}."
+                    f" Found master locations:\n{master_location_descriptions}"
+                )
             vfNameToBaseUfo[vfName] = default_source.font
             for source in vfDoc.sources:
                 sourcesToCompile.add(source.name)
diff --git a/Lib/ufo2ft/featureCompiler.py b/Lib/ufo2ft/featureCompiler.py
index ae35adc..964ee08 100644
--- a/Lib/ufo2ft/featureCompiler.py
+++ b/Lib/ufo2ft/featureCompiler.py
@@ -1,5 +1,8 @@
+from __future__ import annotations
+
 import logging
 import os
+import re
 from collections import OrderedDict
 from inspect import isclass
 from io import StringIO
@@ -21,6 +24,7 @@ from ufo2ft.featureWriters import (
     isValidFeatureWriter,
     loadFeatureWriters,
 )
+from ufo2ft.util import describe_ufo
 
 logger = logging.getLogger(__name__)
 timer = Timer(logging.getLogger("ufo2ft.timer"), level=logging.DEBUG)
@@ -281,6 +285,17 @@ class FeatureCompiler(BaseFeatureCompiler):
             if self.featureWriters:
                 featureFile = parseLayoutFeatures(self.ufo, self.feaIncludeDir)
 
+                # Insertion markers are only considered in "skip" mode.
+                if any(writer.mode == "skip" for writer in self.featureWriters):
+                    markers = {
+                        writer.insertFeatureMarker
+                        for writer in self.featureWriters
+                        if writer.insertFeatureMarker is not None
+                    }
+                    warn_about_miscased_insertion_markers(
+                        describe_ufo(self.ufo), featureFile, markers
+                    )
+
                 path = self.ufo.path
                 for writer in self.featureWriters:
                     try:
@@ -354,3 +369,33 @@ class MtiFeatureCompiler(BaseFeatureCompiler):
             table = mtiLib.build(features.splitlines(), self.ttFont)
             assert table.tableTag == tag
             self.ttFont[tag] = table
+
+
+def warn_about_miscased_insertion_markers(
+    ufo_description: str, feaFile: ast.FeatureFile, patterns: set[str]
+) -> None:
+    """Warn the user about potentially mistyped feature insertion markers."""
+
+    patterns_compiled = tuple(
+        (re.compile(pattern), re.compile(pattern, re.IGNORECASE))
+        for pattern in patterns
+    )
+
+    # NOTE: Insertion markers can only meaningfully occur in top-level feature
+    # blocks.
+    for block in ast.iterFeatureBlocks(feaFile):
+        for statement in block.statements:
+            if not isinstance(statement, ast.Comment):
+                continue
+            for pattern_case, pattern_ignore_case in patterns_compiled:
+                text = str(statement)
+                match_case = re.match(pattern_case, text)
+                match_ignore_case = re.match(pattern_ignore_case, text)
+                if match_ignore_case and not match_case:
+                    logger.warning(
+                        "%s: The insertion comment '%s' in the feature file is "
+                        "miscased (search pattern: %s), ignoring it.",
+                        ufo_description,
+                        text,
+                        pattern_case.pattern,
+                    )
diff --git a/Lib/ufo2ft/featureWriters/baseFeatureWriter.py b/Lib/ufo2ft/featureWriters/baseFeatureWriter.py
index ba79d38..c42e102 100644
--- a/Lib/ufo2ft/featureWriters/baseFeatureWriter.py
+++ b/Lib/ufo2ft/featureWriters/baseFeatureWriter.py
@@ -80,11 +80,13 @@ class BaseFeatureWriter:
           instantiated from a FeatureCompiler);
         - a set of features (tags) to be generated. If self.mode is "skip",
           these are all the features which are _not_ already present.
+        - a set of all existing features (tags) in the feaFile.
 
         Returns the context namespace instance.
         """
         todo = set(self.features)
         insertComments = None
+        existing = set()
         if self.mode == "skip":
             if self.insertFeatureMarker is not None:
                 insertComments = self.collectInsertMarkers(
@@ -104,6 +106,7 @@ class BaseFeatureWriter:
             compiler=compiler,
             todo=todo,
             insertComments=insertComments,
+            existingFeatures=existing,
         )
 
         return self.context
diff --git a/Lib/ufo2ft/featureWriters/kernFeatureWriter.py b/Lib/ufo2ft/featureWriters/kernFeatureWriter.py
index b4c42a0..af3b080 100644
--- a/Lib/ufo2ft/featureWriters/kernFeatureWriter.py
+++ b/Lib/ufo2ft/featureWriters/kernFeatureWriter.py
@@ -11,7 +11,13 @@ from fontTools.unicodedata import script_horizontal_direction
 
 from ufo2ft.constants import COMMON_SCRIPT, INDIC_SCRIPTS, USE_SCRIPTS
 from ufo2ft.featureWriters import BaseFeatureWriter, ast
-from ufo2ft.util import DFLT_SCRIPTS, classifyGlyphs, quantize, unicodeScriptExtensions
+from ufo2ft.util import (
+    DFLT_SCRIPTS,
+    classifyGlyphs,
+    describe_ufo,
+    quantize,
+    unicodeScriptExtensions,
+)
 
 LOGGER = logging.getLogger(__name__)
 
@@ -198,6 +204,23 @@ class KernFeatureWriter(BaseFeatureWriter):
         ctx.gdefClasses = self.getGDEFGlyphClasses()
         ctx.glyphSet = self.getOrderedGlyphSet()
 
+        # Unless we use the legacy append mode (which ignores insertion
+        # markers), if the font contains kerning and the feaFile contains `kern`
+        # or `dist` feature blocks, but we have no insertion markers (or they
+        # were misspelt and ignored), warn the user that the kerning blocks in
+        # the feaFile take precedence and other kerning is dropped.
+        if (
+            self.mode == "skip"
+            and font.kerning
+            and ctx.existingFeatures & self.features
+            and not ctx.insertComments
+        ):
+            LOGGER.warning(
+                "%s: font has kerning, but also manually written kerning features "
+                "without an insertion comment. Dropping the former.",
+                describe_ufo(font),
+            )
+
         # Remember which languages are defined for which OT tag, as all
         # generated kerning needs to be registered for the script's `dflt`
         # language, but also all those the designer defined manually. Otherwise,
diff --git a/Lib/ufo2ft/outlineCompiler.py b/Lib/ufo2ft/outlineCompiler.py
index 8466b00..8f21fef 100644
--- a/Lib/ufo2ft/outlineCompiler.py
+++ b/Lib/ufo2ft/outlineCompiler.py
@@ -382,20 +382,6 @@ class BaseOutlineCompiler:
         self.otf["name"] = name = newTable("name")
         name.names = []
 
-        # Set name records from font.info.openTypeNameRecords
-        for nameRecord in getAttrWithFallback(font.info, "openTypeNameRecords"):
-            nameId = nameRecord["nameID"]
-            platformId = nameRecord["platformID"]
-            platEncId = nameRecord["encodingID"]
-            langId = nameRecord["languageID"]
-            # on Python 2, plistLib (used by ufoLib) returns unicode strings
-            # only when plist data contain non-ascii characters, and returns
-            # ascii-encoded bytes when it can. On the other hand, fontTools's
-            # name table `setName` method wants unicode strings, so we must
-            # decode them first
-            nameVal = nameRecord["string"]
-            name.setName(nameVal, nameId, platformId, platEncId, langId)
-
         # Build name records
         familyName = getAttrWithFallback(font.info, "styleMapFamilyName")
         styleName = getAttrWithFallback(font.info, "styleMapStyleName").title()
@@ -452,6 +438,15 @@ class BaseOutlineCompiler:
                 continue
             name.setName(nameVal, nameId, platformId, platEncId, langId)
 
+        # Set name records from font.info.openTypeNameRecords
+        for nameRecord in getAttrWithFallback(font.info, "openTypeNameRecords"):
+            nameId = nameRecord["nameID"]
+            platformId = nameRecord["platformID"]
+            platEncId = nameRecord["encodingID"]
+            langId = nameRecord["languageID"]
+            nameVal = nameRecord["string"]
+            name.setName(nameVal, nameId, platformId, platEncId, langId)
+
     def setupTable_maxp(self):
         """
         Make the maxp table.
@@ -561,12 +556,7 @@ class BaseOutlineCompiler:
         font = self.ufo
         os2.version = 0x0004
         # average glyph width
-        os2.xAvgCharWidth = 0
-        hmtx = self.otf.get("hmtx")
-        if hmtx is not None:
-            widths = [width for width, _ in hmtx.metrics.values() if width > 0]
-            if widths:
-                os2.xAvgCharWidth = otRound(sum(widths) / len(widths))
+        os2.recalcAvgCharWidth(self.otf)
         # weight and width classes
         os2.usWeightClass = getAttrWithFallback(font.info, "openTypeOS2WeightClass")
         os2.usWidthClass = getAttrWithFallback(font.info, "openTypeOS2WidthClass")
diff --git a/Lib/ufo2ft/util.py b/Lib/ufo2ft/util.py
index c91f90e..f6879a4 100644
--- a/Lib/ufo2ft/util.py
+++ b/Lib/ufo2ft/util.py
@@ -5,7 +5,7 @@ import logging
 import re
 from copy import deepcopy
 from inspect import currentframe, getfullargspec
-from typing import Mapping, Set
+from typing import Any, Mapping, Set
 
 from fontTools import subset, ttLib, unicodedata
 from fontTools.designspaceLib import DesignSpaceDocument
@@ -601,6 +601,12 @@ def getMaxComponentDepth(glyph, glyphSet, maxComponentDepth=0):
     return maxComponentDepth
 
 
+def location_to_string(location):
+    """Reports a designspace location (dictionary mapping axis:loc)
+    in a user-friendly way"""
+    return ", ".join([f"{axis}={loc:g}" for axis, loc in location.items()])
+
+
 def unicodeScriptExtensions(
     codepoint: int, aliases: Mapping[str, str] = UNICODE_SCRIPT_ALIASES
 ) -> set[str]:
@@ -612,3 +618,16 @@ def unicodeScriptExtensions(
     defines "Hrkt" as an alias for both scripts.
     """
     return {aliases.get(s, s) for s in unicodedata.script_extension(chr(codepoint))}
+
+
+def describe_ufo(ufo: Any) -> str:
+    """Returns a description of a UFO suitable for logging."""
+    if (
+        hasattr(ufo, "reader")
+        and hasattr(ufo.reader, "fs")
+        and hasattr(ufo.reader.fs, "root_path")
+    ):
+        return ufo.reader.fs.root_path
+    elif ufo.info.familyName or ufo.info.styleName:
+        return " ".join(n for n in (ufo.info.familyName, ufo.info.styleName) if n)
+    return repr(ufo)
diff --git a/requirements.txt b/requirements.txt
index d090d96..d48e742 100644
--- a/requirements.txt
+++ b/requirements.txt
@@ -1,4 +1,4 @@
-fonttools[lxml,ufo]==4.38.0
+fonttools[lxml,ufo]==4.39.2
 defcon==0.10.2
 cu2qu==1.6.7.post2
 compreffor==0.5.3
diff --git a/setup.py b/setup.py
index e21e1a3..48aede5 100644
--- a/setup.py
+++ b/setup.py
@@ -29,7 +29,7 @@ setup(
     setup_requires=pytest_runner + wheel + ["setuptools_scm"],
     tests_require=["pytest>=2.8"],
     install_requires=[
-        "fonttools[ufo]>=4.38.0",
+        "fonttools[ufo]>=4.39.2",
         "cu2qu>=1.6.7",
         "cffsubr>=0.2.8",
         "booleanOperations>=0.9.0",
@@ -39,7 +39,7 @@ setup(
         "cffsubr": [],  # keep empty for backward compat
         "compreffor": ["compreffor>=0.4.6"],
     },
-    python_requires=">=3.7",
+    python_requires=">=3.8",
     classifiers=[
         "Development Status :: 4 - Beta",
         "Environment :: Console",
diff --git a/tox.ini b/tox.ini
index a8f7977..fed40c4 100644
--- a/tox.ini
+++ b/tox.ini
@@ -1,5 +1,5 @@
 [tox]
-envlist = lint, py3{7,8,9}-cov, htmlcov
+envlist = lint, py3{8,9,10,11}-cov, htmlcov
 skip_missing_interpreters = true
 
 [testenv]


Test patch:
diff --git a/tests/data/DSv5/MutatorSansVariable_Weight_Width-CFF2.ttx b/tests/data/DSv5/MutatorSansVariable_Weight_Width-CFF2.ttx
index d443149..e621dea 100644
--- a/tests/data/DSv5/MutatorSansVariable_Weight_Width-CFF2.ttx
+++ b/tests/data/DSv5/MutatorSansVariable_Weight_Width-CFF2.ttx
@@ -897,6 +897,7 @@
   </STAT>
 
   <avar>
+    <version major="1" minor="0"/>
     <segment axis="wght">
       <mapping from="-1.0" to="-1.0"/>
       <mapping from="0.0" to="0.0"/>
diff --git a/tests/data/DSv5/MutatorSansVariable_Weight_Width-TTF.ttx b/tests/data/DSv5/MutatorSansVariable_Weight_Width-TTF.ttx
index ce05770..3dee6e6 100644
--- a/tests/data/DSv5/MutatorSansVariable_Weight_Width-TTF.ttx
+++ b/tests/data/DSv5/MutatorSansVariable_Weight_Width-TTF.ttx
@@ -808,6 +808,7 @@
   </STAT>
 
   <avar>
+    <version major="1" minor="0"/>
     <segment axis="wght">
       <mapping from="-1.0" to="-1.0"/>
       <mapping from="0.0" to="0.0"/>
diff --git a/tests/data/DSv5/MutatorSansVariable_Width-CFF2.ttx b/tests/data/DSv5/MutatorSansVariable_Width-CFF2.ttx
index 653d42f..fb87fb5 100644
--- a/tests/data/DSv5/MutatorSansVariable_Width-CFF2.ttx
+++ b/tests/data/DSv5/MutatorSansVariable_Width-CFF2.ttx
@@ -525,6 +525,7 @@
   </STAT>
 
   <avar>
+    <version major="1" minor="0"/>
     <segment axis="wdth">
       <mapping from="-1.0" to="-1.0"/>
       <mapping from="0.0" to="0.0"/>
diff --git a/tests/data/DSv5/MutatorSansVariable_Width-TTF.ttx b/tests/data/DSv5/MutatorSansVariable_Width-TTF.ttx
index 73df358..9e4a525 100644
--- a/tests/data/DSv5/MutatorSansVariable_Width-TTF.ttx
+++ b/tests/data/DSv5/MutatorSansVariable_Width-TTF.ttx
@@ -539,6 +539,7 @@
   </STAT>
 
   <avar>
+    <version major="1" minor="0"/>
     <segment axis="wdth">
       <mapping from="-1.0" to="-1.0"/>
       <mapping from="0.0" to="0.0"/>
diff --git a/tests/data/DSv5/MutatorSerifVariable_Width-CFF2.ttx b/tests/data/DSv5/MutatorSerifVariable_Width-CFF2.ttx
index 5f1cc8c..d04c93c 100644
--- a/tests/data/DSv5/MutatorSerifVariable_Width-CFF2.ttx
+++ b/tests/data/DSv5/MutatorSerifVariable_Width-CFF2.ttx
@@ -434,6 +434,7 @@
   </STAT>
 
   <avar>
+    <version major="1" minor="0"/>
     <segment axis="wdth">
       <mapping from="-1.0" to="-1.0"/>
       <mapping from="0.0" to="0.0"/>
diff --git a/tests/data/DSv5/MutatorSerifVariable_Width-TTF.ttx b/tests/data/DSv5/MutatorSerifVariable_Width-TTF.ttx
index 634b241..090c5eb 100644
--- a/tests/data/DSv5/MutatorSerifVariable_Width-TTF.ttx
+++ b/tests/data/DSv5/MutatorSerifVariable_Width-TTF.ttx
@@ -463,6 +463,7 @@
   </STAT>
 
   <avar>
+    <version major="1" minor="0"/>
     <segment axis="wdth">
       <mapping from="-1.0" to="-1.0"/>
       <mapping from="0.0" to="0.0"/>
diff --git a/tests/featureWriters/kernFeatureWriter_test.py b/tests/featureWriters/kernFeatureWriter_test.py
index d99258b..39eed37 100644
--- a/tests/featureWriters/kernFeatureWriter_test.py
+++ b/tests/featureWriters/kernFeatureWriter_test.py
@@ -7,7 +7,7 @@ from fontTools import unicodedata
 
 from ufo2ft.constants import UNICODE_SCRIPT_ALIASES
 from ufo2ft.errors import InvalidFeaturesData
-from ufo2ft.featureCompiler import parseLayoutFeatures
+from ufo2ft.featureCompiler import FeatureCompiler, parseLayoutFeatures
 from ufo2ft.featureWriters import KernFeatureWriter, ast
 from ufo2ft.util import DFLT_SCRIPTS, unicodeScriptExtensions
 
@@ -332,6 +332,42 @@ class KernFeatureWriterTest(FeatureWriterTest):
             """
         )
 
+    def test_comment_wrong_case_or_missing(self, FontClass, caplog):
+        ufo = FontClass()
+        for name in ("a", "b"):
+            ufo.newGlyph(name)
+        ufo.kerning.update({("a", "b"): 25.0})
+        ufo.features.text = dedent(
+            """
+            feature kern {
+                # Automatic code
+            } kern;
+            """
+        ).strip()
+
+        with caplog.at_level(logging.WARNING):
+            compiler = FeatureCompiler(ufo, featureWriters=[KernFeatureWriter])
+            font = compiler.compile()
+
+        # We mis-cased the insertion marker above, so it's ignored and we end up
+        # with an empty `kern` block overriding the other kerning in the font
+        # source and therefore no `GPOS` table.
+        assert "miscased" in caplog.text
+        assert "Dropping the former" in caplog.text
+        assert "GPOS" not in font
+
+        # Append mode ignores insertion markers and so should not log warnings
+        # and have kerning in the final font.
+        caplog.clear()
+        with caplog.at_level(logging.WARNING):
+            compiler = FeatureCompiler(
+                ufo, featureWriters=[KernFeatureWriter(mode="append")]
+            )
+            font = compiler.compile()
+
+        assert not caplog.text
+        assert "GPOS" in font
+
     def test_insert_comment_before_extended(self, FontClass):
         ufo = FontClass()
         for name in ("one", "four", "six", "seven"):
diff --git a/tests/outlineCompiler_test.py b/tests/outlineCompiler_test.py
index b31319c..50f3f4c 100644
--- a/tests/outlineCompiler_test.py
+++ b/tests/outlineCompiler_test.py
@@ -228,6 +228,26 @@ class OutlineTTFCompilerTest:
         assert meta.data["PRIA"] == b"Some private ascii string"
         assert meta.data["PRIU"] == "Some private unicode string…".encode("utf-8")
 
+    def test_setupTable_name(self, testufo):
+        compiler = OutlineTTFCompiler(testufo)
+        compiler.compile()
+        actual = compiler.otf["name"].getName(1, 3, 1, 1033).string
+        assert actual == "Some Font Regular (Style Map Family Name)"
+
+        testufo.info.openTypeNameRecords.append(
+            {
+                "nameID": 1,
+                "platformID": 3,
+                "encodingID": 1,
+                "languageID": 1033,
+                "string": "Custom Name for Windows",
+            }
+        )
+        compiler = OutlineTTFCompiler(testufo)
+        compiler.compile()
+        actual = compiler.otf["name"].getName(1, 3, 1, 1033).string
+        assert actual == "Custom Name for Windows"
+
 
 class OutlineOTFCompilerTest:
     def test_setupTable_CFF_all_blues_defined(self, testufo):