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

manager(gradle): no deps detected when 3 dep strings present in the configuration #30234

Closed
RahulGautamSingh opened this issue Jul 18, 2024 · 11 comments · Fixed by #33487
Closed
Labels
manager:gradle Gradle package manager priority-3-medium Default priority, "should be done" but isn't prioritised ahead of others type:bug Bug fix of existing functionality

Comments

@RahulGautamSingh
Copy link
Collaborator

RahulGautamSingh commented Jul 18, 2024

Describe the proposed change(s).

Ref: #30232

Gradle manager does not detect any dep when 3 dependency strings are present in testImplementation (or, possibly any other configuration).

Reproduction: https://github.com/bh-tt/renovate-tt-gradle-reproducer

@RahulGautamSingh RahulGautamSingh added type:bug Bug fix of existing functionality priority-3-medium Default priority, "should be done" but isn't prioritised ahead of others manager:gradle Gradle package manager labels Jul 18, 2024
@RahulGautamSingh RahulGautamSingh changed the title manager(gradle): no deps detected when 3 deps strings present in the configuration manager(gradle): no deps detected when 3 dep strings present in the configuration Jul 18, 2024
@bh-tt
Copy link

bh-tt commented Oct 25, 2024

This is still broken. I would like to submit a PR myself but I'd need some guidance on how the matching works, since the library Renovate uses has very little documentation.

@rarkins
Copy link
Collaborator

rarkins commented Oct 25, 2024

@zharinov can you advise?

@Churro
Copy link
Collaborator

Churro commented Oct 25, 2024

@bh-tt is this edge-case specific to configurations provided by the java-library plugin? Could you please share API specs or other references that indicate official support for this syntax?

    testImplementation(
            'org.junit.jupiter:junit-jupiter-api:5.6.2',
            'org.junit.jupiter:junit-jupiter-params:5.6.3',
            'jakarta.ws.rs:jakarta.ws.rs-api:2.1.6'
    )

I'm asking because I can't find declarations other than with a single dep in the gradle docs: configuration('<group>:<name>:<version>').

The issue is that renovate's gradle manager has a dependency matching heuristic that searches someMethod("<string>", "<string>", "<string>") and expects to find <group>:<name>:<version>. At the point where this is matched, there is no way to say "oh no, these are actually 3 individual dependencies, not 3 parts of one dependency" (1) because how the parser works (the "reasoning" would come too late to even try matching other patterns) and (2) due to the lax restrictions on allowed characters in package names and versions.

If this edge-case is specific to (some?) gradle configurations of the java-library, it could work to add more specific matching patterns for these configurations that would take precedence over the heuristic matching pattern.

// Update: After digging into this a bit more, I found that this works due to syntactic sugar in Groovy itself. There is no varargs support in gradle's DependencyHandler. Groovy internally wraps multiple arguments in a list if they aren’t matched to a specific method signature with varargs. testImplementation('dep1', 'dep2') gets passed to gradle as testImplementation(['dep1', 'dep2']) and then some magic happens that internally expands this list into separate calls to dependencies.add('testImplementation', 'dep1') and dependencies.add('testImplementation', 'dep2').

Probably it works to tweak the matcher such that the 3rd arg must not include : (which version values don't do). Simply rewriting such version definitions is probably a lot easier than fixing this in renovate.

@bh-tt
Copy link

bh-tt commented Oct 29, 2024

Thanks for the digging, I took a quick look friday and also could not find a method accepting multiple args and had not gone further yet.

I'm aware rewriting the definitions is easier, but that is hard to implement over our 1000+ java projects, all in gradle. I could do a single pass over all of them in a week or so, but having all our developers keep this information in mind when adding a dependency somewhere would not be easy. Eventually someone would add a third dependency to a block, and since gradle simply accepts that it would take a long time before we would notice this.

I agree excluding a : from the 3rd argument would likely work for most cases, although maven version specification is very loose so that might break on some version classifiers.

I did just try a notation like you suggested (implementation("io.etcd", "jetcd-core", "0.8.2")), but gradle does not allow this. If you specify group, artifact and version separately you need to include the group:, name: and version: parameter names, as gradle technically accepts a Map in this case, like (implementation(group: "io.etcd", name:"jetcd-core", version:"0.8.2")). I cannot find support in the docs for specifying 3 strings as a single dependency. Perhaps matching for these 3 names would improve the robustness of the matching?

@bh-tt
Copy link

bh-tt commented Oct 29, 2024

I guess

export const qLongFormDep = q
is the problem? If that is the case, renovate seems to match a format not supported by (perhaps recent versions of) gradle, or not by gradle groovy build files (e.g. .gradle instead of .gradle.kts).

Edit: I've searched backwards up to gradle version 3.5.1 (released in june 2017, long out of support) and I cannot find information allowing this syntax. I've tried using the kotlin syntax but since I'm unfamiliar with that it might take some time to test.

@bh-tt
Copy link

bh-tt commented Oct 29, 2024

The syntax with 3 separate strings, without keys seems to be supported for Kotlin buildscripts, but not for groovy build scripts. Perhaps the simplest solution here is to apply the 3 string matcher only to .gradle.kts files?

@bh-tt
Copy link

bh-tt commented Oct 30, 2024

build.gradle.kts

plugins {
  id("java")
}

dependencies {
  implementation("io.etcd", "jetcd-core", "0.8.2")
}

Results in:

bh@devrd0 ~/testproject $ gradle assemble
Starting a Gradle Daemon (subsequent builds will be faster)

BUILD SUCCESSFUL in 12s
3 actionable tasks: 3 executed

Whereas build.gradle:

plugins {
  id("java")
}

dependencies {
  implementation("io.etcd", "jetcd-core", "0.8.2")
}

Results in:

bh@devrd0 ~/testproject $ gradle assemble

FAILURE: Build failed with an exception.

* Where:
Build file '/home/bh/testproject/build.gradle' line: 6

* What went wrong:
A problem occurred evaluating root project 'testproject'.
> Supplied String module notation 'io.etcd' is invalid. Example notations: 'org.gradle:gradle-core:2.2', 'org.mockito:mockito-core:1.9.5:javadoc'.

* Try:
> Run with --stacktrace option to get the stack trace.
> Run with --info or --debug option to get more log output.
> Run with --scan to get full insights.
> Get more help at https://help.gradle.org.

BUILD FAILED in 1s

I think the first case is also kotlin synthetic sugar, parsing the 3 strings into a map? I'm running with gradle 8.10.1

@bh-tt
Copy link

bh-tt commented Jan 7, 2025

@Churro I wondered if you've had time to look at my comments above? It seems like renovate accepts some notation that gradle itself does not, causing this bug.

@Churro
Copy link
Collaborator

Churro commented Jan 7, 2025

@bh-tt, I'm aware of a bug in renovate's gradle parser that is related to initial issue description and can be reproduced. I have a fix for it ready and will submit a PR when I'm sure that there are no unwanted side effects.

@renovate-release
Copy link
Collaborator

🎉 This issue has been resolved in version 39.96.2 🎉

The release is available on:

Your semantic-release bot 📦🚀

@bh-tt
Copy link

bh-tt commented Jan 9, 2025

@Churro I'm seeing some strange behaviour in renovate now with projects using 3 dep strings present in a single configuration, after updating to 39.96.2. Renovate pushes a new branch to create an update, and then the next run it deletes the branch again, without merging anything. The dependency dashboard is also not updated to reflect these newly found dependencies.

Edit: renovate pushes a branch, but does not push any changes to this branch, likely resulting in an automatic close?

This may need to be reopened, since renovate does not work as expected yet.

Edit: this is likely caused by running 2 different versions of renovate, one with this fix and one without.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
manager:gradle Gradle package manager priority-3-medium Default priority, "should be done" but isn't prioritised ahead of others type:bug Bug fix of existing functionality
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants