no idea what "UNCHECKED" in the subject header means........something apache mailing list put there...................
On Wed, Sep 19, 2018 at 10:49 AM Stephen Mallette <[email protected]> wrote: > Created this: > > https://issues.apache.org/jira/browse/TINKERPOP-2039 > > On Tue, Sep 18, 2018 at 7:48 PM Robert Dale <[email protected]> wrote: > >> Sorry, was skimming and only saw the 3.0 mention. The default is in 3.0. >> I >> thought I read that was new there. I don’t know what’s in 2.5. I’ll try >> to take a look this week. >> >> On Tue, Sep 18, 2018 at 19:20 Stephen Mallette <[email protected]> >> wrote: >> >> > ah - i tried to find some specific mention of that and didn't come >> across >> > it. if that's the case, then i think that maybe we should just head >> > straight to 2.5.2 on master. would you still go to groovy-all if we did >> > that? i presume the indy problem would go away in that case then right? >> > >> > i do think we should just do this all at once as part of one PR - i have >> > 2.5.2. building now with some minor required changes to get it working. >> i >> > can push that to a branch and we can go from there.... >> > >> > On Tue, Sep 18, 2018 at 7:16 PM Robert Dale <[email protected]> wrote: >> > >> > > Indy is gone because invokedynamic is the default in that version. >> > > >> > > On Tue, Sep 18, 2018 at 18:30 Stephen Mallette <[email protected]> >> > > wrote: >> > > >> > > > Looking ahead a bit on this one (I'm interested in what it will >> take to >> > > get >> > > > to Groovy 3.0.0) I noticed that 2.5.x requires <type>pom</type> and >> > > > that groovy-all-x.y.z.jar is no longer available "In order to cater >> to >> > > the >> > > > module system of Java 9+" - i also don't see an "indy" classifier >> for >> > > that >> > > > in central: >> > > > >> > > > >> https://repo1.maven.org/maven2/org/codehaus/groovy/groovy-all/2.5.2/ >> > > > >> > > > atm, i'm not sure if that changes anything in terms of the >> direction we >> > > > were headed. >> > > > >> > > > On Mon, Sep 17, 2018 at 3:06 PM Stephen Mallette < >> [email protected] >> > > >> > > > wrote: >> > > > >> > > > > jeez - hornets nest >> > > > > >> > > > > interesting that going to a single module makes our distribution >> > > smaller. >> > > > > I assume that we must have though the opposite when we started >> using >> > > > pieces >> > > > > of groovy rather than "-all". at this point there shouldn't be >> any >> > > more >> > > > > weirdly licensed files in groovy given that they have been >> releasing >> > > > under >> > > > > apache for as long as we have. i can't really come up with a >> reason >> > not >> > > > to >> > > > > go to "-all" on 3.4.0, so i guess i'm +1 for that. >> > > > > >> > > > > the :install command could support classifiers because grape/ivy >> > > support >> > > > > it as an option. we could treat that as a separate issue i guess - >> > new >> > > > JIRA? >> > > > > >> > > > > >> > > > > >> > > > > >> > > > > >> > > > > On Sun, Sep 16, 2018 at 6:00 PM Robert Dale <[email protected]> >> > wrote: >> > > > > >> > > > >> So, I was taking a closer look at this there's potentially a >> larger >> > > mess >> > > > >> we >> > > > >> want to address. >> > > > >> >> > > > >> TLDR: instead of removing groovy-sql, replace all groovy deps >> with >> > > > >> groovy-all-indy. >> > > > >> >> > > > >> First, a bit of history. tp32 did actually use groovy.sql.Sql >> > > > >> in ConsoleImportCustomizerProvider. This class was deprecated in >> > > 3.2.4. >> > > > >> Later, it was removed in 3.3.0. >> > > > >> >> > > > >> With the plan being to remove groovy-sql completely in 3.4.0, I >> was >> > > > >> testing >> > > > >> my instructions on using the :install feature to reinstall it >> just >> > in >> > > > case >> > > > >> had someone depended on it. It of course downloaded groovy-sql >> but >> > it >> > > > was >> > > > >> different than what was in lib/. It was missing '-indy'. It >> also >> > > > >> downloaded the non-indy version of groovy. I had to go learn >> what >> > > > '-indy' >> > > > >> was about. For those who don't know '-indy' is a maven >> classifier to >> > > > >> denote >> > > > >> that the jar was built with Java 1.7 'invokedymanic' support as >> > > opposed >> > > > >> the >> > > > >> legacy 'call site'. The short of it is that invokedymanic is >> more >> > > > >> performant than call site. >> > > > >> >> > > > >> As I was comparing groovy lib/, I noticed that console and server >> > > > already >> > > > >> include both groovy-2.4.15-indy.jar and groovy-2.4.15.jar. >> However, >> > > the >> > > > >> docs[1] state that only one or the other should be included. Same >> > goes >> > > > for >> > > > >> any 'indy' and 'non-indy' jar of the same module. Luckily, we >> don't >> > > > have >> > > > >> any other conflict like that although there is a mix of other >> indy >> > > > >> non-indy >> > > > >> modules. >> > > > >> >> > > > >> I tried again and was unable to download the indy version of >> > > groovy-sql >> > > > >> because the :install command does not allow a 'classifier' >> > parameter. >> > > It >> > > > >> will only download the non-indy verison of modules. Even if it >> > could >> > > > >> download the 'indy' version, the groovy dependencies are built >> such >> > > that >> > > > >> all 'indy' modules have a dependency on non-indy modules. >> > > > >> >> > > > >> So, there are several issues: >> > > > >> 1) there is included a conflicting mix of indy and non-indy >> builds >> > of >> > > > the >> > > > >> same module - groovy. This is bad and should be fixed. >> > > > >> 2) there is included a mix of indy and non-indy builds of >> different >> > > > >> modules. This is not necessarily bad but does mean we're using >> > > > potentially >> > > > >> less performant pieces of groovy. >> > > > >> indy) groovysh, json, jsr223 >> > > > >> non-indy) console, swing, templates, xml >> > > > >> 3) Install command doesn't support classifier parameter. I'm not >> > > aware >> > > > of >> > > > >> anyone complaining about this before so it's not necessarily an >> > issue >> > > in >> > > > >> itself. However, if we were to desire that all groovy modules be >> > > > >> indy-only, >> > > > >> then classifier support would be required. But then I don't know >> > how >> > > to >> > > > >> prevent the non-indy dependencies of that module from being >> > > downloaded. >> > > > It >> > > > >> seems like a mix would always occur. >> > > > >> >> > > > >> There is one simple solution interestingly enough: depend only >> on >> > > > >> groovy-all-indy. Not only does this keep all indy modules >> > consistent >> > > > and >> > > > >> prevent non-indy conflicts/duplicates, it actually shrinks the >> > > > >> distribution >> > > > >> size by 4M. This is from not distributing the duplicate non-indy >> > > > version >> > > > >> of groovy and because groovy-all isn't much bigger than the >> separate >> > > > >> modules we have now. The other benefit is that we can reduce >> groovy >> > > > >> dependencies down to a single module across the project. The >> > > > alternative >> > > > >> is ugly - having a direct dependency on each module in the >> > dependency >> > > > tree >> > > > >> and having an excludes on each of those dependencies to prevent >> them >> > > > from >> > > > >> loading the non-indy deps. >> > > > >> >> > > > >> In conclusion, instead of removing groovy-sql, I am proposing >> that >> > > > instead >> > > > >> we replace all groovy deps with groovy-all-indy. >> > > > >> >> > > > >> Thoughts? >> > > > >> >> > > > >> Stephen, are the groovy scripts submitted to Gremlin Server >> compiled >> > > > with >> > > > >> invokedynamic? What about Gremlin Console? I know the groovy >> > console >> > > > does >> > > > >> not have this enabled by default. >> > > > >> >> > > > >> 1. >> > > > >> >> > > > >> >> > > > >> > > >> > >> http://docs.groovy-lang.org/docs/groovy-2.4.15/html/documentation/invokedynamic-support.html#_two_jars >> > > > >> >> > > > >> Robert Dale >> > > > >> >> > > > >> >> > > > >> On Fri, Sep 14, 2018 at 9:32 AM Robert Dale <[email protected]> >> > > wrote: >> > > > >> >> > > > >> > No, nothing in the docs. There are some references to >> > > > java.sql.Timestamp >> > > > >> > but only within the context of gryo. I'll just make an upgrade >> > note >> > > > >> that >> > > > >> > it was removed and if required, it can be installed through the >> > > > :install >> > > > >> > command. >> > > > >> > >> > > > >> > Robert Dale >> > > > >> > >> > > > >> > >> > > > >> > On Fri, Sep 14, 2018 at 6:34 AM Stephen Mallette < >> > > > [email protected]> >> > > > >> > wrote: >> > > > >> > >> > > > >> >> I think groovy-sql was there because there was a time when I >> was >> > > into >> > > > >> >> polyglot data transforms in Gremlin and groovy-sql enabled >> you to >> > > > >> connect >> > > > >> >> to JDBC data sources in a nice way. So, I included groovy-sql >> as >> > a >> > > > >> >> convenience. I'm not against removing it, though I think we >> > should >> > > > >> remove >> > > > >> >> it in 3.4.0 only in case someone is currently depending on it >> > > > >> indirectly. >> > > > >> >> Please try to double check that we aren't using groovy-sql in >> any >> > > of >> > > > >> the >> > > > >> >> documentation for some odd example or something. >> > > > >> >> >> > > > >> >> On Thu, Sep 13, 2018 at 5:20 PM Robert Dale < >> [email protected]> >> > > > wrote: >> > > > >> >> >> > > > >> >> > gremlin-driver has a direct dependency on groovy-sql. I >> > removed >> > > it >> > > > >> in >> > > > >> >> > master and all tests pass (docker/build.sh -i -t -n) so >> there >> > > does >> > > > >> not >> > > > >> >> > appear to be any direct or indirect usage. >> > > > >> >> > >> > > > >> >> > Why is it there? Looks like it was pulled in because of >> > > > >> >> > https://issues.apache.org/jira/browse/TINKERPOP-713 >> > > > >> >> > However, groovy-sql does not appear to be a dependency of >> > > > >> >> groovy-console or >> > > > >> >> > anything groovy (except groovy-all). Wasn't then, isn't now. >> > > > >> >> > >> > > > >> >> > Any objections to removing it? >> > > > >> >> > >> > > > >> >> > -- >> > > > >> >> > Robert Dale >> > > > >> >> > >> > > > >> >> >> > > > >> > >> > > > >> >> > > > > >> > > > >> > > -- >> > > Robert Dale >> > > >> > >> -- >> Robert Dale >> >
