To add "closure" to this discussion and appeal to everyone's concerns on this particular JIRA (https://issues.apache.org/jira/browse/GEODE-23), see (now) the *new* https://issues.apache.org/jira/browse/GEODE-27 JIRA ticket I filed to properly address the (remaining) POM issues.
Hope this helps! -j On Mon, May 18, 2015 at 1:31 PM, John Blum <[email protected]> wrote: > @Roman- perhaps this will make more sense... the title of GEODE-23 states > "*POM > file is incorrect.*" The title (which will, or rather should be used for > the eventual "commit message" no less) states nothing about "scope", either > in the Maven sense (dependency "scope") or to @Mark's comments about the > changes I am proposing being "out-of-scope" for this bug. > > Furthermore, this is not only about changing SDG from "compile" to > "provided" (an implementation detail for the fix). As @Mark describes, it > is about preventing SDG from pulling in GemFire 7.0.2 with an "exclusion" > (which technically was not used in the fix). > > This bug also mentions fixing the repository declaration(s) for Geode's > obscure *jline* dependency (missing in the patch, so the "resolution" of > GEODE-23 is what, "incomplete" at best). > > In total, it seems our "in-scope" includes more than just changing the SDG > dependency. Besides, *I* was the one that suggested > <https://svn.gemstone.com/trac/gemfire/ticket/51596> [0] SDG be marked > "provided", or *removed* all together. > > So, w.r.t. "*POM file is incorrect*", it is still *incorrect*. > > Why this matters... "when" you decide to do a release (e.g. GEODE > 1.0.0.M1) with this POM file, whatever state the POM file is in (right or > wrong) at time of release, you are stuck with it! It is inappropriate to > update the geode1.0.0.M1.pom file after it has been released, especially > given the number of hosted Maven artifact servers mirroring and caching > existing, well-known Maven repo (e.g. Maven Central) artifacts. Just saying. > > I find it odd we are debating the merits of what constitutes a fix when we > really ought to be concerned getting things done correctly. > > But, apparently I am arguing for the sake of arguing, so I'll politely bow > out now. > > > [0] - https://svn.gemstone.com/trac/gemfire/ticket/51596 > <https://svn.gemstone.com/trac/gemfire/ticket/51596#comment:10> > > > > On Mon, May 18, 2015 at 10:37 AM, Mark Bretl <[email protected]> wrote: > >> I agree also there is more work to be done, however, that is out of scope >> of this JIRA. The issue in the JIRA was that SDG was pulling in an older >> version of gemfire.jar, which is causing runtime issues. The patch fixes >> that issue. >> >> John, can you open a *new* JIRA ticket to make the modifications you are >> requesting so we can track the work better? >> >> --Mark >> >> On Mon, May 18, 2015 at 10:22 AM, Roman Shaposhnik <[email protected]> >> wrote: >> >> > On Mon, May 18, 2015 at 12:18 AM, John Blum <[email protected]> wrote: >> > > To the contrary, this POM has the potential of causing more conflicts >> > given >> > > the unnecessary declaration of dependencies. I believe if committing >> > early >> > > and often, but when it is warranted, such as with proper testing. >> > > Committing for the sake of committing is does not fuel an OS project. >> > >> > Can you, please, explain to me (hopefully in a way that I would >> understand) >> > how in the world the change that moves a dependency from compile to >> > provided could be "causing more conflicts given the unnecessary >> > declaration of >> > dependencies"? So far, on the subject of patch in question: you seem to >> be >> > making generic statements that don't help to advance anything let alone >> > "fuel an OS project". >> > >> > To be pedantic, let me re-iterate that I agree with you at a higher >> level >> > that we'd have to do additional work on top of the following patch. Its >> > just when this patch is concern you appear to be arguing for the sake >> > of arguing. >> > >> > diff --git a/gemfire-core/build.gradle b/gemfire-core/build.gradle >> > index 7639aa4..a03954a 100755 >> > --- a/gemfire-core/build.gradle >> > +++ b/gemfire-core/build.gradle >> > @@ -46,7 +46,7 @@ dependencies { >> > compile 'org.apache.logging.log4j:log4j-jcl:2.1' >> > compile 'org.slf4j:slf4j-api:1.7.7' >> > compile 'org.springframework.data:spring-data-commons:1.9.1.RELEASE' >> > - compile 'org.springframework.data:spring-data-gemfire:1.5.1.RELEASE' >> > + provided 'org.springframework.data:spring-data-gemfire:1.5.1.RELEASE' >> > compile 'org.springframework:spring-tx:3.2.12.RELEASE' >> > compile 'org.springframework.shell:spring-shell:1.0.0.RELEASE' >> > compile 'org.xerial.snappy:snappy-java:1.1.1.6' >> > >> > >> > >> > >> > > >> > > On Sun, May 17, 2015 at 2:18 PM, Roman Shaposhnik < >> [email protected]> >> > > wrote: >> > > >> > >> On Fri, May 15, 2015 at 5:48 PM, John Blum <[email protected]> wrote: >> > >> > I have a verdict... the Apache Geode "generated" Maven POM files >> > need a >> > >> lot >> > >> > of work yet. >> > >> >> > >> Sure. And thanks for looking into this. My question, however, is >> this: >> > it >> > >> seems >> > >> to me that the current patch proposed on GEODE-23 makes a tiny step >> in >> > >> the right direction. Sure it doesn't address some of the issues >> you're >> > >> referring >> > >> to, but it makes life just a tiny little bit better. So why not >> commit >> > it >> > >> while >> > >> still working on a more complete solution? >> > >> >> > >> Thanks, >> > >> Roman. >> > >> >> > >> > First off, trying to review the geode_dependencies.txt file Mark >> > attached >> > >> > was, well, cumbersome and tedious, prone to error. I'd rather us >> > >> generate >> > >> > appropriate looking POM files and use Mavens tools to inspect the >> > >> dependency >> > >> > tree (i.e. mvn dependency:tree) for each of Geode's POM files >> based on >> > >> the >> > >> > modules, similar to what Andy Wilkinson did in his Spring Boot PR >> > review >> > >> > (https://github.com/spring-projects/spring-boot/issues/2884). >> > >> > >> > >> > Second, it is confusing after running a complete build of Apache >> > Geode, >> > >> that >> > >> > I seem to have duplicate POM files.... >> > >> > >> > >> > $ find . -name "*.pom" >> > >> > >> > >> >> > >> ./gemfire-assembly/build/repo/org/apache/geode/gemfire-core/1.0.0-incubating-SNAPSHOT/gemfire-core-1.0.0-incubating-20150513.224636-1.pom >> > >> > >> > >> >> > >> ./gemfire-assembly/build/repo/org/apache/geode/gemfire-core/1.0.0-incubating-SNAPSHOT/gemfire-core-1.0.0-incubating-20150513.224913-2.pom >> > >> > >> > >> >> > >> ./gemfire-assembly/build/repo/org/apache/geode/gemfire-jgroups/1.0.0-incubating-SNAPSHOT/gemfire-jgroups-1.0.0-incubating-20150513.224637-1.pom >> > >> > >> > >> >> > >> ./gemfire-assembly/build/repo/org/apache/geode/gemfire-jgroups/1.0.0-incubating-SNAPSHOT/gemfire-jgroups-1.0.0-incubating-20150513.224914-2.pom >> > >> > >> > >> >> > >> ./gemfire-assembly/build/repo/org/apache/geode/gemfire-joptsimple/1.0.0-incubating-SNAPSHOT/gemfire-joptsimple-1.0.0-incubating-20150513.224637-1.pom >> > >> > >> > >> >> > >> ./gemfire-assembly/build/repo/org/apache/geode/gemfire-joptsimple/1.0.0-incubating-SNAPSHOT/gemfire-joptsimple-1.0.0-incubating-20150513.224914-2.pom >> > >> > >> > >> >> > >> ./gemfire-assembly/build/repo/org/apache/geode/gemfire-json/1.0.0-incubating-SNAPSHOT/gemfire-json-1.0.0-incubating-20150513.224637-1.pom >> > >> > >> > >> >> > >> ./gemfire-assembly/build/repo/org/apache/geode/gemfire-json/1.0.0-incubating-SNAPSHOT/gemfire-json-1.0.0-incubating-20150513.224914-2.pom >> > >> > ... >> > >> > .. >> > >> > . >> > >> > >> > >> > With seemingly no difference between them, e.g. ... >> > >> > >> > >> > $ diff >> > >> > >> > >> >> > >> ./gemfire-assembly/build/repo/org/apache/geode/gemfire-core/1.0.0-incubating-SNAPSHOT/gemfire-core-1.0.0-incubating-20150513.224636-1.pom >> > >> > >> > >> >> > >> ./gemfire-assembly/build/repo/org/apache/geode/gemfire-core/1.0.0-incubating-SNAPSHOT/gemfire-core-1.0.0-incubating-20150513.224913-2.pom >> > >> > $ >> > >> > >> > >> > Anyway, we have several "chores" to align the Geode and GemFire >> POMs >> > more >> > >> > closely, including, but not limited to the following... >> > >> > >> > >> > 1. The <repositories> declarations need to be cleaned up (reduced). >> > >> > >> > >> > 2. Add proper "scopes", "optional" settings and "exclusions" to the >> > >> > dependencies declared in the POM. >> > >> > >> > >> > 3. Many of the "explicitly" declared dependencies (e.g. >> spring-beans) >> > >> need >> > >> > to be removed; Maven will resolve dependencies transitively. >> > >> > >> > >> > There may be additional tasks we need to look at here; this is on >> > first >> > >> > glance. >> > >> > >> > >> > I have attached *new* GemFire 8.2 Maven POM file as a reference for >> > what >> > >> the >> > >> > Geode POM files should emulate, specifically at the "intersection" >> of >> > >> their >> > >> > dependencies. Note, I realize there are going to be (expected) >> > >> differences >> > >> > between GemFire and Geode, such as, but not limited to, removing >> XOM, >> > >> while >> > >> > adding others. >> > >> > >> > >> > Again, the GemFire 8.2 POM should serve as a model template on >> which >> > to >> > >> > basis the new Geode "generated" POM file. >> > >> > >> > >> > Mark and I will need to work together to make this happen. This is >> > not >> > >> only >> > >> > important, but crucial for projects like Spring Data GemFire, >> Spring >> > >> Boot, >> > >> > and (potentially) many others that will develop applications for >> > Apache >> > >> > Geode. >> > >> > >> > >> > So let's get it right! >> > >> > >> > >> > Thanks, >> > >> > John >> > >> > >> > >> > >> > >> > >> > >> > On Fri, May 15, 2015 at 3:23 PM, Anthony Baker <[email protected]> >> > >> wrote: >> > >> >> >> > >> >> I believe John is pointing out that the Geode POM may have similar >> > >> issues >> > >> >> as were discovered by GemFire users (after all, Geode was recently >> > born >> > >> from >> > >> >> GemFire). >> > >> >> >> > >> >> However I believe those issues are limited to certain use cases >> (e.g. >> > >> >> spring boot). In GEODE-23 the POM is unusable without adding >> exclude >> > >> >> clauses. IMHO, we should fix this now and have a separate >> discussion >> > >> around >> > >> >> making library versions consistent. >> > >> >> >> > >> >> Anthony >> > >> >> >> > >> >> >> > >> >> > On May 15, 2015, at 11:24 AM, Roman Shaposhnik < >> > [email protected]> >> > >> >> > wrote: >> > >> >> > >> > >> >> > Part of the urgency is that we now have nightly artifacts >> > >> >> > that are not easily consumable. >> > >> >> > >> > >> >> > On top of that, I see no reason to hold up anything in >> > >> >> > the Apache project because of a commercial product >> > >> >> > requirements. The way you explained your refactoring >> > >> >> > sounds like it benefits a commercial product. Am I getting >> > >> >> > this wrong? >> > >> >> > >> > >> >> > Thanks, >> > >> >> > Roman. >> > >> >> > >> > >> >> > On Fri, May 15, 2015 at 11:08 AM, John Blum <[email protected]> >> > wrote: >> > >> >> >> I am in the process of refactoring GemFire's (8.0, 8.1 and 8.2) >> > Maven >> > >> >> >> POM >> > >> >> >> files now (along with juggling 5 other things). >> > >> >> >> >> > >> >> >> What is this exactly holding up (what's the urgency)? I don't >> > see an >> > >> >> >> issue >> > >> >> >> with delaying this PR until it is properly reviewed. An >> improper >> > POM >> > >> >> >> file >> > >> >> >> does cause issues for "consumers". >> > >> >> >> >> > >> >> >> I should be able to get to this by EOD. >> > >> >> >> >> > >> >> >> Thanks! >> > >> >> >> John >> > >> >> >> >> > >> >> >> >> > >> >> >> On Fri, May 15, 2015 at 10:30 AM, Roman Shaposhnik >> > >> >> >> <[email protected]> >> > >> >> >> wrote: >> > >> >> >> >> > >> >> >>> Big +1 to that. Commit early commit often. We can always >> > >> >> >>> change it later if needed too. >> > >> >> >>> >> > >> >> >>> Thanks, >> > >> >> >>> Roman. >> > >> >> >>> >> > >> >> >>> On Fri, May 15, 2015 at 8:01 AM, Anthony Baker < >> > [email protected]> >> > >> >> >>> wrote: >> > >> >> >>>> John, have you had a chance to review Mark’s change and the >> > >> generated >> > >> >> >>> POM? I think that we could address updates to library >> versions >> > >> >> >>> through a >> > >> >> >>> follow up issue. >> > >> >> >>>> >> > >> >> >>>> Anthony >> > >> >> >>>> >> > >> >> >>>>> On May 13, 2015, at 10:22 AM, John Blum <[email protected]> >> > wrote: >> > >> >> >>>>> >> > >> >> >>>>> Mark, >> > >> >> >>>>> >> > >> >> >>>>> I plan on working on cleaning up the GemFire Maven POM today >> > for >> > >> >> >>>>> 8.0.0, >> > >> >> >>>>> 8.1.0 and 8.2.0, and we need to make sure that Apache >> Geode's >> > >> Maven >> > >> >> >>>>> POM >> > >> >> >>>>> matches or is similar to clean up the other issues that >> *Andy >> > >> >> >>>>> Wilkinson* >> > >> >> >>>>> <https://github.com/spring-projects/spring-boot/issues/2884 >> > >> > [0] >> > >> >> >>>>> sited. >> > >> >> >>>>> >> > >> >> >>>>> Please hold off on the commit until I get the chance to >> review. >> > >> >> >>>>> >> > >> >> >>>>> Thanks, >> > >> >> >>>>> John >> > >> >> >>>>> >> > >> >> >>>>> >> > >> >> >>>>> On Wed, May 13, 2015 at 10:17 AM, Mark Bretl < >> > [email protected]> >> > >> >> >>>>> wrote: >> > >> >> >>>>> >> > >> >> >>>>>> Hi All, >> > >> >> >>>>>> >> > >> >> >>>>>> I have tried to follow the email threads on the review >> > process, >> > >> not >> > >> >> >>> exactly >> > >> >> >>>>>> sure what to do in this case. I am requesting review of >> > GEODE-23 >> > >> >> >>>>>> and my >> > >> >> >>>>>> patch is attached to the JIRA. It is a one line change >> which >> > >> >> >>>>>> removes >> > >> >> >>> Spring >> > >> >> >>>>>> Data GemFire from the Maven POM. >> > >> >> >>>>>> >> > >> >> >>>>>> I would like to use a 72 hour lazy consensus. >> > >> >> >>>>>> >> > >> >> >>>>>> Best Regards, >> > >> >> >>>>>> >> > >> >> >>>>>> -- >> > >> >> >>>>>> Mark Bretl >> > >> >> >>>>>> Software Build Engineer >> > >> >> >>>>>> Pivotal >> > >> >> >>>>>> 503-533-3869 >> > >> >> >>>>>> www.pivotal.io >> > >> >> >>>>>> >> > >> >> >>>>> >> > >> >> >>>>> >> > >> >> >>>>> >> > >> >> >>>>> -- >> > >> >> >>>>> -John >> > >> >> >>>>> 503-504-8657 >> > >> >> >>>>> john.blum10101 (skype) >> > >> >> >>>> >> > >> >> >>> >> > >> >> >> >> > >> >> >> >> > >> >> >> >> > >> >> >> -- >> > >> >> >> -John >> > >> >> >> 503-504-8657 >> > >> >> >> john.blum10101 (skype) >> > >> >> >> > >> > >> > >> > >> > >> > >> > >> > -- >> > >> > -John >> > >> > 503-504-8657 >> > >> > john.blum10101 (skype) >> > >> >> > > >> > > >> > > >> > > -- >> > > -John >> > > 503-504-8657 >> > > john.blum10101 (skype) >> > >> >> >> >> -- >> Mark Bretl >> Software Build Engineer >> Pivotal >> 503-533-3869 >> www.pivotal.io >> > > > > -- > -John > 503-504-8657 > john.blum10101 (skype) > -- -John 503-504-8657 john.blum10101 (skype)
