Allow different source sets from forbiddenApis#54731
Allow different source sets from forbiddenApis#54731jakelandis merged 6 commits intoelastic:masterfrom
Conversation
ForbiddenApis task via the precommit task currently makes an assumption that only the test and main source sets are present for any given project. This commit removes that assumption and allows for any roject source set's compileClasspath class path to be added to the forbiddenApis classpath configuration.
|
Pinging @elastic/es-core-infra (:Core/Infra/Build) |
mark-vieira
left a comment
There was a problem hiding this comment.
Left a few comments, otherwise LGTM.
buildSrc/src/main/groovy/org/elasticsearch/gradle/precommit/PrecommitTasks.groovy
Outdated
Show resolved
Hide resolved
buildSrc/src/main/groovy/org/elasticsearch/gradle/precommit/PrecommitTasks.groovy
Outdated
Show resolved
Hide resolved
buildSrc/src/main/groovy/org/elasticsearch/gradle/precommit/PrecommitTasks.groovy
Outdated
Show resolved
Hide resolved
buildSrc/src/main/groovy/org/elasticsearch/gradle/precommit/PrecommitTasks.groovy
Outdated
Show resolved
Hide resolved
buildSrc/src/main/groovy/org/elasticsearch/gradle/precommit/PrecommitTasks.groovy
Outdated
Show resolved
Hide resolved
|
thanks @mark-vieira addressed comments via 5d1cc03 |
|
@elasticmachine run elasticsearch-ci/2 |
buildSrc/src/main/groovy/org/elasticsearch/gradle/precommit/PrecommitTasks.groovy
Outdated
Show resolved
Hide resolved
|
@elasticmachine run elasticsearch-ci/2 |
|
@elasticmachine update branch |
|
@rjernst - do the last round of changes look good ? |
rjernst
left a comment
There was a problem hiding this comment.
One more suggested tweak, otherwise LGTM
buildSrc/src/main/groovy/org/elasticsearch/gradle/precommit/PrecommitTasks.groovy
Outdated
Show resolved
Hide resolved
|
@elasticmachine update branch |
|
@elasticmachine run elasticsearch-ci/2 |
ForbiddenApis task via the precommit task currently makes an assumption that only the test and main source sets are present for any given project. This commit removes that assumption and allows for any project source set's compileClasspath class path to be added to the forbiddenApis classpath configuration.
ForbiddenApis task via the precommit task currently makes an assumption that only the test and main source sets are present for any given project. This commit removes that assumption and allows for any project source set's compileClasspath class path to be added to the forbiddenApis classpath configuration.
|
I accidentally deleted a file with this merge. I added it back on 1ff5b85 (fortunately I caught the mistake doing the backport, so it was only missing from master for a few minutes). |
ForbiddenApis task via the precommit task currently makes an assumption that only the test and main source sets are present for any given project. This commit removes that assumption and allows for any project source set's compileClasspath class path to be added to the forbiddenApis classpath configuration.
|
only back-porting as far as 7.x since this builds on #54309 which is only that far back. |
That PR isn't a strict requirement here. I'm pretty sure we can backport this. Folks are actively working in |
|
If I understand correctly, we should explicitly set the classpath for the plugin via the backport even though the old code didn't ? I agree with the desire to backport, just want to ensure that is the correct action here since the original PR changes code that is absent in 7.7. and prior. |
No. What Ryan had to do in that PR was change forbidden APIs to use the runtime classpath because of some weirdness in the way it works and the fact that we leak Guava types in our API, but technically don't actually require it until runtime. If we can, we should prefer the smaller classpath footprint, which in this case means going with the forbiddenapi plugin's default of using the compile classpath. |
|
chatted offline this change only makes sense if applied ontop of #54309, so will not back port. |
ForbiddenApis task via the precommit task currently makes an assumption that only the test and main source sets are present for any given project. This commit removes that assumption and allows for any project source set's compileClasspath class path to be added to the forbiddenApis classpath configuration.
ForbiddenApis task via the precommit task currently makes an assumption
that only the test and main source sets are present for any given project.
This commit removes that assumption and allows for any project source set's
compileClasspath class path to be added to the forbiddenApis classpath
configuration.