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

Rewrite @ScalaSignature when shading #393

Merged
merged 12 commits into from
May 23, 2020
Merged

Conversation

jeroentervoorde
Copy link

@jeroentervoorde jeroentervoorde commented Mar 30, 2020

So shaded artifacts can be used as libraries. See #350
This adds an extra ScalaSigProcessor step to the current jarjar steps. I copied MainProcessor into JJProcessor and scalaified it to be able to add the extra step.

This is still a WIP. We plan to work on the in our next sprint. I made this PR to gather feedback
on the approach.

There are some things that need to be done:

  • Support @ScalaLongSignature|
  • Support .sig files
  • Add tests (scripted + scalatest)
  • Idea: limitation on the rename rules that are supported. For example a new rule: renamePackage
    Reason it i find it difficult to oversee which errors may get introduced by supporting the rename rules as they are right now.
  • Support multiple scala versions. I currently only tested 2.12. Maybe we cannot use PickleBuffer the way we do now.
  • Cleanup, split ScalaSigAnnotationProcessor into multiple class files.

I could really use some insight of a scala compiler developer to check if I've missed some obvious tags to rename.


class ScalaSigProcessor(rules: Seq[Rule]) extends JarProcessor {
override def process(struct: EntryStruct): Boolean = {
if (!struct.name.endsWith(".class") || struct.skipTransform) true
Copy link
Member

Choose a reason for hiding this comment

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

Maybe allow *.sig files here? - scala/scala#7712

Copy link
Author

Choose a reason for hiding this comment

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

I'll add it to the list. I have some reading up to do here.

@eed3si9n
Copy link
Member

Assuming shading is needed by tools other than sbt-assembly, I wonder if it might be useful to split this as a library on its own. When it starts supporting library, there wouldn't be a need to shade only at the end of fat JAR? If so shading could potentially stand on its own as a plugin (we can continue to provide integration from sbt-assembly since it seems to be a popular feature), or even a CLI tool.

@eed3si9n
Copy link
Member

How about calling it Jar Jar Abrams?

@jeroentervoorde
Copy link
Author

@eed3si9n About creating a new library (a can't reply to your comment, maybe we should move discussion to the issue?)

We have only limited time to work on this so i was hoping to continue this as a part of sbt-assembly for now if you don't mind. We'll keep in mind that we want to extract in the future, once it gets some real life testing.

You're right we are not limited to fat jars anymore when this is done. I used sbt-assembly to publish a shaded, fat, library in a test project to test this so currently it already fits that use case. I do like the idea of an sbt-shade plugin but i don't think we have time to build that now (and we do like to start using the shaded jars internally asap)

Something else:

I did run into an issue with ivy though which does not elide all the shaded dependencies. I actually don't know if ivy dependencies can be excluded for an additional artifact with classifier. Do you?

@eed3si9n
Copy link
Member

I did run into an issue with ivy though which does not elide all the shaded dependencies. I actually don't know if ivy dependencies can be excluded for an additional artifact with classifier. Do you?

I have a FAQ entry here that might be relevant - https://github.com/sbt/sbt-assembly#q-despite-the-concerned-friends-i-still-want-publish-fat-jars-what-advice-do-you-have

@jeroentervoorde jeroentervoorde changed the title [WIP] Rewrite @ScalaSignature when shading Rewrite @ScalaSignature when shading Apr 16, 2020
@jeroentervoorde
Copy link
Author

jeroentervoorde commented Apr 16, 2020

Hey @eed3si9n / @xuwei-k , can you take a look at the PR?

I implemented most of the things from the initial todo list. I skipped .sig file support as i am not really sure this is actually useful in this plugin (and also how i'd test it)

Copy link
Member

@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'm guessing it's working thanks to the test.

@eed3si9n
Copy link
Member

Also I'm assuming you consent to releasing with the licensing terms of sbt-assembly? (MIT License)
I am asking this here since as noted above we might want to pull apart shading feature as a stand-alone library.

@jeroentervoorde
Copy link
Author

jeroentervoorde commented Apr 30, 2020

@eed3si9n
Sure! I consent to the licensing terms. Thanks for the review!

@eed3si9n eed3si9n merged commit db69ab5 into sbt:master May 23, 2020
@eed3si9n
Copy link
Member

I've split this into a new library - http://eed3si9n.com/jarjar-abrams

@jeroentervoorde
Copy link
Author

@eed3si9n

Cool! Are you willing to add my company name (Simacan) to the blog post as I got to do most of the work in office hours?

@eed3si9n
Copy link
Member

Absolutely. I just added a line.

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.

2 participants