Skip to content
This repository has been archived by the owner on May 24, 2022. It is now read-only.

More version patters for Gradle #345

Closed
szpak opened this issue Jan 28, 2019 · 13 comments
Closed

More version patters for Gradle #345

szpak opened this issue Jan 28, 2019 · 13 comments

Comments

@szpak
Copy link

szpak commented Jan 28, 2019

In Mockito we keep common dependencies in one place to share them across the (sub)modules. They doesn't seem to be fully supported. It would be useful to extend the property value finder to find unambitious patterns and we would try to adjust the rest to something more generic.

We've seen property_value_finder.rb, but those are regular expressions and in addition you know better how to adjust it properly :).

Sample cases:

def versions = [:]

versions.bytebuddy = '1.9.7'
versions.junitJupiter = '5.1.1'

libraries.junit4 = 'junit:junit:4.12'
libraries.junitJupiterApi = "org.junit.jupiter:junit-jupiter-api:${versions.junitJupiter}"
libraries.junitPlatformLauncher = 'org.junit.platform:junit-platform-launcher:1.1.0'
libraries.junitJupiterEngine = "org.junit.jupiter:junit-jupiter-engine:${versions.junitJupiter}"
libraries.assertj = 'org.assertj:assertj-core:2.9.0'
libraries.hamcrest = 'org.hamcrest:hamcrest-core:1.3'

....
compileOnly libraries.junit4, libraries.hamcrest
compile libraries.objenesis

and

def kotlinVersion = '1.2.10'
libraries.kotlin = [
    stdlib: "org.jetbrains.kotlin:kotlin-stdlib:${kotlinVersion}",
    ...
]

Our file with dependency configuration. Please let us how you see it.

@feelepxyz
Copy link
Contributor

@szpak thanks for getting in touch! Dependabot's Gradle support is still in beta as the file parsing is pretty tricky and we currently don't have much Java knowledge. Looks like we should be evaluating the build file to support your use case, which is a pretty big piece of work so will take some time to get to.

Have you considered removing your version variables and just inlining the versions? Dependabot should then keep these versions up to date individually.

@greysteil
Copy link
Contributor

Thanks @feelepxyz. I'd like to take a quick look at the property value finder to see why it's messing up here, too, though. Will try to get to that this afternoon, and if it's easy I'll get it updated.

(We're going to have a a bunch more resource working on Dependabot's Java support soon, btw, so expect lots of improvements!)

@greysteil
Copy link
Contributor

OK, just digging into this.

It looks like the problem we have here is with Dependabot's file-fetcher for Gradle, rather than the parsing. In particular, Dependabot doesn't have any knowledge of the dependencies.gradle file, because it doesn't have any logic around how to handle apply from statements.

@szpak - could you point me at some docs on how Dependabot should deal with apply from statements? If I can understand those then I should be able to get Dependabot working properly here.

@szpak
Copy link
Author

szpak commented Feb 5, 2019

Thanks @greysteil .

Gradle allows to apply plugins and configuration from other files - both local (most of the cases) and remote. A laconic explanation from the official documentation.

Btw, there is also an example for Kotlin files, however, Kotlin has been added quite recently as a language to configure Gradle and most of the projects still use Groovy (and build.gradle - the first example).

@szpak
Copy link
Author

szpak commented Feb 5, 2019

Btw2, you might be also interested in the way how variables can be created (both local and those in the ext context) in Gradle configuration (again, Groovy is currently much more popular and I propose to support it at the first place).

@greysteil
Copy link
Contributor

That's really useful, thanks @szpak.

I've just shipped this commit which means dependabot will now update dependencies in a plugin file which has dependencies in its name. Assuming that doesn't cause any trouble I'll expand out the plugin files that Dependabot fetches and updates.

@szpak
Copy link
Author

szpak commented Feb 14, 2019

Thanks @greysteil and sorry for delay. I had problem reaching out the repository owner to add Dependabot. In the end I tested it using my fork.

It seems to work in general. However, I have two remarks/suggestions:

  1. For some reason Dependabot created 2 the same looking PRs - https://github.com/szpak/mockito/pull/4/files and https://github.com/szpak/mockito/pull/5/files
  2. What do you think about changing a file selection from *dependencies* to *.gradle? Following the convention, all Gradle configuration files (written in Groovy) should have that extension.

@szpak
Copy link
Author

szpak commented Feb 26, 2019

@greysteil What do you think about my point from the previous comment?

@greysteil
Copy link
Contributor

greysteil commented Feb 28, 2019

  1. That... is really weird. Digging into it now. Thanks for flagging!
  2. Trouble is that that would cause Dependabot to pull down a lot of additional files, which could cause problems with rate limits. I think it's safest to keep it as it is, until we see situations where a dependency update in some other file is being missed.

@greysteil
Copy link
Contributor

FYI, (1) is now fixed (it was a race). Thanks again for the heads up on that one!

@szpak
Copy link
Author

szpak commented Feb 28, 2019

Ad. 1. 👍

Ad. 2. Ok, it should be a big problem.

andrewedstrom added a commit to cloudfoundry/uaa that referenced this issue May 28, 2019
Dependabot will only look at our shared dependency version file if it
has `dependencies` in its name:
dependabot/feedback#345

[#166295186]
andrewedstrom added a commit to cloudfoundry/uaa that referenced this issue May 28, 2019
If this works, dependabot will create a PR to bump the Jackson version
in `dependencies.gradle`.

I'm trying to mimic the structure used by the mockito team:
dependabot/feedback#345

[#166295186]
@andrewedstrom
Copy link

Hey @greysteil :) just wanted to point out that there's nothing in gradle that actually demands you put shared dependencies in a file called dependencies.gradle or one that has dependencies in its name—that's just what the Mockito team happens to do.

I point this out because my team uses a similar pattern. We found it difficult to figure out that in order to get dependabot to detect our shared versions, we had to rename the file we put them in. The only documentation of that seems to be this github issue. Eventually we figured it out in this commit and it started working, but it was quite confusing. It would nice to have this somewhere in the official documentation.

@greysteil
Copy link
Contributor

Sorry about that @andrewedstrom. The team have their work cut out scaling Dependabot's automated security fixes to work on all GitHub repos, but I know coming back to Gradle and improving our support their is high on the priority list once that scaling is done.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants