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.

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

Upgrade to Doxia 2.x stack #1180

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Upgrade to Doxia 2.x stack #1180

wants to merge 1 commit into from

Conversation

slawekjaranowski
Copy link
Member

fix: #1168

@andrzejj0
Copy link
Contributor

andrzejj0 commented Nov 9, 2024

Something's not right. I've tried creating a plugin-updates-report on Modello, and got a problem.

mvn org.codehaus.mojo:versions-maven-plugin:2.18.0-SNAPSHOT:plugin-updates-report -DallowSnapshots=true -X

[ERROR] Failed to execute goal org.codehaus.mojo:versions-maven-plugin:2.18.0-SNAPSHOT:plugin-updates-report (default-cli) on project modello: An error has occurred in Plugin Updates Report report generation.: Failed to create context for skin: Cannot use skin: has [1.11.1,2.0.0-M1) Doxia Sitetools prerequisite, but current is 2.0.0 -> [Help 1]
org.apache.maven.lifecycle.LifecycleExecutionException: Failed to execute goal org.codehaus.mojo:versions-maven-plugin:2.18.0-SNAPSHOT:plugin-updates-report (default-cli) on project modello: An error has occurred in Plugin Updates Report report generation.

and

Caused by: org.apache.maven.doxia.siterenderer.RendererException: Cannot use skin: has [1.11.1,2.0.0-M1) Doxia Sitetools prerequisite, but current is 2.0.0
at org.apache.maven.doxia.siterenderer.DefaultSiteRenderer.createContextForSkin (DefaultSiteRenderer.java:703)
at org.apache.maven.reporting.AbstractMavenReport.createSiteRenderingContext (AbstractMavenReport.java:312)
at org.apache.maven.reporting.AbstractMavenReport.reportToSite (AbstractMavenReport.java:254)
at org.apache.maven.reporting.AbstractMavenReport.execute (AbstractMavenReport.java:212)

See log:

plugin-updates-report.txt

EDIT: I see, it uses the skin as defined in the project's site.xml.

@michael-o
Copy link
Member

Something's not right. I've tried creating a plugin-updates-report on Modello, and got a problem.

mvn org.codehaus.mojo:versions-maven-plugin:2.18.0-SNAPSHOT:plugin-updates-report -DallowSnapshots=true -X

[ERROR] Failed to execute goal org.codehaus.mojo:versions-maven-plugin:2.18.0-SNAPSHOT:plugin-updates-report (default-cli) on project modello: An error has occurred in Plugin Updates Report report generation.: Failed to create context for skin: Cannot use skin: has [1.11.1,2.0.0-M1) Doxia Sitetools prerequisite, but current is 2.0.0 -> [Help 1] org.apache.maven.lifecycle.LifecycleExecutionException: Failed to execute goal org.codehaus.mojo:versions-maven-plugin:2.18.0-SNAPSHOT:plugin-updates-report (default-cli) on project modello: An error has occurred in Plugin Updates Report report generation.

and

Caused by: org.apache.maven.doxia.siterenderer.RendererException: Cannot use skin: has [1.11.1,2.0.0-M1) Doxia Sitetools prerequisite, but current is 2.0.0 at org.apache.maven.doxia.siterenderer.DefaultSiteRenderer.createContextForSkin (DefaultSiteRenderer.java:703) at org.apache.maven.reporting.AbstractMavenReport.createSiteRenderingContext (AbstractMavenReport.java:312) at org.apache.maven.reporting.AbstractMavenReport.reportToSite (AbstractMavenReport.java:254) at org.apache.maven.reporting.AbstractMavenReport.execute (AbstractMavenReport.java:212)

See log:

plugin-updates-report.txt

EDIT: I see, it uses the skin as defined in the project's site.xml.

See

[INFO] Rendering content with org.apache.maven.skins:maven-fluido-skin:jar:1.12.0 skin
You need to upgrade the skin.

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.

As a rule of thumb: Regular mojo classes should end with Mojo, but report mojos with Report only.

@slawekjaranowski
Copy link
Member Author

As a rule of thumb: Regular mojo classes should end with Mojo, but report mojos with Report only.

Renamed

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.

No objections except for VelocityConfigurator.

public void configure(Properties properties) {
// nothing to do
}
}
Copy link
Member

Choose a reason for hiding this comment

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

I don't understand why this happens at all..

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 also don't understand it .... maybe @cstamas, @kwin or @gnodet can help why @Nullable is not respected on DefaultVelocityComponent

One I see - in
AbstractMavenReport we have:

    @Component
    protected Renderer siteRenderer;

but in DependencyUpdatesRepor we use @Inject on constructor ....

Copy link
Contributor

Choose a reason for hiding this comment

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

The injection is failing only for the tests which don't invoke the plugin using site but invoke it directly.

So, only the following tests fail:

[ERROR] The following builds failed:
[ERROR] *  it-dependency-updates-report-issue-951/pom.xml
[ERROR] *  it-dependency-updates-report-issue-755/pom.xml
[ERROR] *  it-dependency-updates-report-issue-684-001/pom.xml

Maybe the problem is not correctly reported? I'm going to try to debug it and compare both cases (via reporting and directly).

Copy link
Contributor

Choose a reason for hiding this comment

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

Debugging is a bit difficult: actual version of sisu depends on the current version of Maven one is using (for 3.9.9 it's 0.9.0.M3). For what it's worth, I've only tested this with Maven 3.9.9 and 3.6.3 so far.

It looks pretty strange, as if @Nullable had no effect:

image

Copy link
Member Author

@slawekjaranowski slawekjaranowski Nov 12, 2024

Choose a reason for hiding this comment

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

one remarks
when report is loading by m-site-p error not occurs,
only exist when report is directly loaded by Maven

Copy link
Member Author

Choose a reason for hiding this comment

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

the same report in one case works and in other not

  • it-dependency-updates-report-issue-951 - direct call report with error
  • it-dependency-updates-report-001 - configured as a site report - ok

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 don't see nothing special in dependencies

Copy link
Member Author

Choose a reason for hiding this comment

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

direct-call.txt
site-report.txt

and diff:

--- direct-call.txt     2024-11-12 20:56:54
+++ site-report.txt     2024-11-12 20:59:23
@@ -57,6 +57,16 @@
 [DEBUG] Created new class realm plugin>org.codehaus.mojo:versions-maven-plugin:2.18.0-SNAPSHOT
 [DEBUG] Importing foreign packages into class realm plugin>org.codehaus.mojo:versions-maven-plugin:2.18.0-SNAPSHOT
 [DEBUG]   Imported:  < maven.api
+[DEBUG]   Imported: org.apache.maven.doxia.logging.Log < plugin>org.apache.maven.plugins:maven-site-plugin:3.20.0
+[DEBUG]   Imported: org.apache.maven.doxia.logging.LogEnabled < plugin>org.apache.maven.plugins:maven-site-plugin:3.20.0
+[DEBUG]   Imported: org.apache.maven.doxia.sink.Sink < plugin>org.apache.maven.plugins:maven-site-plugin:3.20.0
+[DEBUG]   Imported: org.apache.maven.doxia.sink.SinkEventAttributes < plugin>org.apache.maven.plugins:maven-site-plugin:3.20.0
+[DEBUG]   Imported: org.apache.maven.doxia.sink.SinkFactory < plugin>org.apache.maven.plugins:maven-site-plugin:3.20.0
+[DEBUG]   Imported: org.apache.maven.doxia.siterenderer.Renderer < plugin>org.apache.maven.plugins:maven-site-plugin:3.20.0
+[DEBUG]   Imported: org.apache.maven.doxia.siterenderer.SiteRenderer < plugin>org.apache.maven.plugins:maven-site-plugin:3.20.0
+[DEBUG]   Imported: org.apache.maven.reporting.MavenMultiPageReport < plugin>org.apache.maven.plugins:maven-site-plugin:3.20.0
+[DEBUG]   Imported: org.apache.maven.reporting.MavenReport < plugin>org.apache.maven.plugins:maven-site-plugin:3.20.0
+[DEBUG]   Imported: org.codehaus.doxia.sink.Sink < plugin>org.apache.maven.plugins:maven-site-plugin:3.20.0
 [DEBUG] Populating class realm plugin>org.codehaus.mojo:versions-maven-plugin:2.18.0-SNAPSHOT
 [DEBUG]   Included: org.codehaus.mojo:versions-maven-plugin:jar:2.18.0-SNAPSHOT
 [DEBUG]   Included: org.codehaus.mojo.versions:versions-api:jar:2.18.0-SNAPSHOT
@@ -66,13 +76,11 @@
 [DEBUG]   Included: org.apache.commons:commons-collections4:jar:4.4
 [DEBUG]   Included: org.codehaus.plexus:plexus-xml:jar:3.0.1
 [DEBUG]   Included: org.apache.maven.shared:maven-common-artifact-filters:jar:3.4.0
-[DEBUG]   Included: org.apache.maven.reporting:maven-reporting-api:jar:4.0.0
 [DEBUG]   Included: org.apache.maven.reporting:maven-reporting-impl:jar:4.0.0
 [DEBUG]   Included: org.apache.maven.shared:maven-shared-utils:jar:3.4.2
 [DEBUG]   Included: org.apache.maven.doxia:doxia-site-model:jar:2.0.0
 [DEBUG]   Included: org.apache.maven.doxia:doxia-integration-tools:jar:2.0.0
 [DEBUG]   Included: org.codehaus.plexus:plexus-interpolation:jar:1.27
-[DEBUG]   Included: org.apache.maven.doxia:doxia-site-renderer:jar:2.0.0
 [DEBUG]   Included: org.apache.maven.doxia:doxia-skin-model:jar:2.0.0
 [DEBUG]   Included: org.apache.maven.doxia:doxia-module-xhtml5:jar:2.0.0
 [DEBUG]   Included: org.codehaus.plexus:plexus-velocity:jar:2.2.0
@@ -96,7 +104,6 @@
 [DEBUG]   Included: org.apache.maven.doxia:doxia-core:jar:2.0.0
 [DEBUG]   Included: commons-io:commons-io:jar:2.17.0
 [DEBUG]   Included: org.apache.commons:commons-text:jar:1.12.0
-[DEBUG]   Included: org.apache.maven.doxia:doxia-sink-api:jar:2.0.0
 [DEBUG]   Included: org.codehaus.plexus:plexus-i18n:jar:1.0-beta-10
 [DEBUG]   Included: org.codehaus.plexus:plexus-interactivity-api:jar:1.3
 [DEBUG]   Included: com.fasterxml.woodstox:woodstox-core:jar:7.1.0

Copy link
Contributor

Choose a reason for hiding this comment

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

Please add:

<dependency>
  <groupId>org.eclipse.sisu</groupId>
  <artifactId>org.eclipse.sisu.inject</artifactId>
  <!-- use the same version as in Maven core -->
  <version>0.3.4</version>
</dependency>

this solves the issue.

Copy link
Member Author

Choose a reason for hiding this comment

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

VelocityConfigurator removed ... @andrzejj0 thanks for investigate
We need a org.eclipse.sisu.inject on classpath

public void configure(Properties properties) {
// nothing to do
}
}
Copy link
Contributor

Choose a reason for hiding this comment

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

The injection is failing only for the tests which don't invoke the plugin using site but invoke it directly.

So, only the following tests fail:

[ERROR] The following builds failed:
[ERROR] *  it-dependency-updates-report-issue-951/pom.xml
[ERROR] *  it-dependency-updates-report-issue-755/pom.xml
[ERROR] *  it-dependency-updates-report-issue-684-001/pom.xml

Maybe the problem is not correctly reported? I'm going to try to debug it and compare both cases (via reporting and directly).

public void configure(Properties properties) {
// nothing to do
}
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Debugging is a bit difficult: actual version of sisu depends on the current version of Maven one is using (for 3.9.9 it's 0.9.0.M3). For what it's worth, I've only tested this with Maven 3.9.9 and 3.6.3 so far.

It looks pretty strange, as if @Nullable had no effect:

image

public void configure(Properties properties) {
// nothing to do
}
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Hmmm I suppose it might add the injected dependency (VelocityComponentConfigurator) which is otherwise not present.

The real question is why this nullable seems not to work.

But we could also just patch it up somehow by also providing this missing dependency.

Regardless, this doesn't seem like a bug in Versions Maven Plugin, adding this dummy dependency that Jarek created should work around the problem.


Edit: it turns out that when invoked as site, the dependency is null -- @Nullable is suddenly honoured.

public void configure(Properties properties) {
// nothing to do
}
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Please add:

<dependency>
  <groupId>org.eclipse.sisu</groupId>
  <artifactId>org.eclipse.sisu.inject</artifactId>
  <!-- use the same version as in Maven core -->
  <version>0.3.4</version>
</dependency>

this solves the issue.

@slawekjaranowski
Copy link
Member Author

@andrzejj0
Copy link
Contributor

Yes I saw - and approved!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dependencies Pull requests that update a dependency file enhancement
Projects
None yet
3 participants