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 <morg...@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+unsubscr...@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+unsubscr...@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.
