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

Merged
merged 10 commits into from Jul 29, 2016

Conversation

Projects
None yet
4 participants
@benjamn
Member

benjamn commented Jul 22, 2016

For the sake of portability, the files.extractTarGz method is currently implemented in terms of a pure-JavaScript library: https://www.npmjs.com/package/tar

Portability is nice, but speed is also nice. On my machine, it takes 25 seconds to extract the fourseven:scss package using the npm tar package, whereas it takes only 3.5 seconds to extract it using the native tar 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), though tar 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

var extractor = new tar.Extract({
path: files.convertToOSPath(tempDir)
}).on('entry', function (e) {
if (process.platform === "win32" || options.forceConvert) {

This comment has been minimized.

@benjamn

benjamn Jul 22, 2016

Member

The options.forceConvert boolean is no longer passed by any callers.

@@ -695,19 +696,154 @@ files.extractTarGz = function (buffer, destPath, options) {
var tempDir = files.pathJoin(parentDir, '.tmp' + utils.randomToken());
files.mkdir_p(tempDir);

if (! _.has(options, "verbose")) {
options.verbose = require("../console/console.js").Console.verbose;

This comment has been minimized.

@benjamn

benjamn Jul 22, 2016

Member

Note that meteor run --verbose will cause all the output from the tar and/or 7z.exe commands to be displayed in the console, in case they don't seem to be working and we need to get some debugging clues.

@s-devaney

This comment has been minimized.

s-devaney commented Jul 28, 2016

Could the status of the extraction be added to the update process? At the moment it says "downloading" rather than "extracting" etc

@abernix

This comment has been minimized.

Member

abernix commented Jul 28, 2016

@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, meteor run --verbose will be super informative.

@benjamn

This comment has been minimized.

Member

benjamn commented Jul 28, 2016

@s-devaney I'll add a commit that reflects that, thanks!

@benjamn benjamn force-pushed the prefer-native-tar-or-7z branch from 776927e to 536eece Jul 28, 2016

@benjamn

This comment has been minimized.

Member

benjamn commented Jul 28, 2016

The build messages now distinguish between downloading, extracting, and loading packages. Thanks to this PR, the extraction is now by far the cheapest part.

@benjamn

This comment has been minimized.

Member

benjamn commented Jul 28, 2016

@tmeasday should I hold off on merging this until after 1.4.0.1?

@tmeasday

This comment has been minimized.

Contributor

tmeasday commented Jul 29, 2016

Yeah, although I can create a release branch for 1.4.0.1, so merge it whenever

@benjamn benjamn merged commit a9cb9c4 into devel Jul 29, 2016

4 checks passed

CLA Author has signed the Meteor CLA.
Details
ci/circleci Your tests passed on CircleCI!
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment