shama/nodewebkit

Consider using download: https://www.npmjs.org/package/download

Closed this issue · 8 comments

https://www.npmjs.org/package/download as it's a very nice module and would simplify things here greatly. Would fix #15.

Also show a download progress indicator if you're feeling up for it.

adiq commented

It's also fix for #20 so it's not completely enhancement.
I guess download progress indicator is nice-to-have, but not most important atm.

Take a look at ebe27b7.
I don't know how you guys take care of permissions, but it will work with workaround for those who need to develop with nodewebkit.

@adiq Nice! That is basically where my branch stopped on this enhancement/bugfix as well. I think we should send a pull request upstream into download or rather decompress to copy the modes of the files it extracts.

We could manually fs.chmodSync +x the files that need it but copying the mode from the archive seems like an easier path. @kevva is a pretty active and awesome developer too and I bet he'd merge a PR for it if one was sent his way on decompress. I just haven't got around to proposing a PR yet. :)

Is this the .zip file or the .tar.gz? I guess it's the .zip because it currently doesn't apply the mode there. Should fix that. The .tar and .tar.gz is already using the mode on the files it extracts (if not defined).

\o/ Thank you @kevva! You rock!

@adiq Did you want to send a PR for this?

adiq commented

Big thanks to @kevva.
Unfortunately i tried installing with his new fix, and it didn't help at all.

I've tested with 4fbb6ae changes using download@0.1.10 and decompress@0.1.8.

@shama please take a look at this.

Sorry I should have been more clear. With zip files on OSX you can specify the mode in the external file attributes header. Currently we're using zip which exposes this as entry.getMode() to get the mode of the file within the zip so when extracted we can use the zip's permissions. See: https://github.com/kriskowal/zip/blob/master/zip.js#L219

I took a look into adm-zip which doesn't appear to support this ability. So for now, I'll just have it manually set the permissions after it extracts on OSX.

Thanks for the help guys! Commits in bound...

Published as v0.8.5-2. Thanks!