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

Update component #67

Merged
merged 1 commit into from
Dec 16, 2023
Merged

Update component #67

merged 1 commit into from
Dec 16, 2023

Conversation

cstamas
Copy link
Member

@cstamas cstamas commented Dec 11, 2023

Changes:

  • update parent (spotless applied as well)
  • drop p-c-d
  • make it JSR330
  • drop plexus XML mumbo jumbo

Changes:
* drop p-c-d
* make it JSR330
* drop plexus XML mumbo jumbo
@cstamas
Copy link
Member Author

cstamas commented Dec 11, 2023

Parent lags, spotless does not work with Java 21 and this "experimental" build on 21 cannot be disabled...

@slachiewicz
Copy link
Member

slachiewicz commented Dec 11, 2023

You can use plexus parent (not a component) that has all fixed. codehaus-plexus/plexus-components#58

We usually separate formatting commit

The experimental build was red for all, thx to this construction only one experiment failed and all others worked.

Ofc soon I'll remove this one experimental as now spotless works oob.

@michael-o
Copy link
Member

My big question here is: Is it compatible with Maven 3.8.x? If so, I will need to test it through the entire Doxia 2.0.0 on the weekend, if all works this is a good change and will go into Doxia 2.0.0. So, I need a couple of more days for this.

@cstamas
Copy link
Member Author

cstamas commented Dec 13, 2023

This change works with any Maven supporting JSR330 (I guess 3.1+? Or 3.2.x?). For sure is not a drop in replacement, as IF there is some Plexus XML config present, it will be ignored (or worse, will explode?)

@michael-o
Copy link
Member

This change works with any Maven supporting JSR330 (I guess 3.1+? Or 3.2.x?). For sure is not a drop in replacement, as IF there is some Plexus XML config present, it will be ignored (or worse, will explode?)

Alright, I will test and give you notice...

@slachiewicz
Copy link
Member

slachiewicz commented Dec 14, 2023

Would it make sense to have it as a separate component or simply inline into in Doxia?

@cstamas
Copy link
Member Author

cstamas commented Dec 14, 2023

While "site stuff" is the only user of this component, AFAIK is not one single project, but multiple of them. So unsure is it feasible...

@michael-o
Copy link
Member

Would it make sense to have it as a separate component or simply inline into in Doxia?

Not on Doxia. Other components use it as well as far as I remember.

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.

I tried with Doxia Sitetools:

[ERROR] org.apache.maven.doxia.siterenderer.DefaultSiteRendererTest.testRenderExceptionMessageWhenLineNumberIsAvailable -- Time elapsed: 0.664 s <<< ERROR!
org.codehaus.plexus.component.repository.exception.ComponentLookupException:
com.google.inject.ProvisionException: Unable to provision, see the following errors:

1) Error in custom provider, java.lang.TypeNotPresentException: Type org.codehaus.plexus.velocity.DefaultVelocityComponent not present
  at ClassRealm[plexus.core, parent: null] (via modules: org.eclipse.sisu.wire.WireModule -> org.eclipse.sisu.plexus.PlexusBindingModule)
  while locating org.codehaus.plexus.velocity.VelocityComponent
    for field at org.apache.maven.doxia.siterenderer.DefaultSiteRenderer.velocity(Unknown Source)
  at ClassRealm[plexus.core, parent: null] (via modules: org.eclipse.sisu.wire.WireModule -> org.eclipse.sisu.plexus.PlexusBindingModule)
  while locating org.apache.maven.doxia.siterenderer.DefaultSiteRenderer
  while locating java.lang.Object annotated with *

1 error
      role: org.apache.maven.doxia.siterenderer.SiteRenderer
  roleHint:
        at org.codehaus.plexus.DefaultPlexusContainer.lookup(DefaultPlexusContainer.java:268)
        at org.codehaus.plexus.DefaultPlexusContainer.lookup(DefaultPlexusContainer.java:256)
        at org.codehaus.plexus.DefaultPlexusContainer.lookup(DefaultPlexusContainer.java:250)
        at org.apache.maven.doxia.siterenderer.DefaultSiteRendererTest.setUp(DefaultSiteRendererTest.java:101)
        at java.lang.reflect.Method.invoke(Method.java:498)
        at java.util.ArrayList.forEach(ArrayList.java:1259)
        at java.util.ArrayList.forEach(ArrayList.java:1259)
Caused by: com.google.inject.ProvisionException: Unable to provision, see the following errors:

over and over again. What to do?

@cstamas
Copy link
Member Author

cstamas commented Dec 16, 2023

Error is right: this PR makes "org.codehaus.plexus.velocity.DefaultVelocityComponent not present" intentional, it moves it to internal package.

You have somewhere plexus XML I guess (as otherwise, with programmatic lookup you'd fail compilation).

@michael-o
Copy link
Member

I see only:

    @Inject
    private VelocityComponent velocity;

in DefaultSiteRenderer. I fail to the XML. Can you check what is missing? I am a bit lost. Do we need some shim here?

@cstamas
Copy link
Member Author

cstamas commented Dec 16, 2023

Something is mentioning org.codehaus.plexus.velocity.DefaultVelocityComponent type that is gone, so you need to find that thing. Again, is most probably XML, but may be wrong.

@michael-o michael-o self-requested a review December 16, 2023 19:59
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.

All issues resolved locally. This deserves 2.1.0-SNAPSHOT bump.

@cstamas cstamas merged commit e0c3648 into master Dec 16, 2023
22 of 24 checks passed
@cstamas cstamas deleted the update-component branch December 16, 2023 21:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants