Zoltan Chovan has posted comments on this change. ( http://gerrit.cloudera.org:8080/22768 )
Change subject: [java] KUDU-3657: fix publishing ...................................................................... Patch Set 18: > Patch Set 17: > > > > Patch Set 17: > > > > > > (1 comment) > > > > The main reason why many of the implementation scopes were changed > > is that when declaring them as 'api' these dependencies are not > > included in the generated pom.xml. This behaviour is a bit odd > > though, as from the documentation: > > > > "Dependencies appearing in the api configurations will be > > transitively exposed to consumers of the library, and as such will > > appear on the compile classpath of consumers. Dependencies found in > > the implementation configuration will, on the other hand, not be > > exposed to consumers, and therefore not leak into the consumers' > > compile classpath." > > > > Which seems to contradict the actual behavior of the > > configurations. So just to reiterate: when declaring for example > > "implementation libs.netty", then the netty dependency will be > > included in the pom as runtime dependency. These dependencies were > > not present at all before the gradle version upgrade (where the > > scopes compile and runtime are removed from gradle), so after > > discussing with Abhishek, we opted to switch to api since that > > resolved the pom issue. You can check the document I shared earlier > > about the pom differences, please note that the currently generated > > pom is much different than the "post-update" is in the document, > > that is basically after upgrade but before this patch. > > > > Maybe this is a discrepancy between gradle and maven scoping > > behaviour. What do you think? > > I think that using 'api' dependency as a workaround to the observed behavior > isn't quite a good idea because it's semantically incorrect. > > Apparently, there is something wrong with the dependencies after upgrading to > Gradle 7.x, and one would think that a proper way of resolving that might be > finding the root cause of the issue and addressing it. I guess it requires > much deeper analysis of current dependencies, shading and publishing. > Throwing in workarounds like using wrong type of dependency doesn't smell > good, especially if the observed behavior contradicts the documentation -- > it's just another signal that something is wrong. Simply put, doing two > wrongs doesn't make it right. > > Unfortunately, I don't have a recipe for a fix at this time. I haven't taken > deep enough look at the problem and I'm not an expert in Java/Gradle > dependency management. Thanks for the feedback Alexey! I spent some more time on figuring out the root of the issue. I created a bit of a sandbox repository to streamline the investigation a bit: https://github.com/zchovan/gradle_testing/tree/master feel free to mess around with it. In summary, I messed up the pom editing logic. The weird behaviour was present because I used the api config instead of the implementation config to calculate what should be taken out of the poms. I changed that and also fixed another mistake I made in the gradle upgrade commit, relating the compileUnshaded configuration. That should also work correctly now, and as it was originally intended. I can separate this changelist into multiple if you think that would be easier to review and process. With this latest patchset both the poms and the dependency scope declarations should be okay, but please let me know if you find that I missed something. -- To view, visit http://gerrit.cloudera.org:8080/22768 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ie045805a68bfcd16325e1e4dbb38653cefe3a4b0 Gerrit-Change-Number: 22768 Gerrit-PatchSet: 18 Gerrit-Owner: Zoltan Chovan <[email protected]> Gerrit-Reviewer: Abhishek Chennaka <[email protected]> Gerrit-Reviewer: Alexey Serbin <[email protected]> Gerrit-Reviewer: Attila Bukor <[email protected]> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Marton Greber <[email protected]> Gerrit-Reviewer: Zoltan Chovan <[email protected]> Gerrit-Comment-Date: Thu, 15 May 2025 11:33:55 +0000 Gerrit-HasComments: No
