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

Lack of symlink support #125

Closed
palmerc opened this issue Jan 8, 2020 · 13 comments
Closed

Lack of symlink support #125

palmerc opened this issue Jan 8, 2020 · 13 comments
Assignees
Labels
new-feature New feature or request resolved

Comments

@palmerc
Copy link

palmerc commented Jan 8, 2020

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

@dknchris
Copy link

dknchris commented Jan 9, 2020

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

@srikanth-lingala
Copy link
Owner

Looking into it.

@srikanth-lingala srikanth-lingala self-assigned this Jan 13, 2020
@srikanth-lingala srikanth-lingala added the bug Something isn't working label Jan 13, 2020
@palmerc
Copy link
Author

palmerc commented Jan 13, 2020 via email

@srikanth-lingala
Copy link
Owner

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?

@srikanth-lingala srikanth-lingala added want feedback and removed bug Something isn't working labels Jan 18, 2020
@palmerc
Copy link
Author

palmerc commented Jan 18, 2020 via email

@dknchris
Copy link

dknchris commented Jan 18, 2020

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
Copy link
Owner

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
Copy link

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.

@srikanth-lingala srikanth-lingala added new-feature New feature or request and removed want feedback labels Jan 26, 2020
@joorei
Copy link
Contributor

joorei commented Feb 22, 2020

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 added a commit that referenced this issue Feb 28, 2020
@srikanth-lingala
Copy link
Owner

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
Copy link
Owner

Feature included in v2.4.0 released today

@dknchris
Copy link

dknchris commented Mar 6, 2020

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
Copy link
Owner

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
new-feature New feature or request resolved
Projects
None yet
Development

No branches or pull requests

4 participants