@Stuart @Sam https://github.com/google/guice/pull/1553
This adds creation of `guice-X.X.X-shadedasmguava.jar` with both asm and guava shaded. I've chosen `shadedasmguava` instead of just `shaded` as the latter by convention means that *all* deps are shaded, which is not the case: please let me know if you are ok with this. I've tested both `guice-5.0.2-SNAPSHOT.jar` and `guice-5.0.2-SNAPSHOT-shadedasmguava.jar` with some of my projects and they seem to work correctly. btw, I think the current naming scheme of other jars is very confusing: guice-X.X.X.jar -> asm shaded guice-X.X.X-classes.jar -> nothing shaded I think in the future we should make classifiers self-explanatory: guice-X.X.X.jar -> nothing shaded guice-X.X.X-shadedasm.jar -> asm shaded guice-X.X.X-shadedasmguava.jar -> asm and guava shaded Cheers! On 10/12/2021 01:24, Piotr Morgwai Kotarbinski wrote: > Just to make sure: so the current possibility of turning off ASM shading (by > deactivating guice.with.jarjar profile) is not used by anyone? > > @Stuart, thanks for pointing! > > @Sam, exactly: that's what that was supposed to mean. > > Thanks! > > On Friday, December 10, 2021 at 1:08:46 AM UTC+7 Sam Berlin wrote: > > I think I agree on variant (3), but I just want to be clear on what "no > profiles" means. Maven users depending on Guice will get the "with ASM > shaded" version [same as today], and can opt-in to swapping that out for the > "with ASM & Guava shaded" version with some "simple" Maven incantations? > > sam > > On Thu, Dec 9, 2021 at 1:05 PM Stuart McCulloch <mcc...@gmail.com> wrote: > > IMHO for the next release it should be variant 3: no profiles: > > guice-5.0.2.jar -> with ASM shaded (or jarjar'd) > guice-5.0.2-shaded.jar -> with ASM and Guava shaded > > Note the build already produces a jar that has nothing shaded: > > guice-5.0.2-classes.jar > > (see https://repo1.maven.org/maven2/com/google/inject/guice/5.0.1) > > On Thu, 9 Dec 2021 at 17:58, Piotr Morgwai Kotarbinski > <mor...@gmail.com> wrote: > > now I'm a bit confused what should be the final output ;-) > > variant 1: no profiles > guice-5.0.2.jar -> without anything shaded > guice-5.0.2-shaded.jar -> with ASM and Guava shaded > > variant 2: 2 profiles: > default profile: > guice-5.0.2.jar -> without anything shaded > to-be-deployed-to-maven-central-profile: > guice-5.0.2.jar -> with ASM shaded > guice-5.0.2-shaded.jar -> with ASM and Guava shaded > > variant 3: no profiles: > guice-5.0.2.jar -> with ASM shaded > guice-5.0.2-shaded.jar -> with ASM and Guava shaded > > ...or something even else? > > On Thursday, December 9, 2021 at 11:53:34 PM UTC+7 Sam Berlin > wrote: > > I thought we had stopped shading ASM, guess I was wrong. I'm > OK switching fully to shade and dropping jarjar, assuming it works as well or > better. > > Let's give the shaded one a generic "deps-are-shaded"-like > name, so that maybe in the future we can drop the ASM shading and tell folks > to use the shaded-deps version if they want all the deps self-contained. > > sam > > On Thu, Dec 9, 2021 at 10:29 AM Piotr Morgwai Kotarbinski > <mor...@gmail.com> wrote: > > @Sam > adding the below to build plugins in pom.xml > > <plugin> > > <groupId>org.apache.maven.plugins</groupId> > > <artifactId>maven-shade-plugin</artifactId> > <executions> > <execution> > <phase>package</phase> > <goals> > <goal>shade</goal> > </goals> > <configuration> > <artifactSet> > <includes> > > <include>com.google.guava:guava</include> > > </includes> > </artifactSet> > <relocations> > > <relocation> > > <pattern>com.google.common</pattern> > > <shadedPattern>com.google.inject.shadedcommon</shadedPattern> > > </relocation> > </relocations> > > <shadedArtifactAttached>true</shadedArtifactAttached> > > <shadedClassifierName>shadedguava</shadedClassifierName> > > <minimizeJar>true</minimizeJar> > </configuration> > </execution> > </executions> > </plugin> > > should result in an additional jar being produced named > guice-5.0.2-shadedguava.jar that will include all Guava classes that Guice > depends on relocated to com.google.inject.shadedcommon package (excluding > Guava classes not used by Guice and excluding any other dependencies). > Meanwhile guice-5.0.2.jar will not be affected in anyway. > > Later on projects using Guice will be able to do 1 of the > below: > - depend on standard build of Guice that has transitive > compile dependency on Guava that will be included in war/uber-jar: > > <dependencies> > <dependency> > > <groupId>com.google.inject</groupId> > <artifactId>guice</artifactId> > <version>5.0.2</version> > </dependency> > <!-- other deps here --> > </dependencies> > > - depend on -shadedguava build that includes relocated > Guava classes and exclude dependency on full stand-alone Guava: > > <dependencies> > <dependency> > > <groupId>com.google.inject</groupId> > <artifactId>guice</artifactId> > <version>5.0.2</version> > > <classifier>shadedguava</classifier> > <exclusions> > <exclusion> > > <groupId>com.google.guava</groupId> > > <artifactId>guava</artifactId> > </exclusion> > </exclusions> > </dependency> > <!-- other deps here --> > </dependencies> > > I will try to prepare a PR with this. The only potential > problem I see is that Guice already uses JarJar plugin to shade ASM and I'm > not sure if they will not conflict with each other. As I understand jars > deployed to maven-central are build using guice.with.jarjar profile, correct? > In case of conflicts between JarJar and maven-shade-plugin would it be > acceptable to use maven-shade-plugin for shading ASM as well instead of > JarJar? > > Thanks! > > On Thursday, December 9, 2021 at 9:03:22 PM UTC+7 Sam > Berlin wrote: > > I don't think we want to remove the ability to have a > non-shaded Guava (and have that as the default), but if it's possible to > build both at once, and if Maven supports a way of saying: "Hey, every time > my dependency tree depends of Guice, instead use this <other Guava-shaded > version of it>", I see no particular problem with that. > > Assuming both are possible and unless Stuart or > someone else has a strong reason why allowing this is a bad idea, I'd be > happy to accept a PR for it. > > Note that I wouldn't want to do this for *all* the > dependencies. We don't want to get back into the habit of needing to > rerelease Guice whenever a dependency has a new version, even if that version > is backwards compatible. This is particularly true for ASM. > > sam > > > On Thu, Dec 9, 2021, 8:54 AM Piotr Morgwai > Kotarbinski <mor...@gmail.com> wrote: > > Hi, > I don't exactly understand what you mean by > "anyone who wants to shade in Guava can still do that". If you mean "anyone > can make their own build of Guice", then it's quite a burden: you need to > maintain a private maven repo only for this modified build of Guice for > lifetime (at least for the lifetime of your project), rebuild it each time a > new upstream Guice version is released etc... > If you have something else in mind, then please > kindly elaborate when you have a moment: maybe I'm missing some nice and easy > solution ;-) > > OTOH, maven-shade-plugin also has > `shadedArtifactAttached` and `shadedClassifierName` flags which allow to > build and deploy both jars at once (with and without > shaded+relocated+minimized Guava). This way it would be easy for everyone, > right? > > Cheers! > > On Thursday, December 9, 2021 at 7:43:43 PM UTC+7 > mcc...@gmail.com wrote: > > You'll find that while it saves some space it > will still pull in a lot of Guava, which is the main reason why Guice stopped > shading it in for everyone. > > With the current setup anyone who wants to > shade in Guava can still do that, while everyone else is free to use a common > Guava library without the bloat. > > One compromise could be to add a "guice-all" > module to the build which puts Guice and its implementation dependencies into > an uber jar. > > (Although if there's a security issue in the > version of Guava shaded in then you'd then need to wait for a new release of > Guice vs. shading it yourself.) > > > On Thu, 9 Dec 2021 at 12:04, Piotr Morgwai > Kotarbinski <mor...@gmail.com> wrote: > > hmm, it's just come to me that maybe it's > possible to achieve the same result without a need for guice-guava, using > only maven-shade-plugin's filters and "minimizeJar" option: in theory it > should exactly include only classes that the project actually uses: > https://maven.apache.org/plugins/maven-shade-plugin/shade-mojo.html#minimizeJar > (which makes me wonder why wasn't it used > in the first place back when Guava was shaded...) > > On Thursday, December 9, 2021 at 2:10:57 > PM UTC+7 Piotr Morgwai Kotarbinski wrote: > > Unfortunately I think that the part > "get the PR accepted" is the main blocker here: I doubt google would be > interested in such change: internally they build everything from HEAD of each > dependency each time (or at least they used to), so versioning is not an > issue for them. On top of this, they use Guice for almost every piece of Java > code (again, at least they used to), so I'm afraid they will view any change > that does not solve some of *their* problems as an unnecessary risk. On top > of on top of this add general unresponsiveness of google to issues and PRs on > Guice github repo, so even just starting a discussion what could possibly be > an acceptable solution from google's point of view may turn impossible. > As a consequence, anyone undertaking > such task will risk all of his work to never be accepted (or even considered) > due to these "political" reasons regardless of his solution's quality and > usefulness for non-google users :( > > At least for the sake of this > conversation, let's try to discuss possible approaches nevertheless: > - duplicating Guava's functionality > directly into Guice is an ugly solution, introduces maintenance problems and > IMO should NOT be ever accepted. > - putting Guava's functionality > required by Guice into a separate lib (let's call it "guice-guava") and > creating in Guice's pom an additional profile that uses this lib (shaded and > relocated) instead of Guava: this way Guice code stays pure, but this new lib > has exactly the same problems as described in the previous point. > - creating guice-guava by including > Guava as its submodule and linking from `src/main/java` to a subset of java > files in guice-guava submodule folder (hence including only necessary classes > in guice-guava) may be reasonable solution. Whether it's actually feasible > depends how deep is the dependency graph of Guava classes required by Guice. > > Cheers! > > On Friday, December 3, 2021 at > 3:31:40 AM UTC+7 Brian Pontarelli wrote: > > I agree 100%. > > My offer of a bounty still > stands. Seriously. Happy to discuss actual numbers, but it would definitely > be worth it for us to pay a good rate to get this fixed. FusionAuth would be > the corporate sponsor of this effort in case anyone is interested. > > — Brian > >> On Nov 19, 2021, at 1:35 PM, >> Piotr Morgwai Kotarbinski <mor...@gmail.com> wrote: >> >> IMO, if a lib breaks backwards >> compatibility so often as Guava, it must be shaded: otherwise conflicts are >> unavoidable. The real problem is that Guava is too big as mentioned before. >> The right solution IMO would be to split it to smaller sub-libs. This way >> shading a small part that Guice depends on, would not be a problem anymore. >> >> Cheers! >> >> On Monday, April 5, 2021 at >> 9:43:26 PM UTC+7 Brian Pontarelli wrote: >> >> I understand why it would be >> best to leave Guava out of shading, but it really causes issues because of >> the way that Guava manages releases and versioning. The fact that it is on >> major version 27 while many projects are using version 10 is an issue. Every >> time a library upgrades Guava, we are all just hoping that it doesn’t break >> out entire classpath at runtime due to some incompatible change. And >> manually shading everything in our classpath for every upgrade is not >> feasible (hence the reason we are removing Guava deps whenever possible). >> >> I’d be willing to provide a >> bounty if someone is willing to remove Guava completely and can get the PR >> accepted. >> >> — Brian >> >>> On Mar 14, 2021, at 4:57 >>> PM, Stuart McCulloch <mcc...@gmail.com> wrote: >>> >>> >>> On Fri, 19 Feb 2021 at >>> 18:44, Brian Pontarelli <br...@pontarelli.com> wrote: >>> >>> I’d be interested in >>> this as well since we will be upgrading to Java 17 for FusionAuth once it >>> is released. >>> >>> And not to hijack, but >>> will it be possible to remove Guava and all shaded deps? >>> >>> >>> In the latest release >>> (5.0.1) the CGLIB dependency has been removed leaving ASM as the only >>> shaded dependency - because it's shaded it shouldn't conflict with other >>> versions on your classpath. >>> >>> Guava used to be shaded as >>> well, but this caused overhead for users who were also using Guava >>> elsewhere in their application. It also made it harder to pick up Guava >>> fixes in-between releases of Guice. So it's now a direct dependency that >>> people can decide to shade themselves if they want to use different >>> versions on their classpath. >>> >>> Removing Guava completely >>> would be a big undertaking - it's not part of the public API, but the >>> internals use it for caches and multimap support. >>> >>> Guava is a great >>> example of how not to manage a library and it keeps rolling major versions >>> that break compatibility. Tons of libraries uses different versions and >>> regularly break our classpath. We are slowly yanking libraries in >>> FusionAuth that use CGLIB, ASM, and Guava. It would be awesome if Guice >>> could yank those deps. Ideally, Guice would have no deps (except >>> javax.inject) and just be a drop in lib. >>> >>> — Brian >>> >>>> On Feb 19, 2021, at >>>> 11:05 AM, Peter Burka <pe...@quux.net> wrote: >>>> >>>> Java 17 will be >>>> released later this year, and will be the first long term support (LTS) >>>> OpenJDK version since Java 11 in 2018. I imagine that Guice 5 will add >>>> support for Java 17, however the current beta version does not work on >>>> Java 17 early-access as it includes an out-of-date shaded version of ASM. >>>> >>>> What is the >>>> anticipated schedule for releasing Guice 5? Will it be available before >>>> Java 17? Will there be any additional beta releases before GA? >>>> >>>> Are there any plans to >>>> update Guice 4 with Java 17 support, too? >>>> >>>> Thank you! >>>> >>>> >>>> -- >>>> You received this >>>> message because you are subscribed to the Google Groups "google-guice" >>>> group. >>>> To unsubscribe from >>>> this group and stop receiving emails from it, send an email to >>>> google-guice...@googlegroups.com. >>>> To view this >>>> discussion on the web visit >>>> https://groups.google.com/d/msgid/google-guice/88f24d78-f0b0-4633-881b-44deb050610fn%40googlegroups.com >>>> >>>> <https://groups.google.com/d/msgid/google-guice/88f24d78-f0b0-4633-881b-44deb050610fn%40googlegroups.com?utm_medium=email&utm_source=footer>. >>> >>> >>> -- >>> You received this >>> message because you are subscribed to the Google Groups "google-guice" >>> group. >>> To unsubscribe from >>> this group and stop receiving emails from it, send an email to >>> google-guice...@googlegroups.com. >>> To view this discussion >>> on the web visit >>> https://groups.google.com/d/msgid/google-guice/AF183807-C0A0-4C3B-8C23-6B93F89D4BA9%40pontarelli.com >>> >>> <https://groups.google.com/d/msgid/google-guice/AF183807-C0A0-4C3B-8C23-6B93F89D4BA9%40pontarelli.com?utm_medium=email&utm_source=footer>. >>> >>> >>> -- >>> You received this message >>> because you are subscribed to the Google Groups "google-guice" group. >>> To unsubscribe from this >>> group and stop receiving emails from it, send an email to >>> google-guice...@googlegroups.com. >>> To view this discussion on >>> the web visit >>> https://groups.google.com/d/msgid/google-guice/CAMr6Z4%3DLAU4QDiH_gvJ%2BtMm1SpWDhdpPcZ6GB3oqm%3Dnm%2Bc%2BtNQ%40mail.gmail.com >>> >>> <https://groups.google.com/d/msgid/google-guice/CAMr6Z4%3DLAU4QDiH_gvJ%2BtMm1SpWDhdpPcZ6GB3oqm%3Dnm%2Bc%2BtNQ%40mail.gmail.com?utm_medium=email&utm_source=footer>. >> >> >> -- >> You received this message >> because you are subscribed to the Google Groups "google-guice" group. >> To unsubscribe from this group >> and stop receiving emails from it, send an email to >> google-guice...@googlegroups.com. >> To view this discussion on the >> web visit >> https://groups.google.com/d/msgid/google-guice/1eb953bf-675b-4d09-8dea-00c38db8a7f4n%40googlegroups.com >> >> <https://groups.google.com/d/msgid/google-guice/1eb953bf-675b-4d09-8dea-00c38db8a7f4n%40googlegroups.com?utm_medium=email&utm_source=footer>. > > -- > You received this message because you are > subscribed to the Google Groups "google-guice" group. > To unsubscribe from this group and stop > receiving emails from it, send an email to google-guice...@googlegroups.com. > > To view this discussion on the web visit > https://groups.google.com/d/msgid/google-guice/ee3eea18-a9ea-4a21-9ff8-8d2a496e511en%40googlegroups.com > > <https://groups.google.com/d/msgid/google-guice/ee3eea18-a9ea-4a21-9ff8-8d2a496e511en%40googlegroups.com?utm_medium=email&utm_source=footer>. > > -- > You received this message because you are > subscribed to the Google Groups "google-guice" group. > To unsubscribe from this group and stop receiving > emails from it, send an email to google-guice...@googlegroups.com. > > To view this discussion on the web visit > https://groups.google.com/d/msgid/google-guice/8717c4c2-7c53-47af-aee1-012b788b5d6an%40googlegroups.com > > <https://groups.google.com/d/msgid/google-guice/8717c4c2-7c53-47af-aee1-012b788b5d6an%40googlegroups.com?utm_medium=email&utm_source=footer>. > > -- > You received this message because you are subscribed to > the Google Groups "google-guice" group. > To unsubscribe from this group and stop receiving emails > from it, send an email to google-guice...@googlegroups.com. > > To view this discussion on the web visit > https://groups.google.com/d/msgid/google-guice/9829d05d-e453-4740-b643-2686f7b71635n%40googlegroups.com > > <https://groups.google.com/d/msgid/google-guice/9829d05d-e453-4740-b643-2686f7b71635n%40googlegroups.com?utm_medium=email&utm_source=footer>. > > -- > You received this message because you are subscribed to the > Google Groups "google-guice" group. > To unsubscribe from this group and stop receiving emails from it, > send an email to google-guice...@googlegroups.com. > To view this discussion on the web visit > https://groups.google.com/d/msgid/google-guice/91ee31b1-b4f5-44fb-ad04-2c103bf13613n%40googlegroups.com > > <https://groups.google.com/d/msgid/google-guice/91ee31b1-b4f5-44fb-ad04-2c103bf13613n%40googlegroups.com?utm_medium=email&utm_source=footer>. > > -- > You received this message because you are subscribed to the Google > Groups "google-guice" group. > To unsubscribe from this group and stop receiving emails from it, > send an email to google-guice...@googlegroups.com. > > To view this discussion on the web visit > https://groups.google.com/d/msgid/google-guice/CAMr6Z4nE1mYJ2uj%3DLKWcSWM_rq4eejDrNnOGsZv%3DgTyRtT1qKA%40mail.gmail.com > > <https://groups.google.com/d/msgid/google-guice/CAMr6Z4nE1mYJ2uj%3DLKWcSWM_rq4eejDrNnOGsZv%3DgTyRtT1qKA%40mail.gmail.com?utm_medium=email&utm_source=footer>. > > -- > You received this message because you are subscribed to a topic in the Google > Groups "google-guice" group. > To unsubscribe from this topic, visit > https://groups.google.com/d/topic/google-guice/0inCjcpWUwc/unsubscribe. > To unsubscribe from this group and all its topics, send an email to > google-guice+unsubscr...@googlegroups.com > <mailto:google-guice+unsubscr...@googlegroups.com>. > To view this discussion on the web visit > https://groups.google.com/d/msgid/google-guice/8f6689ef-f1d9-49b5-a32a-69c95b11a7ddn%40googlegroups.com > > <https://groups.google.com/d/msgid/google-guice/8f6689ef-f1d9-49b5-a32a-69c95b11a7ddn%40googlegroups.com?utm_medium=email&utm_source=footer>. -- You received this message because you are subscribed to the Google Groups "google-guice" group. To unsubscribe from this group and stop receiving emails from it, send an email to google-guice+unsubscr...@googlegroups.com. To view this discussion on the web visit https://groups.google.com/d/msgid/google-guice/33f7452c-15e0-cb5e-69f0-0a12890621e1%40gmail.com.
