-
Notifications
You must be signed in to change notification settings - Fork 594
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
fix(npm): Introduce installation for cyclone-node-npm in another folder and fallback to cyclonedx/bom to help users generate BOM #4390
Conversation
bf6f30c
to
c3e5abb
Compare
20dd026
to
5ac4d66
Compare
/it-go |
9e2a72a
to
da895bb
Compare
Significant change in code since last review, will be helpful to look again
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 now see in the logs that with the publish
flag set to true, it finds an additional package.json
and publishes that:
info npmExecuteScripts - Discovered package.json file bomFolder/package.json
info npmExecuteScripts - Discovered package.json file package.json
info npmExecuteScripts - triggering publish for bomFolder/package.json
I'm not sure how that package.json
is created in bomFolder
, but I feel it should be excluded when publishing.
Apart from that, it works well, having tested it with a few images.
Good catch. |
fc40afc
to
de79343
Compare
/it-go |
@jliempt thanks for approval, will merge on Monday |
/it-go |
3 similar comments
/it-go |
/it-go |
/it-go |
Kudos, SonarCloud Quality Gate passed! |
This reverts commit 45ac7ca.
/it-go |
…er and fallback to cyclonedx/bom to help users generate BOM (SAP#4390) * Test * Try omit * Introduce global installation and fallback * Extract to a separate function * Fix unit tests * Add root permissions for docker image for Azure * Install in another folder * fix unit tests * Cleanup * introduce back --no-save,change directory name, fix tests * add tmp folder to npmignore * change docker image for guage * Revert "change docker image for guage" This reverts commit 45ac7ca.
Install cyclonedx-npm module in a separate (to avoid extraneous errors ) and try to generate BOM as this is the recommended and generates a more detailed BOM
Lots of users are still facing issues , this is mostly due to how cyclonedx-node-npm module relies on
npm ls
command internally, which is used internally to generate a a dependency tree of infinite depth. For some users with special settings, there are hiccups on how this command works and hence BOM generation is affected. More details can be found here and here.Hence ,we use cyclonedx/bom as a fallback to generate BOM - to facilitate BOM generation when new module throws errors.
To avoid publishing this new folder and packages , an entry to npmignore was added