feat: add switch to add license text to BOM result by CompartMSL · Pull Request #427 · CycloneDX/cyclonedx-node-npm

CompartMSL · 2023-01-10T17:24:08Z

fixes #256

jkowalleck · 2023-01-11T12:19:05Z

❌ Please add proper tests.

PS: this is already ongoing

Signed-off-by: Matthias Schiebel <msl@compart.com>

.gitignore

tests/integration/synthetics/cli.run.test.js

tests/_data/dummy_projects/with-lockfile/node_modules/my-local-a/LICENSE

jkowalleck · 2023-01-19T08:07:10Z

Based on the proposed implementation, the new switch --add-license-text contradicts the existing switch --package-lock-only.

The --package-lock-only (options.packageLockOnly) should have priority - meaning if options.packageLockOnly is true, then options.addLicenseText should be set to false regardless of the user's input. (see how options.packageLockOnly is handled in src/cli.ts)

@CompartMSL, how do you see this?

CompartMSL · 2023-01-23T07:08:22Z

Based on the proposed implementation, the new switch --add-license-text contradicts the existing switch --package-lock-only.

The --package-lock-only (options.packageLockOnly) should have priority - meaning if options.packageLockOnly is true, then options.addLicenseText should be set to false regardless of the user's input. (see how options.packageLockOnly is handled in src/cli.ts)

@CompartMSL, how do you see this?

You are right.
After the implementation of this change I recognized, after the run of the tests, that the chosen test approach for --add-license-texts is disturbed by this change. The files in tests_data\sbom_demo-results\add-license-text\ shows not the wanted effect anymore.
Currently, I see the need to search another test approach and have no idea where to start.

jkowalleck · 2023-01-23T09:29:47Z

@CompartMSL re #427 (comment)

Ignore the tests for a while, and just take care of the needed implementation details.
Maybe just revert the tests you added, for a while. I will assist setting up a test, as soon as the desired feature is shaped properly.

src/licensetexts.ts

jkowalleck · 2023-01-24T13:43:26Z

src/licensetexts.ts

+  if (pkgPath.length < 1) {
+    return licenseFilenamesWType
+  }
+  const typicalFilenames = ['LICENSE', 'License', 'license', 'LICENCE', 'Licence', 'licence', 'NOTICE', 'Notice', 'notice']


Please change the list/implementation to be case-insensitive.
I'd suggest having only lowercased entries in the list.
I'd suggest using a scandir-like behavior where all files are lowercased before they are compared against the list of known files.

I chose the other way first to get a list of all files and match case-insensitive. Maybe there is later an idea to handle multiple files

jkowalleck · 2023-01-24T13:47:20Z

converted PR to "draft", since the implementation is not final, yet.

Signed-off-by: Matthias Schiebel <msl@compart.com>

jkowalleck · 2023-01-25T09:14:59Z

src/licensetexts.ts

+
+import { Enums, Models } from '@cyclonedx/cyclonedx-library'
+import * as fs from 'fs'
+import * as minimatch from 'minimatch'


❓ where does this come from?

jkowalleck · 2023-01-25T09:24:16Z

src/cli.ts

    myConsole
  ).buildFromProjectDir(projectDir, process)

+  if (options.addLicenseText) {


Why do this after everything is set?
I'd suggest putting the code in the existing BomBuilder.makeComponent()

jkowalleck · 2023-01-25T10:00:36Z

@CompartMSL I find the proposed current implementation disturbing, as it does not stick to chosen design principles and architecture of this project.

my suggestion: write the needed function and usage in the builders.ts file.

Add the option whether to add the license texts to existing BomBuilderOptions and act based on it in the exiting BomBuilder. This option could be called shouldAddLicenseTexts.
Create a new class there, that has a method that returns or updates needed structures based on a given directory path.
inject your own builder into BomBuilder - like BomBuilder.componentBuilder.
It could be called LicensBuilder and be available internally as this.licensBuilder.

hook into the existing BomBuilder.makeComponent() and call needed methods like

const component = this.componentBuilder.makeComponent(data, type)
// ...
if (!this.packageLockOnly && this.shouldAddLicenseTexts && data.path) {
  this.licensBuilder.addTexts(component.licenses, data.path)
}
// ...
return component

jkowalleck · 2023-03-02T08:06:40Z

src/licensetexts.ts

+ * @param {Models.DisjunctiveLicense} license
+ * @param {string} installPath
+ */
+function addLicTextBasedOnLicenseFiles (license: Models.DisjunctiveLicense, installPath: string): void {


license text detection is not completely solved.

Therefore, the result shall be an evidence, rather than n license attachment.

As suggested in CycloneDX#427 (comment) Signed-off-by: Christoph Uhland <42832096+cuhland@users.noreply.github.com>

cuhland · 2024-12-09T13:43:34Z

I want to support this feature and created a branch which took these commits here, rebased them on top of current main and did soome refactoring regarding the code structure.
@jkowalleck Is there a chance that this feature can be part of the next release?

jkowalleck · 2024-12-09T16:57:48Z

I want to support this feature and created a branch which took these commits here, rebased them on top of current main and did soome refactoring regarding the code structure. @jkowalleck Is there a chance that this feature can be part of the next release?

I do not see a good to merge the current state of this very PR.

If you have a maintained version of it, feel free to open a competing pull-request.

Since this feature is nothing new, there is no need to reinvent the wheel. It is expected to simply copy existing art.
Please be aware of existing art:

just in case: Please keep the scope of your PR, don't mix unrelated changes into a feature-PR

jkowalleck · 2024-12-17T10:57:00Z

Superseded by #1243

CompartMSL requested a review from a team as a code owner January 10, 2023 17:24

CompartMSL force-pushed the feature/addLicTexts branch from 99f9dde to 1d348e9 Compare January 11, 2023 06:50

CompartMSL closed this Jan 11, 2023

CompartMSL deleted the feature/addLicTexts branch January 11, 2023 06:51

CompartMSL restored the feature/addLicTexts branch January 11, 2023 06:52

CompartMSL reopened this Jan 11, 2023

CompartMSL force-pushed the feature/addLicTexts branch from 1d348e9 to b27a3fe Compare January 11, 2023 06:56

CompartMSL force-pushed the feature/addLicTexts branch from b27a3fe to b20daa5 Compare January 13, 2023 15:11

FEAT: Option to add license text to BOM output (CycloneDX#256)

0d44a40

Signed-off-by: Matthias Schiebel <msl@compart.com>

CompartMSL force-pushed the feature/addLicTexts branch 2 times, most recently from 217a4c8 to b53631e Compare January 16, 2023 12:39

jkowalleck requested changes Jan 18, 2023

View reviewed changes

.gitignore Outdated Show resolved Hide resolved

tests/integration/synthetics/cli.run.test.js Outdated Show resolved Hide resolved

tests/_data/dummy_projects/with-lockfile/node_modules/my-local-a/LICENSE Outdated Show resolved Hide resolved

CompartMSL force-pushed the feature/addLicTexts branch from 7627101 to b53631e Compare January 23, 2023 06:45

jkowalleck changed the title ~~FEAT: Option to add license text to BOM output (#256)~~ feat: Option to add license text to BOM output (#256) Jan 23, 2023

jkowalleck changed the title ~~feat: Option to add license text to BOM output (#256)~~ feat: add switch to add license text to BOM result Jan 23, 2023

jkowalleck reviewed Jan 24, 2023

View reviewed changes

src/licensetexts.ts Outdated Show resolved Hide resolved

jkowalleck reviewed Jan 24, 2023

View reviewed changes

jkowalleck marked this pull request as draft January 24, 2023 13:46

CompartMSL force-pushed the feature/addLicTexts branch 2 times, most recently from d0c1d1c to 2efa5b7 Compare January 25, 2023 08:38

Changed approach to find license text files to be case-insensitive

cf882a1

Signed-off-by: Matthias Schiebel <msl@compart.com>

CompartMSL force-pushed the feature/addLicTexts branch from 2efa5b7 to cf882a1 Compare January 25, 2023 08:54

jkowalleck reviewed Jan 25, 2023

View reviewed changes

jkowalleck reviewed Mar 2, 2023

View reviewed changes

jkowalleck added the enhancement New feature or request label Jun 8, 2023

cuhland added a commit to cuhland/cyclonedx-node-npm that referenced this pull request Dec 9, 2024

Refactor: Create LicenseBuilder

6e81cc3

As suggested in CycloneDX#427 (comment) Signed-off-by: Christoph Uhland <42832096+cuhland@users.noreply.github.com>

cuhland mentioned this pull request Dec 10, 2024

feat: Add license text as evidence #1243

Merged

jkowalleck closed this in #1243 Dec 17, 2024

jkowalleck closed this in 0c7e2d4 Dec 17, 2024

Uh oh!

Conversation

CompartMSL commented Jan 10, 2023 • edited by jkowalleck Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jkowalleck commented Jan 11, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

jkowalleck commented Jan 19, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

CompartMSL commented Jan 23, 2023

Uh oh!

jkowalleck commented Jan 23, 2023

Uh oh!

Uh oh!

jkowalleck Jan 24, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

CompartMSL Jan 25, 2023

Choose a reason for hiding this comment

Uh oh!

jkowalleck commented Jan 24, 2023

Uh oh!

jkowalleck Jan 25, 2023

Choose a reason for hiding this comment

Uh oh!

jkowalleck Jan 25, 2023

Choose a reason for hiding this comment

Uh oh!

jkowalleck commented Jan 25, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jkowalleck Mar 2, 2023

Choose a reason for hiding this comment

Uh oh!

cuhland commented Dec 9, 2024

Uh oh!

jkowalleck commented Dec 9, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jkowalleck commented Dec 17, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

CompartMSL commented Jan 10, 2023 •

edited by jkowalleck

Loading

jkowalleck commented Jan 11, 2023 •

edited

Loading

jkowalleck commented Jan 19, 2023 •

edited

Loading

jkowalleck Jan 24, 2023 •

edited

Loading

jkowalleck commented Jan 25, 2023 •

edited

Loading

jkowalleck commented Dec 9, 2024 •

edited

Loading