switched to tar-fs from node-tar to unpack tars (#2826)

* Set default network concurrency to 8 and switched to tar-fs instead of node-tar

* migrated pack test

* another file migrated to tar-fs

* changed concurrency in another PR

* wip migrating git to use tar-fs

* removed tar dependency

* bump tar-stream

* reverted changes to url in hasArchiveCapability

* made changes according to feedback

* 0o444

* added flow suppress

Author: bestander

__tests__/commands/pack.js

@@ -14,7 +14,7 @@ const os = require('os');
 const stream = require('stream');
 
 const zlib = require('zlib');
-const tar = require('tar');
+const tarFs = require('tar-fs');
 const fs2 = require('fs');
 
 const fixturesLoc = path.join(__dirname, '..', 'fixtures', 'pack');
@@ -83,8 +83,12 @@ export async function getFilesFromArchive(source, destination): Promise<Array<st
   const unzip = new Promise((resolve, reject) => {
     fs2.createReadStream(source)
       .pipe(new zlib.Gunzip())
-      .pipe(tar.Extract({path: destination, strip: 1}))
-      .on('end', resolve)
+      .pipe(tarFs.extract(destination, {
+        strip: 1,
+        dmode: 0o555, // all dirs should be readable
+        fmode: 0o444, // all files should be readable
+      }))
+      .on('finish', resolve)
       .on('error', reject);
   });
   await unzip;

package.json

@@ -35,7 +35,7 @@
     "roadrunner": "^1.1.0",
     "semver": "^5.1.0",
     "strip-bom": "^3.0.0",
-    "tar": "^2.2.1",
+    "tar-fs": "^1.15.1",
     "tar-stream": "^1.5.2",
     "v8-compile-cache": "^1.0.0",
     "validate-npm-package-license": "^3.0.1"

src/constants.js

@@ -31,7 +31,7 @@ export const CACHE_VERSION = 1;
 export const LOCKFILE_VERSION = 1;
 
 // max amount of network requests to perform concurrently
-export const NETWORK_CONCURRENCY = 8;
+export const NETWORK_CONCURRENCY = 16;
 
 // max amount of child processes to execute concurrently
 export const CHILD_CONCURRENCY = 5;

src/fetchers/git-fetcher.js

@@ -7,7 +7,7 @@ import Git from '../util/git.js';
 import * as fsUtil from '../util/fs.js';
 import * as crypto from '../util/crypto.js';
 
-const tar = require('tar');
+const tarFs = require('tar-fs');
 const url = require('url');
 const path = require('path');
 const fs = require('fs');
@@ -34,15 +34,18 @@ export default class GitFetcher extends BaseFetcher {
     }
 
     return new Promise((resolve, reject) => {
-      const untarStream = tar.Extract({path: this.dest});
+      const untarStream = tarFs.extract(this.dest, {
+        dmode: 0o555, // all dirs should be readable
+        fmode: 0o444, // all files should be readable
+      });
 
       const hashStream = new crypto.HashStream();
 
       const cachedStream = fs.createReadStream(localTarball);
       cachedStream
         .pipe(hashStream)
         .pipe(untarStream)
-        .on('end', () => {
+        .on('finish', () => {
           const expectHash = this.hash;
           const actualHash = hashStream.getHash();
           if (!expectHash || expectHash === actualHash) {

src/fetchers/tarball-fetcher.js

@@ -12,7 +12,7 @@ import ROOT_USER from '../util/root-user.js';
 
 const invariant = require('invariant');
 const path = require('path');
-const tar = require('tar');
+const tarFs = require('tar-fs');
 const url = require('url');
 const fs = require('fs');
 
@@ -78,7 +78,11 @@ export default class TarballFetcher extends BaseFetcher {
   } {
     const validateStream = new crypto.HashStream();
     const extractorStream = new UnpackStream();
-    const untarStream = tar.Extract({path: this.dest, strip: 1});
+    const untarStream = tarFs.extract(this.dest, {
+      strip: 1,
+      dmode: 0o555, // all dirs should be readable
+      fmode: 0o444, // all files should be readable
+    });
 
     extractorStream
       .pipe(untarStream)
@@ -89,7 +93,7 @@ export default class TarballFetcher extends BaseFetcher {
           entry.props.gid = entry.gid = 0;
         }
       })
-      .on('end', () => {
+      .on('finish', () => {
         const expectHash = this.hash;
         const actualHash = validateStream.getHash();
         if (!expectHash || expectHash === actualHash) {

src/util/git.js

@@ -11,8 +11,10 @@ import map from './map.js';
 
 const invariant = require('invariant');
 const semver = require('semver');
+const StringDecoder = require('string_decoder').StringDecoder;
+const tarFs = require('tar-fs');
+const tarStream = require('tar-stream');
 const url = require('url');
-const tar = require('tar');
 import {createWriteStream} from 'fs';
 
 type GitRefs = {
@@ -205,9 +207,12 @@ export default class Git {
   async _cloneViaRemoteArchive(dest: string): Promise<void> {
     await child.spawn('git', ['archive', `--remote=${this.url}`, this.ref], {
       process(proc, update, reject, done) {
-        const extractor = tar.Extract({path: dest});
+        const extractor = tarFs.extract(dest, {
+          dmode: 0o555, // all dirs should be readable
+          fmode: 0o444, // all files should be readable
+        });
         extractor.on('error', reject);
-        extractor.on('end', done);
+        extractor.on('finish', done);
 
         proc.stdout.pipe(extractor);
         proc.on('error', reject);
@@ -219,9 +224,13 @@ export default class Git {
     await child.spawn('git', ['archive', this.hash], {
       cwd: this.cwd,
       process(proc, resolve, reject, done) {
-        const extractor = tar.Extract({path: dest});
+        const extractor = tarFs.extract(dest, {
+          dmode: 0o555, // all dirs should be readable
+          fmode: 0o444, // all files should be readable
+        });
+
         extractor.on('error', reject);
-        extractor.on('end', done);
+        extractor.on('finish', done);
 
         proc.stdout.pipe(extractor);
       },
@@ -279,13 +288,25 @@ export default class Git {
     try {
       return await child.spawn('git', ['archive', `--remote=${this.url}`, this.ref, filename], {
         process(proc, update, reject, done) {
-          const parser = tar.Parse();
+          const parser = tarStream.extract();
 
           parser.on('error', reject);
-          parser.on('end', done);
-
-          parser.on('data', (entry: Buffer) => {
-            update(entry.toString());
+          parser.on('finish', done);
+
+          parser.on('entry', (header, stream, next) => {
+            const decoder = new StringDecoder('utf8');
+            let fileContent = '';
+
+            stream.on('data', (buffer) => {
+              fileContent += decoder.write(buffer);
+            });
+            stream.on('end', () => {
+              // $FlowFixMe: suppressing this error due to bug https://github.com/facebook/flow/pull/3483
+              const remaining: string = decoder.end();
+              update(fileContent + remaining);
+              next();
+            });
+            stream.resume();
           });
 
           proc.stdout.pipe(parser);

yarn.lock

@@ -985,6 +985,10 @@ chokidar@^1.4.3, chokidar@^1.6.1:
   optionalDependencies:
     fsevents "^1.0.0"
 
+chownr@^1.0.1:
+  version "1.0.1"
+  resolved "https://registry.yarnpkg.com/chownr/-/chownr-1.0.1.tgz#e2a75042a9551908bebd25b8523d5f9769d79181"
+
 ci-info@^1.0.0:
   version "1.0.0"
   resolved "https://registry.yarnpkg.com/ci-info/-/ci-info-1.0.0.tgz#dc5285f2b4e251821683681c381c3388f46ec534"
@@ -1380,7 +1384,7 @@ end-of-stream@1.0.0:
   dependencies:
     once "~1.3.0"
 
-end-of-stream@^1.0.0:
+end-of-stream@^1.0.0, end-of-stream@^1.1.0:
   version "1.1.0"
   resolved "https://registry.yarnpkg.com/end-of-stream/-/end-of-stream-1.1.0.tgz#e9353258baa9108965efc41cb0ef8ade2f3cfb07"
   dependencies:
@@ -3406,7 +3410,7 @@ object.omit@^2.0.0:
     for-own "^0.1.4"
     is-extendable "^0.1.1"
 
-once@^1.3.0, once@^1.4.0:
+once@^1.3.0, once@^1.3.1, once@^1.4.0:
   version "1.4.0"
   resolved "https://registry.yarnpkg.com/once/-/once-1.4.0.tgz#583b1aa775961d4b113ac17d9c50baef9dd76bd1"
   dependencies:
@@ -3671,6 +3675,13 @@ public-encrypt@^4.0.0:
     parse-asn1 "^5.0.0"
     randombytes "^2.0.1"
 
+pump@^1.0.0:
+  version "1.0.2"
+  resolved "https://registry.yarnpkg.com/pump/-/pump-1.0.2.tgz#3b3ee6512f94f0e575538c17995f9f16990a5d51"
+  dependencies:
+    end-of-stream "^1.1.0"
+    once "^1.3.1"
+
 punycode@1.3.2:
   version "1.3.2"
   resolved "https://registry.yarnpkg.com/punycode/-/punycode-1.3.2.tgz#9653a036fb7c1ee42342f2325cceefea3926c48d"
@@ -4268,6 +4279,15 @@ tapable@^0.2.5, tapable@~0.2.5:
   version "0.2.6"
   resolved "https://registry.yarnpkg.com/tapable/-/tapable-0.2.6.tgz#206be8e188860b514425375e6f1ae89bfb01fd8d"
 
+tar-fs@^1.15.1:
+  version "1.15.1"
+  resolved "https://registry.yarnpkg.com/tar-fs/-/tar-fs-1.15.1.tgz#f4622f5d5e250742b3679a9a8463acfc12cdefd1"
+  dependencies:
+    chownr "^1.0.1"
+    mkdirp "^0.5.0"
+    pump "^1.0.0"
+    tar-stream "^1.1.2"
+
 tar-pack@~3.3.0:
   version "3.3.0"
   resolved "https://registry.yarnpkg.com/tar-pack/-/tar-pack-3.3.0.tgz#30931816418f55afc4d21775afdd6720cee45dae"
@@ -4281,7 +4301,7 @@ tar-pack@~3.3.0:
     tar "~2.2.1"
     uid-number "~0.0.6"
 
-tar-stream@^1.5.2:
+tar-stream@^1.1.2, tar-stream@^1.5.2:
   version "1.5.2"
   resolved "https://registry.yarnpkg.com/tar-stream/-/tar-stream-1.5.2.tgz#fbc6c6e83c1a19d4cb48c7d96171fc248effc7bf"
   dependencies:
@@ -4290,7 +4310,7 @@ tar-stream@^1.5.2:
     readable-stream "^2.0.0"
     xtend "^4.0.0"
 
-tar@^2.0.0, tar@^2.2.1, tar@~2.2.1:
+tar@^2.0.0, tar@~2.2.1:
   version "2.2.1"
   resolved "https://registry.yarnpkg.com/tar/-/tar-2.2.1.tgz#8e4d2a256c0e2185c6b18ad694aec968b83cb1d1"
   dependencies: