Make unpackInto restore metadata

[jchia]

unpackInto will be more useful if it restores metadata, such as timestamps, a la the unzip command.

[mrkkrp]

Two things we need to decide:

• Should this featrue be optional?
• Should saveEntry restore the attributes as well?

It makes sense to teach saveEntry to restore attributes (need to figure out the complete list of them) and then unpackInto will do that automatically as it's defined via saveEntry.

Can you post here all things you'd like to restore? How about making a PR?

[jchia]

• If the feature is optional it will be more useful.
• For consistent design, saveEntry should have the same metadata-setting capabilities as unpackInto.
• For Linux metadata, we can consider these commands for reference: stat, touch, tar, unzip. The metadata restoration candidates I can think of are access permissions (chmod), access timestamp and modify timestamp. touch can change both of these timestamps. I'm not sure about the change timestamp. For the root user, the user and group of the file can also be restored and tar extraction would do that (not sure about unzip). I'm not familiar with Windows metadata. Whatever the case, I think aiming for parity with other well-known tools like tar or unzip would be good.
• Regarding optionality, we could even make the optionality granular -- the user can specify any combination of metadata to restore, or simply all the metadata (that is possible to restore).
• The options could be specified as arguments to withArchive or as monadic actions in ZipArchive. I think the latter is more user-friendly. We could define different monadic actions for specifying the restoration of different metadata.
• What should we do about directories that need to be created in order to extract some file at the right place? Does the zip format specify their metadata?

[mrkkrp]

Well, in fact we don't store permissions even for normal files right now, because zip implements Zip format as per official spec, and this is actually not in Zip format, but something OS-specific if I remember correctly. Must be stored in extra fields. The only thing that is present in the specs is modification time (no creation time though). So we can't restore much metadata at the moment, only modification time (and older versions of directory do not support setting modification time). I rememeber now that I originally intended to restore metadata, but the fact that I only know mod time and directory package is hardwired to GHC and we want to support older GHC versions where corresponding directory doesn't know how to set mod times of files stopped me then. The link to the specification I used seems to be dead now. I'll need to find another copy online for reference.

About directories: we don't store directories by themselves in archives on purpose. They are created as necessary on unpacking. Given that, we don't even have a place to store metadata about directories. It may seem a bit unfortunate, but that decision 1) does not break compatibility with other software 2) makes desgin of the package simpler and more elegant. One limitation is that you can't create empty directories, but that's nothing bad IMO.

[mrkkrp]

I'll keep this open as a feature request, feel free to open a PR.

[jchia]

I can think of two designs:

1. unpackInto & saveEntry each has a variant (e.g. unpackWithModTimeInto) that does the same thing and additionally sets the mod time using setModificationTime.
2. Introduce restoreModTime :: ZipArchive () and ignoreModTime :: ZipArchive () that modulate what subsequent unpackInto & saveEntry do. The new boolean state info is stored as an additional field in ZipState. This field is meaningless for an archive being created, though, just like zsAction is meaningless for an archive being read, speaking of which should we make the new functions illegal to use on an archive being written (throw an IOError)?

Thoughts?

[mrkkrp]

I think adding a boolean argument to the both functions is the best option. I don't want modifying wrappers: additional state, additional confusion.

[jchia]

Are you OK with breaking compatibility with previous versions by adding an argument?

On Jan 6, 2017 21:02, "Mark Karpov" notifications@github.com wrote:

I think adding a boolean argument to the both functions is the best option. I don't want modifying wrappers: additional state, additional confusion.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/mrkkrp/zip/issues/25#issuecomment-270899398, or mute the thread https://github.com/notifications/unsubscribe-auth/AAbQ1MlZZ5NTHOLVj8KcK-kPn18Ogfkkks5rPjtigaJpZM4LZFfJ .

[mrkkrp]

Yes, we can release version 0.2.0 with this breaking change.

[jchia]

I think Path.IO.setModification has a bug. It wrongly calls System.Directory.setAccessTime instead of System.Directory.setModificationTime. I'm using Path.IO as a matter of consistency and I'm going to deal with that first.

[mrkkrp]

New version of path-io is on its way, please bump required version of path-io so we use at least 1.2.2.

[jchia]

I suppose that means we have to wait for path-io-1.2.2 to be picked up by the next lts?

[mrkkrp]

No, users can use extra-deps setting with Stack.