-
Notifications
You must be signed in to change notification settings - Fork 2.7k
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
[MNG-8123] Fix Lifecycle in v3 #1524
Conversation
Map<String, org.apache.maven.lifecycle.Lifecycle> all = | ||
container.lookupMap(org.apache.maven.lifecycle.Lifecycle.class); | ||
return all.keySet().stream() | ||
.filter(id -> !Lifecycle.CLEAN.equals(id) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why this filter? How are these 4 lifecycles published? Or in other words, why distinguish these 4?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We have a kind of chicken/egg situation. Lifecycle can be published by maven 3 extensions, but lifecycles must also be provided through the Maven 3 API. We must avoid rewrapping in a kind of loop the default lifecycles exposed using the maven 4 API because it leads to a lookup error (we end-up with a bean creation cycle).
I can see if there's a better way, such as adding a new sub-interface WrappedLifecycle
to better detect where the lifecycle come from (Maven 3 or 4 api and only wrap if for the other side).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually, I can't use the instance to detect, as getting the instance will cause the lookup error: the map is lazily managed by sisu, and the instance is created when requested only, but that's what causes the lookup error.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Isn't this somewhat similar to Type / ArtifactHandler story? Is same problem present there as well?
IT PR: apache/maven-integration-testing#342