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

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

Closed
wants to merge 11 commits into from

Conversation

jkowalleck
Copy link
Member

@jkowalleck jkowalleck commented Oct 30, 2024

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 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
Copy link

codacy-production bot commented Oct 30, 2024

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
lname = `file: ${file}`
} else {
// `file` could be absolute or relative path - lets resolve it anyway
file = resolve(relativeFrom, file)
Copy link
Member Author

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 ...

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
Member Author

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 ...

Signed-off-by: Jan Kowalleck <[email protected]>
Signed-off-by: Jan Kowalleck <[email protected]>
Signed-off-by: Jan Kowalleck <[email protected]>
Signed-off-by: Jan Kowalleck <[email protected]>
Signed-off-by: Jan Kowalleck <[email protected]>
Signed-off-by: Jan Kowalleck <[email protected]>
Signed-off-by: Jan Kowalleck <[email protected]>
Signed-off-by: Jan Kowalleck <[email protected]>
Signed-off-by: Jan Kowalleck <[email protected]>
Signed-off-by: Jan Kowalleck <[email protected]>
@jkowalleck
Copy link
Member Author

@AugustusKling,

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

@AugustusKling
Copy link
Contributor

@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?

@jkowalleck
Copy link
Member Author

jkowalleck commented Oct 30, 2024

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 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
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

centralized license evidence gatrhering
2 participants