-
Notifications
You must be signed in to change notification settings - Fork 78
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
shade and rewrite codehaus and maven dependencies #2115
shade and rewrite codehaus and maven dependencies #2115
Conversation
jurgenvinju
commented
Jan 3, 2025
•
edited
Loading
edited
- by relocating org.apache.maven and org.codehaus we can run maven instance classes twice without collision later in the rascal-maven-plugin
- removed the unshaded deployment profile
- removed the appenders for maven/codehaus dependency injection files, because the relocator will produce two different files now.
…ance classes twice without collision later in the rascal-maven-plugin
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #2115 +/- ##
=======================================
Coverage 49% 49%
- Complexity 6224 6234 +10
=======================================
Files 663 663
Lines 59181 59181
Branches 8615 8615
=======================================
+ Hits 29458 29479 +21
+ Misses 27497 27475 -22
- Partials 2226 2227 +1 ☔ View full report in Codecov by Sentry. |
I like this better, but we should also revert the changes to build.yaml |
@@ -322,14 +322,6 @@ | |||
<transformer implementation="org.apache.maven.plugins.shade.resource.AppendingTransformer"> | |||
<resource>io/usethesource/vallang/type/types.config</resource> | |||
</transformer> | |||
<transformer implementation="org.apache.maven.plugins.shade.resource.AppendingTransformer"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why did hmyhey get removed? Shouldn't they still be there, but in a new directory?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I tried to remove them for the sake of minimalism. That worked. I guess the rewrites make sure the collisions don't happen anymore.
We should also do a test of the generated jar on Windows, just to make sure it also works there. |
Ok. Are there any volunteers for testing the jar on windows? I had to remove my virtual windows 10 to save disk space. |
Removed unshaded action run
On Monday I'll bake a jar and run it. |
On windows, it doesn't work:
while it should have been (v0.40.17.jar):
|
I've just tested it on linux, it also doesn't work.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It breaks the behavior of the maven project resolution. All you get is the fallback behavior of an empty path config with some defaults filled in from the project and rascal.mf.
(revering the file merging config back to the original has no effect)
It's caused by the logging framework, that tries to (via reflection) load the log class, but that class doesn't exist anymore, as it's been relocated.
|
I've fix this in a different way. Namely properly pubishing a So I propose we abandon this PR? |