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

license checker not ignoring all dev deps #1298

Open
jayvdb opened this issue Oct 5, 2024 · 5 comments
Open

license checker not ignoring all dev deps #1298

jayvdb opened this issue Oct 5, 2024 · 5 comments

Comments

@jayvdb
Copy link

jayvdb commented Oct 5, 2024

I have the following config

[[PackageOverrides]]
group = "dev"
license.ignore = true

When I run osv-scanner --experimental-licenses-summary pnpm-lock.yaml , the summary contains the same list as when I run it without that config.

And when running with --experimental-licenses MIT,ISC,Apache-2.0,.. I see failures in dev dependencies.

@jayvdb jayvdb changed the title license checker not ignoring dev license checker not ignoring all dev deps Oct 5, 2024
@jayvdb
Copy link
Author

jayvdb commented Oct 6, 2024

I noticed when putting in a set of [[PackageOverrides]] explicitly mentioning npm packages, that I needed to remove group = "dev" for the license checker to whitelist them.
So maybe this a generic npm "dev" problem, or user error/understanding.

@G-Rath
Copy link
Collaborator

G-Rath commented Oct 6, 2024

I assume you're using the latest pnpm whose lockfile version no longer marks dev dependencies as such, so currently the scanner has to assume all dependencies are in no groups.

The main path to improving this is supporting extracting information from supporting files (which in this case would be package.json), whichll hopefully be worked on soon after the upcoming V2.

There's an issue open for that if you search (I'm on mobile and can't remember the number off hand 😅)

@jayvdb
Copy link
Author

jayvdb commented Oct 6, 2024

We are using lock 9.0 , which you added support for in #934

It does separate prod vs dev

importers:

  .:
    dependencies:
      '@...':
    devDependencies:
      '@...':

I looked again and didnt find any issues or PRs about it.

@G-Rath
Copy link
Collaborator

G-Rath commented Oct 6, 2024

It does separate prod vs dev

I'm 98% sure that only holds direct dependencies, which is why I didn't try implementing support for it (and eluded to that in my pr) - afaik the lockfile does not provide enough information on its own about the dependency tree to be able to accurately construct "grouped" trees.

I believe this is the sort of thing we might be able to use e.g. deps.dev to look up but given the average number of packages in a npm project I don't think we're structured in a way for that to be efficient in v1.

(Frankly though I think this will still be a problem after we support including package.json, for that underlying reason, but being able to support that will represent an important underlying API change that should make it easier to do other stuff like talk with APIs...)

(After looking back over my paper notes & the pnpm codebase, looks like it might be possible to rebuild the tree manually using all the info in the lockfile, but I mean all the info including parsing the dependency qualifiers - I really wish they'd publish a proper spec or kept the dev field 😵‍💫)

@G-Rath
Copy link
Collaborator

G-Rath commented Oct 6, 2024

You want #416 & #654

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants