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

build: not include not provided deps on pom.xml, scala as provided #216

Merged

Conversation

erizocosmico
Copy link
Contributor

@erizocosmico erizocosmico commented Nov 24, 2017

Fixes #210

  • Not include scala in fatjar
  • Mark scala as provided
  • Rewrite output pom.xml to remove all dependencies without provided scope and add the scala library as provided (since sbt or something else in the process adds it as compile even if you explicitly put it).

NOTE: this only works for maven repositories. That is, if you use publishLocal, it will not work, you need to use publishM2 and use the local maven repository. Since we deploy on maven it should not matter much.

I'll let myself out

Signed-off-by: Miguel Molina <miguel@erizocosmi.co>
@bzz
Copy link
Contributor

bzz commented Nov 24, 2017

Let me try it and get back to you asap!

Quick question - do you think reshading more things included in fatJar, on top netty and grpc/guava would not be a good idea in our case?

@erizocosmico
Copy link
Contributor Author

erizocosmico commented Nov 24, 2017

@bzz reshading is gonna be a pain in the ass and it means every time we add a dependency we need to reshade that as well.
That said, perhaps we should reshade apache commons and jgit that are the most likely to be included. Siva, enry and bblfsh on the other hand, don't think they make sense to reshade them. What do you think?

Signed-off-by: Miguel Molina <miguel@erizocosmi.co>
@erizocosmico
Copy link
Contributor Author

@bzz just reshaded all apache commons and jgit classes

Copy link
Contributor

@bzz bzz left a comment

Choose a reason for hiding this comment

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

@erizocosmico tried commit before re-shading and it solved the issue beautifully - thanks a bunch for a prompt fix!

@erizocosmico erizocosmico merged commit cd19a4d into src-d:master Nov 27, 2017
@erizocosmico erizocosmico deleted the fix/fatjar-not-include-deps-in-pom branch November 27, 2017 09:43
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.

5 participants