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 <[email protected]> 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 <[email protected]> > 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 < >>> [email protected]> 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 < >>>>> [email protected]> 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 [email protected] >>>>>> 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 < >>>>>>> [email protected]> 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 < >>>>>>>>>> [email protected]> 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 <[email protected]> >>>>>>>>>>> wrote: >>>>>>>>>>> >>>>>>>>>>> >>>>>>>>>>> On Fri, 19 Feb 2021 at 18:44, Brian Pontarelli < >>>>>>>>>>> [email protected]> 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 <[email protected]> >>>>>>>>>>>> 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 [email protected]. >>>>>>>>>>>> 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 [email protected]. >>>>>>>>>>>> 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 [email protected]. >>>>>>>>>>> >>>>>>>>>>> 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 [email protected]. >>>>>>>>>> >>>>>>>>>> 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 [email protected]. >>>>>>>> >>>>>>> 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 [email protected]. >>>>>> >>>>> 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 [email protected]. >>>> >>> 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 [email protected]. >> 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 [email protected]. > 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 the Google Groups "google-guice" group. To unsubscribe from this group and stop receiving emails from it, send an email to [email protected]. To view this discussion on the web visit https://groups.google.com/d/msgid/google-guice/CAJEBNUcJFkaJ6PEtQP99f0kB_9RXk9ONF-Rq2pkOa15xWJpX0w%40mail.gmail.com.
