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

Support dependency configurations from java-library #127

Closed
pkubowicz opened this issue Feb 1, 2017 · 16 comments
Closed

Support dependency configurations from java-library #127

pkubowicz opened this issue Feb 1, 2017 · 16 comments
Milestone

Comments

@pkubowicz
Copy link
Contributor

Gradle 3.4 RC introduces 'java-library' as a replacement for 'java'. Running 'dependencyUpdates' on such a project produces errors:

Failed to resolve ::api
Failed to resolve ::apiElements
Failed to resolve ::implementation
Failed to resolve ::runtimeElements
Failed to resolve ::runtimeOnly
Failed to resolve ::testImplementation
Failed to resolve ::testRuntimeOnly

Also, the plugin fails to check the new configurations.

To reproduce use build.gradle with

plugins {
    id 'com.github.ben-manes.versions' version '0.13.0'
    id 'com.github.johnrengelman.shadow' version '1.2.3'
}

apply plugin: 'java-library'

repositories {
    jcenter()
}

dependencies {
    api 'org.slf4j:slf4j-api:1.7.0'
    implementation 'com.google.guava:guava:19.0'
    compileOnly 'javax.servlet:javax.servlet-api:3.0.1'
    runtimeOnly 'ch.qos.logback:logback-classic:1.1.0'

    testImplementation 'junit:junit:4.10'
    testCompileOnly 'org.projectlombok:lombok:1.16.0'
    testRuntimeOnly 'org.slf4j:log4j-over-slf4j:1.7.0'
}

Each dependency in this file (except for the versions plugin) is outdated and should be reported. Currently only plugins, compileOnly and runtimeOnly are checked.

@ben-manes
Copy link
Owner

This is because Gradle disallows resolving those configurations (see #125). I don't know why they did that. I'm hoping that cloning and setting the flag to allow will bypass this. We should probably ask to find out why, in case they'd prefer to revert in rc2/

@pkubowicz
Copy link
Contributor Author

Have you asked them? Do you need some help?

@ben-manes
Copy link
Owner

Yep. I'm a little concerned that validateMutation will reject it, but I haven't tested it yet.

@ben-manes
Copy link
Owner

ben-manes commented Feb 3, 2017

Adding @jochenberger, @oehme

Two approaches seem to work. Both seem a little hacky.

Disable Restriction

configuration = configuration.copyRecursive().setTransitive(false)
configuration.setCanBeResolved(true)

This calls a method only available since Gradle 3.3, so we'd have to check if the method is present first. I could also imagine this setting is disallowed later as perhaps not intended to be modified.

Query with Detatched Configuration

configuration = project.configurations.detachedConfiguration().extendsFrom(configuration)

and replace all usages of configuration.dependencies with configuration.allDependencies.

My concern here is that I have had poor experiences with copying and detached in the past. Now using a super configuration brings in another scenario that likely has bugs. In the past this has been unfixed bugs in repository handling and lately with dependency-management-plugin/ interaction with resolution strategies (caused by bugs in configuration copying). But I think this is more inline with @oehme's intent as he refactors Configuration into multiple distinct concepts.


Using the example shows both approaches work.

------------------------------------------------------------
: Project Dependency Updates (report to plain text file)
------------------------------------------------------------

The following dependencies exceed the version found at the milestone revision level:
 - com.github.ben-manes:gradle-versions-plugin [0.14.0-SNAPSHOT <- 0.13.0]

The following dependencies have later milestone versions:
 - com.google.guava:guava [19.0 -> 21.0]
 - javax.servlet:javax.servlet-api [3.0.1 -> 4.0.0-b01]
 - junit:junit [4.10 -> 4.12]
 - org.slf4j:log4j-over-slf4j [1.7.0 -> 1.7.22]
 - ch.qos.logback:logback-classic [1.1.0 -> 1.1.9]
 - org.projectlombok:lombok [1.16.0 -> 1.16.12]
 - org.slf4j:slf4j-api [1.7.0 -> 1.7.22]

@oehme
Copy link

oehme commented Feb 3, 2017

I'd go with the detached configuration approach, as it is closer to where we want to go in the long run (having a "Query" concept). If there are bugs handling detached Configurations, please let us know.

replace all usages of configuration.dependencies with configuration.allDependencies

I did a quick search and couldn't find usages of configuration.dependencies in this plugin. Did I miss something? As far as I can see you are using the resolved configuration (which makes sense, since otherwise you wouldn't get the actual selected versions).

@oehme
Copy link

oehme commented Feb 3, 2017

Nevermind, GitHub search is broken, I found the places you were referring to.

@oehme
Copy link

oehme commented Feb 3, 2017

You won't need to change to allDependencies. You can always call Configuration.getDependencies, even on ones that are not intended to be resolved. This method will give you the declared dependencies and you'll want to keep calling that on the original one supplied by the user.

@jochenberger
Copy link
Collaborator

jochenberger commented Feb 7, 2017

There's not much I can contribute here in terms of what the best approach might be. But what I take away from this is that we should rewrite the tests to use TestKit to make it easier to test against different Gradle versions.
I'm on it.

@jochenberger
Copy link
Collaborator

See #129

@ben-manes
Copy link
Owner

Thanks @oehme and @jochenberger.

We'll use the detatched configuration method. I'll try to find the time to wrap that up.

@ben-manes
Copy link
Owner

I have a draft that passes the examples (this one, and in examples folder). But it fails on two unit tests, so some debugging is needed. It fails with a custom component selection rule and @jochenberger's TestKit tests. I suspect the detatched configuration isn't honoring the super configuration's resolution strategies which is causing the problems.

@jochenberger
Copy link
Collaborator

You could just create a PR. Travis should build them and we can inspect the build failure and try to fix it.

@ben-manes
Copy link
Owner

Travis builds all branches, but I can add one if it's helpful to iterate on.

@oehme
Copy link

oehme commented Feb 8, 2017

Resolution strategies only work on the configuration they are declared on indeed. If you need them then a copy is the only way.

@ben-manes
Copy link
Owner

Yes, we try to honor the build's configuration. So we'll have to use the setCanBeResolved(true) approach.

@jochenberger jochenberger added this to the 0.14 milestone Feb 8, 2017
@jochenberger
Copy link
Collaborator

See #137 for a new PR.

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

No branches or pull requests

4 participants