-
Notifications
You must be signed in to change notification settings - Fork 40
Updated gradle-dep-tree plugin and added support included builds #611
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
base: dev
Are you sure you want to change the base?
Conversation
# Conflicts: # commands/audit/auditbasicparams.go # commands/audit/auditparams.go # sca/bom/buildinfo/technologies/common.go # sca/bom/buildinfo/technologies/java/gradle.go # sca/bom/buildinfo/technologies/java/resources/gradle-dep-tree.jar
attiasas
left a comment
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.
Nice work, check out my comments
- You should create the new flag in the flag maps and attach it to the related command (similar to other flags in the file)
- I released the new version of the plugin with your changes. make sure you update and run the script that the v
3.2.0will be used and theembedded pluginstest will pass - make sure all tests are passing
| Exclusions = "exclusions" | ||
| IncludeDirs = "include-dirs" | ||
| UseWrapper = "use-wrapper" | ||
| UseIncludedBuilds = "use-included-builds" |
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.
make sure to define the flag and assign it to the related command (similar to other flags in the file)
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.
Added it to the 'flagsMap'
| "-Dcom.jfrog.includeAllBuildFiles=true", | ||
| fmt.Sprintf("-Dcom.jfrog.includeIncludedBuilds=%t", gdt.useIncludedBuilds)} |
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.
There are no issue using both flags, right?
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.
You mean 'includeAllBuildFiles' and 'includeIncludedBuilds' ? They're not exactly related, 'includeAllBuildFiles' was already present before my change. It's used for for the gradle subprojects not for gradle included builds
| Unresolved bool `json:"unresolved,omitempty"` | ||
| Configurations *[]string `json:"configurations,omitempty"` |
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.
Is there a use for them here other than the logs? do we want to skip unresolved?
please make sure Xray can scan those dependencies by adding a test scan on a similar project and making sure Xray can handle those deps
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.
'Configurations' is used to determine the dependency scope (direct, transitive, etc.). It contains values like 'compileOnly', 'testImplementaion', etc. which are in the Gradle spec.
'Unresolved' is not being used yet, I just added it to the struct in case we will want to take it into account.
I want to stress that those values are outputted by the gradle plugin but we just didn't unmarshall them into the struct
devbranch.go vet ./....go fmt ./....Depends on: