-
-
Notifications
You must be signed in to change notification settings - Fork 5
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
Add nested workspace traversal #36
base: 1.0-dev
Are you sure you want to change the base?
Add nested workspace traversal #36
Conversation
@MLSTRM before I review your work, I need to ask: |
357e083
to
c645fa2
Compare
…ecursive flag Signed-off-by: MLSTRM <[email protected]>
Signed-off-by: MLSTRM <[email protected]>
c645fa2
to
ec2ffcf
Compare
@jkowalleck apologies, that should be resolved now. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why is this file needed?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, it's not - I forgot to setup a proper gitignore in this new testbed
@@ -56,7 +57,8 @@ suite('integration', () => { | |||
'--reproducible', | |||
// no intention to test all the spec-versions nor all the output-formats - this would be not our scope. | |||
'--spec-version', latestCdxSpecVersion, | |||
'--output-format', 'JSON' | |||
'--output-format', 'JSON', | |||
'--recursive' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd rather have an own suite for make SBOM recursive
, that only runs on "nested-workspaces", instead of modifying the existing suite make SBOM
pleas create a new suite called make SBOM recursive
that runs with the new CLI switch and targets the test bed "nested-workspaces" only
@@ -70,6 +70,7 @@ $ yarn CycloneDX make-sbom | |||
(choices: "application", "framework", "library", "container", "platform", "device-driver", default: "application") | |||
--reproducible Whether to go the extra mile and make the output reproducible. | |||
This might result in loss of time- and random-based values. | |||
--recursive Scan all nested workspaces within the current project, rather than just the one in the current working directory. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Scan all nested workspaces within the current project, rather than just the one in the current working directory.
my questions:
- Could you explain how this is a use case?
- If the current workspace had no dependency to any other workspace, why would you want the other workspaces be part of the BOM?
- If the current workspace had a dependency on any other workspace, is this not already in the SBOM?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My use case comes from a multi-application project, specifically multiple serverless aws lambda functions, which work together to perform a single task.
As they cannot be deployed separately, it is more useful/accurate to me to have a single SBOM for the overall project, rather than one per function.
Currently the dependencies declared within each sub-workspace are not included at the top level, and without generating multiple separate SBOMs and somehow merging them, I cannot currently represent the full usage state of my application.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Understood. I do not see this as being a feature of the MVP.
So this will probably be not merged for some time.
Workspaces are a way of organizing work, not some architectural or design decision.
Therefore, they actually have no representation in an SBOM.
If you had a product containing of several components, each being an independent application, then you should be using either BOM-Links connecting your product's components/services with eachother, or use a merge-tool to combine individual SBOMs into one.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why are the licenses gone?
✖️ you might have broken something.
see test results: https://github.com/CycloneDX/cyclonedx-node-yarn/actions/runs/8325442279/job/22780468675?pr=36#step:8:38
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I get the same resulting update when running the command CYARN_TEST_UPDATE_SNAPSHOTS=1 yarn test
on 1.0-dev as of commit a70f74e
Not sure if there's something wrong in my local setup around this so I'll dig deeper
fixes #35