-
Notifications
You must be signed in to change notification settings - Fork 3k
Do not set TCCL if there is unlikely to be a discovery phase to close it #48298
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
Do not set TCCL if there is unlikely to be a discovery phase to close it #48298
Conversation
Status for workflow
|
Another horrible hack would be to decide based on the contents of the stack, but I believe what you've done is better. |
Wouldn't there be a way to detect Gradle instead and only install this hack if Gradle? Maybe a system property or env var set by Gradle? |
That was my original idea, too. Detecting gradle is pretty easy, but there's two problems with having an 'is this gradle?' guard on the TCCL-setting-in-session-opened:
What I would love is a gradle-specific pre-load hook, but I can't find any evidence of one. :( |
We couldn't do stack on its own because the 'unwanted' Eclipse stack is most similar to the 'good' IntelliJ stack (grrr), but thinking about it more, we could use the stack to distinguish between the 'wanted' and 'unwanted' Eclipse stacks. If the static variable causes problems, we could reconsider it. I was a bit nervous about how the static would interfere with things like maven surefire re-running, but decided it was ok as long as I only used it in an Eclipse single-run context. |
Exactly |
So should this get in or do we need another patch? |
It should go in. It was just waiting for someone to merge it, so I'll do that now. |
Resolves #48295.
I don't love this fix, but I can't think of a better option. The problem happens because the Eclipse launcher starts a launcher session twice, once with
SessionPerRequestLauncher.discover
and once withSessionPerRequestLauncher.execute
. The second one starts after discovery finished, so there's nothing to set the TCCL back to the app classloader.The problem wouldn't happen if we didn't set a TCCL when launcher sessions are opened, but that has to be done to support gradle.
These are the paths of the first and second call.
First
Second
For comparison, running in IntelliJ, the stack is (so we can't even make a decision based on
SessionPerRequestLauncher.execute
being in the stack).