Skip to content

[WIP] feat: Builders.FromPath.LicenseEvidenceBuilder#1158

Closed
jkowalleck wants to merge 11 commits intomainfrom
feat/builder_evidece_licenses
Closed

[WIP] feat: Builders.FromPath.LicenseEvidenceBuilder#1158
jkowalleck wants to merge 11 commits intomainfrom
feat/builder_evidece_licenses

Conversation

@jkowalleck
Copy link
Copy Markdown
Member

@jkowalleck jkowalleck commented Oct 30, 2024 •
edited
Loading

fixes #1162

Note

on hold - see #1158 (comment)

refactored the evidence collection from our sister project https://github.com/CycloneDX/cyclonedx-webpack-plugin.

original:
https://github.com/CycloneDX/cyclonedx-webpack-plugin/blob/72700f06d00eac79fa3b91fe838bd78c583346a2/src/extractor.ts#L133-L173

the original file is under Apache-2.0 license, and rights owner is OWASP.
All the same applies to this very project. Therefore, no legal issues exist -> we are free to copy/paste/modify the code without the slightest concerns.

need to see if there are additional requirements from CycloneDX/cyclonedx-node-yarn#193

@jkowalleck
feat: builders.FromPath.LicenseEvidenceBuilder
6186d73 
Signed-off-by: Jan Kowalleck <jan.kowalleck@gmail.com>
@jkowalleck jkowalleck added the enhancement New feature or request label Oct 30, 2024
@jkowalleck jkowalleck changed the title feat: builders.FromPath.LicenseEvidenceBuilder [WIP] feat: builders.FromPath.LicenseEvidenceBuilder Oct 30, 2024
@jkowalleck jkowalleck mentioned this pull request Oct 30, 2024
Merged
@codacy-production
Copy link
Copy Markdown

codacy-production bot commented Oct 30, 2024 •
edited
Loading

Coverage summary from Codacy

See diff coverage on Codacy

Coverage variation Diff coverage
✅ -0.63% (target: -1.00%) ❌ 75.56% (target: 90.00%)
Coverage variation details
Coverable lines Covered lines Coverage
Common ancestor commit (414ce5e) 23742 23356 98.37%
Head commit (4d9a100) 24417 (+675) 23866 (+510) 97.74% (-0.63%)

Coverage variation is the difference between the coverage for the head and common ancestor commits of the pull request branch: <coverage of head commit> - <coverage of common ancestor commit>

Diff coverage details
Coverable lines Covered lines Diff coverage
Pull request (#1158) 225 170 75.56%

Diff coverage is the percentage of lines that are covered by tests out of the coverable lines that the pull request added or modified: <covered lines added or modified>/<coverable lines added or modified> * 100%

See your quality gate settings    Change summary preferences

Codacy stopped sending the deprecated coverage status on June 5th, 2024. Learn more

@jkowalleck jkowalleck changed the title [WIP] feat: builders.FromPath.LicenseEvidenceBuilder [WIP] feat: Builders.FromPath.LicenseEvidenceBuilder Oct 30, 2024
jkowalleck
jkowalleck commented Oct 30, 2024
View reviewed changes
src/builders/fromPath.node.ts Outdated
lname = `file: ${file}`
} else {
// `file` could be absolute or relative path - lets resolve it anyway
file = resolve(relativeFrom, file)
Copy link
Copy Markdown
Member Author

@jkowalleck jkowalleck Oct 30, 2024

Choose a reason for hiding this comment

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

unsure if this is a good idea ...

src/builders/fromPath.node.ts Outdated
public * fromDir(dir: string, relativeFrom: string | undefined = undefined): Generator<NamedLicense> {
if ( relativeFrom !== undefined) {
// `dir` could be absolute or relative path - lets resolve it anyway
dir = resolve(relativeFrom, dir)
Copy link
Copy Markdown
Member Author

@jkowalleck jkowalleck Oct 30, 2024

Choose a reason for hiding this comment

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

unsure if this is a good idea ...

@jkowalleck
feat
98973e5 
Signed-off-by: Jan Kowalleck <jan.kowalleck@gmail.com>
@jkowalleck
feat
30f1915 
Signed-off-by: Jan Kowalleck <jan.kowalleck@gmail.com>
@jkowalleck
feat
09cb566 
Signed-off-by: Jan Kowalleck <jan.kowalleck@gmail.com>
@jkowalleck
feat
e68b2bc 
Signed-off-by: Jan Kowalleck <jan.kowalleck@gmail.com>
@jkowalleck
feat
9a94456 
Signed-off-by: Jan Kowalleck <jan.kowalleck@gmail.com>
@jkowalleck
feat
3ec62c7 
Signed-off-by: Jan Kowalleck <jan.kowalleck@gmail.com>
@jkowalleck
feat
de97015 
Signed-off-by: Jan Kowalleck <jan.kowalleck@gmail.com>
@jkowalleck
feat
a9e5615 
Signed-off-by: Jan Kowalleck <jan.kowalleck@gmail.com>
@jkowalleck
feat
d80368c 
Signed-off-by: Jan Kowalleck <jan.kowalleck@gmail.com>
@jkowalleck
feat
4d9a100 
Signed-off-by: Jan Kowalleck <jan.kowalleck@gmail.com>
@jkowalleck
Copy link
Copy Markdown
Member Author

jkowalleck commented Oct 30, 2024

@AugustusKling,

this is the feature for license evidence builder.
Could I ask you for your opinion?

@AugustusKling
Copy link
Copy Markdown
Contributor

AugustusKling commented Oct 30, 2024

@jkowalleck thanks for refactoring this to live in the library.

The code in this pull requests is can be consumed as long as packages are extracted into the file system before your code runs. This is not the case in the cyclonedx-node-yarn plugin which would mean it couldn't consume this part of the library.

Yarn supports 3 different linkers as explained in https://yarnpkg.com/features/linkers of which projects can freely choose:

  • pnp: Uses a loader to make package content available at runtime. Does not extract packages to files on the file system which makes it impossible to access package contents with the Node file system API.
  • pnpm: Uses a combination of symlinks and hard links as detailed in https://pnpm.io/symlinked-node-modules-structure. The Node file system API should work.
  • node-modules: The copies as NPM would produce when writing its node_modules folder.

In other words, I think the LicenseEvidenceBuilder and also the AttachmentFactory should work for the pnpm and node-modules linkers. They would not work using the pnp linker which is essential in a plugin for Yarn.

In the cyclonedx-node-yarn plugin I need to use Yarn's file system abstraction (FakeFS<PortablePath>) which allows to see package contents regardless of the used linker and regardless if a project was ever installed. Doing so I could get the file names and the contents of the files to feed into the CDX library.

What do you think about exposing a isLicenseFile(filename: string): boolean function and one to create a license instance as in makeLicense(filename: string, content: Buffer): CDX.Models.License?

AugustusKling
AugustusKling reviewed Oct 30, 2024
View reviewed changes
@jkowalleck
Copy link
Copy Markdown
Member Author

jkowalleck commented Oct 30, 2024 •
edited
Loading

The work on this generalization with the goal to make it available in the library ...
it feels like premature.... There is just not enough specific code in the field to actually produce any generalized code worth shipping.

I will postpone it until additional art exists.

@jkowalleck jkowalleck mentioned this pull request Nov 4, 2024
Closed
@jkowalleck jkowalleck closed this Nov 18, 2024
@jkowalleck jkowalleck deleted the feat/builder_evidece_licenses branch November 18, 2024 09:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer

@AugustusKling AugustusKling AugustusKling left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

enhancement New feature or request

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

feat: centralized license evidence gatrhering

2 participants

@jkowalleck @AugustusKling