-
Notifications
You must be signed in to change notification settings - Fork 5.2k
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
Extract .tar.gz files using native tar or 7z.exe when possible #7457
Conversation
var extractor = new tar.Extract({ | ||
path: files.convertToOSPath(tempDir) | ||
}).on('entry', function (e) { | ||
if (process.platform === "win32" || options.forceConvert) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The options.forceConvert
boolean is no longer passed by any callers.
f89865d
to
776927e
Compare
Could the status of the extraction be added to the update process? At the moment it says "downloading" rather than "extracting" etc |
@s-devaney This is a good idea. I've looked at this recently, but currently they're in the same sub-task so the message doesn't change. I think assuming that this PR works okay, it will be possible to improve the messaging. For now, as Ben mentioned above, |
@s-devaney I'll add a commit that reflects that, thanks! |
776927e
to
536eece
Compare
The build messages now distinguish between downloading, extracting, and loading packages. Thanks to this PR, the extraction is now by far the cheapest part. |
@tmeasday should I hold off on merging this until after 1.4.0.1? |
Yeah, although I can create a release branch for 1.4.0.1, so merge it whenever |
For the sake of portability, the
files.extractTarGz
method is currently implemented in terms of a pure-JavaScript library: https://www.npmjs.com/package/tarPortability is nice, but speed is also nice. On my machine, it takes 25 seconds to extract the
fourseven:scss
package using the npmtar
package, whereas it takes only 3.5 seconds to extract it using the nativetar
command. On Windows, the time drops from 36 seconds to 8 seconds.The time savings for even larger packages like
meteor-tool
are even more dramatic: 45 seconds vs. 4 seconds on my Mac.This pull request uses the native commands when possible, but falls back to the npm
tar
library if the native commands don't work for any reason.For Windows, we now include a standalone
7z.exe
binary in the dev bundle (i.e.,dev_bundle\bin\7z.exe
), thoughtar
will also work on Windows if it's in the$env:PATH
. This is the standard 7-Zip 16.02 program available here. I believe this usage is compatible with the 7-zip license, according to the FAQ.I think these changes may be too risky to ship with Meteor 1.4, given that it's now in the release candidate stage, but if all goes well we will ship them soon after that release (likely 1.4.1).
cc @tmeasday @zol @abernix