Added (basic) support for build.gradle files #707
Conversation
Added a file in syft/pkg/cataloger/java/, gradle_parser.go, this parses build.gradle files: - First, it finds the 'dependency' parts while also parsing and inserting any `def`ined variables. - Next, it splits the sections into individual dependencies. - Finally, it splits the string by colons, taking the second part as the `Name` and the third as the `Version`. I tried to keep it as close to other code in the repository, particularly syft/pkg/cataloger/apkdb/parse_apk_db.go I've added a `NewJavaGradleCataloger` function to syft/pkg/cataloger/cataloger.go. I changed `"java-cataloger"` to `"java-archive-cataloger"`. I'm not sure about tests, I probably could use some help with them. I've also fixed two typos.
| // integrity check | ||
| var _ common.ParserFn = parseJaveGradle | ||
|
|
||
| func parseJaveGradle(_ string, reader io.Reader) ([]pkg.Package, []artifact.Relationship, error) { |
There was a problem hiding this comment.
can you add tests around this parser function?
| variables := make(map[string][]byte) | ||
|
|
||
| // get each 'dependency {}' section | ||
| onDependency := func(data []byte, atEOF bool) (advance int, token []byte, err error) { |
There was a problem hiding this comment.
I see why this particular parser function is complex, mostly since the build.gradle file is groovy syntax and short of a groovy AST parser there isn't a straightforward way to extract the information in question. The split functions are pretty dense from a reading and understanding point of view, and I'm hesitant to change them without a body of tests around the split functions themselves.
Assuming there isn't any shared state in the stack that is leveraged, can you extract out all of the split functions and put them individually under test?
|
we require that all commits on a PR are signed off as well as signed --can you update your commits? Shout out if you have any questions! |
|
hey @bolshoytoster I just wanted to follow up with the comment here: Are you able to sign the commits and resolve the conflicts? We would love to get this aligned with |
|
hi any idea why this has been closed? really interested in that feature. Maybe i can pick up the pr? |
|
@henrysachs -- it looks like the author closed it, it's unclear why. I'm completely speculating but perhaps the author did not have time to finish this PR by adding tests and such. Being able to catalog Gradle files does sound like something we'd like to have! |
|
Hey, Sounds reasonable. I would like to pick this up. Can you point me to some tests for other cataloggers where I would be able to find the right path for implementing them? |
|
@henrysachs a couple points here:
|
|
@bolshoytoster if you're reading this I would be happy to chat. But I would probably only take the above described algorithm and not the code itself as you said catalogers changed quite a bit. |
I had no clue how to implement tests, and I also definetely needed to simplify and clean up the code. (It was definetely made more complicated by supporting variables.
If you want to, but you'll have to either clean up my code or write a more simplified version. |
|
I just learned parsing gradle files isn't that easy but that will be a fun activity. So give me some time :D |
|
@henrysachs @bolshoytoster -- it looks like you might want to split this into a couple commits, one of which might use |
|
as soon as i have something to show and discuss I will ping both of you guys. I think co-authoring sounds good to me, but lets discuss this as soon as the first pr from my fork is ready. :) |
Added a file in syft/pkg/cataloger/java/, gradle_parser.go, this parses build.gradle files:
defined variables.Nameand the third as theVersion.I tried to keep it as close to other code in the repository, particularly syft/pkg/cataloger/apkdb/parse_apk_db.go
I've added a
NewJavaGradleCatalogerfunction to syft/pkg/cataloger/cataloger.go.I changed
"java-cataloger"to"java-archive-cataloger".I'm not sure about tests, I probably could use some help with writing them.
I've also fixed two typos.