Skip to content

feat(license-scanner): show all licenses for JSON#7528

Merged
zkochan merged 7 commits into
mainfrom
inconsistent-output-of-pnpm-licenses-with-multiple-package-versions-7724
Jan 19, 2024
Merged

feat(license-scanner): show all licenses for JSON#7528
zkochan merged 7 commits into
mainfrom
inconsistent-output-of-pnpm-licenses-with-multiple-package-versions-7724

Conversation

@KSXGitHub

Copy link
Copy Markdown
Contributor

Resolves #7224

@KSXGitHub KSXGitHub force-pushed the inconsistent-output-of-pnpm-licenses-with-multiple-package-versions-7724 branch from 4c3dbcf to ef50602 Compare January 16, 2024 15:29
@KSXGitHub KSXGitHub marked this pull request as ready for review January 16, 2024 15:39
@KSXGitHub KSXGitHub requested a review from zkochan as a code owner January 16, 2024 15:39

function deduplicateTable (table: string[][]): string[][] {
const result: string[][] = []
const rowEqual = (a: string[], b: string[]) => a[0] === b[0] && a[1] === b[1]

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

0 and 1 are the columns, right? Maybe use some constants to name the columns.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I have replaced index numbers with field names of LicensePackage: f5ed0e0

@zkochan

zkochan commented Jan 17, 2024

Copy link
Copy Markdown
Member

So, the solution is that packages will be duplicated, when the pnpm licenses list --json command will be used.

However, with pnpm licenses list the same package will be printed two times only if the different versions will have different licenses.

I would wait for other reviewers to check this too.

@KSXGitHub

KSXGitHub commented Jan 17, 2024

Copy link
Copy Markdown
Contributor Author

However, with pnpm licenses list the same package will be printed two times only if the different versions will have different licenses.

In my opinion, the license change of the later version does not apply to the older versions. If the codebase uses different versions of the same package then it must include both licenses.

@zkochan

zkochan commented Jan 17, 2024

Copy link
Copy Markdown
Member

In my opinion, the license change of the later version does not apply to the older versions. If the codebase uses different versions of the same package then it must include both licenses.

Sure, that is not up to debate.

My concern is more about the difference between --json and the table output. Maybe with --json it should be deduped too.

Also, maybe we should include an array of versions that share the same licenses.

@KSXGitHub

Copy link
Copy Markdown
Contributor Author

My concern is more about the difference between --json and the table output. Maybe with --json it should be deduped too.

Also, maybe we should include an array of versions that share the same licenses.

Would changing the version field from string to string[] be a breaking change?

Do note that JSON is a programming interface, meant to be consumed by another program, whether that program dedupes by itself or pnpm dedupes it for them make little difference in the users' eyes.

@zkochan

zkochan commented Jan 17, 2024

Copy link
Copy Markdown
Member

The main branch is now v9, so we can make breaking changes.

@KSXGitHub

Copy link
Copy Markdown
Contributor Author

Then it shall be done. BTW, how to write tests for these changes? Is there a package in the mocked registry that has different licenses for different versions?

@zkochan

zkochan commented Jan 17, 2024

Copy link
Copy Markdown
Member

No, there are no such packages in the mocked registry now.

You can add them or use real packages from the public registry, if you can find such. Probably easier to mock.

@KSXGitHub

Copy link
Copy Markdown
Contributor Author

@zkochan ac27687 would makes pnpm licenses --json prints versions and paths as arrays from a single name instead of duplicating them.

But I find this structure kinda awkward, I would prefer { [license: string]: { [name: string]: LicensePackageJson } } more. What do you think?

@zkochan

zkochan commented Jan 17, 2024

Copy link
Copy Markdown
Member

But I find this structure kinda awkward, I would prefer { [license: string]: { [name: string]: LicensePackageJson } } more. What do you think?

IMO this structure would be fine.

@KSXGitHub

KSXGitHub commented Jan 17, 2024

Copy link
Copy Markdown
Contributor Author

Here's what the structure currently looks like:

Details
{
  "ISC": [
    {
      "name": "ansi-align",
      "versions": [
        "3.0.1"
      ],
      "paths": [
        "/home/khai/Temp/pnpm-inconsistent-output-of-pnpm-licenses-with-multiple-package-versions-7724/node_modules/.pnpm/ansi-align@3.0.1/node_modules/ansi-align"
      ],
      "license": "ISC",
      "author": "nexdrew",
      "homepage": "https://github.com/nexdrew/ansi-align#readme",
      "description": "align-text with ANSI support for CLIs"
    }
  ],
  "MIT": [
    {
      "name": "ansi-regex",
      "versions": [
        "5.0.1",
        "6.0.1"
      ],
      "paths": [
        "/home/khai/Temp/pnpm-inconsistent-output-of-pnpm-licenses-with-multiple-package-versions-7724/node_modules/.pnpm/ansi-regex@5.0.1/node_modules/ansi-regex",
        "/home/khai/Temp/pnpm-inconsistent-output-of-pnpm-licenses-with-multiple-package-versions-7724/node_modules/.pnpm/ansi-regex@6.0.1/node_modules/ansi-regex"
      ],
      "license": "MIT",
      "author": "Sindre Sorhus",
      "homepage": "https://github.com/chalk/ansi-regex#readme",
      "description": "Regular expression for matching ANSI escape codes"
    },
    {
      "name": "ansi-styles",
      "versions": [
        "4.3.0",
        "6.2.1"
      ],
      "paths": [
        "/home/khai/Temp/pnpm-inconsistent-output-of-pnpm-licenses-with-multiple-package-versions-7724/node_modules/.pnpm/ansi-styles@4.3.0/node_modules/ansi-styles",
        "/home/khai/Temp/pnpm-inconsistent-output-of-pnpm-licenses-with-multiple-package-versions-7724/node_modules/.pnpm/ansi-styles@6.2.1/node_modules/ansi-styles"
      ],
      "license": "MIT",
      "author": "Sindre Sorhus",
      "homepage": "https://github.com/chalk/ansi-styles#readme",
      "description": "ANSI escape codes for styling strings in the terminal"
    },
    {
      "name": "boxen",
      "versions": [
        "5.1.2",
        "7.0.2"
      ],
      "paths": [
        "/home/khai/Temp/pnpm-inconsistent-output-of-pnpm-licenses-with-multiple-package-versions-7724/node_modules/.pnpm/boxen@5.1.2/node_modules/boxen",
        "/home/khai/Temp/pnpm-inconsistent-output-of-pnpm-licenses-with-multiple-package-versions-7724/node_modules/.pnpm/boxen@7.0.2/node_modules/boxen"
      ],
      "license": "MIT",
      "author": "Sindre Sorhus",
      "homepage": "https://github.com/sindresorhus/boxen#readme",
      "description": "Create boxes in the terminal"
    },
    {
      "name": "camelcase",
      "versions": [
        "6.3.0",
        "7.0.1"
      ],
      "paths": [
        "/home/khai/Temp/pnpm-inconsistent-output-of-pnpm-licenses-with-multiple-package-versions-7724/node_modules/.pnpm/camelcase@6.3.0/node_modules/camelcase",
        "/home/khai/Temp/pnpm-inconsistent-output-of-pnpm-licenses-with-multiple-package-versions-7724/node_modules/.pnpm/camelcase@7.0.1/node_modules/camelcase"
      ],
      "license": "MIT",
      "author": "Sindre Sorhus",
      "homepage": "https://github.com/sindresorhus/camelcase#readme",
      "description": "Convert a dash/dot/underscore/space separated string to camelCase or PascalCase: `foo-bar` → `fooBar`"
    },
    {
      "name": "chalk",
      "versions": [
        "4.1.2",
        "5.3.0"
      ],
      "paths": [
        "/home/khai/Temp/pnpm-inconsistent-output-of-pnpm-licenses-with-multiple-package-versions-7724/node_modules/.pnpm/chalk@4.1.2/node_modules/chalk",
        "/home/khai/Temp/pnpm-inconsistent-output-of-pnpm-licenses-with-multiple-package-versions-7724/node_modules/.pnpm/chalk@5.3.0/node_modules/chalk"
      ],
      "license": "MIT",
      "homepage": "https://github.com/chalk/chalk#readme",
      "description": "Terminal string styling done right"
    },
    {
      "name": "cli-boxes",
      "versions": [
        "2.2.1",
        "3.0.0"
      ],
      "paths": [
        "/home/khai/Temp/pnpm-inconsistent-output-of-pnpm-licenses-with-multiple-package-versions-7724/node_modules/.pnpm/cli-boxes@2.2.1/node_modules/cli-boxes",
        "/home/khai/Temp/pnpm-inconsistent-output-of-pnpm-licenses-with-multiple-package-versions-7724/node_modules/.pnpm/cli-boxes@3.0.0/node_modules/cli-boxes"
      ],
      "license": "MIT",
      "author": "Sindre Sorhus",
      "homepage": "https://github.com/sindresorhus/cli-boxes#readme",
      "description": "Boxes for use in the terminal"
    },
    {
      "name": "color-convert",
      "versions": [
        "2.0.1"
      ],
      "paths": [
        "/home/khai/Temp/pnpm-inconsistent-output-of-pnpm-licenses-with-multiple-package-versions-7724/node_modules/.pnpm/color-convert@2.0.1/node_modules/color-convert"
      ],
      "license": "MIT",
      "author": "Heather Arthur",
      "homepage": "https://github.com/Qix-/color-convert#readme",
      "description": "Plain color conversion functions"
    },
    {
      "name": "color-name",
      "versions": [
        "1.1.4"
      ],
      "paths": [
        "/home/khai/Temp/pnpm-inconsistent-output-of-pnpm-licenses-with-multiple-package-versions-7724/node_modules/.pnpm/color-name@1.1.4/node_modules/color-name"
      ],
      "license": "MIT",
      "author": "DY",
      "homepage": "https://github.com/colorjs/color-name",
      "description": "A list of color names and its values"
    },
    {
      "name": "eastasianwidth",
      "versions": [
        "0.2.0"
      ],
      "paths": [
        "/home/khai/Temp/pnpm-inconsistent-output-of-pnpm-licenses-with-multiple-package-versions-7724/node_modules/.pnpm/eastasianwidth@0.2.0/node_modules/eastasianwidth"
      ],
      "license": "MIT",
      "author": "Masaki Komagata",
      "homepage": "https://github.com/komagata/eastasianwidth#readme",
      "description": "Get East Asian Width from a character."
    },
    {
      "name": "emoji-regex",
      "versions": [
        "8.0.0",
        "9.2.2"
      ],
      "paths": [
        "/home/khai/Temp/pnpm-inconsistent-output-of-pnpm-licenses-with-multiple-package-versions-7724/node_modules/.pnpm/emoji-regex@8.0.0/node_modules/emoji-regex",
        "/home/khai/Temp/pnpm-inconsistent-output-of-pnpm-licenses-with-multiple-package-versions-7724/node_modules/.pnpm/emoji-regex@9.2.2/node_modules/emoji-regex"
      ],
      "license": "MIT",
      "author": "Mathias Bynens",
      "homepage": "https://mths.be/emoji-regex",
      "description": "A regular expression to match all Emoji-only symbols as per the Unicode Standard."
    },
    {
      "name": "has-flag",
      "versions": [
        "4.0.0"
      ],
      "paths": [
        "/home/khai/Temp/pnpm-inconsistent-output-of-pnpm-licenses-with-multiple-package-versions-7724/node_modules/.pnpm/has-flag@4.0.0/node_modules/has-flag"
      ],
      "license": "MIT",
      "author": "Sindre Sorhus",
      "homepage": "https://github.com/sindresorhus/has-flag#readme",
      "description": "Check if argv has a specific flag"
    },
    {
      "name": "is-fullwidth-code-point",
      "versions": [
        "3.0.0"
      ],
      "paths": [
        "/home/khai/Temp/pnpm-inconsistent-output-of-pnpm-licenses-with-multiple-package-versions-7724/node_modules/.pnpm/is-fullwidth-code-point@3.0.0/node_modules/is-fullwidth-code-point"
      ],
      "license": "MIT",
      "author": "Sindre Sorhus",
      "homepage": "https://github.com/sindresorhus/is-fullwidth-code-point#readme",
      "description": "Check if the character represented by a given Unicode code point is fullwidth"
    },
    {
      "name": "string-width",
      "versions": [
        "4.2.3",
        "5.1.2"
      ],
      "paths": [
        "/home/khai/Temp/pnpm-inconsistent-output-of-pnpm-licenses-with-multiple-package-versions-7724/node_modules/.pnpm/string-width@4.2.3/node_modules/string-width",
        "/home/khai/Temp/pnpm-inconsistent-output-of-pnpm-licenses-with-multiple-package-versions-7724/node_modules/.pnpm/string-width@5.1.2/node_modules/string-width"
      ],
      "license": "MIT",
      "author": "Sindre Sorhus",
      "homepage": "https://github.com/sindresorhus/string-width#readme",
      "description": "Get the visual width of a string - the number of columns required to display it"
    },
    {
      "name": "strip-ansi",
      "versions": [
        "6.0.1",
        "7.1.0"
      ],
      "paths": [
        "/home/khai/Temp/pnpm-inconsistent-output-of-pnpm-licenses-with-multiple-package-versions-7724/node_modules/.pnpm/strip-ansi@6.0.1/node_modules/strip-ansi",
        "/home/khai/Temp/pnpm-inconsistent-output-of-pnpm-licenses-with-multiple-package-versions-7724/node_modules/.pnpm/strip-ansi@7.1.0/node_modules/strip-ansi"
      ],
      "license": "MIT",
      "author": "Sindre Sorhus",
      "homepage": "https://github.com/chalk/strip-ansi#readme",
      "description": "Strip ANSI escape codes from a string"
    },
    {
      "name": "supports-color",
      "versions": [
        "7.2.0"
      ],
      "paths": [
        "/home/khai/Temp/pnpm-inconsistent-output-of-pnpm-licenses-with-multiple-package-versions-7724/node_modules/.pnpm/supports-color@7.2.0/node_modules/supports-color"
      ],
      "license": "MIT",
      "author": "Sindre Sorhus",
      "homepage": "https://github.com/chalk/supports-color#readme",
      "description": "Detect whether a terminal supports color"
    },
    {
      "name": "widest-line",
      "versions": [
        "3.1.0",
        "4.0.1"
      ],
      "paths": [
        "/home/khai/Temp/pnpm-inconsistent-output-of-pnpm-licenses-with-multiple-package-versions-7724/node_modules/.pnpm/widest-line@3.1.0/node_modules/widest-line",
        "/home/khai/Temp/pnpm-inconsistent-output-of-pnpm-licenses-with-multiple-package-versions-7724/node_modules/.pnpm/widest-line@4.0.1/node_modules/widest-line"
      ],
      "license": "MIT",
      "author": "Sindre Sorhus",
      "homepage": "https://github.com/sindresorhus/widest-line#readme",
      "description": "Get the visual width of the widest line in a string - the number of columns required to display it"
    },
    {
      "name": "wrap-ansi",
      "versions": [
        "7.0.0",
        "8.1.0"
      ],
      "paths": [
        "/home/khai/Temp/pnpm-inconsistent-output-of-pnpm-licenses-with-multiple-package-versions-7724/node_modules/.pnpm/wrap-ansi@7.0.0/node_modules/wrap-ansi",
        "/home/khai/Temp/pnpm-inconsistent-output-of-pnpm-licenses-with-multiple-package-versions-7724/node_modules/.pnpm/wrap-ansi@8.1.0/node_modules/wrap-ansi"
      ],
      "license": "MIT",
      "author": "Sindre Sorhus",
      "homepage": "https://github.com/chalk/wrap-ansi#readme",
      "description": "Wordwrap a string with ANSI escape codes"
    }
  ],
  "(MIT OR CC0-1.0)": [
    {
      "name": "type-fest",
      "versions": [
        "0.20.2",
        "2.19.0"
      ],
      "paths": [
        "/home/khai/Temp/pnpm-inconsistent-output-of-pnpm-licenses-with-multiple-package-versions-7724/node_modules/.pnpm/type-fest@0.20.2/node_modules/type-fest",
        "/home/khai/Temp/pnpm-inconsistent-output-of-pnpm-licenses-with-multiple-package-versions-7724/node_modules/.pnpm/type-fest@2.19.0/node_modules/type-fest"
      ],
      "license": "(MIT OR CC0-1.0)",
      "author": "Sindre Sorhus",
      "homepage": "https://github.com/sindresorhus/type-fest#readme",
      "description": "A collection of essential TypeScript types"
    }
  ]
}

You are fine with it?

@zkochan

zkochan commented Jan 18, 2024

Copy link
Copy Markdown
Member

@weyert are you OK with the new format?

@zkochan zkochan requested a review from weyert January 18, 2024 00:57

@BlackHole1 BlackHole1 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

When there is a null element in paths, it does not provide any useful information, so I would suggest excluding it.

Comment thread reviewing/plugin-commands-licenses/src/outputRenderer.ts
Comment thread reviewing/plugin-commands-licenses/src/outputRenderer.ts
@KSXGitHub

Copy link
Copy Markdown
Contributor Author

@BlackHole1 The indices of paths corresponds to that of versions. If a version doesn't have a path, it will be null.

@weyert weyert left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Yeah, think it's fine. I would need to fix some third party tools that consume this format.

I am personally not sure how the version matters when the license is the same for the package. I understand it matters when generating a SBOM file but that's a different use case.

@zkochan zkochan added this to the v9.0 milestone Jan 19, 2024
@zkochan zkochan merged commit f5766d9 into main Jan 19, 2024
@zkochan zkochan deleted the inconsistent-output-of-pnpm-licenses-with-multiple-package-versions-7724 branch January 19, 2024 08:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Inconsistent output of pnpm licenses with multiple package versions

4 participants