diff --git a/Lib/fontTools/config/__init__.py b/Lib/fontTools/config/__init__.py index 3877742e7..f5a62eaf1 100644 --- a/Lib/fontTools/config/__init__.py +++ b/Lib/fontTools/config/__init__.py @@ -49,9 +49,11 @@ Config.register_option( FontTools tries to use the HarfBuzz Repacker to serialize GPOS/GSUB tables if the uharfbuzz python bindings are importable, otherwise falls back to its slower, less efficient serializer. Set to False to always use the latter. + Set to True to explicitly request the HarfBuzz Repacker (will raise an + error if uharfbuzz cannot be imported). """ ), - default=True, - parse=lambda s: str(s).lower() not in {"0", "no", "false"}, - validate=lambda v: isinstance(v, bool), + default=None, + parse=Option.parse_optional_bool, + validate=Option.validate_optional_bool, ) diff --git a/Lib/fontTools/misc/configTools.py b/Lib/fontTools/misc/configTools.py index dbbd56ce1..38bbada24 100644 --- a/Lib/fontTools/misc/configTools.py +++ b/Lib/fontTools/misc/configTools.py @@ -100,6 +100,21 @@ class Option: validate: Optional[Callable[[Any], bool]] = None """Return true if the given value is an acceptable value.""" + @staticmethod + def parse_optional_bool(v: str) -> Optional[bool]: + s = str(v).lower() + if s in {"0", "no", "false"}: + return False + if s in {"1", "yes", "true"}: + return True + if s in {"auto", "none"}: + return None + raise ValueError("invalid optional bool: {v!r}") + + @staticmethod + def validate_optional_bool(v: Any) -> bool: + return v is None or isinstance(v, bool) + class Options(Mapping): """Registry of available options for a given config system. diff --git a/Lib/fontTools/subset/__init__.py b/Lib/fontTools/subset/__init__.py index 0e2ab898c..ed47c7ceb 100644 --- a/Lib/fontTools/subset/__init__.py +++ b/Lib/fontTools/subset/__init__.py @@ -146,9 +146,11 @@ Output options The Zopfli Python bindings are available at: https://pypi.python.org/pypi/zopfli ---harfbuzz-repacker [default] - Serialize GPOS/GSUB using the HarfBuzz Repacker if uharfbuzz can be - imported [default]. +--harfbuzz-repacker + By default, we serialize GPOS/GSUB using the HarfBuzz Repacker when + uharfbuzz can be imported and is successful, otherwise fall back to + the pure-python serializer. Set the option to force using the HarfBuzz + Repacker (raises an error if uharfbuzz can't be found or fails). --no-harfbuzz-repacker Always use the pure-python serializer even if uharfbuzz is available. diff --git a/Lib/fontTools/ttLib/tables/otBase.py b/Lib/fontTools/ttLib/tables/otBase.py index 64e70ffe7..a99d55997 100644 --- a/Lib/fontTools/ttLib/tables/otBase.py +++ b/Lib/fontTools/ttLib/tables/otBase.py @@ -76,35 +76,49 @@ class BaseTTXConverter(DefaultTable): # If a lookup subtable overflows an offset, we have to start all over. overflowRecord = None + # this is 3-state option: default (None) means automatically use hb.repack or + # silently fall back if it fails; True, use it and raise error if not possible + # or it errors out; False, don't use it, even if you can. use_hb_repack = font.cfg[USE_HARFBUZZ_REPACKER] if self.tableTag in ("GSUB", "GPOS"): - if not use_hb_repack: + if use_hb_repack is False: log.debug( "hb.repack disabled, compiling '%s' with pure-python serializer", self.tableTag, ) elif not have_uharfbuzz: - log.debug( - "uharfbuzz not found, compiling '%s' with pure-python serializer", - self.tableTag, - ) + if use_hb_repack is True: + raise ImportError("No module named 'uharfbuzz'") + else: + assert use_hb_repack is None + log.debug( + "uharfbuzz not found, compiling '%s' with pure-python serializer", + self.tableTag, + ) while True: try: writer = OTTableWriter(tableTag=self.tableTag) self.table.compile(writer, font) - if use_hb_repack and have_uharfbuzz and self.tableTag in ("GSUB", "GPOS"): + if ( + use_hb_repack in (None, True) + and have_uharfbuzz + and self.tableTag in ("GSUB", "GPOS") + ): try: log.debug("serializing '%s' with hb.repack", self.tableTag) return writer.getAllDataUsingHarfbuzz() except (ValueError, MemoryError, hb.RepackerError) as e: - log.error( - "hb.repack failed to serialize '%s', reverting to " - "pure-python serializer; the error message was: %s", - self.tableTag, - e, - ) - return writer.getAllData(remove_duplicate=False) + if use_hb_repack is None: + log.error( + "hb.repack failed to serialize '%s', reverting to " + "pure-python serializer; the error message was: %s", + self.tableTag, + e, + ) + return writer.getAllData(remove_duplicate=False) + # let the error propagate if USE_HARFBUZZ_REPACKER is True + raise return writer.getAllData() except OTLOffsetOverflowError as e: diff --git a/Tests/misc/configTools_test.py b/Tests/misc/configTools_test.py index 844688618..94afb233d 100644 --- a/Tests/misc/configTools_test.py +++ b/Tests/misc/configTools_test.py @@ -57,3 +57,24 @@ def test_options_are_unique(): cfg.get(opt2) with pytest.raises(ConfigUnknownOptionError): cfg.set(opt2, "bar") + + +def test_optional_bool(): + for v in ("yes", "YES", "Yes", "1", "True", "true", "TRUE"): + assert Option.parse_optional_bool(v) is True + + for v in ("no", "NO", "No", "0", "False", "false", "FALSE"): + assert Option.parse_optional_bool(v) is False + + for v in ("auto", "AUTO", "Auto", "None", "none", "NONE"): + assert Option.parse_optional_bool(v) is None + + with pytest.raises(ValueError, match="invalid optional bool"): + Option.parse_optional_bool("foobar") + + assert Option.validate_optional_bool(True) + assert Option.validate_optional_bool(False) + assert Option.validate_optional_bool(None) + assert not Option.validate_optional_bool(1) + assert not Option.validate_optional_bool(0) + assert not Option.validate_optional_bool("1") diff --git a/Tests/subset/subset_test.py b/Tests/subset/subset_test.py index ae9be7e2d..a67a759fa 100644 --- a/Tests/subset/subset_test.py +++ b/Tests/subset/subset_test.py @@ -802,6 +802,8 @@ class SubsetTest: @pytest.mark.parametrize( "installed, enabled, ok", [ + pytest.param(True, None, True, id="installed-auto-ok"), + pytest.param(True, None, True, id="installed-auto-fail"), pytest.param(True, True, True, id="installed-enabled-ok"), pytest.param(True, True, False, id="installed-enabled-fail"), pytest.param(True, False, True, id="installed-disabled"), @@ -839,8 +841,24 @@ class SubsetTest: "--layout-features=*", f"--output-file={subsetpath}", ] - if not enabled: + if enabled is True: + args.append("--harfbuzz-repacker") + elif enabled is False: args.append("--no-harfbuzz-repacker") + # elif enabled is None: ... is the default + + if enabled is True: + if not installed: + # raise if enabled but not installed + with pytest.raises(ImportError, match="uharfbuzz"): + subset.main(args) + return + + elif not ok: + # raise if enabled but fails + with pytest.raises(hb.RepackerError, match="mocking"): + subset.main(args) + return with caplog.at_level(logging.DEBUG, "fontTools.ttLib.tables.otBase"): subset.main(args) @@ -851,14 +869,16 @@ class SubsetTest: subsetfont, self.getpath("expect_harfbuzz_repacker.ttx"), ["GSUB"] ) - if enabled: + if enabled or enabled is None: if installed: assert "serializing 'GSUB' with hb.repack" in caplog.text - else: - assert ( - "uharfbuzz not found, compiling 'GSUB' with pure-python serializer" - ) in caplog.text - else: + + if enabled is None and not installed: + assert ( + "uharfbuzz not found, compiling 'GSUB' with pure-python serializer" + ) in caplog.text + + if enabled is False: assert ( "hb.repack disabled, compiling 'GSUB' with pure-python serializer" ) in caplog.text