You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@groovy.apache.org by GitBox <gi...@apache.org> on 2022/10/25 21:21:01 UTC
[GitHub] [groovy] jonnybot0 opened a new pull request, #1814: Bugfix/groovy 9541 respect parent classloader
jonnybot0 opened a new pull request, #1814:
URL: https://github.com/apache/groovy/pull/1814
Helpful background may be found in the comments of https://issues.apache.org/jira/browse/GROOVY-9541.
As @eric-milles noted there, we may need to do a more careful consideration of each consumer of `StaticTypeCheckingSupport#evaluateExpression`, since the `source` object's classloader may not actually be correct in every case.
I can attest that letting `org.codehaus.groovy.transform.ASTTransformationCollectorCodeVisitor#findCollectedAnnotations` use the context classloader will result in difficult-to-resolve cases of GROOVY-9541.
A word about 4616fc8: without that, you'll get a [cast_failure](https://github.com/apache/groovy/files/9864104/cast_failure.txt) when trying to compile the groovy-macro module. I was a bit nervous about it, but I think `java.util.Optional#map` provides enough null safety that it should work.
My local test suite is still finishing execution, but I thought a concrete diff & a PR would be helpful for the discussion.
--
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: notifications-unsubscribe@groovy.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [groovy] jonnybot0 commented on pull request #1814: Bugfix/groovy 9541 respect parent classloader
Posted by GitBox <gi...@apache.org>.
jonnybot0 commented on PR #1814:
URL: https://github.com/apache/groovy/pull/1814#issuecomment-1291223195
> Have you searched for each use of evaluateExpression or is there a subset that was handled?
Yes, this is all the usages in the source code I could find.
> The cast failure implies that you used a different class loader to create the instance of AnnotationCollectorMode than the loader the compiler used to load that class. The transform loader is what you want if the compiler is going to be assigning references that aren't just strings and numbers.
Right. That's why I moved to using `.valueOf(val.toString())` to get the annotation value without a cast. But I see what you mean. I should be able to use the `transformLoader` on the class. I'll do that in my follow-up PR to master.
--
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: notifications-unsubscribe@groovy.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [groovy] sonatype-lift[bot] commented on pull request #1814: Bugfix/groovy 9541 respect parent classloader
Posted by GitBox <gi...@apache.org>.
sonatype-lift[bot] commented on PR #1814:
URL: https://github.com/apache/groovy/pull/1814#issuecomment-1291168193
:warning: **185 God Classes** were detected by Lift in this project. [Visit the Lift web console](https://lift.sonatype.com/results/github.com/apache/groovy/01GG8GA9EV124R0V0Y66TM1NT8?tab=technical-debt&utm_source=github.com&utm_campaign=lift-comment&utm_content=apache\%20groovy) for more details.
--
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: notifications-unsubscribe@groovy.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [groovy] eric-milles commented on pull request #1814: Bugfix/groovy 9541 respect parent classloader
Posted by GitBox <gi...@apache.org>.
eric-milles commented on PR #1814:
URL: https://github.com/apache/groovy/pull/1814#issuecomment-1291195299
Could you recreate the PR against the `master` branch? That is where we would first evaluate it and then we can cherry-pick it back to `4_0_X` and `3_0_X` for you if risk is minimal.
The changes look fine in general. Have you search for each use of `evaluateExpression` or is there a subset that was handled?
--
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: notifications-unsubscribe@groovy.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [groovy] jonnybot0 commented on pull request #1814: Bugfix/groovy 9541 respect parent classloader
Posted by GitBox <gi...@apache.org>.
jonnybot0 commented on PR #1814:
URL: https://github.com/apache/groovy/pull/1814#issuecomment-1292252517
I'll close this and reopen once I can get the patch working against the `master` branch.
--
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: notifications-unsubscribe@groovy.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [groovy] eric-milles commented on pull request #1814: Bugfix/groovy 9541 respect parent classloader
Posted by GitBox <gi...@apache.org>.
eric-milles commented on PR #1814:
URL: https://github.com/apache/groovy/pull/1814#issuecomment-1291198310
The cast failure implies that you used a different class loader to create the instance of `AnnotationCollectorMode` than the loader the compiler used to load that class. The transform loader is what you want if the compiler is going to be assigning references that aren't just strings and numbers.
--
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: notifications-unsubscribe@groovy.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [groovy] jonnybot0 closed pull request #1814: Bugfix/groovy 9541 respect parent classloader
Posted by GitBox <gi...@apache.org>.
jonnybot0 closed pull request #1814: Bugfix/groovy 9541 respect parent classloader
URL: https://github.com/apache/groovy/pull/1814
--
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: notifications-unsubscribe@groovy.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org