search

srikanth-lingala / zip4j

A Java library for zip files and streams
Apache License 2.0
2.07k stars 312 forks source link

Lack of symlink support #125

Closed palmerc closed 4 years ago

palmerc commented 4 years ago

When zipping certain file types like .frameworks on macOS zip4j does not support adding the symlink to the current version of the library.

dknchris commented 4 years ago

Willing to test on Android/Linux if this is possible for zip4j.

srikanth-lingala commented 4 years ago

Looking into it.

palmerc commented 4 years ago

Let me know what you think, I’ve started looking into it myself.

On 13 Jan 2020, at 19:46, Srikanth Reddy Lingala notifications@github.com wrote:

Looking into it.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/srikanth-lingala/zip4j/issues/125?email_source=notifications&email_token=AAECE7QRTLAGDQBSLU6Y7OLQ5SZJZA5CNFSM4KEEOVC2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEIZ2ZEI#issuecomment-573811857, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAECE7QKVZ2UUSORMSHEMUTQ5SZJZANCNFSM4KEEOVCQ.

srikanth-lingala commented 4 years ago

At the moment, zip4j adds the linked file and not the link itself. I presume this is desired behaviour? Your expectation is that the link is added and not the linked file?

palmerc commented 4 years ago

My expectation is that the linked file and the symlink would be created. Symlinks should be preserved generally, if desired

On 18 Jan 2020, at 12:53, Srikanth Reddy Lingala notifications@github.com wrote:

 At the moment, zip4j adds the linked file and not the link itself. I presume this is desired behaviour? Your expectation is that the link is added and not the linked file?

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub, or unsubscribe.

dknchris commented 4 years ago

It should be optional. Some users would want to copy the actual referenced file instead of the link, which zip4j currently does by default. However, copying symlinks as-is is important too.

Perhaps, you can add a boolean flag in ZipParameters to indicate what to do when encountering symlinks.

Man page here for linux zip command for context: https://linux.die.net/man/1/zip. Check out the -y or --symlinks option.

srikanth-lingala commented 4 years ago

I agree with providing an option to the user. I will include an enum in ZipParameters which will have the options: INCLUDE_ONLY_LINK, INCLUDE_ONLY_LINKED_FILE, INCLUDE_LINK_AND_LINKED_FILE or something similarly named.

I am investigating the possibilities and different scenarios. What I am currently stuck with and investigating is that FileInputStream.read(link) seems to only read the linked file and not the link. If user chooses the option to include link, then we will be needing a way to read the content of the link file. I am currently looking into this, hopefully there is an easy way around this. I need to also investigate into adding appropriate headers (if any).

dknchris commented 4 years ago

Great options. Thanks for considering this feature.

If user chooses the option to include link, then we will be needing a way to read the content of the link file

Ya, I'm not sure if you can do this without java.nio.file apis. Perhaps one approach is to use Files.readSymbolicLink() and store it somehow in the zip and use Files.createSymbolicLink() during extraction.

This will also mean that the new options would require atleast Java 7, Android Oreo.

joorei commented 4 years ago

I need to also investigate into adding appropriate headers (if any).

4.5.7 -UNIX Extra Field (0x000d) in https://pkware.cachefly.net/webdocs/casestudies/APPNOTE.TXT may be relevant:

Currently the only values allowed are the original "linked to" file names for hard or symbolic links, […] Link files will have the name of the original file stored. This name is NOT NULL terminated.

srikanth-lingala commented 4 years ago

I added this feature in zip4j. As discussed above, you have three options, to only include link, to only include target (linked file) and to include both link and linked file. You can set this via ZipParameters.setSymbolicLinkAction() and the enum is ZipParameters.SymbolicLinkAction.

I made a SNAPSHOT release 2.3.3-SNAPSHOT. I already did a lot of testing, and obviously also added some tests to the test suite, but would be great if you could test it as well. Thanks.

P.S: If you have trouble accessing this SNAPSHOT version, you might want to have a look here

srikanth-lingala commented 4 years ago

Feature included in v2.4.0 released today

dknchris commented 4 years ago

Sorry for the late response here Srikant. I tested the update and yes it works as intended. Some notes below.

Unfortunately, there is a huge roadblock on Android's user visible storage (one that goes /storage/emulated/0 or /sdcard. You can't create symbolic links on this partition, so the tests will fail if this is the location of source/extraction files.

Symbolic links, however, are allowed in the private app directories obtained from Context.getFilesDir() or Context.getCacheDir(). When performing tests on Android, users need to make sure they are performing the tests within these directories.

srikanth-lingala commented 4 years ago

@dknchris Thanks for testing this, and for the hints. I am sure, some of the Android developers will find your hints useful.