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

Support for service providers #21

Merged
merged 6 commits into from
Oct 25, 2021

Conversation

gabrielrussoc
Copy link
Contributor

@gabrielrussoc gabrielrussoc commented Oct 21, 2021

I believe this has been a known issue since 2009: https://code.google.com/archive/p/jarjar/issues/30

This seems to be the only actively maintained fork of jarjar I could find, so I decided to contribute here.

I added some tests to the ResourceProcessor.


Since we are only using the jarjar cli, I also added the assembly plugin in order to build an uber jar. There were some module-info.class clashes so I simply ignored them since we will build it with jdk 8 (and they are most likely useless):

[error] 1 error was encountered during merge
[error] stack trace is suppressed; run last assembly for the full output
[error] (assembly) deduplicate: different file contents found in the following:
[error] /Users/gabriel.russo/Library/Caches/Coursier/v1/https/repo1.maven.org/maven2/org/ow2/asm/asm-analysis/9.2/asm-analysis-9.2.jar:module-info.class
[error] /Users/gabriel.russo/Library/Caches/Coursier/v1/https/repo1.maven.org/maven2/org/ow2/asm/asm-commons/9.2/asm-commons-9.2.jar:module-info.class
[error] /Users/gabriel.russo/Library/Caches/Coursier/v1/https/repo1.maven.org/maven2/org/ow2/asm/asm-tree/9.2/asm-tree-9.2.jar:module-info.class
[error] /Users/gabriel.russo/Library/Caches/Coursier/v1/https/repo1.maven.org/maven2/org/ow2/asm/asm/9.2/asm-9.2.jar:module-info.class
[error] Total time: 2 s, completed 21 Oct 2021, 15:12:28

@eed3si9n
Copy link
Owner

Thanks for the contribution!

@er1c
Copy link
Collaborator

er1c commented Oct 21, 2021

For the failure, I htink you just need .linesIterator although maybe I'm thinking about the scala method and not the java one

[error] /home/runner/work/jarjar-abrams/jarjar-abrams/jarjar/src/main/java/com/eed3si9n/jarjar/ResourceProcessor.java:59:1: cannot find symbol
[error]   symbol:   method lines()
[error]   location: variable s of type java.lang.String
[error] s.lines

Copy link
Contributor

@ericdotdata ericdotdata left a comment

Choose a reason for hiding this comment

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

LGTM, thank you @gabrielrussoc I have a question for @eed3si9n about the uber jar usage, and whether we should depend on sbt-assembly here or not, but this looks great!

Repository owner deleted a comment from ericdotdata Oct 25, 2021
Repository owner deleted a comment from ericdotdata Oct 25, 2021
build.sbt Show resolved Hide resolved
Copy link
Owner

@eed3si9n eed3si9n left a comment

Choose a reason for hiding this comment

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

I think the PR looks good as-is. We'll follow up on how we should publish an über JAR.

@er1c er1c merged commit 1c34e69 into eed3si9n:develop Oct 25, 2021
@er1c
Copy link
Collaborator

er1c commented Oct 26, 2021

@gabrielrussoc thanks again for the contribution! Going to add an uber assembly on the next published version via: #23

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.

4 participants