From 0dd518eba01495acbd302c889ab10b8cbfcf2e95 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kat=20March=C3=A1n?= Date: Fri, 18 Aug 2017 17:09:12 -0700 Subject: [PATCH] fix(tar): bring back the .gitignore -> .npmignore logic (#113) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit pacote@5 dropped support for a Very Legacy™ thing where npm would, on extract, rename .gitignore to .npmignore if there was not already an .npmignore. This was primarily to support the use case where folks would use npm install on github release tarballs. Ref: https://github.com/npm/npm/issues/5658 BREAKING CHANGE: this reverts a previous change to disable this feature. --- lib/extract-stream.js | 21 ++++++++++- package-lock.json | 6 +-- package.json | 2 +- test/extract-stream.js | 83 ++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 107 insertions(+), 5 deletions(-) diff --git a/lib/extract-stream.js b/lib/extract-stream.js index eb6e75b..1175319 100644 --- a/lib/extract-stream.js +++ b/lib/extract-stream.js @@ -1,10 +1,12 @@ 'use strict' +const path = require('path') const tar = require('tar') module.exports = extractStream -function extractStream (dest, opts, cb) { +function extractStream (dest, opts) { opts = opts || {} + const sawIgnores = new Set() return tar.x({ cwd: dest, filter: (name, entry) => !entry.header.type.match(/^.*link$/i), @@ -18,6 +20,23 @@ function extractStream (dest, opts, cb) { } else if (entry.type.toLowerCase() === 'directory') { entry.mode = opts.dmode & ~(opts.umask || 0) } + + // Note: This mirrors logic in the fs read operations that are + // employed during tarball creation, in the fstream-npm module. + // It is duplicated here to handle tarballs that are created + // using other means, such as system tar or git archive. + if (entry.type.toLowerCase() === 'file') { + const base = path.basename(entry.path) + if (base === '.npmignore') { + sawIgnores.add(entry.path) + } else if (base === '.gitignore') { + const npmignore = entry.path.replace(/\.gitignore$/, '.npmignore') + if (!sawIgnores.has(npmignore)) { + // Rename, may be clobbered later. + entry.path = npmignore + } + } + } } }) } diff --git a/package-lock.json b/package-lock.json index d54bfc8..df4fc5f 100644 --- a/package-lock.json +++ b/package-lock.json @@ -5534,9 +5534,9 @@ } }, "tar": { - "version": "3.2.0", - "resolved": "https://registry.npmjs.org/tar/-/tar-3.2.0.tgz", - "integrity": "sha512-YRHTZzumkYuLx/lXsWZVowi9WrYbOQvxbEDtuFjq+LPCtLLGleWLkL/pdCnTwZMA92+Vg3UwVRBjpVQNcg6H3A==", + "version": "4.0.0", + "resolved": "https://registry.npmjs.org/tar/-/tar-4.0.0.tgz", + "integrity": "sha512-oHhf8rlu/1pLR0fEaUvkjTFScTBPWxyH6rxCURo4NWHjM7T0WzPvVaIZ1cw3VfdKc5852QAWavADhR14XeoaEA==", "requires": { "chownr": "1.0.1", "minipass": "2.2.1", diff --git a/package.json b/package.json index 3b04578..240523f 100644 --- a/package.json +++ b/package.json @@ -59,7 +59,7 @@ "safe-buffer": "^5.1.1", "semver": "^5.4.1", "ssri": "^4.1.6", - "tar": "^3.2.0", + "tar": "^4.0.0", "unique-filename": "^1.1.0", "which": "^1.3.0" }, diff --git a/test/extract-stream.js b/test/extract-stream.js index 0578d04..76a47ac 100644 --- a/test/extract-stream.js +++ b/test/extract-stream.js @@ -3,6 +3,7 @@ const BB = require('bluebird') const fs = BB.promisifyAll(require('fs')) +const mkdirp = BB.promisify(require('mkdirp')) const mockTar = require('./util/mock-tarball') const npmlog = require('npmlog') const path = require('path') @@ -118,6 +119,88 @@ test('excludes symlinks', t => { }) }) +// Yes, this logic is terrible and seriously confusing, but +// I'm pretty sure this is exactly what npm is doing. +// ...we should really deprecate this cluster. +test('renames .gitignore to .npmignore if not present', t => { + return mkdirp('./no-npmignore').then(() => { + return mockTar({ + 'package.json': JSON.stringify({ + name: 'foo', + version: '1.0.0' + }), + 'index.js': 'console.log("hello world!")', + '.gitignore': 'tada!' + }, {stream: true}).then(tarStream => { + return pipe(tarStream, extractStream('./no-npmignore', OPTS)) + }).then(() => { + return fs.readFileAsync( + './no-npmignore/.npmignore', 'utf8' + ).then(data => { + t.deepEqual(data, 'tada!', '.gitignore renamed to .npmignore') + }) + }) + }).then(() => { + return mkdirp('./has-npmignore1') + }).then(() => { + return mockTar({ + 'package.json': JSON.stringify({ + name: 'foo', + version: '1.0.0' + }), + 'index.js': 'console.log("hello world!")', + '.gitignore': 'git!', + '.npmignore': 'npm!' + }, {stream: true}).then(tarStream => { + return pipe(tarStream, extractStream('./has-npmignore1', OPTS)) + }).then(() => { + return BB.join( + fs.readFileAsync( + './has-npmignore1/.npmignore', 'utf8' + ).then(data => { + t.deepEqual(data, 'npm!', '.npmignore left intact if present') + }), + fs.readFileAsync( + './has-npmignore1/.gitignore', 'utf8' + ).then( + () => { throw new Error('expected an error') }, + err => { + t.ok(err, 'got expected error on reading .gitignore') + t.equal(err.code, 'ENOENT', '.gitignore missing') + } + ) + ) + }) + }).then(() => { + return mkdirp('./has-npmignore2') + }).then(() => { + return mockTar({ + 'package.json': JSON.stringify({ + name: 'foo', + version: '1.0.0' + }), + 'index.js': 'console.log("hello world!")', + '.npmignore': 'npm!', + '.gitignore': 'git!' + }, {stream: true}).then(tarStream => { + return pipe(tarStream, extractStream('./has-npmignore2', OPTS)) + }).then(() => { + return BB.join( + fs.readFileAsync( + './has-npmignore2/.npmignore', 'utf8' + ).then(data => { + t.deepEqual(data, 'npm!', '.npmignore left intact if present') + }), + fs.readFileAsync( + './has-npmignore2/.gitignore', 'utf8' + ).then(data => { + t.deepEqual(data, 'git!', '.gitignore intact if we previously had an .npmignore') + }) + ) + }) + }) +}) + test('accepts dmode/fmode/umask opts', { skip: process.platform === 'win32' }, t => {