Skip to content
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

Revert "install: do not descend into directory deps' child modules" #242

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 6 additions & 14 deletions lib/install/inflate-shrinkwrap.js
Original file line number Diff line number Diff line change
Expand Up @@ -50,12 +50,8 @@ function inflateShrinkwrap (topPath, tree, swdeps, opts) {

return BB.each(Object.keys(swdeps), (name) => {
const sw = swdeps[name]
const dependencies = sw.dependencies || {}
const requested = realizeShrinkwrapSpecifier(name, sw, topPath)
// We should not muck about in the node_modules folder of
// symlinked packages. Treat its dependencies as if they
// were empty, since it's none of our business.
const dependencies = requested.type === 'directory' ? {}
: sw.dependencies || {}
return inflatableChild(
onDisk[name], name, topPath, tree, sw, requested, opts
).then((child) => {
Expand Down Expand Up @@ -145,10 +141,6 @@ function isGit (sw) {
}

function makeFakeChild (name, topPath, tree, sw, requested) {
// We should not muck about in the node_modules folder of
// symlinked packages. Treat its dependencies as if they
// were empty, since it's none of our business.
const isDirectory = requested.type === 'directory'
const from = sw.from || requested.raw
const pkg = {
name: name,
Expand All @@ -164,7 +156,7 @@ function makeFakeChild (name, topPath, tree, sw, requested) {
_spec: requested.rawSpec,
_where: topPath,
_args: [[requested.toString(), topPath]],
dependencies: isDirectory ? {} : sw.requires
dependencies: sw.requires
}

if (!sw.bundled) {
Expand All @@ -175,16 +167,16 @@ function makeFakeChild (name, topPath, tree, sw, requested) {
}
const child = createChild({
package: pkg,
loaded: isDirectory,
loaded: true,
parent: tree,
children: [],
fromShrinkwrap: requested,
fakeChild: sw,
fromBundle: sw.bundled ? tree.fromBundle || tree : null,
path: childPath(tree.path, pkg),
realpath: isDirectory ? requested.fetchSpec : childPath(tree.realpath, pkg),
realpath: requested.type === 'directory' ? requested.fetchSpec : childPath(tree.realpath, pkg),
location: (tree.location === '/' ? '' : tree.location + '/') + pkg.name,
isLink: isDirectory,
isLink: requested.type === 'directory',
isInLink: tree.isLink || tree.isInLink,
swRequires: sw.requires
})
Expand All @@ -203,7 +195,7 @@ function fetchChild (topPath, tree, sw, requested) {
var isLink = pkg._requested.type === 'directory'
const child = createChild({
package: pkg,
loaded: isLink,
loaded: false,
parent: tree,
fromShrinkwrap: requested,
path: childPath(tree.path, pkg),
Expand Down
6 changes: 1 addition & 5 deletions test/tap/install-from-local-multipath.js
Original file line number Diff line number Diff line change
Expand Up @@ -167,13 +167,9 @@ test('\'npm install\' should install local packages', function (t) {
'install', '.'
],
EXEC_OPTS,
function (err, code, stdout, stderr) {
function (err, code) {
t.ifError(err, 'error should not exist')
t.notOk(code, 'npm install exited with code 0')
// if the test fails, let's see why
if (err || code) {
console.error({code, stdout, stderr})
}
t.end()
}
)
Expand Down
52 changes: 52 additions & 0 deletions test/tap/install-link-metadeps-locally.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
// XXX Remove in npm v7, when this is no longer how we do things
const t = require('tap')
const common = require('../common-tap.js')
const pkg = common.pkg
const mkdirp = require('mkdirp')
const { writeFileSync, statSync } = require('fs')
const { resolve } = require('path')
const mr = require('npm-registry-mock')
const rimraf = require('rimraf')

t.test('setup', t => {
mkdirp.sync(resolve(pkg, 'node_modules'))
mkdirp.sync(resolve(pkg, 'foo'))
writeFileSync(resolve(pkg, 'foo', 'package.json'), JSON.stringify({
name: 'foo',
version: '1.2.3',
dependencies: {
underscore: '*'
}
}))

writeFileSync(resolve(pkg, 'package.json'), JSON.stringify({
name: 'root',
version: '1.2.3',
dependencies: {
foo: 'file:foo'
}
}))

mr({ port: common.port }, (er, s) => {
if (er) {
throw er
}
t.parent.teardown(() => s.close())
t.end()
})
})

t.test('initial install to create package-lock',
t => common.npm(['install', '--registry', common.registry], { cwd: pkg })
.then(([code]) => t.equal(code, 0, 'command worked')))

t.test('remove node_modules', t =>
rimraf(resolve(pkg, 'node_modules'), t.end))

t.test('install again from package-lock', t =>
common.npm(['install', '--registry', common.registry], { cwd: pkg })
.then(([code]) => {
t.equal(code, 0, 'command worked')
const underscore = resolve(pkg, 'node_modules', 'underscore')
t.equal(statSync(underscore).isDirectory(), true, 'underscore installed')
}))
68 changes: 68 additions & 0 deletions test/tap/install-link-metadeps-subfolders.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,68 @@
const t = require('tap')
const common = require('../common-tap.js')
const mkdirp = require('mkdirp')
const { writeFileSync, readFileSync } = require('fs')
const { resolve } = require('path')
const pkg = common.pkg
const app = resolve(pkg, 'app')
const lib = resolve(pkg, 'lib')
const moda = resolve(lib, 'module-a')
const modb = resolve(lib, 'module-b')

const rimraf = require('rimraf')

t.test('setup', t => {
mkdirp.sync(app)
mkdirp.sync(moda)
mkdirp.sync(modb)

writeFileSync(resolve(app, 'package.json'), JSON.stringify({
name: 'app',
version: '1.2.3',
dependencies: {
moda: 'file:../lib/module-a'
}
}))

writeFileSync(resolve(moda, 'package.json'), JSON.stringify({
name: 'moda',
version: '1.2.3',
dependencies: {
modb: 'file:../module-b'
}
}))

writeFileSync(resolve(modb, 'package.json'), JSON.stringify({
name: 'modb',
version: '1.2.3'
}))

t.end()
})

t.test('initial install to create package-lock',
t => common.npm(['install'], { cwd: app })
.then(([code]) => t.equal(code, 0, 'command worked')))

t.test('remove node_modules', t =>
rimraf(resolve(pkg, 'node_modules'), t.end))

t.test('install again from package-lock', t =>
common.npm(['install'], { cwd: app })
.then(([code]) => {
t.equal(code, 0, 'command worked')
// verify that module-b is linked under module-a
const depPkg = resolve(
app,
'node_modules',
'moda',
'node_modules',
'modb',
'package.json'
)
const data = JSON.parse(readFileSync(depPkg, 'utf8'))
t.strictSame(data, {
name: 'modb',
version: '1.2.3'
})
}))
127 changes: 0 additions & 127 deletions test/tap/install-symlink-leave-children-alone.js

This file was deleted.