snyk / snyk-gradle-plugin

Basic Snyk CLI plugin for Gradle support
Other
25 stars 19 forks source link

Feat/dot notation #113

Closed aviadhahami closed 4 years ago

aviadhahami commented 4 years ago

With very large projects, this initscript has some problems with regards to memory usage since it is attempting to keep the entire dependency tree in memory through any included configurations or attributes. In my Gradle version at least it also appeared that the snykMergedDepsConf would result in a cycle, but dependency cycles in Gradle are not allowed within a configuration, so the combination of all configurations should also not have any cycles.

What does this PR do?

Converts the graph generation to use dot format. This alleviates the memory consumption problem for large projects as well as speeding up graph generation.

Where should the reviewer start?

Most of the code is in the init.gradle file. However, the parse-gradle.ts was scavenged pretty close to verbatim from the snyk-mvn-plugin.

How should this be manually tested?

I wasn't sure how to run the plugin through the CLI directly, so I was mostly running this directly via the Gradle commands:

./gradlew -I lib/init.gradle snykResolvedDeps -q --no-daemon -Dorg.gradle.parallel=
./gradlew -I lib/init.gradle snykResolvedDeps -q --no-daemon -Dorg.gradle.parallel= -Pconfiguration=testRuntime
./gradlew -I lib/init.gradle snykResolvedDeps -q --no-daemon -Dorg.gradle.parallel= -PonlySubProject=sub-project

All commands should remain identical to their previous counterparts.

What are the relevant tickets?

Snyk support ticket 3464

claassistantio commented 4 years ago

CLA assistant check
All committers have signed the CLA.

shanman190 commented 4 years ago

@aviadhahami and @orsagie, if there's anything I can do to help please let me know. I'm more than eager to learn! :)

anthogez commented 4 years ago

Hi @shanman190 , I would like to update you that we are current working on Gradle Improvements :)

shanman190 commented 4 years ago

I've also got availability to help if anything is needed. :)

anthogez commented 4 years ago

@shanman190 I would like to say thank you very much for your insights and collaboration with our team, but we decided of move forward with an alternative solution that is current available on the latest version of snyk-cli, would be nice to have some feedback :)

Thanks again!

shanman190 commented 4 years ago

Yep. I've been keeping up with it. :)

The first time I tested I had raised the issue on the Gradle wrapper bug. The model from a Gradle standpoint was much more performance though at that time it was still using the depTree format so was very nodejs memory intensive. I'll try out the depGraph updates tomorrow and let you know how that testing goes for me.

I do think the PRs that you've been putting in supercede the work done here quite nicely and as always more than happy to jump in and get my handy dirty! :)

anthogez commented 4 years ago

Yep. I've been keeping up with it. :)

The first time I tested I had raised the issue on the Gradle wrapper bug. The model from a Gradle standpoint was much more performance though at that time it was still using the depTree format so was very nodejs memory intensive. I'll try out the depGraph updates tomorrow and let you know how that testing goes for me.

I do think the PRs that you've been putting in supercede the work done here quite nicely and as always more than happy to jump in and get my handy dirty! :)

I agree the usage of gradle wrapper is important, I am happy to hear it thanks! I am looking forward for your feedback :)