search

nfriedly / node-bestzip

Provides a `bestzip` command that uses the system `zip` if avaliable, and a Node.js implimentation otherwise.
MIT License
80 stars 16 forks source link

Excludes the destination zip file #8

Closed Lampei closed 6 years ago

Lampei commented 6 years ago

When doing the archiver bulk creation of the zip file, the destination zip file is included within the zip file being created (multiple times). This will exclude the destination zip file being included in the final zip file being created, but will still allow any zip files in sub directories that happen to have the same name as the destination zip file to still be created. e.g. if my destination zip is my-new-zip.zip, then /my-new-zip.zip will be excluded but /my/sub/directory/my-new-zip.zip will still be added.

Lampei commented 6 years ago

Looks like if you specify the sources and there happens to be a zip file in there named the same as the destination zip, that it will still exclude it using the method I have in the pull request. I will try tweaking the request slightly so that it will only check when in the 'root' directory.

Lampei commented 6 years ago

2nd commit will now only exclude the zip being created, but not any in sub directories.

nfriedly commented 6 years ago

Hey, thanks,

Does this maintain parity with various native zip commands? (Admittedly, it's hard to test them all, but at least mac or linux?)

Lampei commented 6 years ago

I looked over the code and it appeared that you weren't actually calling the "NATIVE" section of the zip commands. You are referring to nodeZip in both https://github.com/nfriedly/node-bestzip/blob/master/lib/bestzip.js#L88 and https://github.com/nfriedly/node-bestzip/blob/master/lib/bestzip.js#L90 I wasn't sure if this was intentional or not, so I did not submit a pull request for that. That being said, both of my changes should not affect any of the native functionality. The first change uses the path package to check whether the current directory location that is being zipped is not the spot where the destination zip is being created. And the second change is passing an extra 'exclude' to the 'archiver' package rather than including everything. I tested via bash on windows, so did not do any testing against linux or mac.

nfriedly commented 6 years ago

Well, that's embarrassing. I probably did that during some debugging and then accidentally shipped it :(

This thing needs some tests like nobody's business...

Lampei commented 6 years ago

np :) I thought maybe you had run into issues using the native vs the nodeZip way, so I left it alone. It's probably testament that you only have 2 issues in your queue and 1K downloads/day that it is running pretty well at the moment.

nfriedly commented 6 years ago

Heh, yea, but one of those issues is a cross-platform thing that I thought was due to differences between the native and node zip methods...

BTW, I just tagged you on #9 - if you want to do a little more with this lib, I'd welcome the help.