Add ZIP support to UFO (rebased)

[anthrotype]

this is #26 after merging origin/master and fixing up merge conflicts

[anthrotype]

Can somebody remind me why we need to contain all the files inside a root folder in the zip archive? It seems like the only use of the FileSystem class is to add that root folder at the beginning of all paths. I wish we do away with that further indirection and use the fs objects directly (or mock them when not available)

[typemytype]

i guess: when unzipped manually it has the same structure as an existing ufo

On Wed, 3 Oct 2018 at 19:57, Cosimo Lupo notifications@github.com wrote:

Can somebody remind me why we need to contain all the files inside a root folder in the zip archive? It seems like the only use of the FileSystem class is to add that root folder at the beginning of all paths. I wish we do away with that further indirection and use the fs objects directly (or mock them when not available)

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/unified-font-object/ufoLib/pull/172#issuecomment-426734892, or mute the thread https://github.com/notifications/unsubscribe-auth/ABIp1otL2Ymq2-W0h54oGMCs48SdjSnzks5uhPp1gaJpZM4XGOjZ .

-- gr Frederik www.typemytype.com

[anthrotype]

i guess: when unzipped manually it has the same structure as an existing ufo

well, I just tried to zip the content of a UFO from the Finder (after "Show Package Content", select all files inside, and then "Compress") and when I unzip the resulting zip archive, Finder always creates a folder with the same name as the zip file.

Also, using unzip command from the console, one can use the -d exdir option to extract into a directory, which is created if it doesn't exist.

[anthrotype]

I understand the convenience of compressing a UFO directory as is straight from Finder/Explorer and renaming the archive to .ufoz — as opposed to having to show package contents and compress only those, or even better using a script or a one line zip command from the terminal.

But for me this really isn’t enough reason to complicate the code like that, adding another layer of indirection (with the risk of making thing even slower than they already are), forcing users to having to make subclasses of ufoLib.FileSystem just to be able to use a compatible but different filesystem format. When it would be so much easier to simply have UFOReader and UFOWriter directly use an pyfilesystem2 fs instance (or an object that has the same API), and do other ufo-like things (like reading a plist) on top of that, instead of delegating that to this intermediate FileSystem wrapper.

I think I’m going to try and remove that class and see how it plays out.

[anthrotype]

And I would be soooo happy if we made pyfilesystem2 a hard requirement. Would make this so much easier and consistent. With that _NOFS implementation basically we have to reimplement the same code as fs (but worse), and then the code using it has to catch both fs.errors and regular IOError exceptions and reraise both of them as UFOLib errors.

[anthrotype]

Reading through the pyfilesystem2 docs, there so many cool features and we are only using less than 10% of its rich API. Of course we are, otherwise we would have to rewrite it from scratch given our historical reluctance on adding third party dependencies.. The whole point of fs module is to abstract away from the details of the OS filesystem, but having to maintain a separate (and subpar) concrete implementation of the OSFS kind of defies the purpose.

So.. does anybody have objection to making pyfilesystem2 an installation requirement to use ufoLib?

[moyogo]

What about UFO4?

[moyogo]

I mean, in the currently planned UFO4, multiple UFOs are in that folder.

[anthrotype]

But that’s a different issue altogether. Here I’m talking about removing the need to have an intermediate FileSystem wrapper around the actual pyfilesystem2 fs instance (and also about requiring pyfilesystem2).

[anthrotype]

And also as far as I remember that proposed ufo4 “unified font set” is not a container but a separate file (or package) sitting next to the ufos

[anthrotype]

I just discovered that I can keep the root "contents" directory for the .ufoz zip filesystem, by simply using the opendir method (which returns an instance of SubFS, actually fs.subfs.ClosingSubFS which autocloses its parent on closing self). nice! :+1: So no need to change the spec proposal, we can keep it as is.

[anthrotype]

I want to move this forward (don't want to spend a month cleaning up ufoLib), so I think I'm going to merge this later on today. Feel free to leave comments here.

[anthrotype]

I'm thinking we should set the version to 3.0.0, even though the changes here are backward compatible. My reasoning is that clients of this new fs-powered ufoLib are expected to never reach to os.path API and mess with the UFO filesystem's internal files, because the UFO is no longer guaranteed to be a folder on the local filesystem. They must only use the public methods of UFOReader and UFOWriter, or if they wish they may use their self.fs attribute directly, which is an FS object with a clear documented API (check pyfilesystem2 docs). This is the only way we can reliably and consistently support zip (or or any other FS that does not map to local OSFS, e.g. MemoryFS).

[anthrotype]

also, the ufoLib methods for reading/writing files in the "data" subdirectory of UFO used to take OS-like paths (e.g. backslashes on windows). This is going to change, and can't be otherwise. Pyfilesystem2 API requires forward-slashes for paths (it takes care internally of the translation to native OS filesystem if needed). We just can't blindly find-replace "\" with "/" because the former is a valid character in path names on UNIX. So, e.g., even though the defcon test suite passes on Mac/Linux with the new ufoLib (I tested both py2 and py3), on Windows it will likely fail where it attempts to pass native OS paths to ufoLib methods that internally use the pyfilesystem2 API. Another reason to make it 3.0.0.

Also, this is going to be the last version anyway. The next one, will be as fontTools.ufoLib.

[anthrotype]

"I agree" (with myself) :laughing:

I would have liked another pair of eyes, but I really want to get moving, so I'll take the responsibility and go on and merge. I'll sleep over it so you can have another chance to test this out, then tomorrow I'll make the last final 3.0 release of ufoLib as standalone module, and start preparing the removal to fontTools.ufoLib.

[benkiel]

Agree that it should be version 3