garydgregory commented on PR #1328: URL: https://github.com/apache/commons-lang/pull/1328#issuecomment-2523069892
> > Hello @struberg > > Checking against an interface in `isJavaInternalClass()` is not going to work. > > I updated `EqualsBuilderReflectJreImplementationTest` in git master. > > Gary, this has already long been resolved in [f20639a](https://github.com/apache/commons-lang/commit/f20639a039476826b9c146d96f64e6979ea7d772) and [ec4150b](https://github.com/apache/commons-lang/commit/ec4150bc13cfd2705e5d672f47706930af578c39) > > I've even merged your changes (and removed that new --add-opens again...) from main to this very branch. > > This branch clearly improves the situation with standard jpms setup for a lot of common cases. Of course, this does not yet fix ALL the problems (albeit it's a good start). But if people come across another `InaccessibleObjectException` then they can still --add-opens as needed. So the current version is clearly an improvement. If you agree, then we should merge it into main instead of yet more parallel work in different branches... Hello @struberg The issue is not that this PR is a wants to start improving something, the issue is that this PR _breaks_ existing behavior. You cannot check for interfaces to short-circuit logic in `Reflection.isJavaInternalClass()`. This will break existing applications. The branch is not up to date with master. This fails: ``` git clone https://github.com/struberg/commons-lang.git cd commons-lang git checkout fb_jpms git remote add upstream https://github.com/apache/commons-lang.git git pull upstream master mvn clean test -Dtest=EqualsBuilderReflectJreImplementationTest ``` -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
