added option for winZip similar to macZip

[speedskater]

As large projects might result in slow application startup times, I suggest the following changes to introduce a winZip option. This option defaults to true (which leaves node-webkit-builder's behavior unchanged) and can be set to false to allow putting the application uncompressed next to the executable.

See:
#195

[felicienfrancois]

Great job but I have a few remarks:

What about linux ? Maybe the macZip and winZIp option should be unified in a general zip option ? (with backward compatibility for the macZip option)
On top of that the option name may be misleading as it is also the name of a well known software.

[CamiloMM]

Vehemently endorse zip instead of having to specify winZip, macZip and eventually linuxZip. Either way this pull is a very welcome thing.

[sanderboom]

+1

[sanderboom]

Thanks for looking into this.
Question: I want to disable zipping while building for win32 on a linux64 box. Just setting zip: true still zipped the build. I added zipping = false to mergeAppFiles in index.js to make it work. Probably I missed something, any ideas? Thanks.

[ChristianWeyer]

@mllrsohn will this ever the merged? This is a highly needed feature :)
Thanks!

[matthew-dean]

I came here looking exactly for this. I'm running into this issue: nwjs/nw.js#514

It appears the problem is that Windows is taking 2-3 minutes (for my app) to extract every time, so it looks like the best option is to not zip the files at all (as mentioned in that thread). I would love to specify zip: false or zip: [osx64, linux32] to specify specific platforms to zip. (Default: zip: [osx64])

[matthew-dean]

I used the @speedskater fork and it worked wonderfully. Epic fast Windows builds now!

[conceptualspace]

👍

[CamiloMM]

These changes haven't been merged into nw-builder yet, have they?

[matthew-dean]

Can this be merged in? The @speedskater fork no longer works, I guess with current versions of Node.

[ngjermundshaug]

Also waiting for this to be merged in.

[elliott-jenkins]

Also interested in this to be merged 👍

[matheussampaio]

👍

[allanharris]

This feature is a must have!

[matthew-dean]

Note: I've done another PR that merged a more recent version of nw-builder with the code changes that @speedskater made. The PR is here: #256. This is all I use now since the regular nw-builder doesn't work for me at all (since it turns Windows app start times > 3 mins).

[meriturva]

No news here? Waiting for this to be merged in!!!!

[matthew-dean]

@meriturva - It does seem rather non-controversial. It just adds the option to other platforms. ¯_(ツ)_/¯

[rBurgett]

I'm working on a production application which will ship to thousands of anthropologists/humanitarian workers around the world after the new year and will be used cross-platform. I can't possibly accept a 30 second load time in Windows! How has this not been merged in yet? Don't the makers of NW want it to be a viable platform on Windows?

[ngjermundshaug]

@rBurgett Look at the excellent Pull Request of @matthew-dean and use his code. No need to sit still waiting for this to be merged in when you can fix your loading problem within minutes 😃

[rBurgett]

@ngjermundshaug That is exactly what I am using right now and I am extremely thankful for the work of @matthew-dean. But I don't see why the nwjs team won't make it official. My client wants the software as reliable as possible. It is meant to be used for many years. I don't mind using a fork right now, but down the road it makes me nervous.

[ngjermundshaug]

@rBurgett Totally agree - this PR is a no-brainer that should have been merged in months ago.

[matthew-dean]

@ngjermundshaug @rBurgett Don't give me too much credit. All I did really was merge @speedskater's PR to a more recent version of nw-builder.

The crazy thing about this whole issue: why would it ever be a good idea to unzip a bunch of files on EACH load of an application? Not only does zipping slow down Windows apps to a crawl (if they have any amount of node modules and those modules' dependencies), but the unzip happens every time the app is opened. How is this a good idea under any scenario? I don't get it.

[CamiloMM]

@matthew-dean It would be a good idea if you were unzipping files as they get requested, the zip file was non-solid and used a lightweight compression scheme (like DEFLATE) - essentially treating the zip as a compressed filesystem. This would be significantly faster on HDDs since CPU is cheaper than IO under these circumstances (some node modules have a lot of unused stuff like test dirs).

(edit: but yes, unzipping all of it, all the time is just amateur programming)

However, the real issue here is that nw-builder's dev team is probably a bunch of macfag half-wits who can't see the merge button behind the starbucks cups.

[matthew-dean]

@CamiloMM Errr I'm not sure the best way to motivate an open source team is to insult them. But I would obviously agree that there is probably a smarter approach.

I'd probably suggest forking to a new project, but my understanding is that NW13 will have some sort of "packaging" solution ready to go.

[k0ff33]

When I try to use the SpeedSkater fork I get this error:

[...]\node-webkit-builder\node_modules\graceful-ncp\node_modules\proxyquire\lib\proxyquire.js:13
if (!mdl[key]) mdl[key] = original[key];
^
TypeError: Cannot assign to read only property 'F_OK' of #
at [...]\node-webkit-builder\node_modules\gracefulncp\node_modules\proxyquire\lib\proxyquire.js:13:30

Anyone knows what am I doing wrong?

[ngjermundshaug]

@Lsks Grab these files instead - they are newer than the SpeedSkater fork: https://github.com/Crunch/nw-builder/tree/master/lib

[k0ff33]

Great, thanks, I thought that the SpeedSkater's fork is the newest one. Works fine!

[matthew-dean]

@Lsks @ngjermundshaug I'll leave that fork up, then, and I'll probably pull in newer nw-builder updates if they're critical. This is the fork I'm currently using for Crunch 2, so it seems to work fine.

[matthew-dean]

@CamiloMM Have you seen this? - https://github.com/atom/asar. It's an Atom shell utility that allows you to extract a file without unpacking the whole archive.

[CamiloMM]

@matthew-dean while unrelated to the current issue (unless someone is up for a full-blown fork and not just a pull request attempt), that is seriously nice. Simple format, extensible... it's great to know it exists, and it would be awesome to have a nw-builder with such a format (moreover because without compression, you can redistribute your app with the external compression you desire).

[adam-lynch]

Closing in favour of #193. Thanks everyone for the effort! I promise quicker replies in future. Just need to test #193 (should be able to test on Windows tomorrow).

[matthew-dean]

@adam-lynch It wasn't clear if #193 was the same thing, since it said it was about disabling merge of zip file with executable, and not disabling zipping entirely. Is that what "disabling merge" means, that is doesn't zip the files?

[adam-lynch]

Yeah I think so @matthew-dean. Have a look at my test results in the #193 thread. That might clear things up?

[matthew-dean]

From what I'm seeing in the code changes, all it does is separate package.nw (a zip of app files) from the executable. That's not what this thread or these pull requests are at all.

This request is about _not creating a zipped package in the first place._ Simply separating the zip does not address the performance problem on Windows. This issue should not be closed.

[adam-lynch]

@matthew-dean Ugh :( ok, thanks. Sorry, I've come back to this module after spending too much time away. So this performance is exclusive to Windows right? Maybe #270 is better to merge since it's calling the option winZip. The only problem I see with it is that it should default to true.

[polpo]

I don't think it's exclusive to Windows. I've had to stop using zipped application bundles on Linux due to the increased startup time.

[adam-lynch]

@polpo thanks! What do you two think... we should add winZip and linuxZip options which default to true and have a separate function for checking if the zipping should happen based on the platform (like this PR does).

[matthew-dean]

I think it should be a simple "zip" property which accepts either Boolean true/false (default true) or an array of platforms.

[matthew-dean]

(Or "compress")

[adam-lynch]

@matthew-dean but then it would conflict with macZip?

[matthew-dean]

macZip would be deprecated, and in the short term, would be the equivalent of "zip: ['osx']". There's no need to have separate properties to set one behaviour.

[matthew-dean]

Look at the code:

NwBuilder.prototype.isPlatformNeedingZip = function(name, platform) {
    var self = this,
        needsZip = platform.needsZip;

    if(name.indexOf('osx') === 0 && self.options.macZip != null) {
        needsZip = self.options.macZip;
    } else if(self.options.zip != null) {
        needsZip = self.options.zip;
    }

    return needsZip;
};

@speedskater already had all of this covered. I updated to a later nw-builder version, but otherwise, this PR is ready to go, as is. There's no need to overthink it. It doesn't break anything previously, and if someone has macZip as a previous option, it will still work. It just adds the ability to turn it off for other platforms, or off for all. Don't reinvent the wheel here with "linuxZip" and "winZip". Those options were already discussed in this very thread, with the consensus being "zip" instead, which the first PR fulfilled. So, basically the only thing needed is to merge https://github.com/Crunch/nw-builder/tree/master/lib and this issue is closeable.

[adam-lynch]

Ok this is out in v2.2.0. Really, thanks everybody!

[matthew-dean]

Thanks @adam-lynch!