Zhenya, I think that this problem exists regardless of whether we have our own version of Guava or not: multiple dependencies can depend on Guava which will lead to clashes on the classpath. If a transitive version is updated in an incompatible way, we can try updating our version as well. And if that doesn't work, we can take some drastic measures, like forking it, but I doubt that this should ever happen.
On Thu, Sep 2, 2021 at 2:58 PM Zhenya Stanilovsky <arzamas...@mail.ru.invalid> wrote: > > Alex, what about : «compatibility when the transitive Guava dependency > will be updated to a new version.» ? I don`t know is it an issue for us, > but we need to answer it before move forward. > > > > >Andrey, > >I think that the benefits of using Guava methods instead of copy-pasting > >them are quite obvious: you don't have to copy-paste code and support it. > I > >also find this situation quite strange: we have a dependency and > copy-paste > >code from it instead of using it directly. > > > > > >On Thu, Sep 2, 2021 at 1:58 PM Andrey Mashenkov < > andrey.mashen...@gmail.com > > >wrote: > > > >> Alex, > >> > >> As I understand we agree to shade Guava transitive dependency and > >> you said a 'maven-shade-plugin' can drop unused Guava methods to reduce > the > >> footprint of Ignite jar. > >> > >> At this point, there is no difference between copy-pasting Guava method > to > >> Ignite and use Guava one. > >> The resulted jar will have one copy of such method, but in the second > case, > >> we have to care about compatibility when the transitive Guava dependency > >> will be updated to a new version. > >> > >> So, I see no benefits from using Guava directly. > >> > >> > >> On Thu, Sep 2, 2021 at 11:56 AM Alexander Polovtcev < > >> alexpolovt...@gmail.com > > >> wrote: > >> > >> > Hi, Andrey! > >> > I mostly agree with your proposal, but, since we already have some > >> > copy-paste in our code, can we at least use Guava to remove it? So I > >> would > >> > propose to allow at least *some* methods that we consider useful, > while > >> > disallowing everything else. I understand that it may be difficult to > >> > formalize, so maybe we can create some kind of a whitelist of Guava > >> > methods? What do you think? > >> > > >> > On Mon, Aug 30, 2021 at 2:54 PM Andrey Gura < ag...@apache.org > > wrote: > >> > > >> > > Follow up > >> > > > >> > > On Mon, Aug 23, 2021 at 1:22 PM Andrey Gura < ag...@apache.org > > wrote: > >> > > > > >> > > > Igniters, > >> > > > > >> > > > What I actually didn't understand from this thread: Is Guava > allowed > >> > > > in production code of Ignite 3 modules (not dependencies like > >> > > > Calcite)? > >> > > > > >> > > > While we agree with using shading I don't see any arguments about > >> > > > using Guava library in our code base except for the fact that we > have > >> > > > some copy-paste of Guava code in the project. > >> > > > > >> > > > Guava is rich enough library and it has both advantages and > >> > > > disadvantages. Ignite code base always strived to be not > overloaded > >> > > > and minimalistic. For example we usually use immutability very > rare, > >> > > > we try to avoid complex and overloaded APIs on hot path, we try > >> reduce > >> > > > GC pressure, etc. In turn, Guava is some sense forces using of > >> > > > immutable collections, has a lot of utility methods which produces > >> > > > temp object (yes, I don't trust to escap analysys), etc. Rich set > of > >> > > > collections and utility functions will also lead to different > >> > > > programming styles during Ignite development. I can't predict it, > but > >> > > > it is usual enough when some developers force immutablity while > >> others > >> > > > remove static type declaration and replace it by var :) > >> > > > > >> > > > This is a common pratice to reduce language possibilies subset and > >> > > > limiting using of some libraries in order to provide more or less > >> > > > unified approach to coding. So I propose to disallow using Guava > in > >> > > > our code base. > >> > > > > >> > > > WDYT? > >> > > > > >> > > > > >> > > > On Fri, Aug 20, 2021 at 7:37 PM Alexander Polovtcev > >> > > > < alexpolovt...@gmail.com > wrote: > >> > > > > > >> > > > > Guys, > >> > > > > > >> > > > > Thanks again for your responses. I've created a ticket about > using > >> > the > >> > > > > Shade plugin in order to understand if it is possible to shade > and > >> > > minimize > >> > > > > Guava, I will start working on it shortly: > >> > > > > https://issues.apache.org/jira/browse/IGNITE-15354 > >> > > > > > >> > > > > On Tue, Aug 10, 2021 at 1:51 PM Courtney Robinson < > >> > > courtney.robin...@hypi.io > > >> > > > > wrote: > >> > > > > > >> > > > > > I think since Calcite brings it in already then your arguments > >> make > >> > > sense. > >> > > > > > Would it be pinned to the same version as Calcite? Risk > >> > > NoSuchMethodError > >> > > > > > at runtime if not. > >> > > > > > +1 > >> > > > > > > >> > > > > > On Mon, Aug 9, 2021 at 9:56 AM Alexander Polovtcev < > >> > > > > > alexpolovt...@gmail.com > > >> > > > > > wrote: > >> > > > > > > >> > > > > > > Zhenya, Courtney, Andrey, > >> > > > > > > > >> > > > > > > What do you think about my arguments, was I able to convince > >> you? > >> > > I would > >> > > > > > > like to reach some consensus here. At the moment, my > original > >> > > points > >> > > > > > still > >> > > > > > > stand, I'm also ok with shading Guava if needed, though I > think > >> > it > >> > > is not > >> > > > > > > necessary at this point. > >> > > > > > > > >> > > > > > > On Fri, Aug 6, 2021 at 12:45 PM Alexander Polovtcev < > >> > > > > > > alexpolovt...@gmail.com > > >> > > > > > > wrote: > >> > > > > > > > >> > > > > > > > Zhenya, > >> > > > > > > > > >> > > > > > > > > But there is no restrictions from running ignite server > >> nodes > >> > > from > >> > > > > > some > >> > > > > > > > other code with it`s own guava version seems we obtain > fast > >> > path > >> > > to jar > >> > > > > > > > hell here? > >> > > > > > > > > >> > > > > > > > I'm not sure if I fully understand your question, but it > >> looks > >> > > like we > >> > > > > > > are > >> > > > > > > > in this situation already, because we have some > dependencies > >> > > that use > >> > > > > > > > Guava. That's why I propose to add Guava explicitly to at > >> least > >> > > have a > >> > > > > > > > deterministic runtime configuration (see this link > >> > > > > > > > < > >> > > > > > > > >> > > > > > > >> > > > >> > > >> > https://maven.apache.org/guides/introduction/introduction-to-dependency-mechanism.html#Dependency_Management > >> > > > > > > > > >> > > > > > > > for an explanation). > >> > > > > > > > > >> > > > > > > > On Fri, Aug 6, 2021 at 12:25 PM Zhenya Stanilovsky > >> > > > > > > > < arzamas...@mail.ru.invalid > wrote: > >> > > > > > > > > >> > > > > > > >> > >> > > > > > > >> Alexander, first of all looks like Ivan Daschinsky > approach > >> > > about thin > >> > > > > > > >> client use only and shadow plugin are cover all Andrey > >> > Mashenkov > >> > > > > > listing > >> > > > > > > >> problems. > >> > > > > > > >> But there is no restrictions from running ignite server > >> nodes > >> > > from > >> > > > > > some > >> > > > > > > >> other code with it`s own guava version seems we obtain > fast > >> > > path to > >> > > > > > jar > >> > > > > > > >> hell here? > >> > > > > > > >> > >> > > > > > > >> > >> > > > > > > >> >Zhenya, > >> > > > > > > >> > > >> > > > > > > >> >My intentions are the following: > >> > > > > > > >> > > >> > > > > > > >> >1. Remove some copy-pasted code (like the "bytecode" > module > >> > or > >> > > some > >> > > > > > > >> utility > >> > > > > > > >> >methods). Please see my original message for the links > to > >> the > >> > > code. > >> > > > > > > >> >2. Explicitly pin the Guava version to avoid conflicts > in > >> the > >> > > > > > runtime. > >> > > > > > > >> > > >> > > > > > > >> >About allowing to use Guava in the codebase, my thoughts > >> are > >> > > the > >> > > > > > > >> following: > >> > > > > > > >> > > >> > > > > > > >> >1. We *already* use some code from Guava either directly > >> > (like > >> > > in the > >> > > > > > > >> >"calcite" module) or by copy-pasting it into a utility > >> class. > >> > > > > > > >> >2. I understand that some Guava methods are obsolete as > of > >> > > Java 11, > >> > > > > > but > >> > > > > > > >> >some of them still don't have any standard library > >> > > counterparts, in > >> > > > > > > which > >> > > > > > > >> >case I think using Guava is justified (which is > supported > >> by > >> > > point > >> > > > > > 1). > >> > > > > > > >> > > >> > > > > > > >> >Can you please explain why you would disapprove of my > >> > proposal? > >> > > > > > > >> > > >> > > > > > > >> >On Thu, Aug 5, 2021 at 7:56 PM Zhenya Stanilovsky > >> > > > > > > >> >< arzamas...@mail.ru.invalid > wrote: > >> > > > > > > >> > > >> > > > > > > >> >> > >> > > > > > > >> >> alexpolovtcev please clarify what do you mean under : > >> > > «possibility > >> > > > > > of > >> > > > > > > >> >> using Guava in Ignite 3», using how necessary > dependency > >> of > >> > > calcite > >> > > > > > > or > >> > > > > > > >> >> using like «using in our code» ? If using in code, i > -1 > >> > here. > >> > > > > > > >> >> thanks. > >> > > > > > > >> >> > >> > > > > > > >> >> > >> > > > > > > >> >> >Hello, dear Igniters! > >> > > > > > > >> >> > > >> > > > > > > >> >> >I would like to discuss the possibility of using > Guava > >> > > > > > > >> >> >< https://github.com/google/guava > in Ignite 3. I > >> know > >> > > about > >> > > > > > the > >> > > > > > > >> >> restrictive > >> > > > > > > >> >> >policy of using it in Ignite 2, but I have the > following > >> > > reasons: > >> > > > > > > >> >> > > >> > > > > > > >> >> >1. We are de-facto using it already as an implicit > >> > > dependency, > >> > > > > > since > >> > > > > > > >> the > >> > > > > > > >> >> >Calcite module depends on it, and Calcite is going to > >> stay > >> > > for a > >> > > > > > > >> while =) > >> > > > > > > >> >> >2. AFAIK, the "bytecode" module is copied into the > >> > codebase > >> > > only > >> > > > > > to > >> > > > > > > >> strip > >> > > > > > > >> >> >Guava away from it. We can remove this module, which > >> will > >> > > improve > >> > > > > > > the > >> > > > > > > >> >> >maintainability of the project. > >> > > > > > > >> >> >3. We have some copy-paste of Guava code in the > project. > >> > For > >> > > > > > > example, > >> > > > > > > >> see > >> > > > > > > >> >> >this > >> > > > > > > >> >> >< > >> > > > > > > >> >> > >> > > > > > > >> > >> > > > > > > > >> > > > > > > >> > > > >> > > >> > https://github.com/apache/ignite-3/blob/main/modules/core/src/main/java/org/apache/ignite/internal/util/IgniteUtils.java#L136 > >> > > > > > > >> >> > > >> > > > > > > >> >> >and this > >> > > > > > > >> >> >< > >> > > > > > > >> >> > >> > > > > > > >> > >> > > > > > > > >> > > > > > > >> > > > >> > > >> > https://github.com/apache/ignite-3/blob/main/modules/core/src/main/java/org/apache/ignite/internal/util/IgniteUtils.java#L428 > >> > > > > > > >> >> > > >> > > > > > > >> >> >. > >> > > > > > > >> >> >4. Regarding security concerns, this report > >> > > > > > > >> >> >< > >> > > > > > > >> >> > >> > > > > > > >> > >> > > > > > > > >> > > > > > > >> > > > >> > > >> > https://www.cvedetails.com/product/52274/Google-Guava.html?vendor_id=1224 > >> > > > > > > >> >> > > >> > > > > > > >> >> >shows no major vulnerability issues for the last > three > >> > > years. > >> > > > > > > >> >> > > >> > > > > > > >> >> >Taking these points into account, I propose to allow > >> using > >> > > Guava > >> > > > > > > both > >> > > > > > > >> in > >> > > > > > > >> >> >production and test code and to add it as an explicit > >> > > dependency. > >> > > > > > > >> >> > > >> > > > > > > >> >> >What do you think? > >> > > > > > > >> >> > > >> > > > > > > >> >> >-- > >> > > > > > > >> >> >With regards, > >> > > > > > > >> >> >Aleksandr Polovtcev > >> > > > > > > >> >> > >> > > > > > > >> >> > >> > > > > > > >> >> > >> > > > > > > >> >> > >> > > > > > > >> > > >> > > > > > > >> > > >> > > > > > > >> >-- > >> > > > > > > >> >With regards, > >> > > > > > > >> >Aleksandr Polovtcev > >> > > > > > > >> > >> > > > > > > >> > >> > > > > > > >> > >> > > > > > > >> > >> > > > > > > > > >> > > > > > > > > >> > > > > > > > > >> > > > > > > > -- > >> > > > > > > > With regards, > >> > > > > > > > Aleksandr Polovtcev > >> > > > > > > > > >> > > > > > > > >> > > > > > > > >> > > > > > > -- > >> > > > > > > With regards, > >> > > > > > > Aleksandr Polovtcev > >> > > > > > > > >> > > > > > > >> > > > > > >> > > > > > >> > > > > -- > >> > > > > With regards, > >> > > > > Aleksandr Polovtcev > >> > > > >> > > >> > > >> > -- > >> > With regards, > >> > Aleksandr Polovtcev > >> > > >> > >> > >> -- > >> Best regards, > >> Andrey V. Mashenkov > >> > > > >-- > >With regards, > >Aleksandr Polovtcev > > > > -- With regards, Aleksandr Polovtcev
