You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@geode.apache.org by Dan Smith <ds...@pivotal.io> on 2016/03/15 22:43:31 UTC
Review Request 44865: GEODE-27: Enabling transitive dependencie and
marking jars as optional in the generated pom
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/44865/
-----------------------------------------------------------
Review request for geode, Anthony Baker, Jason Huynh, Jens Deppe, and Mark Bretl.
Repository: geode
Description
-------
This is the first step in cleaning up the pom. Enabling transitive dependencies allows us to reduce the number of dependencies we add, as well as making it easier to change the dependencies because we don't have few dependencies in our build file that aren't direct dependencies of geode.
Adding excludes and forcing some versions to clean up the list of jars
we are pulling in to match what it was before I enabled transitive
dependencies.
Some of these excludes and all of the forced versions look suspect.
They need to be reviewed to see if we really need them.
Adding support to mark jars as optional by setting an ext.optional
property in the dependency declaration.
Adding the optional property to jars in the geode-core project that are
not required for geode to work.
Some of these optional dependencies will go away if we can move things
like the REST API, gfsh, and management classes into another subproject.
Diffs
-----
build.gradle 85e5c3564e0a347f4442d064f800f45822d53302
extensions/geode-modules-hibernate/build.gradle PRE-CREATION
extensions/geode-modules-tomcat7/build.gradle PRE-CREATION
extensions/geode-modules/build.gradle 1f9bff8d7d37700627ca1b6e8aa74827d59b76d4
geode-assembly/build.gradle 5930f13c5307e0cd70396b019263176a377d415a
geode-core/build.gradle 041dc07c860c008f117d37969ee688375c2a348d
geode-core/src/main/java/com/gemstone/gemfire/cache/query/internal/CumulativeNonDistinctResults.java 3735956f3f96f1d7a4fe4a7d2d9119947154a17f
geode-junit/build.gradle PRE-CREATION
geode-lucene/build.gradle 6218e55c73f771a46a74ae31606ce1ace5b2008f
geode-pulse/build.gradle PRE-CREATION
geode-rebalancer/build.gradle e3ba6abb47fee30f9b3fca978aac51c8b4ee95d9
geode-web-api/build.gradle 940fddd7b653ca3fc49916b6ff463c89f6e2f4aa
geode-web/build.gradle 596590b1c96e016e2dd927a21dc50d2e373b62b2
gradle/dependency-resolution.gradle PRE-CREATION
gradle/java.gradle PRE-CREATION
gradle/publish.gradle PRE-CREATION
Diff: https://reviews.apache.org/r/44865/diff/
Testing
-------
Thanks,
Dan Smith
Re: Review Request 44865: GEODE-27: Enabling transitive dependencie
and marking jars as optional in the generated pom
Posted by Mark Bretl <mb...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/44865/#review124075
-----------------------------------------------------------
Ship it!
Ship It!
- Mark Bretl
On March 15, 2016, 2:43 p.m., Dan Smith wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44865/
> -----------------------------------------------------------
>
> (Updated March 15, 2016, 2:43 p.m.)
>
>
> Review request for geode, Anthony Baker, Jason Huynh, Jens Deppe, and Mark Bretl.
>
>
> Repository: geode
>
>
> Description
> -------
>
> This is the first step in cleaning up the pom. Enabling transitive dependencies allows us to reduce the number of dependencies we add, as well as making it easier to change the dependencies because we don't have few dependencies in our build file that aren't direct dependencies of geode.
>
> Adding excludes and forcing some versions to clean up the list of jars
> we are pulling in to match what it was before I enabled transitive
> dependencies.
>
> Some of these excludes and all of the forced versions look suspect.
> They need to be reviewed to see if we really need them.
>
>
> Adding support to mark jars as optional by setting an ext.optional
> property in the dependency declaration.
>
> Adding the optional property to jars in the geode-core project that are
> not required for geode to work.
>
> Some of these optional dependencies will go away if we can move things
> like the REST API, gfsh, and management classes into another subproject.
>
>
> Diffs
> -----
>
> build.gradle 85e5c3564e0a347f4442d064f800f45822d53302
> extensions/geode-modules-hibernate/build.gradle PRE-CREATION
> extensions/geode-modules-tomcat7/build.gradle PRE-CREATION
> extensions/geode-modules/build.gradle 1f9bff8d7d37700627ca1b6e8aa74827d59b76d4
> geode-assembly/build.gradle 5930f13c5307e0cd70396b019263176a377d415a
> geode-core/build.gradle 041dc07c860c008f117d37969ee688375c2a348d
> geode-core/src/main/java/com/gemstone/gemfire/cache/query/internal/CumulativeNonDistinctResults.java 3735956f3f96f1d7a4fe4a7d2d9119947154a17f
> geode-junit/build.gradle PRE-CREATION
> geode-lucene/build.gradle 6218e55c73f771a46a74ae31606ce1ace5b2008f
> geode-pulse/build.gradle PRE-CREATION
> geode-rebalancer/build.gradle e3ba6abb47fee30f9b3fca978aac51c8b4ee95d9
> geode-web-api/build.gradle 940fddd7b653ca3fc49916b6ff463c89f6e2f4aa
> geode-web/build.gradle 596590b1c96e016e2dd927a21dc50d2e373b62b2
> gradle/dependency-resolution.gradle PRE-CREATION
> gradle/java.gradle PRE-CREATION
> gradle/publish.gradle PRE-CREATION
>
> Diff: https://reviews.apache.org/r/44865/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Dan Smith
>
>
Re: Review Request 44865: GEODE-27: Enabling transitive dependencie
and marking jars as optional in the generated pom
Posted by Dan Smith <ds...@pivotal.io>.
> On March 16, 2016, 9:23 p.m., Mark Bretl wrote:
> > gradle/publish.gradle, lines 63-67
> > <https://reviews.apache.org/r/44865/diff/1/?file=1300248#file1300248line63>
> >
> > More of a clarification question trying to understand this 'black magic'...
> >
> > foreach dependency which has 'optional' property, check if.optional = true, then set a value to true otherwise add the optional node?
> >
> > Am I reading that right?
> >
> > Feels like if the dependency has the optional property, then add the optional node for the Maven POM.
> >
> > I see this solution in GRADLE-1749 issue, but still doesn't help the black magic :)
> >
> > Does the gradle-extra-configurations-plugin help in this situation?
>
> Dan Smith wrote:
> You read that right.
>
> Hmm, that gradle-extra-configurations plugin might do the trick, I'll have to try that. Part of the trickiness is that we're using a different maven publishing plugin that built in one, so I don't know if they will play nicely together. But if they do, maybe we don't need to create our own provided scope either - we're using that all over the place too...
I tested the gradle-extra-configurations plugin to see if the optional flag would work. But it doesn't look like it plays well with our different maven publishing plugin - https://github.com/bmuschko/gradle-nexus-plugin. The optional flag was not added to the entries in the pom.
- Dan
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/44865/#review123933
-----------------------------------------------------------
On March 15, 2016, 9:43 p.m., Dan Smith wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44865/
> -----------------------------------------------------------
>
> (Updated March 15, 2016, 9:43 p.m.)
>
>
> Review request for geode, Anthony Baker, Jason Huynh, Jens Deppe, and Mark Bretl.
>
>
> Repository: geode
>
>
> Description
> -------
>
> This is the first step in cleaning up the pom. Enabling transitive dependencies allows us to reduce the number of dependencies we add, as well as making it easier to change the dependencies because we don't have few dependencies in our build file that aren't direct dependencies of geode.
>
> Adding excludes and forcing some versions to clean up the list of jars
> we are pulling in to match what it was before I enabled transitive
> dependencies.
>
> Some of these excludes and all of the forced versions look suspect.
> They need to be reviewed to see if we really need them.
>
>
> Adding support to mark jars as optional by setting an ext.optional
> property in the dependency declaration.
>
> Adding the optional property to jars in the geode-core project that are
> not required for geode to work.
>
> Some of these optional dependencies will go away if we can move things
> like the REST API, gfsh, and management classes into another subproject.
>
>
> Diffs
> -----
>
> build.gradle 85e5c3564e0a347f4442d064f800f45822d53302
> extensions/geode-modules-hibernate/build.gradle PRE-CREATION
> extensions/geode-modules-tomcat7/build.gradle PRE-CREATION
> extensions/geode-modules/build.gradle 1f9bff8d7d37700627ca1b6e8aa74827d59b76d4
> geode-assembly/build.gradle 5930f13c5307e0cd70396b019263176a377d415a
> geode-core/build.gradle 041dc07c860c008f117d37969ee688375c2a348d
> geode-core/src/main/java/com/gemstone/gemfire/cache/query/internal/CumulativeNonDistinctResults.java 3735956f3f96f1d7a4fe4a7d2d9119947154a17f
> geode-junit/build.gradle PRE-CREATION
> geode-lucene/build.gradle 6218e55c73f771a46a74ae31606ce1ace5b2008f
> geode-pulse/build.gradle PRE-CREATION
> geode-rebalancer/build.gradle e3ba6abb47fee30f9b3fca978aac51c8b4ee95d9
> geode-web-api/build.gradle 940fddd7b653ca3fc49916b6ff463c89f6e2f4aa
> geode-web/build.gradle 596590b1c96e016e2dd927a21dc50d2e373b62b2
> gradle/dependency-resolution.gradle PRE-CREATION
> gradle/java.gradle PRE-CREATION
> gradle/publish.gradle PRE-CREATION
>
> Diff: https://reviews.apache.org/r/44865/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Dan Smith
>
>
Re: Review Request 44865: GEODE-27: Enabling transitive dependencie
and marking jars as optional in the generated pom
Posted by Dan Smith <ds...@pivotal.io>.
> On March 16, 2016, 9:23 p.m., Mark Bretl wrote:
> > gradle/publish.gradle, lines 63-67
> > <https://reviews.apache.org/r/44865/diff/1/?file=1300248#file1300248line63>
> >
> > More of a clarification question trying to understand this 'black magic'...
> >
> > foreach dependency which has 'optional' property, check if.optional = true, then set a value to true otherwise add the optional node?
> >
> > Am I reading that right?
> >
> > Feels like if the dependency has the optional property, then add the optional node for the Maven POM.
> >
> > I see this solution in GRADLE-1749 issue, but still doesn't help the black magic :)
> >
> > Does the gradle-extra-configurations-plugin help in this situation?
You read that right.
Hmm, that gradle-extra-configurations plugin might do the trick, I'll have to try that. Part of the trickiness is that we're using a different maven publishing plugin that built in one, so I don't know if they will play nicely together. But if they do, maybe we don't need to create our own provided scope either - we're using that all over the place too...
- Dan
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/44865/#review123933
-----------------------------------------------------------
On March 15, 2016, 9:43 p.m., Dan Smith wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44865/
> -----------------------------------------------------------
>
> (Updated March 15, 2016, 9:43 p.m.)
>
>
> Review request for geode, Anthony Baker, Jason Huynh, Jens Deppe, and Mark Bretl.
>
>
> Repository: geode
>
>
> Description
> -------
>
> This is the first step in cleaning up the pom. Enabling transitive dependencies allows us to reduce the number of dependencies we add, as well as making it easier to change the dependencies because we don't have few dependencies in our build file that aren't direct dependencies of geode.
>
> Adding excludes and forcing some versions to clean up the list of jars
> we are pulling in to match what it was before I enabled transitive
> dependencies.
>
> Some of these excludes and all of the forced versions look suspect.
> They need to be reviewed to see if we really need them.
>
>
> Adding support to mark jars as optional by setting an ext.optional
> property in the dependency declaration.
>
> Adding the optional property to jars in the geode-core project that are
> not required for geode to work.
>
> Some of these optional dependencies will go away if we can move things
> like the REST API, gfsh, and management classes into another subproject.
>
>
> Diffs
> -----
>
> build.gradle 85e5c3564e0a347f4442d064f800f45822d53302
> extensions/geode-modules-hibernate/build.gradle PRE-CREATION
> extensions/geode-modules-tomcat7/build.gradle PRE-CREATION
> extensions/geode-modules/build.gradle 1f9bff8d7d37700627ca1b6e8aa74827d59b76d4
> geode-assembly/build.gradle 5930f13c5307e0cd70396b019263176a377d415a
> geode-core/build.gradle 041dc07c860c008f117d37969ee688375c2a348d
> geode-core/src/main/java/com/gemstone/gemfire/cache/query/internal/CumulativeNonDistinctResults.java 3735956f3f96f1d7a4fe4a7d2d9119947154a17f
> geode-junit/build.gradle PRE-CREATION
> geode-lucene/build.gradle 6218e55c73f771a46a74ae31606ce1ace5b2008f
> geode-pulse/build.gradle PRE-CREATION
> geode-rebalancer/build.gradle e3ba6abb47fee30f9b3fca978aac51c8b4ee95d9
> geode-web-api/build.gradle 940fddd7b653ca3fc49916b6ff463c89f6e2f4aa
> geode-web/build.gradle 596590b1c96e016e2dd927a21dc50d2e373b62b2
> gradle/dependency-resolution.gradle PRE-CREATION
> gradle/java.gradle PRE-CREATION
> gradle/publish.gradle PRE-CREATION
>
> Diff: https://reviews.apache.org/r/44865/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Dan Smith
>
>
Re: Review Request 44865: GEODE-27: Enabling transitive dependencie
and marking jars as optional in the generated pom
Posted by Anthony Baker <ab...@apache.org>.
> On March 16, 2016, 9:23 p.m., Mark Bretl wrote:
> > gradle/publish.gradle, lines 63-67
> > <https://reviews.apache.org/r/44865/diff/1/?file=1300248#file1300248line63>
> >
> > More of a clarification question trying to understand this 'black magic'...
> >
> > foreach dependency which has 'optional' property, check if.optional = true, then set a value to true otherwise add the optional node?
> >
> > Am I reading that right?
> >
> > Feels like if the dependency has the optional property, then add the optional node for the Maven POM.
> >
> > I see this solution in GRADLE-1749 issue, but still doesn't help the black magic :)
> >
> > Does the gradle-extra-configurations-plugin help in this situation?
>
> Dan Smith wrote:
> You read that right.
>
> Hmm, that gradle-extra-configurations plugin might do the trick, I'll have to try that. Part of the trickiness is that we're using a different maven publishing plugin that built in one, so I don't know if they will play nicely together. But if they do, maybe we don't need to create our own provided scope either - we're using that all over the place too...
>
> Dan Smith wrote:
> I tested the gradle-extra-configurations plugin to see if the optional flag would work. But it doesn't look like it plays well with our different maven publishing plugin - https://github.com/bmuschko/gradle-nexus-plugin. The optional flag was not added to the entries in the pom.
We aren't using the newer publishing plugin because it isn't able to sign poms or upload signature files. In short, it's still really immature despite being "incubating" for several years.
- Anthony
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/44865/#review123933
-----------------------------------------------------------
On March 15, 2016, 9:43 p.m., Dan Smith wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44865/
> -----------------------------------------------------------
>
> (Updated March 15, 2016, 9:43 p.m.)
>
>
> Review request for geode, Anthony Baker, Jason Huynh, Jens Deppe, and Mark Bretl.
>
>
> Repository: geode
>
>
> Description
> -------
>
> This is the first step in cleaning up the pom. Enabling transitive dependencies allows us to reduce the number of dependencies we add, as well as making it easier to change the dependencies because we don't have few dependencies in our build file that aren't direct dependencies of geode.
>
> Adding excludes and forcing some versions to clean up the list of jars
> we are pulling in to match what it was before I enabled transitive
> dependencies.
>
> Some of these excludes and all of the forced versions look suspect.
> They need to be reviewed to see if we really need them.
>
>
> Adding support to mark jars as optional by setting an ext.optional
> property in the dependency declaration.
>
> Adding the optional property to jars in the geode-core project that are
> not required for geode to work.
>
> Some of these optional dependencies will go away if we can move things
> like the REST API, gfsh, and management classes into another subproject.
>
>
> Diffs
> -----
>
> build.gradle 85e5c3564e0a347f4442d064f800f45822d53302
> extensions/geode-modules-hibernate/build.gradle PRE-CREATION
> extensions/geode-modules-tomcat7/build.gradle PRE-CREATION
> extensions/geode-modules/build.gradle 1f9bff8d7d37700627ca1b6e8aa74827d59b76d4
> geode-assembly/build.gradle 5930f13c5307e0cd70396b019263176a377d415a
> geode-core/build.gradle 041dc07c860c008f117d37969ee688375c2a348d
> geode-core/src/main/java/com/gemstone/gemfire/cache/query/internal/CumulativeNonDistinctResults.java 3735956f3f96f1d7a4fe4a7d2d9119947154a17f
> geode-junit/build.gradle PRE-CREATION
> geode-lucene/build.gradle 6218e55c73f771a46a74ae31606ce1ace5b2008f
> geode-pulse/build.gradle PRE-CREATION
> geode-rebalancer/build.gradle e3ba6abb47fee30f9b3fca978aac51c8b4ee95d9
> geode-web-api/build.gradle 940fddd7b653ca3fc49916b6ff463c89f6e2f4aa
> geode-web/build.gradle 596590b1c96e016e2dd927a21dc50d2e373b62b2
> gradle/dependency-resolution.gradle PRE-CREATION
> gradle/java.gradle PRE-CREATION
> gradle/publish.gradle PRE-CREATION
>
> Diff: https://reviews.apache.org/r/44865/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Dan Smith
>
>
Re: Review Request 44865: GEODE-27: Enabling transitive dependencie
and marking jars as optional in the generated pom
Posted by Mark Bretl <mb...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/44865/#review123933
-----------------------------------------------------------
gradle/publish.gradle (lines 63 - 67)
<https://reviews.apache.org/r/44865/#comment186263>
More of a clarification question trying to understand this 'black magic'...
foreach dependency which has 'optional' property, check if.optional = true, then set a value to true otherwise add the optional node?
Am I reading that right?
Feels like if the dependency has the optional property, then add the optional node for the Maven POM.
I see this solution in GRADLE-1749 issue, but still doesn't help the black magic :)
Does the gradle-extra-configurations-plugin help in this situation?
- Mark Bretl
On March 15, 2016, 2:43 p.m., Dan Smith wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44865/
> -----------------------------------------------------------
>
> (Updated March 15, 2016, 2:43 p.m.)
>
>
> Review request for geode, Anthony Baker, Jason Huynh, Jens Deppe, and Mark Bretl.
>
>
> Repository: geode
>
>
> Description
> -------
>
> This is the first step in cleaning up the pom. Enabling transitive dependencies allows us to reduce the number of dependencies we add, as well as making it easier to change the dependencies because we don't have few dependencies in our build file that aren't direct dependencies of geode.
>
> Adding excludes and forcing some versions to clean up the list of jars
> we are pulling in to match what it was before I enabled transitive
> dependencies.
>
> Some of these excludes and all of the forced versions look suspect.
> They need to be reviewed to see if we really need them.
>
>
> Adding support to mark jars as optional by setting an ext.optional
> property in the dependency declaration.
>
> Adding the optional property to jars in the geode-core project that are
> not required for geode to work.
>
> Some of these optional dependencies will go away if we can move things
> like the REST API, gfsh, and management classes into another subproject.
>
>
> Diffs
> -----
>
> build.gradle 85e5c3564e0a347f4442d064f800f45822d53302
> extensions/geode-modules-hibernate/build.gradle PRE-CREATION
> extensions/geode-modules-tomcat7/build.gradle PRE-CREATION
> extensions/geode-modules/build.gradle 1f9bff8d7d37700627ca1b6e8aa74827d59b76d4
> geode-assembly/build.gradle 5930f13c5307e0cd70396b019263176a377d415a
> geode-core/build.gradle 041dc07c860c008f117d37969ee688375c2a348d
> geode-core/src/main/java/com/gemstone/gemfire/cache/query/internal/CumulativeNonDistinctResults.java 3735956f3f96f1d7a4fe4a7d2d9119947154a17f
> geode-junit/build.gradle PRE-CREATION
> geode-lucene/build.gradle 6218e55c73f771a46a74ae31606ce1ace5b2008f
> geode-pulse/build.gradle PRE-CREATION
> geode-rebalancer/build.gradle e3ba6abb47fee30f9b3fca978aac51c8b4ee95d9
> geode-web-api/build.gradle 940fddd7b653ca3fc49916b6ff463c89f6e2f4aa
> geode-web/build.gradle 596590b1c96e016e2dd927a21dc50d2e373b62b2
> gradle/dependency-resolution.gradle PRE-CREATION
> gradle/java.gradle PRE-CREATION
> gradle/publish.gradle PRE-CREATION
>
> Diff: https://reviews.apache.org/r/44865/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Dan Smith
>
>