-
Notifications
You must be signed in to change notification settings - Fork 24.7k
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
Build: Fix repos.mavenLocal casing #29289
Conversation
The sysprop repos.mavenLocal may be used to add the local .m2 maven repository for testing snapshots of locally build dependencies. Unfortunately this has to be checked in two different places (they cannot be shared, due to buildSrc being built essentially as a separate project), and the casing of the string sysprop lookups did not align. This commit fixes BuildPlugin's checking of repos.mavenLocal to use the correct casing (camelCase, to match the gradle dsl element).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM.
Pinging @elastic/es-core-infra |
The sysprop repos.mavenLocal may be used to add the local .m2 maven repository for testing snapshots of locally build dependencies. Unfortunately this has to be checked in two different places (they cannot be shared, due to buildSrc being built essentially as a separate project), and the casing of the string sysprop lookups did not align. This commit fixes BuildPlugin's checking of repos.mavenLocal to use the correct casing (camelCase, to match the gradle dsl element).
Thanks @rjernst, I will check this with a snapshot build of forbiddenapis. It looks like the reason for the problem is the spelling difference. If I had passed both sysprops, it may have worked, right? It would be good to also have a documentation of the repos.mavenLocal sysprop somewhere. |
Hi,
Here is my test setup:
Thanks, this makes testing releases of forbiddenapis before pushing the artifacts to Maven Central much easier (this would have prevented the 2.4 regression last year). |
A bit unrelated: When looking at the tasks in "server" module, I did not find a "forbiddenApisJava9" task, but a "checkStyleJava9" one. I have no idea why it's not setup automatically, because the plugin automatically creates a task for every sourceSet. I think the reason for this is that the build plugin is loaded before the sourceSets are defined, so it does not see the source sets at that time. I am digging into this. A current workaround is to define the Java 9 sourceSet before the |
Thanks for the further investigation. Is there an issue in forbiddenapis we can follow? I don't remember how forbiddenapis finds the source sets now, but normally one would use the |
I will open the issue now. I already solved the problem, was a bug on my side (lack of knowledge)! Instead of using I will open a separate issue in Elasticsearch and also add the forbidden issue there. |
Here is the issue: policeman-tools/forbidden-apis#138 |
Here is the ES issue: #29292 |
The sysprop repos.mavenLocal may be used to add the local .m2 maven
repository for testing snapshots of locally build dependencies.
Unfortunately this has to be checked in two different places (they cannot
be shared, due to buildSrc being built essentially as a separate
project), and the casing of the string sysprop lookups did not align.
This commit fixes BuildPlugin's checking of repos.mavenLocal to use the
correct casing (camelCase, to match the gradle dsl element).