gnodet commented on PR #743:
URL: https://github.com/apache/maven/pull/743#issuecomment-1131622335
> We have TLS-based solutions in past releases and we had to revert. What
makes this one different?
It's not a new TLS, it's a move of the existing TLS from SessionScope to
MavenSession.
The problem I'm trying to solve is really MNG-7474 : the fact that session
scoped beans are not singletons anymore, since the introduction of _multiple_
sessions when introducing the concurrent builds. This is what introduced the
TLS in the SessionScope, thereby breaking its purpose, i.e. beans are not
cached within the session scope.
So there are several solutions to this problem, but I think the SessionScope
is useless in the current form. So if we don't want to use a single session
and keep things the way they are, I'd need to introduce a more global scope
with a real singleton session object. We can't simply change the SessionScope,
because beans are injected with the MavenSession, so if we want singletons, we
need to have a consistent MavenSession object across mojo executions, which is
not the case because of the multiple session clones.
So the possibilities are:
* revert the use of multiple sessions, i.e. use a single session
throughout the whole build, which is what this PR is about. The possible
drawback is breakages of code that would use `MavenSession.getCurrentProject()`
from a different thread
* introduce a new concept of singleton session + scope. if we want it
immutable, it would basically be the MavenSession without the current project.
The obvious drawback
* do nothing to fix the problem: this is a possibility and I'd have to
work around the problem in m-build-cache-e by keeping state inside the
MavenSession, or most probably in the `RepositorySystemSession.getData()`
structure. In such a case, I think it would be better to deprecate the
`@SessionScope` and related classes.
I think the cleaner state would be to fix the `@SessionScope` and use a
singleton for `MavenSession` because that's semantically the most desirable
state. Scopes are natural for DI, so it makes sense to use them, and the
`MavenSession` cloning stuff has only been introduced to solve the
`getCurrentProject()` problem. We should maybe also deprecate this method and
suggest that plugins should have the project injected in a field. It would be
difficult to get rid of it in the very short term, as that one is used a lot
internally for the moment. We could also move it the `LegacySupport` along
with the access to the current session.
--
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]
