Report Misses Dependencies because of Unchecked Configurations

[mroeppis]

I'm missing dependencies when checking the report. When debugging the plugin using an included build I discovered that the configurations holding the missing dependencies are being skipped because they are not classified "useful":

gradle-versions-plugin/src/main/groovy/com/github/benmanes/gradle/versions/updates/DependencyUpdates.groovy

Lines 70 to 78 in 6f7b95d

	configurations.collect { Configuration config ->
	def isUsefulConfiguration = !config.canBeResolved || config.canBeConsumed ||
	config.name == 'annotationProcessor' || config.name == 'kapt'
	if (isUsefulConfiguration) {
	resolve(resolver, proj, config)
	} else {
	[]
	}

When running the above code with resolve(...) for all configs more dependencies are reported.

Since the reason behind the isUsefulConfiguration flag is not self-explanatory it would be nice to know why it's necessary. However, there should be another way to manage this.

I also found out that Failed to resolve ... error messages may occur because some constraints inside selected configurations can't be copied. It happens when trying to copy the configuration itself:

gradle-versions-plugin/src/main/groovy/com/github/benmanes/gradle/versions/updates/Resolver.groovy

Line 273 in 6f7b95d

Configuration copy = configuration.copyRecursive().setTransitive(transitive)

Since this is a Gradle issue I raised one on their end.

---

I use the latest plugin version 0.38.0 running with Gradle 6.7.1

[ben-manes]

I believe the problem that we had is the whole canBeResolved and canBeConsumed mess when Gradle changed configurations to not be resolvable. However that isn't properly set and, if I recall correctly, some backdoors allow some to bypass it. As this only applied to later versions Gradle, it was also a compatibility pain.

I am fine if we can refine this or somehow do it correctly. You can see the discussion in #378 and the linked issues. Maybe we were running into that copy bug and fixing that would have solved our problems.

[mroeppis]

What would happen if isUsefulConfiguration was removed? Is this a Kotlin issue? If so, it might be necessary for some people to restrict configurations as is done right now. On the other hand it would only be fair to allow more configurations to be checked if wanted by a user.

Could we define an input closure allowing to configure configurations?

[ben-manes]

By removing we would try to resolve configurations which are not allowed for dependency resolution. Prior versions of Gradle allowed all to be resolved, so we iterated over all of them to generate the report. If you try removing now, you'll observe a lot of errors when Gradle rejects resolution. It might be that we should try to resolve, catch the error, and suppress it. I find the documentation on this concept very confusing and it seems to be ad hoc in usages by Kotlin & Android, and that Configuration.copy sometimes misses necessary metadata so it fails weirdly.

Since the failure is an immediate throwing of an exception, I kind of think the best is just to try and log.info that we skipped it. That way we get the most accuracy and don't try to detect on whatever the metadata claims. That should be fast and maybe least error prone. What do you think?

[mroeppis]

Potential errors shouldn't bother us. That's why we've got tests.
My intent was not to disadvantage somebody. After all you and the other contributors from #378 made a joint decision, I guess. And yes, maybe there are users saying: "Trying to resolve that doesn't make sense at all. So why?".

I have nothing against your suggestion. It's more like the opposite, i.e. I'd actually prefer it. Speaking of my projects I don't see the point in skipping a configuration. I also like your idea because it would take only a minimal change to make it happen. We could start by reverting the change to sources from 1c6627e.

Are you fine with changing the log level right here?

gradle-versions-plugin/src/main/groovy/com/github/benmanes/gradle/versions/updates/DependencyUpdates.groovy

Lines 83 to 91 in 9bef5cb

	private Set<DependencyStatus> resolve(Resolver resolver, Project proj, Configuration config) {
	try {
	return resolver.resolve(config, revision)
	} catch (Exception e) {
	String msg = "Failed to resolve ${proj.path}:${config.name}"
	project.logger.error(msg, project.logger.isInfoEnabled() ? e : null)
	return Collections.emptySet()
	}
	}

Maybe we should also change the wording to what you wrote. Like Skipping configuration x:y because ...

[ben-manes]

Yep, I'm fine with the log level being changed and using your new message format.

[mroeppis]

Closing since agreed changes have already been merged (see #522).