mrkkrp / zip

Efficient library for manipulating zip archives
Other
81 stars 27 forks source link

How to preserve file permissions using packDirRecur #66

Closed saurabhnanda closed 4 years ago

saurabhnanda commented 4 years ago

The following is not working as I am expecting it to. Am I doing anything wrong?

prepareZipFile :: FilePath -> IO FilePath
prepareZipFile dir = do
  Zip.createArchive zipFileName $ Zip.packDirRecur Zip.Deflate createSelector dir
  pure zipFileName
  where
    zipFileName = "/tmp/upload.zip"

    createSelector fpath = do
      s <- Zip.mkEntrySelector fpath
      Zip.setExternalFileAttrs ((0x100000 .|. 0o0755) `shiftL` 16) s
      -- fmode <- liftIO $ (fmap Unix.fileMode $ Unix.getFileStatus $ dir <> "/" <> fpath)
      -- Zip.setExternalFileAttrs (traceShowId $ fromIntegral $ (toInteger fmode) `shiftL` 16) s
      pure s
saurabhnanda commented 4 years ago

This is something that works, but IMO is too clunky. Why is packRecurDir not respecting all external file attributes?

prepareZipFile :: FilePath -> IO FilePath
prepareZipFile dir = do
  Zip.createArchive zipFileName $ do
    Zip.packDirRecur Zip.Deflate createSelector dir
    Zip.commit
    x <- toList <$> Zip.getEntries
    forM x $ \(es, _) -> Zip.setExternalFileAttrs ((0x100000 .|. 0o0755) `shiftL` 16) es
  pure zipFileName
  where
    zipFileName = "/tmp/upload.zip"

    createSelector fpath = do
      s <- Zip.mkEntrySelector fpath
      -- Zip.setExternalFileAttrs ((0x100000 .|. 0o0755) `shiftL` 16) s
      -- fmode <- liftIO $ (fmap Unix.fileMode $ Unix.getFileStatus $ dir <> "/" <> fpath)
      -- Zip.setExternalFileAttrs (traceShowId $ fromIntegral $ (toInteger fmode) `shiftL` 16) s
      pure s
mrkkrp commented 4 years ago

The callback createSelector is called before the entries are created. Event though we do not execute the actions immediately, the code tries to follow "common sense", so if you try to edit something (in your case, by setting external attributes) that doesn't yet exist, nothing happens.

Here is the code that clears editing actions when an entry is created:

https://github.com/mrkkrp/zip/blob/dddc3a83b9671994853b4bf78ab3a3c775da7139/Codec/Archive/Zip/Internal.hs#L327-L331

Your approach from the second snippet looks good. I'd recommend to use forEntries to traverse all entries:

prepareZipFile :: FilePath -> IO FilePath
prepareZipFile dir = do
  Zip.createArchive zipFileName $ do
    Zip.packDirRecur Zip.Deflate Zip.mkEntrySelector dir
    Zip.commit
    forEntries $ Zip.setExternalFileAttrs ((0x100000 .|. 0o0755) `shiftL` 16)
  pure zipFileName
  where
    zipFileName = "/tmp/upload.zip"

BTW, why do you do ((0x100000 .|. 0o0755)shiftL16) instead of using toFileMode from Codec.Archive.Zip.Unix? I'd guess that it does exactly what you need.

mrkkrp commented 4 years ago

The only problem with that approach is that the archive will be written two times. We could add an extra call back to the packDirRecur to make it possible to perform editing actions on not-yet-created entries. Tell me if you're interested in that feature.