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

Basic multi-release jar handling #18

Merged
merged 1 commit into from
Aug 29, 2021
Merged

Basic multi-release jar handling #18

merged 1 commit into from
Aug 29, 2021

Conversation

zml2008
Copy link
Contributor

@zml2008 zml2008 commented Apr 5, 2021 •
edited
Loading

This adds multi-release jar handling to the model and
updates the remapping transformer to be aware of that information.

There are a few things I'm unsure of here:

  • What should the default name of a jar entry be? Currently the name is the raw name to maintain existing behavior, but the JDK's JarEntry uses the version-stripped name as default, exposing the raw name as the real name
  • Naming? does "unversioned" name make sense?
  • Does it make sense to add any special handling to Atlas? With these model changes atlas shouldn't need any changes to function, but it might be nice to add multi-release aware lookup methods to the atlas JarFile. Especially in its role as a ClassProvider, it might be helpful for Atlas to be aware of multi-release jars.

@zml2008
Basic multi-release jar handling
c822ed1 
This adds multi-release jar handling to the model and
updates the remapping transformer to be aware of that information.
@zml2008 zml2008 requested a review from jamierocks as a code owner April 5, 2021 05:22
@codecov
Copy link

codecov bot commented Apr 5, 2021

Codecov Report

Merging #18 (c822ed1) into develop (7adf3c4) will increase coverage by 2.81%.
The diff coverage is 76.59%.

Impacted file tree graph

@@             Coverage Diff             @@
##           develop      #18      +/-   ##
===========================================
+ Coverage    25.50%   28.31%   +2.81%     
===========================================
  Files           44       44              
  Lines          694      731      +37     
  Branches        88       93       +5     
===========================================
+ Hits           177      207      +30     
- Misses         498      502       +4     
- Partials        19       22       +3
Impacted Files Coverage Δ
.../java/org/cadixdev/bombe/jar/JarManifestEntry.java 36.36% <0.00%> (ø)
.../java/org/cadixdev/bombe/jar/JarResourceEntry.java 50.00% <0.00%> (-16.67%) ⬇️
...ombe/jar/JarServiceProviderConfigurationEntry.java 28.57% <0.00%> (ø)
...ain/java/org/cadixdev/bombe/jar/JarClassEntry.java 77.77% <75.00%> (+11.11%) ⬆️
.../java/org/cadixdev/bombe/jar/AbstractJarEntry.java 76.92% <88.57%> (+15.01%) ⬆️
...ev/bombe/jar/asm/JarEntryRemappingTransformer.java 73.80% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 7adf3c4...c822ed1. Read the comment docs.

jamierocks
jamierocks reviewed Apr 7, 2021
View reviewed changes
Copy link
Member

@jamierocks jamierocks left a comment

Choose a reason for hiding this comment

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

This looks good to me, will clone it soon and try it out :) Thanks!

@jamierocks jamierocks merged commit aef4659 into CadixDev:develop Aug 29, 2021
@jamierocks jamierocks linked an issue Aug 29, 2021 that may be closed by this pull request
Multi-release jar remapping support #14
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jamierocks jamierocks jamierocks left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Multi-release jar remapping support
2 participants
@zml2008 @jamierocks