[ufoLib] Needless writing of metainfo.plist

[justvanrossum]

I notice that when using UFOWriter (aka UFOReaderWriter) on an existing UFO, it unconditionally rewrites the metainfo.plist file. This can cause formatting differences (for example: source files uses tabs, ufoLib writes spaces).

(Context: Fontra is using UFOReaderWriter. When opening a UFO, it can not know whether the user will maybe edit things. The goal is to not touch anything unless the user makes an edit.)

The following would fix it, but is perhaps too naive: the meta info may need updating if we're upgrading from an older format?

diff --git a/Lib/fontTools/ufoLib/__init__.py b/Lib/fontTools/ufoLib/__init__.py
index b0e425ff0..48617b24d 100755
--- a/Lib/fontTools/ufoLib/__init__.py
+++ b/Lib/fontTools/ufoLib/__init__.py
@@ -1102,8 +1102,9 @@ class UFOWriter(UFOReader):
             # imply the layer contents
             if self.fs.exists(DEFAULT_GLYPHS_DIRNAME):
                 self.layerContents = {DEFAULT_LAYER_NAME: DEFAULT_GLYPHS_DIRNAME}
-        # write the new metainfo
-        self._writeMetaInfo()
+        if not self._havePreviousFile:
+            # write the new metainfo
+            self._writeMetaInfo()
 
     # properties

[Hoolean]

This issue solved a mystery for me, thanks Just 😄 I'd seen these diffs popping up in a few places in my local repositories, but hadn't narrowed it down to them being the ones I'd been previewing in Fontra yet (which it excels at, thank you for the fab tool)

[justvanrossum]

I’ll make a PR.

[justvanrossum]

Ah, the issue is slightly more subtle: it only writes the (any) plist if the written data would be different, and looks at the bytes, not the object to be serialized.

I could change UFOWriter._writePlist() so that it would only overwrite the previous file if the object is different (so it wouldn't write if only the formatting was different).

This approach has two downsides:

1. it is more expensive
2. it prevents ufoLib from enforcing its own preferred formatting, which may be desired behavior.

What if I do implement this behavior, but make it opt-in, with a new UFOWriter constructor argument: ignoreExistingPlistFormatting=False?

I'll try that.

[anthrotype]

Why not simply delay the writing of metainfo.plist until any of the other public writing methods are used? In that case, you wouldn't need an option (the name of the latter is a bit confusing because it may be misunderstood as preserving original formatting even when modifying, which is not the case)

[justvanrossum]

Why not simply delay the writing of metainfo.plist until any of the other public writing methods are used?

That sounds like a much bigger change. We'd need to keep track of "did we already write metainfo.plist?". Also, when we create the UFO we do want it to be written immediately.

Also, I kindof like it if this change could work with all plist files. I can try to think of a better option name, though.

[justvanrossum]

Why not simply delay the writing of metainfo.plist until any of the other public writing methods are used?

But yes, this is doable.

[justvanrossum]

The other approach is to always deserialize when a plist exists, and compare the object instead of the bytes contents. That would work for me.

[justvanrossum]

Hm, maybe this goes further: UFOWriter always sets the "creator" meta info field to com.github.fonttools.ufoLib, even if the UFO already exists. Arguably ufoLib isn't the "creator" in that case. So maybe UFOWriter should only write metainfo.plist if the format info differs.

Although: this can also bee seen as an argument for "only write the metaplist.plist file once any actual writing happens".

[justvanrossum]

"only write the metaplist.plist file once any actual writing happens".

Hm, this is hard, because "actual writing" could mean calling glyphSet.writeGlyph().

I'll let this rest now for a bit, and reconsider how I could perhaps solve it in Fontra itself.

[justvanrossum]

I have now fixed this in Fontra itself.

I am not sure what would the best approach to fix it in fonttools, but since it no longer affects my situation, we can also choose to close this as "won't fix".

If people would like this fixed anyway, and we can agree on an approach, I can still make a PR.