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

Enhance NPM dependency resolver to resolve all the dependent packages #51

Merged
merged 4 commits into from
Aug 3, 2021

Conversation

zooltd
Copy link
Contributor

@zooltd zooltd commented Aug 1, 2021

This patch enhances the NPM dependency resolver to resolve all the dependent packages.
First, it runs the npm command npm ls --all --parseable to list all the packages' absolute paths.
Then, each package's name is inferred from its relative path from the node_modules dir.
Finally, walk through each package's root path to resolve licenses from the package.json file or the license file.

@zooltd zooltd changed the title Enhance Go dependency resolver to resolve all the dependent packages Enhance NPM dependency resolver to resolve all the dependent packages Aug 1, 2021
@kezhenxu94 kezhenxu94 self-requested a review August 1, 2021 10:48
@kezhenxu94 kezhenxu94 added this to the 0.2.0 milestone Aug 1, 2021
@wu-sheng
Copy link
Member

wu-sheng commented Aug 1, 2021

What happens if npm command is not available?

@zooltd
Copy link
Contributor Author

zooltd commented Aug 1, 2021

What happens if npm command is not available?

The error msg will print in the console, and the program continues regardlessly. However, it will not crash, it just prints an empty list.
Snipaste_2021-08-01_21-08-02

pkg/deps/npm.go Show resolved Hide resolved
pkg/deps/npm.go Outdated Show resolved Hide resolved
@zooltd
Copy link
Contributor Author

zooltd commented Aug 3, 2021

devDeps and transitive devDeps are now excluded

Copy link
Member

@kezhenxu94 kezhenxu94 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks much better, just some nits. Can you please run this tool on http://github.com/apache/skywalking-client-js and http://github.com/apache/skywalking-nodejs to see the results and paste here?

Comment on lines +136 to +143
func (resolver *NpmResolver) ListPkgPaths() (io.Reader, error) {
buffer := &bytes.Buffer{}
cmd := exec.Command("npm", "ls", "--all", "--production", "--parseable")
cmd.Stderr = os.Stderr
cmd.Stdout = buffer
// Error occurs all the time in npm commands, so no return statement here
err := cmd.Run()
return buffer, err
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe we can remove the return value error because

  • the error is mostly expected
  • we don't deal with the error when invoking this function
  • the stderr is printed and we don't need to get the error when invoking this function

Copy link
Contributor Author

@zooltd zooltd Aug 3, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the error returned from cmd.Run() is exist error, like exit status 1
it differs from Stderr , which records runtime error
so the same error will not be printed twice
same as the problem below

if err := cmd.Run(); err != nil {
return err
logger.Log.Errorln(err)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we want to log it again because the stderr has already printed this right?

@zooltd
Copy link
Contributor Author

zooltd commented Aug 3, 2021

It seems in some cases, the license field in package.json is like:

"licenses": [
     {
         "type": "AFLv2.1",
         "url": "http://trac.dojotoolkit.org/browser/dojo/trunk/LICENSE#L43"
     },
     {
         "type": "BSD",
         "url": "http://trac.dojotoolkit.org/browser/dojo/trunk/LICENSE#L13"
     }

It can not be parsed right now. Plan to fix this in next patch

@zooltd
Copy link
Contributor Author

zooltd commented Aug 3, 2021

This looks much better, just some nits. Can you please run this tool on http://github.com/apache/skywalking-client-js and http://github.com/apache/skywalking-nodejs to see the results and paste here?

For skywalking-client-js, the total output is shown as below:

root@Samaritan:~/skywalking-eyes# ./bin/linux/license-eye -c ~/testPlace/skywalking-client-js/.licenserc.yaml d r
INFO GITHUB_TOKEN is not set, license-eye won't comment on the pull request
INFO Loading configuration from file: /root/testPlace/skywalking-client-js/.licenserc.yaml
INFO Try to install nodejs packages in 5 seconds, press [s/S] and ENTER to skip
INFO Time out, try to install packages
INFO Run command: /usr/local/bin/npm install, please wait
npm WARN old lockfile
npm WARN old lockfile The package-lock.json file was created with an old version of npm,
npm WARN old lockfile so supplemental metadata must be fetched from the registry.
npm WARN old lockfile
npm WARN old lockfile This is a one-time fix-up, please be patient...
npm WARN old lockfile
npm WARN deprecated urix@0.1.0: Please see https://github.com/lydell/urix#deprecated
npm WARN deprecated resolve-url@0.2.1: https://github.com/lydell/resolve-url#deprecated
npm WARN deprecated uuid@3.4.0: Please upgrade to version 7 or higher. Older versions may use Math.random() in certain circumstances, which is known to be problematic. See https://v8.dev/blog/math-random for details.
npm WARN deprecated querystring@0.2.0: The querystring API is considered Legacy. new code should use the URLSearchParams API instead.
npm WARN deprecated chokidar@2.1.8: Chokidar 2 will break on node v14+. Upgrade to chokidar 3 with 15x less dependencies.

added 652 packages, and audited 653 packages in 29s

50 packages are looking for funding
run npm fund for details

5 vulnerabilities (4 moderate, 1 high)

To address issues that do not require attention, run:
npm audit fix

To address all issues (including breaking changes), run:
npm audit fix --force

Run npm audit for details.

Dependency License
js-base64 BSD-3-Clause

And for skywalking-nodejs:

(npm run generate-source failed somehow)

root@Samaritan:~/skywalking-eyes# ./bin/linux/license-eye -c ~/testPlace/skywalking-nodejs/.licenserc.yaml d r
INFO GITHUB_TOKEN is not set, license-eye won't comment on the pull request
INFO Loading configuration from file: /root/testPlace/skywalking-nodejs/.licenserc.yaml
INFO Try to install nodejs packages in 5 seconds, press [s/S] and ENTER to skip
INFO Time out, try to install packages
INFO Run command: /usr/local/bin/npm install, please wait

skywalking-backend-js@0.4.0 prepare
npm run generate-source

skywalking-backend-js@0.4.0 generate-source
scripts/protoc.sh

Could not make proto path relative: **/*.proto: No such file or directory
/root/testPlace/skywalking-nodejs/node_modules/grpc-tools/bin/protoc.js:41
throw error;
^

Error: Command failed: /root/testPlace/skywalking-nodejs/node_modules/grpc-tools/bin/protoc --plugin=protoc-gen-grpc=/root/testPlace/skywalking-nodejs/node_modules/grpc-tools/bin/grpc_node_plugin --js_out=import_style=commonjs,binary:/root/testPlace/skywalking-nodejs/src/proto/ --grpc_out=/root/testPlace/skywalking-nodejs/src/proto/ --plugin=protoc-gen-grpc=/root/testPlace/skywalking-nodejs/node_modules/.bin/grpc_tools_node_protoc_plugin **/.proto
Could not make proto path relative: **/
.proto: No such file or directory

at ChildProcess.exithandler (child_process.js:390:12)
at ChildProcess.emit (events.js:400:28)
at maybeClose (internal/child_process.js:1055:16)
at Socket.<anonymous> (internal/child_process.js:441:11)
at Socket.emit (events.js:400:28)
at Pipe.<anonymous> (net.js:675:12) {

killed: false,
code: 1,
signal: null,
cmd: '/root/testPlace/skywalking-nodejs/node_modules/grpc-tools/bin/protoc --plugin=protoc-gen-grpc=/root/testPlace/skywalking-nodejs/node_modules/grpc-tools/bin/grpc_node_plugin --js_out=import_style=commonjs,binary:/root/testPlace/skywalking-nodejs/src/proto/ --grpc_out=/root/testPlace/skywalking-nodejs/src/proto/ --plugin=protoc-gen-grpc=/root/testPlace/skywalking-nodejs/node_modules/.bin/grpc_tools_node_protoc_plugin **/.proto'
}
Could not make proto path relative: **/
.proto: No such file or directory
/root/testPlace/skywalking-nodejs/node_modules/grpc-tools/bin/protoc.js:41
throw error;
^

Error: Command failed: /root/testPlace/skywalking-nodejs/node_modules/grpc-tools/bin/protoc --plugin=protoc-gen-grpc=/root/testPlace/skywalking-nodejs/node_modules/grpc-tools/bin/grpc_node_plugin --plugin=protoc-gen-ts=/root/testPlace/skywalking-nodejs/node_modules/.bin/protoc-gen-ts --ts_out=/root/testPlace/skywalking-nodejs/src/proto/ **/.proto
Could not make proto path relative: **/
.proto: No such file or directory

at ChildProcess.exithandler (child_process.js:390:12)
at ChildProcess.emit (events.js:400:28)
at maybeClose (internal/child_process.js:1055:16)
at Socket.<anonymous> (internal/child_process.js:441:11)
at Socket.emit (events.js:400:28)
at Pipe.<anonymous> (net.js:675:12) {

killed: false,
code: 1,
signal: null,
cmd: '/root/testPlace/skywalking-nodejs/node_modules/grpc-tools/bin/protoc --plugin=protoc-gen-grpc=/root/testPlace/skywalking-nodejs/node_modules/grpc-tools/bin/grpc_node_plugin --plugin=protoc-gen-ts=/root/testPlace/skywalking-nodejs/node_modules/.bin/protoc-gen-ts --ts_out=/root/testPlace/skywalking-nodejs/src/proto/ **/*.proto'
}
npm ERR! code 1
npm ERR! path /root/testPlace/skywalking-nodejs
npm ERR! command failed
npm ERR! command sh -c npm run generate-source

npm ERR! A complete log of this run can be found in:
npm ERR! /root/.npm/_logs/2021-08-03T05_38_55_303Z-debug.log
ERROR exit status 1

Dependency License
google-protobuf BSD-3-Clause
grpc Apache-2.0
semver ISC
tslib 0BSD
uuid MIT
winston MIT
@types/bytebuffer MIT
lodash.camelcase MIT
lodash.clone MIT
nan MIT
node-pre-gyp BSD-3-Clause
protobufjs Apache-2.0
lru-cache ISC
@dabh/diagnostics MIT
async MIT
is-stream MIT
logform MIT
one-time MIT
winston/node_modules/readable-stream MIT
stack-trace MIT
triple-beam MIT
winston-transport MIT
@types/long MIT
@types/node MIT
detect-libc Apache-2.0
mkdirp MIT
needle MIT
nopt ISC
npm-packlist ISC
npmlog ISC
rc (BSD-2-Clause OR MIT OR Apache-2.0)
rimraf ISC
node-pre-gyp/node_modules/semver ISC
tar ISC
ascli Apache-2.0
bytebuffer Apache-2.0
glob ISC
yargs MIT
lru-cache/node_modules/yallist ISC
colorspace MIT
enabled MIT
kuler MIT
colors MIT
fast-safe-stringify MIT
fecha MIT
ms MIT
fn.name MIT
inherits ISC
string_decoder MIT
util-deprecate MIT
readable-stream MIT
minimist MIT
debug MIT
iconv-lite MIT
sax ISC
abbrev ISC
osenv ISC
ignore-walk ISC
npm-bundled ISC
npm-normalize-package-bin ISC
are-we-there-yet ISC
console-control-strings ISC
gauge ISC
set-blocking ISC
deep-extend MIT
ini ISC
strip-json-comments MIT
chownr ISC
fs-minipass ISC
minipass ISC
minizlib MIT
safe-buffer MIT
yallist ISC
colour MIT
optjs MIT
long Apache-2.0
fs.realpath ISC
inflight ISC
minimatch ISC
once ISC
path-is-absolute MIT
camelcase MIT
cliui ISC
decamelize MIT
os-locale MIT
string-width MIT
window-size MIT
y18n ISC
color MIT
text-hex MIT
core-util-is MIT
isarray MIT
process-nextick-args MIT
safer-buffer MIT
os-homedir MIT
os-tmpdir MIT
delegates MIT
aproba ISC
has-unicode ISC
object-assign MIT
signal-exit ISC
strip-ansi MIT
wide-align ISC
wrappy ISC
brace-expansion MIT
wrap-ansi MIT
lcid MIT
code-point-at MIT
is-fullwidth-code-point MIT
color-convert MIT
color-string MIT
ansi-regex MIT
balanced-match MIT
concat-map MIT
invert-kv MIT
number-is-nan MIT
color-name MIT
simple-swizzle MIT
is-arrayish MIT

@kezhenxu94
Copy link
Member

It seems in some cases, the license field in package.json shows as:

"licenses": [
     {
         "type": "AFLv2.1",
         "url": "http://trac.dojotoolkit.org/browser/dojo/trunk/LICENSE#L43"
     },
     {
         "type": "BSD",
         "url": "http://trac.dojotoolkit.org/browser/dojo/trunk/LICENSE#L13"
     }

It can not be parsed right now. Plan to fix this in next patch

Good catch! Projects are able to be dual-licensed or licensed under multiple licenses, let’s tackle this case in next PR

@kezhenxu94 kezhenxu94 merged commit 804fb3e into apache:main Aug 3, 2021
@zooltd zooltd deleted the enhance/npm branch August 6, 2021 14:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants