@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)
