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.

Reply via email to