From 761887f6bf4ee1abc239a0cba48a0cc8ef8ab236 Mon Sep 17 00:00:00 2001 From: David Freilich Date: Wed, 13 Feb 2019 15:34:09 -0500 Subject: [PATCH] Add unmet deps warning - Refactor runner to include quiet mode - Refactor npm version to use quiet mode [#163528537] Co-authored-by: Danny Joyce --- go.mod | 4 +- go.sum | 4 + integration/missing_node_modules_test.go | 13 + integration/testdata/unmet_dep_npm/.gitignore | 1 + integration/testdata/unmet_dep_npm/.npmrc | 1 + integration/testdata/unmet_dep_npm/Procfile | 1 + integration/testdata/unmet_dep_npm/README.md | 1 + .../testdata/unmet_dep_npm/package-lock.json | 248 ++++++++++++++++++ .../testdata/unmet_dep_npm/package.json | 23 ++ integration/testdata/unmet_dep_npm/resolve | 1 + integration/testdata/unmet_dep_npm/server.js | 14 + integration/versioning_test.go | 2 +- npm/mocks_test.go | 16 +- npm/npm.go | 29 +- npm/npm_test.go | 20 +- utils/utils.go | 30 ++- 16 files changed, 375 insertions(+), 33 deletions(-) create mode 100644 integration/testdata/unmet_dep_npm/.gitignore create mode 100644 integration/testdata/unmet_dep_npm/.npmrc create mode 100644 integration/testdata/unmet_dep_npm/Procfile create mode 100644 integration/testdata/unmet_dep_npm/README.md create mode 100644 integration/testdata/unmet_dep_npm/package-lock.json create mode 100644 integration/testdata/unmet_dep_npm/package.json create mode 100644 integration/testdata/unmet_dep_npm/resolve create mode 100644 integration/testdata/unmet_dep_npm/server.js diff --git a/go.mod b/go.mod index feed1d3c..3af26b73 100644 --- a/go.mod +++ b/go.mod @@ -3,9 +3,9 @@ module github.com/cloudfoundry/npm-cnb require ( github.com/Masterminds/semver v1.4.2 github.com/buildpack/libbuildpack v1.10.0 - github.com/cloudfoundry/dagger v0.0.0-20190211213328-5a14d6d7438f + github.com/cloudfoundry/dagger v0.0.0-20190213192159-4348188594f6 github.com/cloudfoundry/libcfbuildpack v1.42.0 - github.com/cloudfoundry/nodejs-cnb v0.0.3-0.20181217224909-e1533a296e07 + github.com/cloudfoundry/nodejs-cnb v0.0.4 github.com/golang/mock v1.2.0 github.com/kr/pty v1.1.3 // indirect github.com/onsi/gomega v1.4.3 diff --git a/go.sum b/go.sum index e8f66d8c..e0003b79 100644 --- a/go.sum +++ b/go.sum @@ -13,6 +13,8 @@ github.com/cloudfoundry/dagger v0.0.0-20190204200826-d194e96e09ab h1:jx7sikzaAZA github.com/cloudfoundry/dagger v0.0.0-20190204200826-d194e96e09ab/go.mod h1:lew8J5SVLq1xe9PXy+38XTiuuW31NWHU92oP6YRkn1w= github.com/cloudfoundry/dagger v0.0.0-20190211213328-5a14d6d7438f h1:xhUh7luzCWNnXQqj3JtJ+gO+iFWoK4nTEW+y/x24Jc8= github.com/cloudfoundry/dagger v0.0.0-20190211213328-5a14d6d7438f/go.mod h1:lew8J5SVLq1xe9PXy+38XTiuuW31NWHU92oP6YRkn1w= +github.com/cloudfoundry/dagger v0.0.0-20190213192159-4348188594f6 h1:aT5ZbNHQdfqQKwq8aH93Z/57m2UgkKYxMc5dhCTd+vA= +github.com/cloudfoundry/dagger v0.0.0-20190213192159-4348188594f6/go.mod h1:lew8J5SVLq1xe9PXy+38XTiuuW31NWHU92oP6YRkn1w= github.com/cloudfoundry/libcfbuildpack v1.29.0/go.mod h1:OEwAwCqppAMwWrcrnxE0j0llq5HcvQHrUa6J5n8XMMc= github.com/cloudfoundry/libcfbuildpack v1.37.0 h1:/jeSWneCY2QD0n91OgaEHJrv9FUTZV+uoorSAdPg0Ik= github.com/cloudfoundry/libcfbuildpack v1.37.0/go.mod h1:WqW/cPLKqNnxhBt0JLTCQGpcKlFfC1T6vLHg0gNRtAA= @@ -22,6 +24,8 @@ github.com/cloudfoundry/libcfbuildpack v1.42.0 h1:vHVF9t0Px4p0WsgCG2EkUTTyXxI3Qg github.com/cloudfoundry/libcfbuildpack v1.42.0/go.mod h1:C2AOm63Ca1GIBP7NdVhd8Q2gKenz600PVWqtOAbrN4o= github.com/cloudfoundry/nodejs-cnb v0.0.3-0.20181217224909-e1533a296e07 h1:o1rOurj5HLkRABMapPeXwlxUtwwLZlHcuurSDdMtH2U= github.com/cloudfoundry/nodejs-cnb v0.0.3-0.20181217224909-e1533a296e07/go.mod h1:jIMmJN7byxXmRCqlTmhZ9IB2lZU3MpyVcWDIx6PGiVo= +github.com/cloudfoundry/nodejs-cnb v0.0.4 h1:1iet2dfSC+fZ/qNQKGkI2kMCb9QbEAUoHjNm4HdGjvU= +github.com/cloudfoundry/nodejs-cnb v0.0.4/go.mod h1:BkYMGwKyuBpqJMaxgipXOJctYRWtc4yutQVID/CGzeg= github.com/fatih/color v1.7.0 h1:DkWD4oS2D8LGGgTQ6IvwJJXSL5Vp2ffcQg58nFV38Ys= github.com/fatih/color v1.7.0/go.mod h1:Zm6kSWBoL9eyXnKyktHP6abPY2pDugNf5KwzbycvMj4= github.com/fsnotify/fsnotify v1.4.7 h1:IXs+QLmnXW2CcXuY+8Mzv/fWEsPGWxqefPtCP5CnV9I= diff --git a/integration/missing_node_modules_test.go b/integration/missing_node_modules_test.go index 338bccea..c6340304 100644 --- a/integration/missing_node_modules_test.go +++ b/integration/missing_node_modules_test.go @@ -59,4 +59,17 @@ func missingNodeModulesIntegration(t *testing.T, when spec.G, it spec.S) { Expect(err).NotTo(HaveOccurred()) }) }) + + when("package manager is npm", func() { + it("warns that unmet dependencies may cause issues", func() { + app, err := dagger.PackBuild(filepath.Join("testdata", "unmet_dep_npm"), nodeBP, bp) + Expect(err).ToNot(HaveOccurred()) + defer app.Destroy() + + Expect(app.Start()).To(Succeed()) + + Expect(app.BuildLogs()).To(ContainSubstring("Unmet dependencies don't fail npm install but may cause runtime issues")) + }) + }) + } diff --git a/integration/testdata/unmet_dep_npm/.gitignore b/integration/testdata/unmet_dep_npm/.gitignore new file mode 100644 index 00000000..c2658d7d --- /dev/null +++ b/integration/testdata/unmet_dep_npm/.gitignore @@ -0,0 +1 @@ +node_modules/ diff --git a/integration/testdata/unmet_dep_npm/.npmrc b/integration/testdata/unmet_dep_npm/.npmrc new file mode 100644 index 00000000..1e8fc641 --- /dev/null +++ b/integration/testdata/unmet_dep_npm/.npmrc @@ -0,0 +1 @@ +ca= diff --git a/integration/testdata/unmet_dep_npm/Procfile b/integration/testdata/unmet_dep_npm/Procfile new file mode 100644 index 00000000..6f86b16c --- /dev/null +++ b/integration/testdata/unmet_dep_npm/Procfile @@ -0,0 +1 @@ +web: node server.js \ No newline at end of file diff --git a/integration/testdata/unmet_dep_npm/README.md b/integration/testdata/unmet_dep_npm/README.md new file mode 100644 index 00000000..e42c2640 --- /dev/null +++ b/integration/testdata/unmet_dep_npm/README.md @@ -0,0 +1 @@ +This file here to suppress "npm WARN package.json node_web_app@0.0.0 No README data" diff --git a/integration/testdata/unmet_dep_npm/package-lock.json b/integration/testdata/unmet_dep_npm/package-lock.json new file mode 100644 index 00000000..682e73fa --- /dev/null +++ b/integration/testdata/unmet_dep_npm/package-lock.json @@ -0,0 +1,248 @@ +{ + "name": "node_web_app", + "version": "0.0.0", + "lockfileVersion": 1, + "requires": true, + "dependencies": { + "accepts": { + "version": "1.0.0", + "resolved": "https://registry.npmjs.org/accepts/-/accepts-1.0.0.tgz", + "integrity": "sha1-NgTHZVhsO5z3h3tpN829RYf5R9w=", + "requires": { + "mime": "~1.2.11", + "negotiator": "~0.3.0" + } + }, + "ansi-styles": { + "version": "0.2.0", + "resolved": "https://registry.npmjs.org/ansi-styles/-/ansi-styles-0.2.0.tgz", + "integrity": "sha1-NZq0sV3NZLptdHNLcsNjYKmvLBk=" + }, + "buffer-crc32": { + "version": "0.2.1", + "resolved": "https://registry.npmjs.org/buffer-crc32/-/buffer-crc32-0.2.1.tgz", + "integrity": "sha1-vj5TgvwCttYySVasGvmKqYsIU0w=" + }, + "chalk": { + "version": "0.3.0", + "resolved": "https://registry.npmjs.org/chalk/-/chalk-0.3.0.tgz", + "integrity": "sha1-HJhDdzfxGZ68wdTEj9Qbn5yOjyM=", + "requires": { + "ansi-styles": "~0.2.0", + "has-color": "~0.1.0" + } + }, + "cookie": { + "version": "0.1.0", + "resolved": "https://registry.npmjs.org/cookie/-/cookie-0.1.0.tgz", + "integrity": "sha1-kOtGndzpBchm3mh+/EMTHYgB+dA=" + }, + "cookie-signature": { + "version": "1.0.3", + "resolved": "https://registry.npmjs.org/cookie-signature/-/cookie-signature-1.0.3.tgz", + "integrity": "sha1-kc2ZfMUftkFZVzjGnNoCAyj1D/k=" + }, + "debug": { + "version": "0.8.1", + "resolved": "https://registry.npmjs.org/debug/-/debug-0.8.1.tgz", + "integrity": "sha1-IP9NJvXkIstoobrLu2EDmtjBwTA=" + }, + "escape-html": { + "version": "1.0.1", + "resolved": "https://registry.npmjs.org/escape-html/-/escape-html-1.0.1.tgz", + "integrity": "sha1-GBoobq05ejmpKFfPsdQwUuNWv/A=" + }, + "express": { + "version": "4.0.0", + "resolved": "https://registry.npmjs.org/express/-/express-4.0.0.tgz", + "integrity": "sha1-J03IKTPJ9XTMOKDOXqgXK+nGsJQ=", + "requires": { + "accepts": "1.0.0", + "buffer-crc32": "0.2.1", + "cookie": "0.1.0", + "cookie-signature": "1.0.3", + "debug": ">= 0.7.3 < 1", + "escape-html": "1.0.1", + "fresh": "0.2.2", + "merge-descriptors": "0.0.2", + "methods": "0.1.0", + "parseurl": "1.0.1", + "path-to-regexp": "0.1.2", + "qs": "0.6.6", + "range-parser": "1.0.0", + "send": "0.2.0", + "serve-static": "1.0.1", + "type-is": "1.0.0", + "utils-merge": "1.0.0" + } + }, + "fresh": { + "version": "0.2.2", + "resolved": "https://registry.npmjs.org/fresh/-/fresh-0.2.2.tgz", + "integrity": "sha1-lzHc9WeMf660T7kDxPct9VGH+nc=" + }, + "grunt-steroids": { + "version": "0.2.3", + "resolved": "https://registry.npmjs.org/grunt-steroids/-/grunt-steroids-0.2.3.tgz", + "integrity": "sha1-rCTYWXu+rkDKuXLq0KSk7F5qoKk=", + "requires": { + "chalk": "0.3.0", + "lodash": "2.4.1", + "wrench": "1.5.4", + "xml2js": "0.4.1" + } + }, + "has-color": { + "version": "0.1.7", + "resolved": "https://registry.npmjs.org/has-color/-/has-color-0.1.7.tgz", + "integrity": "sha1-ZxRKUmDDT8PMpnfQQdr1L+e3iy8=" + }, + "lodash": { + "version": "2.4.1", + "resolved": "https://registry.npmjs.org/lodash/-/lodash-2.4.1.tgz", + "integrity": "sha1-W3cjA03aTSYuWkb7LFjXzCL3FCA=" + }, + "logfmt": { + "version": "1.1.3", + "resolved": "https://registry.npmjs.org/logfmt/-/logfmt-1.1.3.tgz", + "integrity": "sha1-oo5IJTXSe4oyS/hLiFahHvilrLY=", + "requires": { + "lodash": "~2.4.1", + "split": "0.2.x", + "through": "2.3.x" + } + }, + "merge-descriptors": { + "version": "0.0.2", + "resolved": "https://registry.npmjs.org/merge-descriptors/-/merge-descriptors-0.0.2.tgz", + "integrity": "sha1-w2pSp4FDdRPFcnXzndnTF1FKyMc=" + }, + "methods": { + "version": "0.1.0", + "resolved": "https://registry.npmjs.org/methods/-/methods-0.1.0.tgz", + "integrity": "sha1-M11Cnu/SG3us8unJIqjSvRSjDk8=" + }, + "mime": { + "version": "1.2.11", + "resolved": "https://registry.npmjs.org/mime/-/mime-1.2.11.tgz", + "integrity": "sha1-WCA+7Ybjpe8XrtK32evUfwpg3RA=" + }, + "negotiator": { + "version": "0.3.0", + "resolved": "https://registry.npmjs.org/negotiator/-/negotiator-0.3.0.tgz", + "integrity": "sha1-cG1pLv7d9XTVfqn7GriaT6fuj2A=" + }, + "parseurl": { + "version": "1.0.1", + "resolved": "https://registry.npmjs.org/parseurl/-/parseurl-1.0.1.tgz", + "integrity": "sha1-Llfc5u/dN8NRhwEDCUTCK/OIt7Q=" + }, + "path-to-regexp": { + "version": "0.1.2", + "resolved": "https://registry.npmjs.org/path-to-regexp/-/path-to-regexp-0.1.2.tgz", + "integrity": "sha1-mysVH5zDAYye6lDKlXKeBXgXErQ=" + }, + "qs": { + "version": "0.6.6", + "resolved": "https://registry.npmjs.org/qs/-/qs-0.6.6.tgz", + "integrity": "sha1-bgFQmP9RlouKPIGQAdXyyJvEsQc=" + }, + "range-parser": { + "version": "1.0.0", + "resolved": "https://registry.npmjs.org/range-parser/-/range-parser-1.0.0.tgz", + "integrity": "sha1-pLJkz+C+XONqvjdlrJwqJIdG28A=" + }, + "sax": { + "version": "0.5.8", + "resolved": "https://registry.npmjs.org/sax/-/sax-0.5.8.tgz", + "integrity": "sha1-1HLbIo6zMcJQaw6MFVJK25OdEsE=" + }, + "send": { + "version": "0.2.0", + "resolved": "https://registry.npmjs.org/send/-/send-0.2.0.tgz", + "integrity": "sha1-Bnq/Rc/4v/spy9t0OXJbMjiKLFg=", + "requires": { + "debug": "*", + "fresh": "~0.2.1", + "mime": "~1.2.9", + "range-parser": "~1.0.0" + } + }, + "serve-static": { + "version": "1.0.1", + "resolved": "https://registry.npmjs.org/serve-static/-/serve-static-1.0.1.tgz", + "integrity": "sha1-ENy/1Es+ApGhMfyatKslqfWnikI=", + "requires": { + "send": "0.1.4" + }, + "dependencies": { + "fresh": { + "version": "0.2.0", + "resolved": "https://registry.npmjs.org/fresh/-/fresh-0.2.0.tgz", + "integrity": "sha1-v9lALPPfEsSkwxDHn5mj3eE9NKc=" + }, + "range-parser": { + "version": "0.0.4", + "resolved": "https://registry.npmjs.org/range-parser/-/range-parser-0.0.4.tgz", + "integrity": "sha1-wEJ//vUcEKy6B4KkbJYC50T/Ygs=" + }, + "send": { + "version": "0.1.4", + "resolved": "https://registry.npmjs.org/send/-/send-0.1.4.tgz", + "integrity": "sha1-vnDY0b4B3mGCGvE3gLUDRaT3Gr0=", + "requires": { + "debug": "*", + "fresh": "0.2.0", + "mime": "~1.2.9", + "range-parser": "0.0.4" + } + } + } + }, + "split": { + "version": "0.2.10", + "resolved": "https://registry.npmjs.org/split/-/split-0.2.10.tgz", + "integrity": "sha1-Zwl8YB1pfOE2j0GPBs0gHPBSGlc=", + "requires": { + "through": "2" + } + }, + "through": { + "version": "2.3.8", + "resolved": "https://registry.npmjs.org/through/-/through-2.3.8.tgz", + "integrity": "sha1-DdTJ/6q8NXlgsbckEV1+Doai4fU=" + }, + "type-is": { + "version": "1.0.0", + "resolved": "https://registry.npmjs.org/type-is/-/type-is-1.0.0.tgz", + "integrity": "sha1-T/Qk6XNJoe4ZELS/xIhZXs3EQ/w=", + "requires": { + "mime": "~1.2.11" + } + }, + "utils-merge": { + "version": "1.0.0", + "resolved": "https://registry.npmjs.org/utils-merge/-/utils-merge-1.0.0.tgz", + "integrity": "sha1-ApT7kiu5N1FTVBxPcJYjHyh8ivg=" + }, + "wrench": { + "version": "1.5.4", + "resolved": "https://registry.npmjs.org/wrench/-/wrench-1.5.4.tgz", + "integrity": "sha1-Lo2dPbNWjMHAAaTI3OyncuXSFkM=" + }, + "xml2js": { + "version": "0.4.1", + "resolved": "https://registry.npmjs.org/xml2js/-/xml2js-0.4.1.tgz", + "integrity": "sha1-3uBjAXOlxupieX+EhhX9Vca8aZI=", + "requires": { + "sax": "0.5.x", + "xmlbuilder": ">=0.4.2" + } + }, + "xmlbuilder": { + "version": "10.1.1", + "resolved": "https://registry.npmjs.org/xmlbuilder/-/xmlbuilder-10.1.1.tgz", + "integrity": "sha512-OyzrcFLL/nb6fMGHbiRDuPup9ljBycsdCypwuyg5AAHvyWzGfChJpCXMG88AGTIMFhGZ9RccFN1e6lhg3hkwKg==" + } + } +} diff --git a/integration/testdata/unmet_dep_npm/package.json b/integration/testdata/unmet_dep_npm/package.json new file mode 100644 index 00000000..ee7954e5 --- /dev/null +++ b/integration/testdata/unmet_dep_npm/package.json @@ -0,0 +1,23 @@ +{ + "name": "node_web_app", + "version": "0.0.0", + "description": "hello, world", + "main": "server.js", + "scripts": { + "test": "echo \"Error: no test specified\" && exit 1" + }, + "author": "", + "license": "BSD-2-Clause", + "dependencies": { + "logfmt": "~1.1.2", + "express": "~4.0.0", + "grunt-steroids": "0.2.3" + }, + "engines": { + "node": "~>6" + }, + "repository": { + "type": "git", + "url": "http://github.com/cloudfoundry/nodejs-buildpack.git" + } +} diff --git a/integration/testdata/unmet_dep_npm/resolve b/integration/testdata/unmet_dep_npm/resolve new file mode 100644 index 00000000..8e6eba22 --- /dev/null +++ b/integration/testdata/unmet_dep_npm/resolve @@ -0,0 +1 @@ +0.10.26 \ No newline at end of file diff --git a/integration/testdata/unmet_dep_npm/server.js b/integration/testdata/unmet_dep_npm/server.js new file mode 100644 index 00000000..2138d1ad --- /dev/null +++ b/integration/testdata/unmet_dep_npm/server.js @@ -0,0 +1,14 @@ +var port = Number(process.env.PORT || 8080); +var express = require("express"); +var logfmt = require("logfmt"); +var app = express(); + +app.use(logfmt.requestLogger()); + +app.listen(port, function() { + console.log("Listening on " + port); +}); + +app.get('/', function(req, res) { + res.send('Hello, World!'); +}); diff --git a/integration/versioning_test.go b/integration/versioning_test.go index a6c8b320..a03d8dd5 100644 --- a/integration/versioning_test.go +++ b/integration/versioning_test.go @@ -42,7 +42,7 @@ func testVersioningIntegration(t *testing.T, when spec.G, it spec.S) { defer app.Destroy() Expect(app.Start()).To(Succeed()) - Expect(app.BuildStdout.String()).To(MatchRegexp(`NodeJS[^\.\n]*6\.`)) //Allows it to ignore the control characters for color + Expect(app.BuildLogs()).To(MatchRegexp(`NodeJS 6\.`)) Expect(app.HTTPGetBody("/")).To(ContainSubstring("Hello, World!")) }) }) diff --git a/npm/mocks_test.go b/npm/mocks_test.go index e895deb4..060ac00e 100644 --- a/npm/mocks_test.go +++ b/npm/mocks_test.go @@ -33,8 +33,8 @@ func (m *MockRunner) EXPECT() *MockRunnerMockRecorder { } // Run mocks base method -func (m *MockRunner) Run(bin, dir string, args ...string) error { - varargs := []interface{}{bin, dir} +func (m *MockRunner) Run(bin, dir string, quiet bool, args ...string) error { + varargs := []interface{}{bin, dir, quiet} for _, a := range args { varargs = append(varargs, a) } @@ -44,14 +44,14 @@ func (m *MockRunner) Run(bin, dir string, args ...string) error { } // Run indicates an expected call of Run -func (mr *MockRunnerMockRecorder) Run(bin, dir interface{}, args ...interface{}) *gomock.Call { - varargs := append([]interface{}{bin, dir}, args...) +func (mr *MockRunnerMockRecorder) Run(bin, dir, quiet interface{}, args ...interface{}) *gomock.Call { + varargs := append([]interface{}{bin, dir, quiet}, args...) return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Run", reflect.TypeOf((*MockRunner)(nil).Run), varargs...) } // RunWithOutput mocks base method -func (m *MockRunner) RunWithOutput(bin, dir string, args ...string) (string, error) { - varargs := []interface{}{bin, dir} +func (m *MockRunner) RunWithOutput(bin, dir string, quiet bool, args ...string) (string, error) { + varargs := []interface{}{bin, dir, quiet} for _, a := range args { varargs = append(varargs, a) } @@ -62,8 +62,8 @@ func (m *MockRunner) RunWithOutput(bin, dir string, args ...string) (string, err } // RunWithOutput indicates an expected call of RunWithOutput -func (mr *MockRunnerMockRecorder) RunWithOutput(bin, dir interface{}, args ...interface{}) *gomock.Call { - varargs := append([]interface{}{bin, dir}, args...) +func (mr *MockRunnerMockRecorder) RunWithOutput(bin, dir, quiet interface{}, args ...interface{}) *gomock.Call { + varargs := append([]interface{}{bin, dir, quiet}, args...) return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "RunWithOutput", reflect.TypeOf((*MockRunner)(nil).RunWithOutput), varargs...) } diff --git a/npm/npm.go b/npm/npm.go index 74b5a5ba..965ac6fa 100644 --- a/npm/npm.go +++ b/npm/npm.go @@ -4,6 +4,7 @@ import ( "fmt" "os" "path/filepath" + "strings" "github.com/cloudfoundry/libcfbuildpack/buildpack" @@ -13,8 +14,8 @@ import ( ) type Runner interface { - Run(bin, dir string, args ...string) error - RunWithOutput(bin, dir string, args ...string) (string, error) + Run(bin, dir string, quiet bool, args ...string) error + RunWithOutput(bin, dir string, quiet bool, args ...string) (string, error) } type Logger interface { @@ -44,7 +45,7 @@ func (n NPM) Install(modulesLayer, cacheLayer, location string) error { } func (n NPM) Rebuild(cacheLayer, location string) error { - if err := n.Runner.Run("npm", location, "rebuild"); err != nil { + if err := n.Runner.Run("npm", location, false, "rebuild"); err != nil { return fmt.Errorf("failed running npm rebuild %s", err.Error()) } @@ -85,7 +86,7 @@ func (n NPM) moveDir(source, target, name string) error { } func (n NPM) getNPMVersion(location string) (buildpack.Version, error) { - out, err := n.Runner.RunWithOutput("npm", location, "-v") + out, err := n.Runner.RunWithOutput("npm", location, true, "-v") if err != nil { return buildpack.Version{}, err } @@ -97,7 +98,13 @@ func (n NPM) getNPMVersion(location string) (buildpack.Version, error) { } func (n NPM) runInstall(location string, cacheLocation string) error { - return n.Runner.Run("npm", location, "install", "--unsafe-perm", "--cache", cacheLocation) + installLogs, err := n.Runner.RunWithOutput("npm", location, false, "install", "--unsafe-perm", "--cache", cacheLocation) + if err != nil { + return err + } + n.warnUnmetDependencies(installLogs) + + return nil } func (n NPM) runCacheVerify(location, cacheLocation string) error { @@ -115,5 +122,15 @@ func (n NPM) runCacheVerify(location, cacheLocation string) error { return nil } - return n.Runner.Run("npm", location, "cache", "verify", "--cache", cacheLocation) + return n.Runner.Run("npm", location, false, "cache", "verify", "--cache", cacheLocation) +} + +func (n NPM) warnUnmetDependencies(installLog string) { + installLog = strings.ToLower(installLog) + unmet := strings.Contains(installLog, "unmet dependency") || strings.Contains(installLog, "unmet peer dependency") + if unmet { + warning := "Unmet dependencies don't fail npm install but may cause runtime issues\n" + warning += "See: https://github.com/npm/npm/issues/7494" + n.Logger.Info(warning) + } } diff --git a/npm/npm_test.go b/npm/npm_test.go index 4d13ab25..5d09f8e6 100644 --- a/npm/npm_test.go +++ b/npm/npm_test.go @@ -49,9 +49,9 @@ func testNPM(t *testing.T, when spec.G, it spec.S) { location := filepath.Join("some", "fake", "dir") npmCache := filepath.Join(location, modules.CacheDir) - mockRunner.EXPECT().Run("npm", location, "install", "--unsafe-perm", "--cache", npmCache) - mockRunner.EXPECT().Run("npm", location, "cache", "verify", "--cache", npmCache) - mockRunner.EXPECT().RunWithOutput("npm", location, "-v").Return("5.0.0", nil) + mockRunner.EXPECT().RunWithOutput("npm", location, false, "install", "--unsafe-perm", "--cache", npmCache) + mockRunner.EXPECT().Run("npm", location, false, "cache", "verify", "--cache", npmCache) + mockRunner.EXPECT().RunWithOutput("npm", location, true, "-v").Return("5.0.0", nil) Expect(pkgManager.Install("", "", location)).To(Succeed()) }) @@ -59,8 +59,8 @@ func testNPM(t *testing.T, when spec.G, it spec.S) { location := filepath.Join("some", "fake", "dir") npmCache := filepath.Join(location, modules.CacheDir) - mockRunner.EXPECT().Run("npm", location, "install", "--unsafe-perm", "--cache", npmCache) - mockRunner.EXPECT().RunWithOutput("npm", location, "-v").Return("4.3.2", nil) + mockRunner.EXPECT().RunWithOutput("npm", location, false, "install", "--unsafe-perm", "--cache", npmCache) + mockRunner.EXPECT().RunWithOutput("npm", location, true, "-v").Return("4.3.2", nil) Expect(pkgManager.Install("", "", location)).To(Succeed()) }) }) @@ -86,9 +86,9 @@ func testNPM(t *testing.T, when spec.G, it spec.S) { Expect(ioutil.WriteFile(filepath.Join(cacheLayer, modules.CacheDir, "cache-item"), []byte(""), os.ModePerm)).To(Succeed()) npmCache := filepath.Join(location, modules.CacheDir) - mockRunner.EXPECT().Run("npm", location, "install", "--unsafe-perm", "--cache", npmCache) - mockRunner.EXPECT().Run("npm", location, "cache", "verify", "--cache", npmCache) - mockRunner.EXPECT().RunWithOutput("npm", location, "-v").Return("5.0.1", nil) + mockRunner.EXPECT().RunWithOutput("npm", location, false, "install", "--unsafe-perm", "--cache", npmCache) + mockRunner.EXPECT().Run("npm", location, false, "cache", "verify", "--cache", npmCache) + mockRunner.EXPECT().RunWithOutput("npm", location, true, "-v").Return("5.0.1", nil) Expect(pkgManager.Install(modulesLayer, cacheLayer, location)).To(Succeed()) @@ -110,8 +110,8 @@ func testNPM(t *testing.T, when spec.G, it spec.S) { cacheLayer, err := ioutil.TempDir("", "") Expect(err).ToNot(HaveOccurred()) - mockRunner.EXPECT().Run("npm", location, "rebuild") - mockRunner.EXPECT().Run("npm", location, "install", "--unsafe-perm", "--cache", npmCache) + mockRunner.EXPECT().Run("npm", location, false, "rebuild") + mockRunner.EXPECT().RunWithOutput("npm", location, false, "install", "--unsafe-perm", "--cache", npmCache) Expect(pkgManager.Rebuild(cacheLayer, location)).To(Succeed()) }) diff --git a/utils/utils.go b/utils/utils.go index 55c99dd2..9e003f16 100644 --- a/utils/utils.go +++ b/utils/utils.go @@ -1,6 +1,9 @@ package utils import ( + "bytes" + "io" + "io/ioutil" "os" "os/exec" "strings" @@ -9,17 +12,32 @@ import ( type CommandRunner struct { } -func (r CommandRunner) Run(bin, dir string, args ...string) error { +func (r CommandRunner) Run(bin, dir string, quiet bool, args ...string) error { cmd := exec.Command(bin, args...) cmd.Dir = dir - cmd.Stdout = os.Stdout - cmd.Stderr = os.Stderr + if quiet { + cmd.Stdout = ioutil.Discard + cmd.Stderr = ioutil.Discard + } else { + cmd.Stdout = os.Stdout + cmd.Stderr = os.Stderr + } return cmd.Run() } -func (r CommandRunner) RunWithOutput(bin, dir string, args ...string) (string, error) { +func (r CommandRunner) RunWithOutput(bin, dir string, quiet bool, args ...string) (string, error) { + logs := &bytes.Buffer{} + cmd := exec.Command(bin, args...) cmd.Dir = dir - out, err := cmd.CombinedOutput() - return strings.TrimSpace(string(out)), err + if quiet { + cmd.Stdout = io.MultiWriter(ioutil.Discard, logs) + cmd.Stderr = io.MultiWriter(ioutil.Discard, logs) + } else { + cmd.Stdout = io.MultiWriter(os.Stdout, logs) + cmd.Stderr = io.MultiWriter(os.Stderr, logs) + } + err := cmd.Run() + + return strings.TrimSpace(logs.String()), err }