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 <morg...@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+unsubscr...@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+unsubscr...@googlegroups.com. To view this discussion on the web visit https://groups.google.com/d/msgid/google-guice/CAJEBNUe6hnGmPNpdFTGjLcC4UHoHPLF8%2Bs4LyCtu2bWgZ7FT4g%40mail.gmail.com.
