drop execute() override workaround #137
Conversation
|
looking at
it looks you found a bug in plugin-tools on |
I wonder why this is the case? Because one so and the other one so? Mixed one? All other reporting plugins don't fail with Doxia 2.0.0 deps... |
|
looking at apache/maven-javadoc-plugin@master...doxia-2.0.0 you added a new oh, |
Well, the site renderer has been in
... I tried to remove the now redundant field and its getter, but it did not solve the problem with |
|
One more funny thing that I noticed was that it seems all protected fields are affected, not just @Override
public void execute() throws MojoExecutionException {
super.siteTool = this.siteTool;
super.siteRenderer = this.siteRenderer;
super.project = this.project;
super.reactorProjects = this.reactorProjects;
super.repoSession = this.repoSession;
super.remoteProjectRepositories = this.remoteProjectRepositories;
super.siteDirectory = this.siteDirectory;
super.mojoExecution = this.mojoExecution;
super.execute();
}If any of these assignments are missing, the |
|
Instead of redundantly adding the fields and initialisations, it also helps to manually patch plugin.xml to contain the relevant parent class |
|
@hboutemy, @michael-o: If I copy the source code of |
While I don't have an answer from the top of my head, did you compare your plugin with the other reporting plugins in the |
@michael-o, I am not sure what you are talking about. Where did you provide anything? |
Here: https://www.mail-archive.com/users@maven.apache.org/msg145060.html |
|
Sorry, if I sound stupid, but I never understood that remark on the mailing list:
@michael-o: Which repository? Which files? Do you have a link for me? I am utterly lost. How can any of your changes contain migration magic relating to my reporting plugins? I really do not want to get on your nerves or play stupid, I simply do not have enough information to understand what you are talking about. Terse is nice, but too terse means lenghty discussions like these. I am keen to follow your advice, but first I need to understand it and find what you are trying to point me to. |
My apologies, the text contains a significant typo. It should read:
This refers to all reporting plugin under Again, my bad, I did not proof-read. |
@michael-o: No. Maven Central lists 67 entries for that group ID. Firstly, I do not know in which plugins I could find a situation comparable to this one. Secondly, I would not know what to look for and what to compare. Thirdly, here is a reproducer with some additional findings, which is debuggable by someone as knowledeable and experienced in building and converting plugins as you are. Why was I asked to provide a reproducer on the mailing list, if now is is not being used? Last but not least, what about MPLUGIN-483? It has neither been worked on nor closed as invalid. Why are you assuming that the problem sits in front of the computer, if @hboutemy thinks that there is a bug in Maven Plugin Plugin or its infrastructure? |
A good amount of them is reporting only and documented on our project page. I encourage you to investigate yourself. |
|
@michael-o, are you trying to mock me by responding fuzzily and refusing to share a link to some piece of code you want me to look at? How am I supposed to know what you are talking about what what "our project" is? And why are you ignoring my new finding from #137 (comment)? That should be a clue for you to get to the bottom of this. Why does it make a difference, if the parent class is in a dependency (maven-reporting-impl) vs. the same project as the plugin, if the source code of both is identical? Is that not proof that I did in fact investigate and try to help you guys find the root cause of the problem? |
|
Here is the listing: https://maven.apache.org/plugins/index.html. Filter for type |
The root cause for the problem was the 'mojoDependencies' config for maven-plugin-plugin, which aimed to fix https://issues.apache.org/jira/browse/MPLUGIN-328 by the workaround from mojohaus/aspectj-maven-plugin#34. That old workaround was actually bad to begin with, because it only included dev.aspectj:aspectj-maven-plugin, therefore implicitly excluding other dependencies like maven-reporting-impl. The latter, however, contains class AbstractMavenReport, which led to all super class @parameter and @component properties to not be included in the generated plugin.xml. Relates to #137. Solves https://issues.apache.org/jira/browse/MPLUGIN-483.
|
I managed to solve the problem, see the explanation in my MPLUGIN-483 comment. @michael-o, @hboutemy, the problem was not in the code but in the POM. I had hoped that one of you would help me identify the root cause, because it was quite tricky to find for a plugin noob like me, especially because the bug was old, but only manifested itself now after the upgrade to Doxia 2. I am sure, you would have found it in a fraction of the time that I needed. Anyway, thanks for the well-meant advice, even though it did not help me in this particular case. |
That is fantastic news. I know that kind of hacks from elsewhere, but not this problem. We all have learned that every hack can incur a very high cost at the end... |
@kriegaex are you serious? Do you really think we did not work hard to help? |
|
I said it (the advice) did not help me, not you did not help me. That pronoun makes a difference, I think. So please, read carefully. I do appreciate that at least you (Hervé) actually checked out my reproducer, found out about the missing plugin.xml entries and raised a bug over there, albeit an invalid one, because the root cause was not in Maven but in the reproducer project. That someone dismissed MPLUGIN-483 without so much as a look at the project, falsely stating
was not helpful at all. Thanks also for supporting my objection over there. I do, just like I said, appreciate Michael's hints, too, even though they were not helpful either. He meant well, which is why I called them "well-meant" before. I have, however, problems understanding why usually, just like in this case, people on the mailing list, ask for a reproducer, but then in the face of one instead of using it to debug the problem, they tell me things like (paraphrasing): "I never had that problem. Just look at how I did it in other (unnamed) projects and compare." Combine that with a back-and-forth meta discussion, me trying finding out which exact project and piece(s) of code Michael was suggesting me to look at. I actually looked at 5 projects with doxia-2.0 branches and did not find any major differences in their code, compared to this one. Nobody, including Michael, could have known that the problem was not in the code or in bumped version numbers in the POM, but in another place in the POM. Hence the reproducer! What I did say and still believe to be true, however, is that if an expert like Michael would have debugged this case instead of me, he would have found the root cause in a fraction of the time I needed, because I had no idea where to look. I was hoping that Michael, an expert with an impressive track record of having upgraded many major Maven plugins to Doxia 2, would - without even "working hard", as Hervé called it - have found the root cause in a very short time frame. Sorry for explaining so much, but I do not want to be misunderstood again and certainly not risk pissing anyone off, refusing to help me in the future. BTW, I do not find it particularly nice to threaten users like that. Please do accept some criticism along with my sincere thanks, though. I think, that should be permitted. |
@michael-o, I could not agree more, especially because workarounds, even if originally meant to be temporary ones, often remain in projects longer than their creators, leaving the task to figure out problems caused by them to future generations. Either way, it was an opportunity to leave the camp ground behind a little cleaner than I found it, according to the old boyscout rule. That old plugin probably needs more maintenance work by someone who knows much better than I how to develop good Maven plugins. I am simply trying to keep the plugin up to date and working with new dependency versions (AspectJ, JDK, libraries, plugins), adding features rather infrequently, because the plugin basically does what it is supposed to already. |
The root cause for the problem was the 'mojoDependencies' config for maven-plugin-plugin, which aimed to fix https://issues.apache.org/jira/browse/MPLUGIN-328 by the workaround from mojohaus/aspectj-maven-plugin#34. That old workaround was actually bad to begin with, because it only included dev.aspectj:aspectj-maven-plugin, therefore implicitly excluding other dependencies like maven-reporting-impl. The latter, however, contains class AbstractMavenReport, which led to all super class @parameter and @component properties to not be included in the generated plugin.xml. Relates to #137. Solves https://issues.apache.org/jira/browse/MPLUGIN-483.
The root cause for the problem was the 'mojoDependencies' config for maven-plugin-plugin, which aimed to fix https://issues.apache.org/jira/browse/MPLUGIN-328 by the workaround from mojohaus/aspectj-maven-plugin#34. That old workaround was actually bad to begin with, because it only included dev.aspectj:aspectj-maven-plugin, therefore implicitly excluding other dependencies like maven-reporting-impl. The latter, however, contains class AbstractMavenReport, which led to all super class @parameter and @component properties to not be included in the generated plugin.xml. Relates to #137. Solves https://issues.apache.org/jira/browse/MPLUGIN-483.
see discussion https://lists.apache.org/thread/ko6g9sgwysm46t9j0nqw4bj6krj72q2b
for quick run limited to the failing IT:
issue visible in
target/it/CreateReport/build.log:AbstractMavenReport.siteToolis not injected as it should as it is declared as@Componenthttps://github.com/apache/maven-reporting-impl/blob/maven-reporting-impl-4.0.0-M10/src/main/java/org/apache/maven/reporting/AbstractMavenReport.java#L150