From 8d2a3ae6d22897d0b012d219d3a0b496b9c816d1 Mon Sep 17 00:00:00 2001 From: Nikolaus Waxweiler Date: Mon, 23 Nov 2020 13:57:32 +0000 Subject: [PATCH] Add expectContentsFile parameter to GlyphSet For use when reading existing UFOs, to comply with the specification stating that a contents.plist file must exist in a glyph set. Closes https://github.com/fonttools/fonttools/issues/2111. --- Lib/fontTools/ufoLib/__init__.py | 46 +++++++++++++++++++++++++++++--- Lib/fontTools/ufoLib/glifLib.py | 3 +++ Tests/ufoLib/UFO3_test.py | 20 ++++++++++++++ Tests/ufoLib/glifLib_test.py | 10 +++++++ 4 files changed, 75 insertions(+), 4 deletions(-) diff --git a/Lib/fontTools/ufoLib/__init__.py b/Lib/fontTools/ufoLib/__init__.py index 4b92102d9..9a6113dc6 100755 --- a/Lib/fontTools/ufoLib/__init__.py +++ b/Lib/fontTools/ufoLib/__init__.py @@ -683,7 +683,13 @@ class UFOReader(_UFOBaseIO): # this will already have been raised during __init__ raise UFOLibError("The default layer is not defined in layercontents.plist.") - def getGlyphSet(self, layerName=None, validateRead=None, validateWrite=None): + def getGlyphSet( + self, + layerName=None, + validateRead=None, + validateWrite=None, + expectContentsFile=False + ): """ Return the GlyphSet associated with the glyphs directory mapped to layerName @@ -695,6 +701,10 @@ class UFOReader(_UFOBaseIO): class's validate value, can be overridden. ``validateWrite`` will validate the written data, by default it is set to the class's validate value, can be overridden. + ``expectContentsFile`` will raise a GlifLibError if a contents.plist file is + not found on the glyph set file system. This should be set to ``True`` if you + are reading an existing UFO and ``False`` if you use ``getGlyphSet`` to create + a fresh glyph set. """ from fontTools.ufoLib.glifLib import GlyphSet @@ -723,6 +733,7 @@ class UFOReader(_UFOBaseIO): ufoFormatVersion=self._formatVersion, validateRead=validateRead, validateWrite=validateWrite, + expectContentsFile=expectContentsFile ) def getCharacterMapping(self, layerName=None, validate=None): @@ -1422,7 +1433,15 @@ class UFOWriter(UFOReader): raise UFOLibError("Could not locate a glyph set directory for the layer named %s." % layerName) return foundDirectory - def getGlyphSet(self, layerName=None, defaultLayer=True, glyphNameToFileNameFunc=None, validateRead=None, validateWrite=None): + def getGlyphSet( + self, + layerName=None, + defaultLayer=True, + glyphNameToFileNameFunc=None, + validateRead=None, + validateWrite=None, + expectContentsFile=False, + ): """ Return the GlyphSet object associated with the appropriate glyph directory in the .ufo. @@ -1435,6 +1454,10 @@ class UFOWriter(UFOReader): class's validate value, can be overridden. ``validateWrte`` will validate the written data, by default it is set to the class's validate value, can be overridden. + ``expectContentsFile`` will raise a GlifLibError if a contents.plist file is + not found on the glyph set file system. This should be set to ``True`` if you + are reading an existing UFO and ``False`` if you use ``getGlyphSet`` to create + a fresh glyph set. """ if validateRead is None: validateRead = self._validate @@ -1459,7 +1482,12 @@ class UFOWriter(UFOReader): raise UFOLibError("A layer name must be provided for non-default layers.") # move along to format specific writing if self._formatVersion < UFOFormatVersion.FORMAT_3_0: - return self._getDefaultGlyphSet(validateRead, validateWrite, glyphNameToFileNameFunc=glyphNameToFileNameFunc) + return self._getDefaultGlyphSet( + validateRead, + validateWrite, + glyphNameToFileNameFunc=glyphNameToFileNameFunc, + expectContentsFile=expectContentsFile + ) elif self._formatVersion.major == UFOFormatVersion.FORMAT_3_0.major: return self._getGlyphSetFormatVersion3( validateRead, @@ -1467,11 +1495,18 @@ class UFOWriter(UFOReader): layerName=layerName, defaultLayer=defaultLayer, glyphNameToFileNameFunc=glyphNameToFileNameFunc, + expectContentsFile=expectContentsFile, ) else: raise NotImplementedError(self._formatVersion) - def _getDefaultGlyphSet(self, validateRead, validateWrite, glyphNameToFileNameFunc=None): + def _getDefaultGlyphSet( + self, + validateRead, + validateWrite, + glyphNameToFileNameFunc=None, + expectContentsFile=False, + ): from fontTools.ufoLib.glifLib import GlyphSet glyphSubFS = self.fs.makedir(DEFAULT_GLYPHS_DIRNAME, recreate=True) @@ -1481,6 +1516,7 @@ class UFOWriter(UFOReader): ufoFormatVersion=self._formatVersion, validateRead=validateRead, validateWrite=validateWrite, + expectContentsFile=expectContentsFile, ) def _getGlyphSetFormatVersion3( @@ -1490,6 +1526,7 @@ class UFOWriter(UFOReader): layerName=None, defaultLayer=True, glyphNameToFileNameFunc=None, + expectContentsFile=False, ): from fontTools.ufoLib.glifLib import GlyphSet @@ -1529,6 +1566,7 @@ class UFOWriter(UFOReader): ufoFormatVersion=self._formatVersion, validateRead=validateRead, validateWrite=validateWrite, + expectContentsFile=expectContentsFile, ) def renameGlyphSet(self, layerName, newLayerName, defaultLayer=False): diff --git a/Lib/fontTools/ufoLib/glifLib.py b/Lib/fontTools/ufoLib/glifLib.py index 385f54bfe..0810d2ed8 100755 --- a/Lib/fontTools/ufoLib/glifLib.py +++ b/Lib/fontTools/ufoLib/glifLib.py @@ -135,6 +135,7 @@ class GlyphSet(_UFOBaseIO): ufoFormatVersion=None, validateRead=True, validateWrite=True, + expectContentsFile=False, ): """ 'path' should be a path (string) to an existing local directory, or @@ -191,6 +192,8 @@ class GlyphSet(_UFOBaseIO): self.fs = filesystem # if glyphSet contains no 'contents.plist', we consider it empty self._havePreviousFile = filesystem.exists(CONTENTS_FILENAME) + if expectContentsFile and not self._havePreviousFile: + raise GlifLibError(f"{CONTENTS_FILENAME} is missing.") # attribute kept for backward compatibility self.ufoFormatVersion = ufoFormatVersion.major self.ufoFormatVersionTuple = ufoFormatVersion diff --git a/Tests/ufoLib/UFO3_test.py b/Tests/ufoLib/UFO3_test.py index bed527dba..8a146b754 100644 --- a/Tests/ufoLib/UFO3_test.py +++ b/Tests/ufoLib/UFO3_test.py @@ -3940,6 +3940,26 @@ class UFO3WriteLayersTestCase(unittest.TestCase): result = list(writer.getGlyphSet("layer 2", defaultLayer=False).keys()) self.assertEqual(expected, result) + def testGetGlyphSetNoContents(self): + self.makeUFO() + os.remove(os.path.join(self.ufoPath, "glyphs.layer 1", "contents.plist")) + + reader = UFOReader(self.ufoPath, validate=True) + with self.assertRaises(GlifLibError): + reader.getGlyphSet("layer 1", expectContentsFile=True) + + writer = UFOWriter(self.ufoPath, validate=True) + with self.assertRaises(GlifLibError): + writer.getGlyphSet("layer 1", defaultLayer=False, expectContentsFile=True) + + # There's a separate code path for < v3 UFOs. + with open(os.path.join(self.ufoPath, "metainfo.plist"), "wb") as f: + plistlib.dump(dict(creator="test", formatVersion=2), f) + os.remove(os.path.join(self.ufoPath, "glyphs", "contents.plist")) + writer = UFOWriter(self.ufoPath, validate=True, formatVersion=2) + with self.assertRaises(GlifLibError): + writer.getGlyphSet(expectContentsFile=True) + # make a new font with two layers def testNewFontOneLayer(self): diff --git a/Tests/ufoLib/glifLib_test.py b/Tests/ufoLib/glifLib_test.py index 4c8d587c6..3af0256cd 100644 --- a/Tests/ufoLib/glifLib_test.py +++ b/Tests/ufoLib/glifLib_test.py @@ -54,6 +54,16 @@ class GlyphSetTests(unittest.TestCase): added, removed, "%s.glif file differs after round tripping" % glyphName) + def testContentsExist(self): + with self.assertRaises(GlifLibError): + GlyphSet( + self.dstDir, + ufoFormatVersion=2, + validateRead=True, + validateWrite=True, + expectContentsFile=True, + ) + def testRebuildContents(self): gset = GlyphSet(GLYPHSETDIR, validateRead=True, validateWrite=True) contents = gset.contents