-
-
Notifications
You must be signed in to change notification settings - Fork 172
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
Slow fetching of licenses (cdxgen -t dotnet) #352
Comments
@andreycha Great ticket. Will definitely look into this. Do you happen to have any sample project to test this? |
cyclonedx-dotnet repo can be used as a sample project, there you already see all the items I've described. One more note: while the ticket is described as .NET specific, it looks to me like some of these items might benefit all project types. |
@andreycha do you have a project where cdxgen is slower than cyclonedx-dotnet? Looking at the code, cdx-dotnet is first calling nuget to get the github license url, then queries github to get the spdx id. cdxgen in contrast is only querying nuget and then sets the license name as Also, the grouping feature you noticed is merely multiple libraries specifying the same vcs url. Let me see if there is a way to group and reduce lookups without losing accuracy. |
@prabhu cyclonedx-dotnet is such a project :). When I run it over itself, it runs several times faster than cdxgen (for bigger project difference is even bigger). |
@andreycha Sorry didn't build the project. Got the issue now. |
Signed-off-by: Prabhu Subramanian <[email protected]>
@andreycha PR 358 is ready for your review. If you could test the branch with multiple repos and compare the result with cyclonedx-dotnet, that would be awesome! |
Signed-off-by: Prabhu Subramanian <[email protected]>
@prabhu thanks for a rapid fix! Changes looks good to me. What's the best way to compile and run it locally? Docker Compose? |
@andreycha, Yes docker compose is a good idea. Alternatively, you can git clone cdxgen repo and switch to the branch fix/issue-352 . Or use github cli as shown Then npm install followed by invoking cdxgen using |
@andreycha, any luck with the testing? |
Signed-off-by: Prabhu Subramanian <[email protected]>
Signed-off-by: Prabhu Subramanian <[email protected]>
Signed-off-by: Prabhu Subramanian <[email protected]>
Signed-off-by: Prabhu Subramanian <[email protected]>
Signed-off-by: Prabhu Subramanian <[email protected]>
Signed-off-by: Prabhu Subramanian <[email protected]>
* Fixes #352. Cache nuget metadata lookup with some safe defaults Signed-off-by: Prabhu Subramanian <[email protected]> --------- Signed-off-by: Prabhu Subramanian <[email protected]>
Hi,
I'm considering to switch from cyclonedx-dotnet to cdxgen when generating BOM for .NET applications which also contain frontend dependencies. With cyclonedx-dotnet I'm specifying Github username and token to include package information and licenses in particular into resulting BOM file.
So first I've tried to generate BOM for .NET dependencies using cdxgen: I set
FETCH_LICENSE
andGITHUB_TOKEN
variables and rancdxgen -t dotnet -o ...
. And it runs very slowly, at least x10 comparing to cyclonedx-dotnet.I've set
SCAN_DEBUG_MODE=debug
and noticed few things comparing to cyclonedx-dotnet output:Retrieving GitHub license for repository dotnetcore/NPOI and ref master - URL: https://api.github.com/repos/dotnetcore/NPOI/license?ref=master
while cdxgen does it for every single package, e.g.:And then it gets multiplied by a number of projects where they are referenced.
It again gets multiplied by a number of projects where they are referenced.
MyCompany.*
.I initially thought that cdxgen just serves like a proxy to a set of concrete tools, but looks like it has its own implementation at least when it comes to fetching package information. Would it be possible to improve the performance here?
Meanwhile I guess I'd be better off with running cyclonedx-dotnet to obtain a BOM file with .NET deps, then cdxgen to obtain a BOM file with frontend only dependencies, and then merge them.
Thank you!
The text was updated successfully, but these errors were encountered: