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

Trying to implement #1380: remove use of Moditect plug-in for 3.0 #1381

Merged
merged 11 commits into from
Jan 10, 2025

Conversation

cowtowncoder
Copy link
Member

@cowtowncoder cowtowncoder commented Jan 9, 2025

For #1380

@cowtowncoder
Copy link
Member Author

@GedMarc Ok, I managed to put this together so that it finally builds & tests run. But for some reason, module-info.class is not included it seems.

I was hoping you could sanity check whether changes make sense (plus if you have an idea of where module-info disappears, that'd be nice bonus too :) )

@cowtowncoder
Copy link
Member Author

FUDGE. It is all thanks to %)@$(#)^$#^# maven-shade-plugin which ALWAYS drops module-info.class: first if "minimize" is enabled and then, well, drop it anyway no matter what.
Apparently no way to configure it to be retained.

Will see if there's a way to RE-ADD it tho.

@cowtowncoder
Copy link
Member Author

Ok and the blocker is this issue:

https://issues.apache.org/jira/browse/MSHADE-261

Because Maven shade plug-in will drop module-info.class (starting with 3.1.0) if:

  1. "minimize" is used, OR
  2. ... just for funsies even if not (there's second check)

@cowtowncoder
Copy link
Member Author

Ok this has been tough one, but at this point build fully works, ending up with module-info.class under JDK 17 folder (META-INF/versions/17/module-info.class). To make that happen:

  1. Build uses src/main/java/module-info.java as it should
  2. Tests use src/main/java/module-info.java as they should
  3. Jar plugin is instructed to drop /module-info.class because...
  4. we need Moditect to re-build and stash module-info.java as META-INF/versions/17/module-info.class (and it won't if there is already module-info.class hence previous removal)
  5. maven-shade-plugin cannot forcibly remove module-info.class (it does from main level but NOT from jvm-specific ones. There!)

which is pretty convoluted way to make it. But I could not figure a simpler way... :-(
Still, seems fine.

Also had to simplify FastDoubleParser shading; use of names did not work for some reason.

Next to tackle FasterXML/jackson-databind#4891 (will try to get that working before merging this in).

@cowtowncoder
Copy link
Member Author

@GedMarc @pjfanning @JooHyukKim Just FYI: getting things properly JPMS modularized is pretty challenging. But after jackson-core and jackson-databind, hopefully bit more straight-forward (will see if shading should be removed from Afterburner/Blackbird/MrBean tho)

@cowtowncoder cowtowncoder merged commit fcd97eb into master Jan 10, 2025
6 checks passed
@cowtowncoder cowtowncoder deleted the tatu/3.0/1380-rm-moditect-add-module-info branch January 10, 2025 22:54
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.

1 participant