Re: [PR] SLING-12313 bump oak.version to 1.62.0 for compatibility [sling-org-apache-sling-jcr-oak-server]
reschke commented on PR #10: URL: https://github.com/apache/sling-org-apache-sling-jcr-oak-server/pull/10#issuecomment-2112861725 FTR: 1. Yes, we want to completely remove even the shaded Guava variant, but it'll take time. 2. *Major* version updates of Guava happened ~ once every year since 2020, so the situation might not be that bad, -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: dev-unsubscr...@sling.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] SLING-12313 bump oak.version to 1.62.0 for compatibility [sling-org-apache-sling-jcr-oak-server]
rombert commented on PR #10: URL: https://github.com/apache/sling-org-apache-sling-jcr-oak-server/pull/10#issuecomment-2112842362 Echoing a direct conversation I had with @reschke - it's hard for me to formulate an ask to the Oak project since we can pull in a lot of components (from oak-core?) which may be using Guava without exposing it as an API. One thing we could improve for this bundle, if we chose to go the way of removing Guava usages, is to fail the build if such imports are found. I haven't found a good way of doing this though. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: dev-unsubscr...@sling.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] SLING-12313 bump oak.version to 1.62.0 for compatibility [sling-org-apache-sling-jcr-oak-server]
reschke commented on PR #10: URL: https://github.com/apache/sling-org-apache-sling-jcr-oak-server/pull/10#issuecomment-2112496633 It's not clear to me yet what makes those Oak objects special. Or are they really special? At the end of the day, each repostiory instance will end up with some sort of (shaded) Guava requirement, until we are done with the removal. So it's not even clear whether fixing particular ones will get us anywhere. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: dev-unsubscr...@sling.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] SLING-12313 bump oak.version to 1.62.0 for compatibility [sling-org-apache-sling-jcr-oak-server]
reschke commented on PR #10: URL: https://github.com/apache/sling-org-apache-sling-jcr-oak-server/pull/10#issuecomment-2109422262 @rombert - is there something we can do systematically in Oak (other than removing Guava completely) at this point? Also, can you file tickets for the classes where you noticed issues? -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: dev-unsubscr...@sling.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] SLING-12313 bump oak.version to 1.62.0 for compatibility [sling-org-apache-sling-jcr-oak-server]
reschke commented on PR #10: URL: https://github.com/apache/sling-org-apache-sling-jcr-oak-server/pull/10#issuecomment-2107970625 Yes, indeed. So it seems we we missed the case implementation objects leak the dependency. For now we should open tickets for each that we encounter, we might be able to address those in Oak 1.64.0. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: dev-unsubscr...@sling.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] SLING-12313 bump oak.version to 1.62.0 for compatibility [sling-org-apache-sling-jcr-oak-server]
rombert commented on PR #10: URL: https://github.com/apache/sling-org-apache-sling-jcr-oak-server/pull/10#issuecomment-2107641629 > Yes, that's why we have removed Oak from the public Oak API. I think you meant 'removed Guava', right? At any rate, for closely integrated bundles like this one it looks that we need more that public APIs. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: dev-unsubscr...@sling.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] SLING-12313 bump oak.version to 1.62.0 for compatibility [sling-org-apache-sling-jcr-oak-server]
reschke commented on PR #10: URL: https://github.com/apache/sling-org-apache-sling-jcr-oak-server/pull/10#issuecomment-2107614606 Yes, that's why we have removed Oak from the public Oak API. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: dev-unsubscr...@sling.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] SLING-12313 bump oak.version to 1.62.0 for compatibility [sling-org-apache-sling-jcr-oak-server]
rombert commented on PR #10: URL: https://github.com/apache/sling-org-apache-sling-jcr-oak-server/pull/10#issuecomment-2107549956 > Oak's shaded Guava lib should be treated just as any other Oak component, such as [oak-core](https://issues.apache.org/jira/browse/OAK-core). I'm not sure what's different? Because of the simplistic way the OSGi package versions are managed. Almost each shaded Guava update is a major version bump to consumers which causes headaches when trying to support multiple Oak versions. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: dev-unsubscr...@sling.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] SLING-12313 bump oak.version to 1.62.0 for compatibility [sling-org-apache-sling-jcr-oak-server]
reschke commented on PR #10: URL: https://github.com/apache/sling-org-apache-sling-jcr-oak-server/pull/10#issuecomment-2107533908 Oak's shaded Guava lib should be treated just as any other Oak component, such as oak-core. I'm not sure what's different? -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: dev-unsubscr...@sling.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] SLING-12313 bump oak.version to 1.62.0 for compatibility [sling-org-apache-sling-jcr-oak-server]
rombert commented on PR #10: URL: https://github.com/apache/sling-org-apache-sling-jcr-oak-server/pull/10#issuecomment-2107526113 @reschke - the problem is that there seems to be no way to have a SlingRepository implementation without pulling in Guava transitively. If you do see a way of doing that without linking to various core classes, please let me know. And the inlining is only about the shaded Guava classes, not other internals. What concerns would you have with that? -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: dev-unsubscr...@sling.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] SLING-12313 bump oak.version to 1.62.0 for compatibility [sling-org-apache-sling-jcr-oak-server]
reschke commented on PR #10: URL: https://github.com/apache/sling-org-apache-sling-jcr-oak-server/pull/10#issuecomment-2107521572 And no, by all means do not inline internal Oak classes! -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: dev-unsubscr...@sling.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] SLING-12313 bump oak.version to 1.62.0 for compatibility [sling-org-apache-sling-jcr-oak-server]
reschke commented on PR #10: URL: https://github.com/apache/sling-org-apache-sling-jcr-oak-server/pull/10#issuecomment-2107516285 The goal definitively way for Oak not to leak any Guava-ish stuff in the public APIs. AFAICT, this has been the case for approximately one year now. That said, there are indeed runtime dependencies; and removing them is only a mid-term goal. So... I'm not really sure what problem we're discussing here. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: dev-unsubscr...@sling.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] SLING-12313 bump oak.version to 1.62.0 for compatibility [sling-org-apache-sling-jcr-oak-server]
rombert commented on PR #10: URL: https://github.com/apache/sling-org-apache-sling-jcr-oak-server/pull/10#issuecomment-2107499868 @enapps-enorman - good catch with the `AuthInfoImpl`. I hand-rolled a different implementation (which does not seem to work, not sure why). But we still have the guava classess pulled in, e.g. via `org.apache.jackrabbit.oak.plugins.commit.JcrConflictHandler`. I think it is going to be very hard to make sure we don't transitively pull in Guava classes from Oak, unfortunately. Perhaps @reschke can comment on that. Regarding the actual fix, we can either - go with this fix and keep making new releases for new Oak versions - go with a wider bnd range ( and probably add extra tests ) ; we would still need to keep making releases for new Oak versions but users of older versions of Oak can upgrade to newer versions of the bundle - inline the required guava classes in this bundle so we can get wide compatibility; a good start for this is below ``` Private-Package: org.apache.jackrabbit.guava.common.base, \ org.apache.jackrabbit.guava.common.collect, \ org.apache.jackrabbit.guava.common.math, \ org.apache.jackrabbit.guava.common.base.internal, \ org.apache.jackrabbit.guava.common.primitives ``` I'll leave this with you. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: dev-unsubscr...@sling.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] SLING-12313 bump oak.version to 1.62.0 for compatibility [sling-org-apache-sling-jcr-oak-server]
enapps-enorman commented on PR #10: URL: https://github.com/apache/sling-org-apache-sling-jcr-oak-server/pull/10#issuecomment-2099118388 > Thanks for looking into this. I wonder though whether it makes more sense to extend the version range that we accept for the `org.apache.jackrabbit.guava.common.base`. and stay compatible with more Oak versions. > I don't have any way of knowing if that would be safe or not. It is probably ok, but it would just be a guess with each new release. A better solution would be for OAK to stop using guava in the code path that causes that import to exist. Why does anyone still use guava at this point? As far as I can tell it is the reference to "[org.apache.jackrabbit.oak.spi.security.authentication.AuthInfoImpl](https://github.com/apache/jackrabbit-oak/blob/trunk/oak-security-spi/src/main/java/org/apache/jackrabbit/oak/spi/security/authentication/AuthInfoImpl.java)" in the OakSlingRepository class that drags guava into the required dependencies. And the usage of guava there seems trivial and could be replaced with standard java apis. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: dev-unsubscr...@sling.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] SLING-12313 bump oak.version to 1.62.0 for compatibility [sling-org-apache-sling-jcr-oak-server]
sonarcloud[bot] commented on PR #10: URL: https://github.com/apache/sling-org-apache-sling-jcr-oak-server/pull/10#issuecomment-2094365049 ## [![Quality Gate Passed](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/QualityGateBadge/qg-passed-20px.png 'Quality Gate Passed')](https://sonarcloud.io/dashboard?id=apache_sling-org-apache-sling-jcr-oak-server&pullRequest=10) **Quality Gate passed** Issues ![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/passed-16px.png '') [0 New issues](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-jcr-oak-server&pullRequest=10&resolved=false&inNewCodePeriod=true) ![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/accepted-16px.png '') [0 Accepted issues](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-jcr-oak-server&pullRequest=10&metric=new_accepted_issues&view=list) Measures ![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/passed-16px.png '') [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_sling-org-apache-sling-jcr-oak-server&pullRequest=10&resolved=false&inNewCodePeriod=true) ![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/no-data-16px.png '') No data about Coverage ![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/passed-16px.png '') [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-jcr-oak-server&pullRequest=10&metric=new_duplicated_lines_density&view=list) [See analysis details on SonarCloud](https://sonarcloud.io/dashboard?id=apache_sling-org-apache-sling-jcr-oak-server&pullRequest=10) -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: dev-unsubscr...@sling.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[PR] SLING-12313 bump oak.version to 1.62.0 for compatibility [sling-org-apache-sling-jcr-oak-server]
enapps-enorman opened a new pull request, #10: URL: https://github.com/apache/sling-org-apache-sling-jcr-oak-server/pull/10 Oak 1.62.0 has bumped the version of the shaded guava packages to a new major version (from 32 to 33) which results in a failure to resolve the o.a.sling.jcr.oak.server bundle with that version of the Oak bundles. These errors are reported: ```org.osgi.framework.BundleException: Unable to resolve org.apache.sling.jcr.oak.server [144](R 144.0): missing requirement [org.apache.sling.jcr.oak.server [144](R 144.0)] osgi.wiring.package; (&(osgi.wiring.package=org.apache.jackrabbit.guava.common.base)(version>=32.1.0)(!(version>=33.0.0))) Unresolved requirements: [[org.apache.sling.jcr.oak.server [144](R 144.0)] osgi.wiring.package; (&(osgi.wiring.package=org.apache.jackrabbit.guava.common.base)(version>=32.1.0)(!(version>=33.0.0)))]``` -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: dev-unsubscr...@sling.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org