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.
