Skip to content
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

Detect dependencies in Gradle included builds #5028

Merged

Conversation

gabrielfeo
Copy link
Contributor

@gabrielfeo gabrielfeo commented Apr 22, 2022

Detect already supported Gradle dependency files inside included builds and buildSrc.

Goal

Dependabot currently supports dependencies in build.gradle and other buildscripts in the main build. However, these files can be present in the same format in other builds part of the main one: included builds. They can be easily supported by the current file parsing and updating implementation, since they're in the same format, most changes being in fetching files in the additional folders.

Closes #4375

What are included builds?

In Gradle, a single build is composed of one root project with zero or more subprojects. A single build can also include other independent builds, each with its own settings file and root project (and maybe their own included builds):

main-build
\_settings.gradle
\_subproject
\_included-build
    \_settings.gradle
   \_build.gradle
   \_subproject
   \_nested-included-build
      \_settings.gradle
...

An included build is declared explicitly in settings.gradle:

includeBuild './included'
include ':app'
// ...

buildSrc is a special case. It's "treated as an included build". It's included in the build if the directory is named buildSrc regardless of being declared in settings.gradle. Not to be confused with the common pattern of declaring dependencies in code such as Kotlin classes inside buildSrc (#2180, example). That pattern is not supported and not a goal of this PR.

Changes

  • c6bf538 Detect includeBuild in Gradle SettingsFileParser
  • 0267202 Refactor FileFetcher to support transitive search. Supported files were always considered to be in the main build root project or main build subprojects. They should now be relative to the dir of current build being examined, the main one or included ones.
  • c66e1e827 Fetch files of included builds in Gradle FileFetcher. Read includeBuild in every build's settings file and walk. Nothing changes if there's no included builds.
  • 842c66a99 Support buildSrc as an included build. buildSrc is an implicit included build, check for it even if it's not manually declared.

Dry run examples

In regards to finding the update of Kotlin 1.6.10 -> 1.6.21 in this build.gradle.kts, which is inside an included build directory.

Current behavior: doesn't detect the Kotlin update because it doesn't detect and fetch files of included builds (test branch)

Screen Shot 2022-04-22 at 17 00 22

New behavior: detects the Kotlin update because it parses included build paths and fetches supported files inside (test branch 1 and 2)

Screen Shot 2022-04-22 at 17 04 20

Screen Shot 2022-04-22 at 17 05 32

@gabrielfeo gabrielfeo requested a review from a team April 22, 2022 16:24
@gabrielfeo
Copy link
Contributor Author

Hey @jakecoffman, maybe you can take look at this? :)

@gabrielfeo gabrielfeo requested a review from a team as a code owner May 18, 2022 21:59
@LeoColman
Copy link

Hey guys!
This is important and the work is already done. Could you take a look? @jakecoffman

@dgaillard2
Copy link

Any news about with this PR ? Waiting for such changes

@Nishnha Nishnha force-pushed the feature/gradle-detect-included-builds branch from e022f50 to c9f526f Compare September 30, 2022 20:44
@Nishnha Nishnha force-pushed the feature/gradle-detect-included-builds branch from c9f526f to 4cd5677 Compare October 14, 2022 19:21
@Nishnha Nishnha merged commit 2401f2b into dependabot:main Oct 14, 2022
@Nishnha
Copy link
Member

Nishnha commented Oct 14, 2022

Hi @gabrielfeo, we really apologize for the delays here. There is no reason this could't have been deployed earlier.

Since this is now in production, it would be great if you could check that this is working as intended since you have familiarity with the standard.

Thank you for the contribution!

@gabrielfeo
Copy link
Contributor Author

gabrielfeo commented Oct 19, 2022

Hi @gabrielfeo, we really apologize for the delays here. There is no reason this could't have been deployed earlier.

Since this is now in production, it would be great if you could check that this is working as intended since you have familiarity with the standard.

Thank you for the contribution!

Working as intended, @Nishnha! E.g. gabrielfeo/50-72#61. Thank you for reviewing.

We might be able to close #2180 also, but that wasn't the goal. I can test it further, not sure OP and latest comments are talking about the same thing.

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

Successfully merging this pull request may close these issues.

buildSrc project will not be parsed
4 participants