Merge pull request #393 from fonttools/drop-fs

don't import fs module, replace tempfs with tempfile.TemporaryDir

Author: anthrotype

src/ufoLib2/objects/font.py

@@ -2,6 +2,7 @@
 
 import os
 import shutil
+import tempfile
 from typing import (
     Any,
     Dict,
@@ -16,8 +17,6 @@
     cast,
 )
 
-import fs.base
-import fs.tempfs
 from attrs import define, field
 from fontTools.ufoLib import UFOFileStructure, UFOReader, UFOWriter
 
@@ -48,7 +47,7 @@
     _setstate_attrs,
 )
 from ufoLib2.serde import serde
-from ufoLib2.typing import HasIdentifier, PathLike, T
+from ufoLib2.typing import PATH_TYPES, HasIdentifier, PathLike, T
 
 
 def _convert_LayerSet(value: LayerSet | Iterable[Layer]) -> LayerSet:
@@ -532,7 +531,7 @@ def write(self, writer: UFOWriter, saveAs: bool | None = None) -> None:
 
     def save(
         self,
-        path: PathLike | fs.base.FS | None = None,
+        path: PathLike | None = None,
         formatVersion: int = 3,
         structure: UFOFileStructure | None = None,
         overwrite: bool = False,
@@ -541,9 +540,8 @@ def save(
         """Saves the font to ``path``.
 
         Args:
-            path: The target path. If it is None, the path from the last save (except
-                when that was a ``fs.base.FS``) or when the font was first opened will
-                be used.
+            path: The target path. If it is None, the path from the last save or when
+                the font was first opened will be used.
             formatVersion: The version to save the UFO as. Only version 3 is supported
                 currently.
             structure (fontTools.ufoLib.UFOFileStructure): How to store the UFO.
@@ -567,8 +565,8 @@ def save(
             structure = self._fileStructure
 
         # Normalize path unless we're given a fs.base.FS, which we pass to UFOWriter.
-        if path is not None and not isinstance(path, fs.base.FS):
-            path = os.path.normpath(os.fspath(path))
+        if path is not None and isinstance(path, PATH_TYPES):
+            path = os.fsdecode(os.path.normpath(os.fspath(path)))
 
         overwritePath = tmp = None
 
@@ -577,8 +575,8 @@ def save(
             if isinstance(path, str) and os.path.exists(path):
                 if overwrite:
                     overwritePath = path
-                    tmp = fs.tempfs.TempFS()
-                    path = tmp.getsyspath(os.path.basename(path))
+                    tmp = tempfile.TemporaryDirectory()
+                    path = os.path.join(tmp.name, os.path.basename(path))
                 else:
                     import errno
 
@@ -608,12 +606,12 @@ def save(
         finally:
             # clean up the temporary directory
             if tmp is not None:
-                tmp.close()
+                tmp.cleanup()
 
         # Only remember path if it isn't a fs.base.FS because not all FS objects are
         # OsFS with a corresponding filesystem path. E.g. think about MemoryFS.
         # If you want, you can call getsyspath("") method of OsFS object and set that to
         # self._path. But you then have to catch the fs.errors.NoSysPath and skip if
         # the FS object does not implement a filesystem path.
-        if not isinstance(path, fs.base.FS):
+        if isinstance(path, str):
             self._path = path

src/ufoLib2/typing.py

@@ -12,6 +12,9 @@
 PathLike = Union[str, bytes, "os.PathLike[str]", "os.PathLike[bytes]"]
 """Represents a path in various possible forms."""
 
+# can be used with isinstance at runtime to check if something is a path
+PATH_TYPES = (str, bytes, os.PathLike)
+
 
 class Drawable(Protocol):
     """Stand-in for an object that can draw itself with a given pen.

tests/test_ufoLib2.py

@@ -1,5 +1,6 @@
 from __future__ import annotations
 
+import zipfile
 from copy import deepcopy
 from pathlib import Path
 from typing import Any, Type
@@ -284,3 +285,27 @@ def test_convert_on_setattr(
     assert not isinstance(obj, attr_type)
     setattr(o, attr_name, obj)
     assert isinstance(getattr(o, attr_name), attr_type)
+
+
+@pytest.mark.parametrize(
+    "structure", [ufoLib.UFOFileStructure.PACKAGE, ufoLib.UFOFileStructure.ZIP]
+)
+def test_save_again_to_existing_path(
+    tmp_path: Path, structure: ufoLib.UFOFileStructure
+) -> None:
+    font = Font()
+    ufo_name = "foo.ufoz" if structure == ufoLib.UFOFileStructure.ZIP else "foo.ufo"
+    font.save(tmp_path / ufo_name, structure=structure)
+
+    # without the overwrite flag, this should fail
+    with pytest.raises(OSError, match="already exists"):
+        font.save(tmp_path / ufo_name, structure=structure)
+
+    # with the overwrite flag, it should succeed
+    font.save(tmp_path / ufo_name, overwrite=True, structure=structure)
+
+    if structure == ufoLib.UFOFileStructure.PACKAGE:
+        assert (tmp_path / ufo_name).is_dir()
+    else:
+        assert (tmp_path / ufo_name).is_file()
+        assert zipfile.is_zipfile(tmp_path / ufo_name)