Skip to content

Conversation

@nebulade
Copy link
Contributor

Hi, this will add support for symlinks.

I added test checks for tar and a symlink fixture. Currently the tests fail for zip, as the test will receive a stream error correctly for non-supported type 'symlink'. This is why the pull request is marked WIP.

As of archiverjs/node-compress-commons#23 there seems to be a fix for zip, but I am not sure how to exactly use this. It would be great if someone could provide a example or further explanation how to deal with symlinks in zips.

Let me know what needs to be fixed up otherwise for this to go through. Thanks!

@ef4
Copy link

ef4 commented May 31, 2017

This looks like it's fully working if you use the master branch of node-zip-stream. I have the test suite passing by applying this small patch on top of this PR:

diff --git a/lib/plugins/zip.js b/lib/plugins/zip.js
index 8eea00d..2474ed7 100644
--- a/lib/plugins/zip.js
+++ b/lib/plugins/zip.js
@@ -30,7 +30,7 @@ var Zip = function(options) {

   this.supports = {
     directory: true,
-    symlink: false
+    symlink: true
   };

   this.engine = new engine(options);
diff --git a/package.json b/package.json
index 9763f0b..237a0f3 100644
--- a/package.json
+++ b/package.json
@@ -36,7 +36,7 @@
     "lodash": "^4.8.0",
     "readable-stream": "^2.0.0",
     "tar-stream": "^1.5.0",
-    "zip-stream": "^1.1.0",
+    "zip-stream": "https://github.com/archiverjs/node-zip-stream#722ed7b54a3bc83cf77dff830ff5853af641036e",
     "walkdir": "^0.0.11"
   },
   "devDependencies": {
diff --git a/test/plugins.js b/test/plugins.js
index 86f3243..e083a54 100644
--- a/test/plugins.js
+++ b/test/plugins.js
@@ -127,7 +127,7 @@ describe('plugins', function() {

     it('should append multiple entries', function() {
       assert.isArray(actual);
-      assert.lengthOf(actual, 8);
+      assert.lengthOf(actual, 9);
     });

     it('should append buffer', function() {

ef4 added a commit to ef4/node-archiver that referenced this pull request May 31, 2017
@ctalkington
Copy link
Member

thanks for taking an initiative here. i do think we are very close to getting this all working. ill check into doing some merging this weekend.

@unicron
Copy link

unicron commented Mar 30, 2018

Does this follow the link, and zip an actual file into the archive? Or just zip a symlink in there and no file?

@ef4
Copy link

ef4 commented Mar 30, 2018

It allows you to add symlinks directly into the archive. It doesn't auto-follow.

@unicron
Copy link

unicron commented Mar 30, 2018

Thanks @ef4 for the quick response. Is there a way to zip the actual file rather than the symlink? I love the performance of this tool but need support to zip the resolved files. I saw #311 which seems related? That's actually my use-case as well, packages in node_modules created via npm link command.

@ef4
Copy link

ef4 commented Mar 30, 2018

You can certainly use your own directory-walking code that follows symlinks and just repeatedly call archive.append or archive.file.

Or you could make a PR for this library that would add an option to archive.directory that does the symlink resolution, but whether it's what the maintainers would merge is not up to me, I am merely a fellow passerby. 😁

@newlukai
Copy link

The documentation doesn't exactly describe the processing of symlinks. So please let me know if symlinks can be added to zip files using file() or directory(). From the tests it seems, that it's not possible, but I'm not sure if this really is the expected behavior.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants