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

Bad resolution for BOM-style dependencies #97

Closed
the4yeast opened this issue Mar 21, 2016 · 26 comments
Closed

Bad resolution for BOM-style dependencies #97

the4yeast opened this issue Mar 21, 2016 · 26 comments

Comments

@the4yeast
Copy link

It looks like there's an issue with dependencies declared as mavenBom, e.g.

dependencyManagement {
    imports {
        mavenBom("com.amazonaws:aws-java-sdk-bom:$awsSdkVersion")
    }
}

In my case, the version is 1.10.56, whereas 1.10.62 is the most recent one at time of writing. I'm almost sure that the plugin stopped reporting newer versions of this dependency when I switched to the BOM style.

Also, not explicitly a BOM, but it works in a similar manner - Spring Boot dependency.
The plugin is imported and appliesjust like any other plugin:

buildscript {
    dependencies {
        classpath("org.springframework.boot:spring-boot-gradle-plugin:$springBootVersion")
    }
}

apply plugin: 'spring-boot'

with my current version being 1.3.3.RELEASE. Dependencies are added without versions, e.g. compile('org.springframework.boot:spring-boot-starter-web').

The plugin (when executed with -Drevision=release) now reports 1.4.0.M1 as the latest release version of the plugin, but correctly indicates that 1.3.3.RELEASE is the latest release version of each dependency.

As much as the Spring Boot issue is not a big problem - the dependencies themselves are reported correctly - the AWS SDK issue is causing some inconvenience.

@ben-manes
Copy link
Owner

You can inspect what Gradle and the task is doing using the --info flag. The Spring Boot issue might be related to #73 where Gradle's comparator only knows the Maven proper version tags, whereas Spring uses a custom convention. We can't do anything but keep bugging Gradle to improve their comparator.

The mavenBom is probably resolving the full dependency late in the life cycle, e.g. just before compilation, instead of being consumable at configuration resolution time. That might work in general but break anyone else inspecting the dependencies. I'd first look at the info log to understand what the task is consuming and, assuming its the mavenBom issue, open a ticket on their side to play nice.

An example project with the bugs that I could play with would be helpful.

@ben-manes
Copy link
Owner

It looks like the mavenBom plugin using configuration.incoming.beforeResolve hook to lazily set the version. Perhaps the incoming dependencies are not picked up by this plugin and are not listed by the configuration's regular dependency set. Alternatively it may be that the configuration.copy doesn't clone that information, so the dependencies are skipped as being not fully defined. Experimenting would tell us what is going on.

@the4yeast
Copy link
Author

Thanks @ben-manes.

In case of Spring, the logs don't tell me much. Clearly, the fact that I'm using their snapshot and milestone repositories, together with Spring's custom convention you mentioned, causes the plugin to report 1.4.0.M1 as the latest release version.

However, I want to keep using those repositories, as I like to play with new features they release and occasionally change my dependency version to a SNAPSHOT one.

In terms of the AWS dependency, logs tell me quite nothing - all I see is the plugin finding a locally available resource with matching checksum. Does the plugin not check with remote repositories every time and cache those checks? Can we force it to hit the repositories every time?

Sample with additional Spring repos is below. I've added a Jackson dependency to have a "regular" one in there.

buildscript {
    repositories {
        jcenter()
        maven { url 'http://repo.spring.io/snapshot' }
        maven { url 'http://repo.spring.io/milestone' }
    }
    dependencies {
        classpath('org.springframework.boot:spring-boot-gradle-plugin:1.3.3.RELEASE')
        classpath('com.github.ben-manes:gradle-versions-plugin:0.12.0')
        classpath('io.spring.gradle:dependency-management-plugin:0.5.6.RELEASE')
    }
}

apply plugin: 'java'
apply plugin: 'spring-boot'
apply plugin: 'com.github.ben-manes.versions'
apply plugin: 'io.spring.dependency-management'

repositories {
    jcenter()
    maven { url 'http://repo.spring.io/snapshot' }
    maven { url 'http://repo.spring.io/milestone' }
}

dependencyManagement {
    imports {
        mavenBom('com.amazonaws:aws-java-sdk-bom:1.10.56')
    }
}

dependencies {
    compile('org.springframework.boot:spring-boot-starter-web')
    compile('com.amazonaws:aws-java-sdk-dynamodb')
    compile('com.fasterxml.jackson.datatype:jackson-datatype-jdk8:2.6.3')
}

task wrapper(type: Wrapper) {
    gradleVersion = '2.11'
}

@ben-manes
Copy link
Owner

You can force Gradle to skip its cache by using --refresh-dependencies. Then you should see it resolve all the remote information again.

@the4yeast
Copy link
Author

I just fixed the sample - for whatever reason, I omitted the io.spring.dependency-management plugin.

Great, will use that and look at the logs.

@the4yeast
Copy link
Author

Logs say:

Cached resource Fri Feb 26 12:12:37 AEDT 2016 is up-to-date (lastModified: https://jcenter.bintray.com/com/amazonaws/aws-java-sdk-bom/1.10.56/aws-java-sdk-bom-1.10.56.pom).
Cached resource Fri Feb 26 08:50:45 AEDT 2016 is up-to-date (lastModified: https://jcenter.bintray.com/com/amazonaws/aws-java-sdk-pom/1.10.56/aws-java-sdk-pom-1.10.56.pom).
Cached resource Fri Feb 26 08:50:45 AEDT 2016 is up-to-date (lastModified: https://jcenter.bintray.com/com/amazonaws/aws-java-sdk-pom/1.10.56/aws-java-sdk-pom-1.10.56.pom).

Cached resource Fri Feb 26 09:09:01 AEDT 2016 is up-to-date (lastModified: https://jcenter.bintray.com/com/amazonaws/aws-java-sdk-dynamodb/1.10.56/aws-java-sdk-dynamodb-1.10.56.pom).

Do you know what the lastModified flag means? Is that a repository/dependency-level flag for marking dependencies? Is it used to sort versions of a given dependency?

@ben-manes
Copy link
Owner

Probably its from the HTTP header. The dependencyUpdates task should be printing out its own log information of what its getting, so that's what I would be looking at to see why it is obtaining the wrong versions.

@the4yeast
Copy link
Author

There's actually not much more in the logs...
Does the plugin have a log level switch?

@ben-manes
Copy link
Owner

I tried running your example. It looks like the mavenBom is doing a lot of magic and confusing anyone else inspecting the dependencies. For example it rewrites jackson-datatype-jdk8 as,

Using version '2.6.5' for dependency 'com.fasterxml.jackson.datatype:jackson-datatype-jdk8:2.6.3'

So the dependencyUpdates task reports the local one as 2.6.5 even though perhaps you expected 2.6.3.

This rewriting may be confusing the AWS listing as I see,

Using version '1.10.56' for dependency 'com.amazonaws:aws-java-sdk-dynamodb:'
Using version '1.10.56' for dependency 'com.amazonaws:aws-java-sdk-dynamodb:none'

and my plugin is printing out,

Comparing dependency (current: com.amazonaws:aws-java-sdk-dynamodb:1.10.56, latest: 1.10.56)

indicating that the maximum resolution for a + search was forced to 1.10.56 by the mavenPom plugin. So my search was constrained by that plugin and its told the latest is an older version.

All the dependencies declared as mavenBom are not visible to this plugin because they are not part of the configuration's dependency set. They are stashed somewhere else (incoming) that I don't inspect. Perhaps I should, but I'm not sure what its used for.

So the big issue seems to be that the mavenPom plugin restricts anyone else trying to query artifacts by forcing them to the BOM's version. I haven't used BOMs before, so perhaps that is their intended purpose. If it is, then its intentionally incompatible.

@the4yeast
Copy link
Author

Thanks for the walkthrough, I have a bigger picture now.
I've run htmlDependencyReport for an overview or declared and resolved dependencies - now I feel I know what's going on.

@ben-manes
Copy link
Owner

From their README,

This configuration will cause all dependencies (direct or transitive) on spring-core and commons-logging to have the versions 4.0.3.RELEASE and 1.1.2 respectively...

That indicates the incompatibility is the desired capability of this plugin.

An option might be to scope the BOM to the desired configurations. The restrictions may (or may not) apply due to using configuration.copy() which is given a unique name.

@ben-manes
Copy link
Owner

All experiments I've tried can't bypass this plugin, as it applies itself to all project configurations. The buildscript output is expected since it doesn't restrict those configurations.

@wilkinsona please advise if there is any way to make our plugins compatible.

@wilkinsona
Copy link

So the big issue seems to be that the mavenPom plugin restricts anyone else trying to query artifacts by forcing them to the BOM's version. I haven't used BOMs before, so perhaps that is their intended purpose.

The intention of a bom is to force the versions of a project's dependencies to match those declared in the bom.

mavenBom plugin using configuration.incoming.beforeResolve hook to lazily set the version

That's not strictly accurate. The dependency management plugin uses beforeResolve to gather information about dependency versions, but versions are not set until the plugin's resolution strategy is called.

I wonder if there could be an ordering issue here? @ben-manes when does your plugin perform its inspection of the versions?

@wilkinsona
Copy link

I had a bit more time so I've looked at this plugin's code a little, as well as re-reading the above. As I result, I think I may now have a better understanding of the problem.

Am I correct in thinking that the problem is that the Configuration that's created by Resolver to determine the latest versions, still has the dependency management plugin's resolution strategy applied to it? That would mean that the attempt to resolve the query dependency (with + or latest.${revision}) actually turns into an attempt to resolve whatever version's specified in the dependency management.

The dependency management plugin actually had the same problem at one point due to resolution strategies being copied over when a copy is made of a Configuration. I avoided the problem by creating a detached configuration with the desired dependencies instead. A similar approach may work here too.

Another option would be to make a change to the dependency management plugin and have it ignore dependencies with + and latest.whatever as their version. That's perhaps not an unreasonable change to make anyway as it would be strange to be using dependency management on the one hand while asking Gradle for the latest version on the other.

@ben-manes
Copy link
Owner

The original version of this plugin used a detached configuration and latest.whatever only. It aggregated all the dependencies and repositories across the configurations. That was a tiny script that I made into a plugin and suited my needs (2012).

Unfortunately over time that approach had numerous issues, some due to the plugin and others by Gradle bugs. For example, when resolution strategies were added to Gradle some users expected them to be honored by the dependency report. Another is that copying over repositories turned out to be error prone because RepositoryHandler is extremely buggy (incorrectly drops additions as duplicates).

The only safe, reliable, and forward looking approach was to copy the configuration. Then I clear out and rewrite the associated dependencies because that works. As some users want a to filter the dependency report (e.g. ignore betas) but not use that constraint globally, I added the ability to attach a custom resolution strategy.

I'm hesitant to go back to a detached configuration as I think it might re-introduce bugs or cause more problems. I think you're right that it would avoid the conflict, but arguably that is an intended behavior if we are to honor their configurations.

I'd rather see if your idea of ignoring + or latest.whatever pans out. If not, then a hack would be ask users to disable your plugin somehow in a doFirst block of the report. Otherwise I'd want to ask the Gradle team for their advise on how best to proceed.

@wilkinsona
Copy link

I've started working on spring-gradle-plugins/dependency-management-plugin#77. The idea of ignoring dynamic versions (+, latest.integration, [1.0,2.0], etc) looks like it'll work. However, I've hit a snag.

As shown above, the dependency management plugin allows a dependency to be declared without a version. The snag is that this plugin replaces the null version with none rather than with +. As far as Gradle is concerned, none isn't a dynamic version so I don't want the dependency management plugin to treat it as such. This means that the dependency management plugin doesn't ignore the dependency and manages its version.

Considering the sample from @the4yeast, the end result is that this plugin continues to report that aws-java-sdk-dynamodb is up-to-date:

The following dependencies are using the latest release version:
 - com.amazonaws:aws-java-sdk-dynamodb:1.10.56

It does, however, report that a later version of jackson-datatype-jdk8 is available:

The following dependencies have later release versions:
 - com.fasterxml.jackson.datatype:jackson-datatype-jdk8 [2.6.5 -> 2.7.4]

As I said above, I don't want to treat none as a dynamic version in the dependency management plugin as there's no guarantee that it is. However, I'd also like to get the two plugins playing nicely together. Is there anything that can be done in this plugin so that + is used rather than none for a dependency that was declared without a version?

@ben-manes
Copy link
Owner

A dependency without a version is commonly a local file dependency. These are common in Android projects. It might be confusing to assume it is therefore the latest? Or perhaps not assuming that the coordinates wouldn't match a remote dependency?

@wilkinsona
Copy link

Perhaps you could ignore local file dependencies entirely by checking to see if each Dependency is an instance of SelfResolvingDependency?

@ben-manes
Copy link
Owner

Users would expect to see them in the report, but we could treat them differently if that helps? I'd be fine making a SelfResolvingDependency use none and otherwise use + for external dependencies.

@wilkinsona
Copy link

Yes, that would help. Thanks. The dependency mangagement plugin could/should be ignoring SelfResolvingDependency instances anyway.

@the4yeast
Copy link
Author

@wilkinsona any idea when spring-gradle-plugins/dependency-management-plugin#77 might be released in a build?

thanks for the enhancement!

@wilkinsona
Copy link

@the4yeast It's available in 0.6.0.BUILD-SNAPSHOT builds now. You can get those from https://repo.spring.io/plugins-snapshot. All being well, I suspect that 0.6.0.RELEASE will be released in the next week or so.

@the4yeast
Copy link
Author

thanks @wilkinsona

@ben-manes
Copy link
Owner

Using @the4yeast example, @wilkinsona's latest release and his requested changes, the update report comes out properly. Note that I had to check dependency.artifacts.empty because file dependencies don't implement SelfResolvingDependency (as not "independent of a repository").

The following dependencies are using the latest milestone version:
 - io.spring.gradle:dependency-management-plugin:0.6.0.RELEASE
 - com.github.ben-manes:gradle-versions-plugin:0.13.0

The following dependencies have later milestone versions:
 - com.amazonaws:aws-java-sdk-dynamodb [1.10.56 -> 1.11.10]
 - com.fasterxml.jackson.datatype:jackson-datatype-jdk8 [2.6.5 -> 2.8.0.rc2]
 - org.springframework.boot:spring-boot-gradle-plugin [1.3.3.RELEASE -> 1.4.0.M3]
 - org.springframework.boot:spring-boot-starter-web [1.3.3.RELEASE -> 1.4.0.M3]

@jochenberger Can you please perform the release (0.13.0)?

@the4yeast
Copy link
Author

Sounds awesome, thanks a lot gentlemen!

@ben-manes
Copy link
Owner

0.13.0 is live, thanks @jochenberger!

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

3 participants