SOLR-17161: build: remove needless permitTestUnusedDeclared #4048

dsmiley · 2026-01-14T06:50:08Z

And remove:

testImplementation libs.jakarta.servlet.api
testImplementation libs.eclipse.jetty.http2.client

https://issues.apache.org/jira/browse/SOLR-17161

Background: while we collaborated on cleaning up the permitTestUnusedDeclared mess in solrj-jetty/build.gradle, we neglected to do similarly in solrj/build.gradle.

Process: (loop until done; took me 2 iterations; went super fast)

remove the junk.
write locks
check
respond to results.

And remove: * testImplementation libs.jakarta.servlet.api * testImplementation libs.eclipse.jetty.http2.client

risdenk

whoops definitely missed this thanks!

Remove additional needless "permit": solr-core, modules/cuvs, modules/extraction, modules/sql, solrj, solrj-streaming, test-framework

* clarify opentelemetry

dsmiley

As this has increased in scope to be wholistic house-cleaning, maybe I should unlink to SOLR-17161. Albeit that JIRA roughly doubled the use of permitBlahBlah, which is the thing that should be avoided.

dsmiley · 2026-01-14T16:36:18Z

solr/modules/cuvs/build.gradle

  testImplementation libs.junit.junit
  testImplementation libs.commonsio.commonsio
-
-  // lucene-backward-codecs is a transitive dependency from cuvs-lucene but required in lockfile


This explanation doesn't make sense IMO; instead it tells me that the author of the comment doesn't know to write locks first. Maybe our build is complicated for many folks :-(

dsmiley · 2026-01-14T16:39:49Z

solr/modules/sql/build.gradle


 dependencies {
  implementation platform(project(':platform'))
-  permitUnusedDeclared platform(project(":platform"))


Very weird -- using permitBlahBlah on a platform. A platform based declaration isn't really a dependency, it just imports constraints on dependencies (not actually depending on those things).

dsmiley · 2026-01-14T17:56:14Z

dev-docs/gradle-help/dependencies.txt

@risdenk please review this; I'd love to have your input

This looks great. I went down this rabbit hole for a bit too the other day. I didn't make progress. I went through and ripped out all the "permit" and ended up with test failures that I didn't have time to debug. I agree with what you are trying to do. We should not have permit* anywhere for the most part. There may be a few cases where its needed but it should be very rare.

dsmiley · 2026-01-14T17:56:45Z

solr/core/build.gradle


 dependencies {
  api platform(project(":platform"))
-  permitUnusedDeclared platform(project(":platform"))


this was a bizarre one since it's a "platform"

dsmiley · 2026-01-14T17:57:17Z

solr/core/build.gradle

  // It is not included in the release so exclude from checks.
  compileOnly libs.spotbugs.annotations
  testCompileOnly libs.spotbugs.annotations
-  permitUnusedDeclared libs.spotbugs.annotations


because recent plugin update considers compileOnly as such (i.e. automatically)

dsmiley · 2026-01-14T17:59:25Z

solr/core/build.gradle

-  implementation libs.apache.lucene.backward.codecs
-  permitUnusedDeclared libs.apache.lucene.backward.codecs


nonetheless, it's still on the runtime classpath (lockfile is truth on such matters).

dsmiley · 2026-01-14T18:01:45Z

solr/modules/extraction/build.gradle

-  permitUsedUndeclared libs.eclipse.jetty.http
-  permitUsedUndeclared libs.eclipse.jetty.util
-  permitUsedUndeclared libs.eclipse.jetty.io
+  implementation libs.eclipse.jetty.http
+  implementation libs.eclipse.jetty.util
+  implementation libs.eclipse.jetty.io


@janhoy were you trying to avoid simply using implementation for some reason?

Strange. I must have added libs.eclipse.jetty.client first (which brings in the other three transitively), and then reacted to the error message by permitting them instead of adding explicit. I don't recall exactly what happened in october, but I may have misinterpreted the error message from the plugin during implementation, and then it was never revisited before merge.

dsmiley · 2026-01-14T18:02:39Z

solr/solrj-streaming/build.gradle

-  })
-  permitTestUnusedDeclared libs.apache.zookeeper.zookeeper
-
  permitTestUsedUndeclared project(':solr:solrj-streaming') // duh!


so weird...

I dont think this is needed.

Try for yourself ;-) It's the weirdest inexplicable thing that I've seen this plugin require me to do. There's one more of these -- search for "// duh!". Ideally this gets reported to the plugin author but I'm skeptical anything becomes of it without us doing the legwork

Add it as an testImplementation. similar to the other testImplementation around line 30. I think it was just the wrong choice to do it.

Sure I could switch to testImplementation, and maybe I should but the reason the line is there at all is because of this plugin's flaw. And the "permit" use is basically always a flaw, so the line as-is shows that this plugin is flawed.

There is no flaw here. For the tests, the tests require classes from solr:solrj-streaming to compile. You could conceivably write tests without requiring the classes from solr:solrj-streaming.

If you put testImplementation as intended in Gradle, it would show that this module for the tests requires solr:solrj-streaming classes to compile. Thats not the plugin being wrong.

The wrong thing is that we don't have a testImplementation line that references solr:solrj-streaming

I see that other modules (e.g. SolrJ) have testImplementation with an explicit dependency on the module itself, which you had added back when you added the cutterslade plugin. So we have cutterslade to thank for this verbosity. Yes of course a module's tests depend on the module! This is how Gradle or any build system would be expected to work. This is a bad side of the plugin -- a flaw; requiring needless verbosity to state the obvious.

I'll switch it to for consistency with our other files (thanks to cutterslade), and to remove a sad "permit", and report the bug.

dsmiley · 2026-01-14T18:03:18Z

solr/solrj-streaming/gradle.lockfile

-org.apache.yetus:audience-annotations:0.12.0=permitTestUnusedDeclared
-org.apache.zookeeper:zookeeper-jute:3.9.4=jarValidation,permitTestUnusedDeclared,testCompileClasspath,testRuntimeClasspath
-org.apache.zookeeper:zookeeper:3.9.4=jarValidation,permitTestUnusedDeclared,testCompileClasspath,testRuntimeClasspath
+org.apache.zookeeper:zookeeper-jute:3.9.4=jarValidation,testCompileClasspath,testRuntimeClasspath


see; it's still on the test runtime classpath, so may be consulted in tests at runtime

dsmiley · 2026-01-15T01:17:41Z

dev-docs/gradle-help/dependencies.txt

+
+We use the "cutterslade.analyze" build plugin to help maintain good dependency hygiene.
+It tries to identify when a dependency is needlessly declared, and then fail the build to complain.
+Unfortunately, it also requires us to add declarations that Gradle would otherwise deem unnecessary,


@risdenk I saw a notification about a comment that I can't find in GitHub. You said:

I don't think this statement is true?

The plugin is requiring that dependencies used in a module are actually declared. The api spec has nothing to do with it. It makes each module self sufficient. It means that even if an api dependency removes a dependency (say commons-io) that the module doesn't break if it uses commons-io. it just makes the usage of dependency explicit. This is only for compile time dependencies where its a compile time dependency to actually build the module in question.

How does an "api dependency" remove a dependency?

RE "api": An illustrative example would be :solrj-jetty having api libs.eclipse.jetty.http2.client (Jetty's HttpClient).
The use of "api" is a convenience to a user of a solrj-jetty to spare them from adding an additional implementation scoped dependencies that they are likely to use in conjunction with the lib. But thanks to cutterslade, within Solr, that distinction is defeated. Do you agree and/or am I missing something?

There's a spectrum of developer prerogative from conservative to liberal, in terms of use of "api". I say "likely to use in conjunction with" but another person's definition might simply be the mere existence of a public signature that includes the target lib. I've increasingly preferred to use it conservatively. At work it's used liberally in a mono-repo and the extensive dependency web that doing so creates is out of control.

SOLR-17161: build: remove needless permitTestUnusedDeclared

aaf9e20

And remove: * testImplementation libs.jakarta.servlet.api * testImplementation libs.eclipse.jetty.http2.client

dsmiley requested review from janhoy and risdenk January 14, 2026 06:50

github-actions bot added dependencies Dependency upgrades client:solrj tool:build labels Jan 14, 2026

dsmiley added the no-changelog label Jan 14, 2026

risdenk approved these changes Jan 14, 2026

View reviewed changes

dependencies.txt docs on the analyze plugin.

20b43da

Remove additional needless "permit": solr-core, modules/cuvs, modules/extraction, modules/sql, solrj, solrj-streaming, test-framework

github-actions bot added documentation Improvements or additions to documentation test-framework module:extraction module:sql admin-ui labels Jan 14, 2026

test-framework

41f971c

* clarify opentelemetry

dsmiley commented Jan 14, 2026

View reviewed changes

dsmiley commented Jan 15, 2026

View reviewed changes

		implementation libs.apache.lucene.backward.codecs
		permitUnusedDeclared libs.apache.lucene.backward.codecs

SOLR-17161: build: remove needless permitTestUnusedDeclared #4048

Are you sure you want to change the base?

SOLR-17161: build: remove needless permitTestUnusedDeclared #4048

Uh oh!

Conversation

dsmiley commented Jan 14, 2026

Uh oh!

risdenk left a comment

Choose a reason for hiding this comment

Uh oh!

dsmiley left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants