Skip to content
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.

Sign up for GitHub

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

Jump to bottom

MJAVADOC-731 update parent, get rid of legacy #164

Merged
merged 2 commits into from
Sep 27, 2022
Merged

MJAVADOC-731 update parent, get rid of legacy #164

merged 2 commits into from
Sep 27, 2022

Conversation

kwin
Copy link
Member

@kwin kwin commented Sep 18, 2022 •
edited

No description provided.

@kwin kwin requested a review from michael-o September 18, 2022 17:53
@kwin
Copy link
Member Author

kwin commented Sep 18, 2022

@michael-o Any idea why the version is managed to 3.3.1 in

maven-javadoc-plugin/pom.xml

Line 415 in 3032299

<version>3.3.1</version>
? Couldn't we always use the current version for the report as well?

@kwin kwin force-pushed the feature/update-to-parent-37 branch from 3032299 to 34b9d77 Compare September 19, 2022 17:26
@kwin kwin changed the title Update parent pom to maven-plugins 37 MJAVADOC-731 update parent, get rid of legacy Sep 19, 2022
@michael-o
Copy link
Member

@michael-o Any idea why the version is managed to 3.3.1 in

maven-javadoc-plugin/pom.xml

Line 415 in 3032299

<version>3.3.1</version>

? Couldn't we always use the current version for the report as well?

Yes, it should actually work. At least other reporting plugins reuse themselves in the reactor for reporting. One of the problem is that if you haven't installed the plugin at least once locally, site phase won't include the plugin classes in the reactor. Try and should see it fail.

@kwin kwin requested a review from cstamas September 19, 2022 17:29
@kwin kwin marked this pull request as ready for review September 19, 2022 17:29
kwin
kwin commented Sep 19, 2022
View reviewed changes
pom.xml
<scope>provided</scope>
<groupId>javax.inject</groupId>
<artifactId>javax.inject</artifactId>
<version>1</version>
Copy link
Member Author

@kwin kwin Sep 19, 2022 •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@cstamas Shouldn't this have scope "provided" as well (although not that critical, as unlikely to ever get another version)

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@kwin It did get other versions, the GAV changed...

Latest in 1.x is

<dependency>
  <groupId>jakarta.inject</groupId>
  <artifactId>jakarta.inject-api</artifactId>
  <version>1.0.5</version>
</dependency>

The original has problems with fact it has no meta data. I forget specific issue that caused in the past. Would be better to use the officially supported one though regardless. Note this is still javax internals.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just depend on the artifact which is embedded (https://github.com/apache/maven/blob/84538c9988a25aec085021c365c560670ad80f63/pom.xml#L263-L267) and exported (https://github.com/apache/maven/blob/84538c9988a25aec085021c365c560670ad80f63/maven-core/src/main/resources/META-INF/maven/extension.xml#L96) by Maven Core. If there is an issue with that dependency we should fix this upstream. I didn't face any particular issue due to this, what metadata is missing?
Please report a bug at https://issues.apache.org/jira/projects/MNG/issues if there is an issue with this dependency.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok thanks, the original jar has no meta-inf. If I recall, tools that scan jars such as tomcat have issues with that jar. Its better to use the correct api though that jakarta released for javax stream. Makes sense to fix up stream and anywhere else and exclude the other if any legacy brings it in.

michael-o
michael-o requested changes Sep 19, 2022
View reviewed changes
Copy link
Member

@michael-o michael-o left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Massive..

src/main/java/org/apache/maven/plugins/javadoc/resolver/ResourceResolver.java Outdated Show resolved Hide resolved
@kwin kwin requested a review from michael-o September 20, 2022 06:01
@michael-o
Copy link
Member

going through...

michael-o
michael-o requested changes Sep 20, 2022
View reviewed changes
Copy link
Member

@michael-o michael-o left a comment •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does not work for me. Both behind Nexus and direct connect to Central I always get:

84170 [ERROR] Tests run: 23, Failures: 0, Errors: 1, Skipped: 0, Time elapsed: 49.635 s <<< FAILURE! - in org.apache.maven.plugins.javadoc.JavadocReportTest
84171 [ERROR] org.apache.maven.plugins.javadoc.JavadocReportTest.testTagletArtifacts  Time elapsed: 1.746 s  <<< ERROR!
84172 org.apache.maven.plugin.MojoExecutionException:
84173 An error has occurred in Javadoc report generation: Unable to resolve artifact:groupId = 'org.codehaus.plexus'
84174 artifactId = 'plexus-javadoc'
84175 version = '1.0'
84176 classifier = 'null'
84177     at org.apache.maven.plugins.javadoc.AbstractJavadocMojo.failOnError(AbstractJavadocMojo.java:6994)
84178     at org.apache.maven.plugins.javadoc.JavadocReport.doExecute(JavadocReport.java:351)
84179     at org.apache.maven.plugins.javadoc.AbstractJavadocMojo.execute(AbstractJavadocMojo.java:1860)
84180     at org.apache.maven.plugins.javadoc.JavadocReportTest.testTagletArtifacts(JavadocReportTest.java:1212)
84181     at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
84182     at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
84183     at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
84184     at java.lang.reflect.Method.invoke(Method.java:498)
84185     at junit.framework.TestCase.runTest(TestCase.java:177)
84186     at junit.framework.TestCase.runBare(TestCase.java:142)
84187     at junit.framework.TestResult$1.protect(TestResult.java:122)
84188     at junit.framework.TestResult.runProtected(TestResult.java:142)
84189     at junit.framework.TestResult.run(TestResult.java:125)
84190     at junit.framework.TestCase.run(TestCase.java:130)
84191     at junit.framework.TestSuite.runTest(TestSuite.java:241)
84192     at junit.framework.TestSuite.run(TestSuite.java:236)
84193     at org.junit.internal.runners.JUnit38ClassRunner.run(JUnit38ClassRunner.java:90)
84194     at org.apache.maven.surefire.junit4.JUnit4Provider.execute(JUnit4Provider.java:377)
...
84202 Caused by: org.apache.maven.reporting.MavenReportException: Unable to resolve artifact:groupId = 'org.codehaus.plexus'
84203 artifactId = 'plexus-javadoc'
84204 version = '1.0'
84205 classifier = 'null'
84206     at org.apache.maven.plugins.javadoc.AbstractJavadocMojo.addTagletsFromTagletArtifacts(AbstractJavadocMojo.java:5721)
84207     at org.apache.maven.plugins.javadoc.AbstractJavadocMojo.addStandardDocletOptions(AbstractJavadocMojo.java:5537)
84208     at org.apache.maven.plugins.javadoc.AbstractJavadocMojo.executeReport(AbstractJavadocMojo.java:2020)
84209     at org.apache.maven.plugins.javadoc.JavadocReport.generate(JavadocReport.java:149)
84210     at org.apache.maven.plugins.javadoc.JavadocReport.doExecute(JavadocReport.java:346)
84211     ... 23 more
84212 Caused by: org.eclipse.aether.resolution.ArtifactResolutionException: Could not find artifact org.codehaus.plexus:plexus-javadoc:jar:1.0
84213     at org.eclipse.aether.internal.impl.DefaultArtifactResolver.resolve(DefaultArtifactResolver.java:444)
84214     at org.eclipse.aether.internal.impl.DefaultArtifactResolver.resolveArtifacts(DefaultArtifactResolver.java:246)
84215     at org.eclipse.aether.internal.impl.DefaultArtifactResolver.resolveArtifact(DefaultArtifactResolver.java:223)
84216     at org.eclipse.aether.internal.impl.DefaultRepositorySystem.resolveArtifact(DefaultRepositorySystem.java:294)
84217     at org.apache.maven.plugins.javadoc.AbstractJavadocMojo.createAndResolveArtifact(AbstractJavadocMojo.java:3648)
84218     at org.apache.maven.plugins.javadoc.AbstractJavadocMojo.addTagletsFromTagletArtifacts(AbstractJavadocMojo.java:5717)
84219     ... 27 more
84220 Caused by: org.eclipse.aether.transfer.ArtifactNotFoundException: Could not find artifact org.codehaus.plexus:plexus-javadoc:jar:1.0
84221     at org.eclipse.aether.internal.impl.DefaultArtifactResolver.resolve(DefaultArtifactResolver.java:434)
84222     ... 32 more
84223

@michael-o
Copy link
Member

@kwin Should I test again?

@kwin
Copy link
Member Author

kwin commented Sep 22, 2022

It still fails for me locally and on Jenkins and I don't know why exactly. The mrm-maven-plugin should IMHO deliver the missing artifact but the source in https://github.com/apache/maven-javadoc-plugin/tree/master/src/it/mrm/3rdparty seems incomplete to me. I run into

DEBUG] Resolving artifact org.apache.maven.plugins.javadoc.its:lib:jar:1.0 from [mrm-maven-plugin (http://localhost:51146, default, releases+snapshots)]
[DEBUG] Skipped remote request for org.apache.maven.plugins.javadoc.its:lib:jar:1.0, already updated during this session
[INFO] ------------------------------------------------------------------------
[INFO] Reactor Summary for mjavadoc494 1.0-SNAPSHOT:
[INFO] 
[INFO] mjavadoc494 ........................................ FAILURE [  1.394 s]
[INFO] mjavadoc494-module1 ................................ SKIPPED
[INFO] ------------------------------------------------------------------------
[INFO] BUILD FAILURE
[INFO] ------------------------------------------------------------------------
[INFO] Total time:  1.531 s
[INFO] Finished at: 2022-09-22T18:09:27+02:00
[INFO] ------------------------------------------------------------------------
[ERROR] Failed to execute goal org.apache.maven.plugins:maven-javadoc-plugin:3.4.2-SNAPSHOT:aggregate (default-cli) on project mjavadoc494: An error has occurred in Javadoc report generation: org.apache.maven.plugins.javadoc.its:lib:jar:1.0 was not found in http://localhost:51146 during a previous attempt. This failure was cached in the local repository and resolution is not reattempted until the update interval of mrm-maven-plugin has elapsed or updates are forced -> [Help 1]
org.apache.maven.lifecycle.LifecycleExecutionException: Failed to execute goal org.apache.maven.plugins:maven-javadoc-plugin:3.4.2-SNAPSHOT:aggregate (default-cli) on project mjavadoc494: An error has occurred in Javadoc report generation: org.apache.maven.plugins.javadoc.its:lib:jar:1.0 was not found in http://localhost:51146 during a previous attempt. This failure was cached in the local repository and resolution is not reattempted until the update interval of mrm-maven-plugin has elapsed or updates are forced...

@kwin
[MJAVADOC-731] Update parent pom to maven-plugins 37
738c97b 
Remove legacy plexus component annotations
Remove usage of m-artifact-transfer and directly leverage m-resolver API
Mark Maven Core plugins as provided dependencies
@kwin kwin force-pushed the feature/update-to-parent-37 branch from 81f0ce6 to 738c97b Compare September 26, 2022 10:19
@kwin kwin requested a review from michael-o September 26, 2022 10:32
@kwin
Copy link
Member Author

kwin commented Sep 26, 2022

Last IT fixed, squashed and rebased. Ready for final review.

@kwin kwin merged commit 8b58932 into master Sep 27, 2022
@kwin kwin deleted the feature/update-to-parent-37 branch September 27, 2022 12:20
@michael-o
Copy link
Member

I knew, I had premonition that this PR does too much. Replacing MAT has now caused: https://issues.apache.org/jira/browse/MJAVADOC-742

@hboutemy hboutemy mentioned this pull request Sep 18, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@hazendaz hazendaz hazendaz left review comments

@cstamas cstamas cstamas approved these changes

@michael-o michael-o michael-o approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
4 participants
@kwin @michael-o @cstamas @hazendaz