Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feature: Add archiving functionality (zip/tar/..) #65

Closed
gsantner opened this issue Mar 21, 2021 · 16 comments
Closed

Feature: Add archiving functionality (zip/tar/..) #65

gsantner opened this issue Mar 21, 2021 · 16 comments
Assignees
Labels

Comments

@gsantner
Copy link

Hey, just looked at the combined-header file and the available functionalities/modules at readme.

I thought maybe some kind of archiving functionality could fit well for this library as well, due there are File related functionalites. So quickly zip up a folder, or i.e. two files to create a backup of i.e. application configuration + some images.

Don't really implying any preference, whatever is easy and short (LOC/singleheader) to implement ... zip?tar?
Maybe minizip has something to offer.

Feel free to close if you don't think it fits for the project.

@zpl-zak
Copy link
Member

zpl-zak commented Mar 22, 2021

Hi @gsantner,

Thank you for your input! We've discussed a possible solution with @inlife and concluded we could expand the file API by providing the necessary tools to manipulate archived files.

While we won't roll our compression library for ZPL, we will support the most common libraries we would use internally to manipulate the data. As a user, one can enable the support of a specific user-supplied third-party compression library without the need of having to use the third-party library themselves directly.

We didn't discuss the design yet, but it is something worth looking into in the future. Please, feel free to share any design suggestions if you can think of something. It would be very appreciated! :)

@gsantner
Copy link
Author

Sorry to say, but disagree.

  • Will be hardly possible to get a "one thing fits it all" approach with third party libraries. Everything and each has it's own file parameters, types and stuff.
  • What happens if you suggest to -l one third party lib: You start collecting a list of external libs, because everybody starts suggesting things for his favourite thing. This totally destroys your project goal - which is include ONLY this one single header file and you get everything without additional dependencies. I suggest to not go that path, then you get no different to other frameworks/function collections like boost...glib/gtk...qt..etc
  • I shortly checked what is available, I found zip too complicated and in terms of LOC to add as a single file. Even the small ones are ~5k LOC miniz. So I'd say tar is definitely one of the better options.
  • However there are tar implementations with ~300-400 LOC, which I think would fit for the project. At same time, LOC might even go down by the file stuff zpl already has. Examples: calcrypto, bebuch, rxi.
  • I'd say the focus should be on archiving, not compressing. High compression requires strong algo, thus much code. The simplest way might be tar, which is just concating data. (not tar.xz / tar.gz ... XZ/GZ compression on top of tar).

@zpl-zak
Copy link
Member

zpl-zak commented Mar 22, 2021

Thank you, we will re-iterate the approach in that case. TAR support sounds reasonable indeed, thank you for providing some examples.

As for compression, fortunately, it's a topic that can be dealt with outside of ZPL, so I agree the focus should stay on archiving itself.

I've opened another issue that would introduce some internal changes in ZPL to allow for TAR (and possibly ZIP) manipulation in the future, but feel free to provide any suggestions regarding the interface. To keep it simple, we won't focus on that part.

We could introduce a basic set of methods to pack/unpack a set of files/folders and manipulate the archive (add/remove/edit entries).

@gsantner
Copy link
Author

Only thing I could think off for the "VFS" task is: Taking care of "vendor" file restrictions, like on iOS & Android with StorageAccessFramework - but I guess thats a too big point for ZPL.
Otherwise, for VFS things like file/folder access over NFS or SSH comes to my mind - which again is guess too much for ZPL.
So not really seeing atm something which is a) helpful to abstract and taking away efforts, b) small enough to put in a single header.

As noted on description, what I see as "main goal": To be able to quickly zip up some folders and files to have a single file that can be copied to a backup folder...can be sent over network .. like that. And reversed, being able to unpack it.

@zpl-zak
Copy link
Member

zpl-zak commented Mar 22, 2021

The set of methods I'd propose to introduce:

zpl_tar_init(zpl_tar*)
zpl_tar_destroy(zpl_tar*)
zpl_tar_add_file(zpl_tar*, path, zpl_file*)
zpl_tar_add_dir(zpl_tar*, path, zpl_dir_info*)
zpl_tar_add_path(zpl_tar*, path, char const* os_path) // resolves path and adds file/folder
zpl_tar_rem_entry(zpl_tar*, path)
zpl_tar_pack(zpl_tar*, zpl_allocator, &size)
zpl_tar_unpack(zpl_tar*, zpl_file*)
zpl_tar_unpack_buffer(zpl_tar*, buffer, len)

would this work out for the feature to be implemented? I think this wouldn't bring much of an overhead to the library and it should still provide all the necessary tools to support the criteria. The list above should give a rough idea of the design but is not final.

@gsantner
Copy link
Author

I guess

zipstruct tar = init("file.tar");
add_path(tar, "config");
pack(tar);

unpack(tar, "/tmp/unpacked");

is enough yes.

Note that I currently don't make use of zpl yet, I just discovered this project these days. But this is one of the things that came to my mind and thought to suggest 😄

@zpl-zak
Copy link
Member

zpl-zak commented Mar 22, 2021

That would work fine, thank you again for the suggestion, I will keep you updated on the progress. :)

@zpl-zak
Copy link
Member

zpl-zak commented Mar 23, 2021

please see the comment below

@zpl-zak
Copy link
Member

zpl-zak commented Mar 23, 2021

Hi @gsantner, please check out https://github.com/zpl-c/zpl/releases/tag/12.5.0-pre.0 to see whether this approach is suitable.

Example code can be found at https://github.com/zpl-c/zpl/blob/review/tar-archive/code/apps/examples/tar.c

Thank you for your feature request!

Note: there is no unpacking code (for filesystem) written yet, but it's currently a work in progress, until then, the example code only showcases how to list all packed files. The example code now also contains a call that unpacks all files to the filesystem.

@gsantner
Copy link
Author

@zaklaus

  • Bad that folders are not supported, so guess a pity choice of some existing implementation. But you already have chosen and guess won't do it over again. (i.e. cannot do `compress("configs/");)
  • If stuff copy/pasted, don't forget about license notice - guess others are not happy if there is no proper license attribution.
  • At same time, if somebody is gonna use ZPL in his project, he will also have to explicitley notice open source license of the tar library you forked. (Same goes for anything else that gets mixed into zpl project from other "lightweight libraries")

@zpl-zak
Copy link
Member

zpl-zak commented Mar 24, 2021

@zaklaus

  • Bad that folders are not supported, so guess a pity choice of some existing implementation. But you already have chosen and guess won't do it over again. (i.e. cannot do `compress("configs/");)
  • If stuff copy/pasted, don't forget about license notice - guess others are not happy if there is no proper license attribution.
  • At same time, if somebody is gonna use ZPL in his project, he will also have to explicitley notice open source license of the tar library you forked. (Same goes for anything else that gets mixed into zpl project from other "lightweight libraries")

You can pack folders as well, using zpl_tar_pack_dir, zpl_tar_unpack_dir is also an option, it's just that empty folders are not recognized and won't be added to the tar archive. As for the license, fortunately no code was copied, however rxi's microtar library was a great inspiration for the design choices, so credit was given indeed. :)

@gsantner
Copy link
Author

OK haven't compared the code, just saw the mention :D.

@zpl-zak
Copy link
Member

zpl-zak commented Mar 24, 2021

This is the final proposed API:

zpl_isize zpl_tar_pack(zpl_file *archive, char const **paths, zpl_isize paths_len);
zpl_isize zpl_tar_pack_dir(zpl_file *archive, char const *path, zpl_allocator alloc);
zpl_isize zpl_tar_unpack(zpl_file *archive, zpl_tar_unpack_proc *unpack_proc, void *user_data);
zpl_isize zpl_tar_unpack_dir(zpl_file *archive, char const *dest);

zpl_tar_pack_dir and zpl_tar_unpack_dir are helper methods to pack/unpack files on the filesystem.

@gsantner
Copy link
Author

I would swap paths and length, but thats totally personal taste.

functioncall(something, [ 
   [a,b,c],
   [d,e,f]
], 123);

vs 

functioncall(something, 123, [ 
   [a,b,c],
   [d,e,f]
]);

Overall I guess it's fine, but still sorry didn't make it to try zpl yet :D

@zpl-zak
Copy link
Member

zpl-zak commented Mar 24, 2021

Overall I guess it's fine, but still sorry didn't make it to try zpl yet :D

That's fine, thanks a lot for the feature request, I think it's a great addition to the library!

@inlife and I will review the PR soon, once merged we can possibly close this issue. 👍🏻

@zpl-zak zpl-zak self-assigned this Mar 24, 2021
@zpl-zak
Copy link
Member

zpl-zak commented Mar 27, 2021

TAR functionality has been incorporated into https://github.com/zpl-c/zpl/releases/tag/12.3.0. Closing this issue now, but feel free to re-open it if need be.

@zpl-zak zpl-zak closed this as completed Mar 27, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants