elharo commented on code in PR #12634:
URL: https://github.com/apache/maven/pull/12634#discussion_r3737233412
##########
impl/maven-core/src/main/java/org/apache/maven/DefaultMaven.java:
##########
@@ -221,9 +219,13 @@ private MavenExecutionResult
doExecute(MavenExecutionRequest request) {
sessionScope.seed(Session.class, session.getSession());
sessionScope.seed(InternalMavenSession.class,
InternalMavenSession.from(session.getSession()));
+ MavenSession previousSession = legacySupport.getSession();
legacySupport.setSession(session);
-
- return doExecute(request, session, result, chainedWorkspaceReader);
+ try {
+ return doExecute(request, session, result,
chainedWorkspaceReader);
+ } finally {
+ legacySupport.setSession(previousSession);
Review Comment:
Fixed: `legacySupport.setSession(session)` is now inside the try block, so
the previous session is restored even if setSession throws after
previousSession is captured.
##########
impl/maven-core/src/main/java/org/apache/maven/DefaultMaven.java:
##########
@@ -221,9 +219,13 @@ private MavenExecutionResult
doExecute(MavenExecutionRequest request) {
sessionScope.seed(Session.class, session.getSession());
sessionScope.seed(InternalMavenSession.class,
InternalMavenSession.from(session.getSession()));
+ MavenSession previousSession = legacySupport.getSession();
legacySupport.setSession(session);
-
- return doExecute(request, session, result, chainedWorkspaceReader);
+ try {
+ return doExecute(request, session, result,
chainedWorkspaceReader);
+ } finally {
+ legacySupport.setSession(previousSession);
Review Comment:
Fixed: setSession(session) is now inside the try block, so the previous
session is restored even if setSession throws.
##########
impl/maven-core/src/main/java/org/apache/maven/DefaultMaven.java:
##########
@@ -221,9 +219,13 @@ private MavenExecutionResult
doExecute(MavenExecutionRequest request) {
sessionScope.seed(Session.class, session.getSession());
sessionScope.seed(InternalMavenSession.class,
InternalMavenSession.from(session.getSession()));
+ MavenSession previousSession = legacySupport.getSession();
legacySupport.setSession(session);
-
- return doExecute(request, session, result, chainedWorkspaceReader);
+ try {
+ return doExecute(request, session, result,
chainedWorkspaceReader);
Review Comment:
Fixed: added testThatLegacySupportSessionIsRestoredAfterExecute in
DefaultMavenTest, which pre-sets a session, runs maven.execute(), and asserts
the pre-set session is restored afterward. Verified the test fails on the
previous code (which unconditionally called setSession(null)).
--
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]
