Re: [Discuss] Transitive dependencies and internal .pom changes

[Patrick Rhomberg <pr...@apache.org>]

> Patrick, you just mentioned the java-library plugin's ability to
> distinguish between api and implementation dependencies. Geode doesn't use
> that plugin yet AFAIK, nor does the PR introduce that plugin right?

That is correct, we do not yet make use of the `java-library` plugin (which
has the `implementation` and `api` configurations) and are still using the
`java` plugin (and rely on the `compile` configurations).  I brought it up
only to mention that there will be a great deal of inter-module build
breakages if we were to try to transition to using `implementation` while
our actual dependencies of each module were not yet explicit.

And yeah, all of this is on the road to Java9 modules.

As for accepting this PR, there are still multiple issues with it as it
stands -- I mostly opened it in its current state to spur this discussion.
I'm was going to go open a handful of tickets so that each individual PR
and commit are more bite-sized.

On Fri, Sep 28, 2018 at 4:26 PM, Bill Burcham <bb...@pivotal.io> wrote:

> So it sounds like (per Robert) we use the gradle-lint-plugin's
> unused-dependency rule to warn us of unused dependencies. That handles one
> side of the equation (only list *necessary* dependencies).
>
> I haven't heard anyone mention tool support for the other side of the
> equation: ensuring that we list the *sufficient* list of dependencies per
> build.gradle.
>
> Patrick, you just mentioned the java-library plugin's ability to
> distinguish between api and implementation dependencies. Geode doesn't use
> that plugin yet AFAIK, nor does the PR introduce that plugin right?
>
> All this seems of a piece with the march toward modularity, exemplified by
> Java 9 modules. Patrick, is your vision something like:
>
> 1. accept this PR
> …
> 2. introduce the java-library plugin and segregate dependencies into api
> vs. implementation ones
> …
> N. define Java modules
>
>
> On Fri, Sep 28, 2018 at 10:39 AM Patrick Rhomberg <pr...@apache.org>
> wrote:
>
> > Bill has the heart of it, yes.
> >
> > I should have also mentioned that this ties into java-library plugin
> > configuration, notably that the `compile` configuration is deprecated.
> > For dependences that we do not wish to leak, we will need to use
> > `implementation`.  For dependencies which we intentionally elect to share
> > with our consumers, we should use `api`.
> >
> > For modularity, it should not break one module's ability to build for
> > another module to declare a `compile` configuration to be
> `implementation`,
> > unless of course that dependency is an active and necessary component
> part
> > of that module's consumption.  In that case, it should be declared part
> of
> > the modules `api`.
> > In this vein and as Bill pointed out, a module should have an accurate
> > representation of its own build-time dependencies.
> >
> > In time, this will allow a much improved build experience, since
> > `implementation` and `api` are positioned for better granularity of
> > incremental building.
> >
> > On Fri, Sep 28, 2018 at 10:23 AM, Robert Houghton <rh...@pivotal.io>
> > wrote:
> >
> > > Hi Bill,
> > >
> > > We are using a Gradle plug-in to identify dependencies that are unused,
> > or
> > > are declared in the wrong module or scope.
> > > This is called out by the Gradle documents on improving build [
> > > https://guides.gradle.org/performance/#avoid_unnecessary_and_unused_
> > > dependencies].
> > > The plug-in documentation is here [
> > > https://github.com/nebula-plugins/gradle-lint-plugin/
> > > wiki/Unused-Dependency-Rule
> > > ]
> > >
> > > Thank you,
> > > -Robert Houghton
> > >
> > > On Fri, Sep 28, 2018 at 10:18 AM Bill Burcham <bb...@pivotal.io>
> > wrote:
> > >
> > > > From the PR, Anthony, it seems to me that Patrick is proposing that
> > each
> > > > build.gradle be explicit about mentioning the "things" it depends on.
> > For
> > > > example:
> > > >
> > > > [image: image.png]
> > > >
> > > > Look at how geode-connectors goes from mentioning a few dependencies
> to
> > > > mentioning many more. The value of this is that it ostensibly shows
> us
> > > all
> > > > the things that geode-connectors actually depends on.
> > > >
> > > > The challenge I see is two-fold:
> > > >
> > > > • as with Java imports and "unused imports", there is the risk of
> > listing
> > > > more dependencies than we actually need
> > > > • there's also the risk that we still don't have a complete list of
> > > > dependencies
> > > >
> > > > Patrick: do you have tool support for this? Is there a tool that can
> > > > identify or even remove unused dependencies? What process do you use
> > for
> > > > finding these heretofore hidden dependencies? Did you run gradle
> > > > app:dependencies?
> > > >
> > >
> >
>

Re: [Discuss] Transitive dependencies and internal .pom changes

[Anthony Baker <ab...@pivotal.io>]

I think it would be easier to review and discuss these changes with smaller, more focused PR’s.  Is there a way to split up these changes?  That would help me a lot and I would be less concerned about introducing a problem with our POM’s / dependencies.

Thanks,
Anthony


> On Oct 1, 2018, at 2:04 PM, Dan Smith <ds...@pivotal.io> wrote:
> 
> If what we are doing is just making sure that each geode module is
> explicitly declaring all of its first order dependencies (things that
> directly show up in import statements) in its build.gradle, then this seems
> good to me.
> 
> I don't think we want to stop using gradle's ability to pull in all of the
> transitive implementation dependencies that we don't directly use. We don't
> want to get into the business of maintaining those manually.
> 
> Removing unused dependencies and pulling in this lint plugin also seem like
> good improvements.
> 
> -Dan
> 
> On Mon, Oct 1, 2018 at 1:44 PM Patrick Rhomberg <pr...@apache.org>
> wrote:
> 
>>> Patrick, you just mentioned the java-library plugin's ability to
>>> distinguish between api and implementation dependencies. Geode doesn't
>> use
>>> that plugin yet AFAIK, nor does the PR introduce that plugin right?
>> 
>> That is correct, we do not yet make use of the `java-library` plugin (which
>> has the `implementation` and `api` configurations) and are still using the
>> `java` plugin (and rely on the `compile` configurations).  I brought it up
>> only to mention that there will be a great deal of inter-module build
>> breakages if we were to try to transition to using `implementation` while
>> our actual dependencies of each module were not yet explicit.
>> 
>> And yeah, all of this is on the road to Java9 modules.
>> 
>> As for accepting this PR, there are still multiple issues with it as it
>> stands -- I mostly opened it in its current state to spur this discussion.
>> I'm was going to go open a handful of tickets so that each individual PR
>> and commit are more bite-sized.
>> 
>> On Fri, Sep 28, 2018 at 4:26 PM, Bill Burcham <bb...@pivotal.io> wrote:
>> 
>>> So it sounds like (per Robert) we use the gradle-lint-plugin's
>>> unused-dependency rule to warn us of unused dependencies. That handles
>> one
>>> side of the equation (only list *necessary* dependencies).
>>> 
>>> I haven't heard anyone mention tool support for the other side of the
>>> equation: ensuring that we list the *sufficient* list of dependencies per
>>> build.gradle.
>>> 
>>> Patrick, you just mentioned the java-library plugin's ability to
>>> distinguish between api and implementation dependencies. Geode doesn't
>> use
>>> that plugin yet AFAIK, nor does the PR introduce that plugin right?
>>> 
>>> All this seems of a piece with the march toward modularity, exemplified
>> by
>>> Java 9 modules. Patrick, is your vision something like:
>>> 
>>> 1. accept this PR
>>> …
>>> 2. introduce the java-library plugin and segregate dependencies into api
>>> vs. implementation ones
>>> …
>>> N. define Java modules
>>> 
>>> 
>>> On Fri, Sep 28, 2018 at 10:39 AM Patrick Rhomberg <pr...@apache.org>
>>> wrote:
>>> 
>>>> Bill has the heart of it, yes.
>>>> 
>>>> I should have also mentioned that this ties into java-library plugin
>>>> configuration, notably that the `compile` configuration is deprecated.
>>>> For dependences that we do not wish to leak, we will need to use
>>>> `implementation`.  For dependencies which we intentionally elect to
>> share
>>>> with our consumers, we should use `api`.
>>>> 
>>>> For modularity, it should not break one module's ability to build for
>>>> another module to declare a `compile` configuration to be
>>> `implementation`,
>>>> unless of course that dependency is an active and necessary component
>>> part
>>>> of that module's consumption.  In that case, it should be declared part
>>> of
>>>> the modules `api`.
>>>> In this vein and as Bill pointed out, a module should have an accurate
>>>> representation of its own build-time dependencies.
>>>> 
>>>> In time, this will allow a much improved build experience, since
>>>> `implementation` and `api` are positioned for better granularity of
>>>> incremental building.
>>>> 
>>>> On Fri, Sep 28, 2018 at 10:23 AM, Robert Houghton <
>> rhoughton@pivotal.io>
>>>> wrote:
>>>> 
>>>>> Hi Bill,
>>>>> 
>>>>> We are using a Gradle plug-in to identify dependencies that are
>> unused,
>>>> or
>>>>> are declared in the wrong module or scope.
>>>>> This is called out by the Gradle documents on improving build [
>>>>> https://guides.gradle.org/performance/#avoid_unnecessary_and_unused_
>>>>> dependencies].
>>>>> The plug-in documentation is here [
>>>>> https://github.com/nebula-plugins/gradle-lint-plugin/
>>>>> wiki/Unused-Dependency-Rule
>>>>> ]
>>>>> 
>>>>> Thank you,
>>>>> -Robert Houghton
>>>>> 
>>>>> On Fri, Sep 28, 2018 at 10:18 AM Bill Burcham <bb...@pivotal.io>
>>>> wrote:
>>>>> 
>>>>>> From the PR, Anthony, it seems to me that Patrick is proposing that
>>>> each
>>>>>> build.gradle be explicit about mentioning the "things" it depends
>> on.
>>>> For
>>>>>> example:
>>>>>> 
>>>>>> [image: image.png]
>>>>>> 
>>>>>> Look at how geode-connectors goes from mentioning a few
>> dependencies
>>> to
>>>>>> mentioning many more. The value of this is that it ostensibly shows
>>> us
>>>>> all
>>>>>> the things that geode-connectors actually depends on.
>>>>>> 
>>>>>> The challenge I see is two-fold:
>>>>>> 
>>>>>> • as with Java imports and "unused imports", there is the risk of
>>>> listing
>>>>>> more dependencies than we actually need
>>>>>> • there's also the risk that we still don't have a complete list of
>>>>>> dependencies
>>>>>> 
>>>>>> Patrick: do you have tool support for this? Is there a tool that
>> can
>>>>>> identify or even remove unused dependencies? What process do you
>> use
>>>> for
>>>>>> finding these heretofore hidden dependencies? Did you run gradle
>>>>>> app:dependencies?
>>>>>> 
>>>>> 
>>>> 
>>> 
>> 

Re: [Discuss] Transitive dependencies and internal .pom changes

[Dan Smith <ds...@pivotal.io>]

If what we are doing is just making sure that each geode module is
explicitly declaring all of its first order dependencies (things that
directly show up in import statements) in its build.gradle, then this seems
good to me.

I don't think we want to stop using gradle's ability to pull in all of the
transitive implementation dependencies that we don't directly use. We don't
want to get into the business of maintaining those manually.

Removing unused dependencies and pulling in this lint plugin also seem like
good improvements.

-Dan

On Mon, Oct 1, 2018 at 1:44 PM Patrick Rhomberg <pr...@apache.org>
wrote:

> > Patrick, you just mentioned the java-library plugin's ability to
> > distinguish between api and implementation dependencies. Geode doesn't
> use
> > that plugin yet AFAIK, nor does the PR introduce that plugin right?
>
> That is correct, we do not yet make use of the `java-library` plugin (which
> has the `implementation` and `api` configurations) and are still using the
> `java` plugin (and rely on the `compile` configurations).  I brought it up
> only to mention that there will be a great deal of inter-module build
> breakages if we were to try to transition to using `implementation` while
> our actual dependencies of each module were not yet explicit.
>
> And yeah, all of this is on the road to Java9 modules.
>
> As for accepting this PR, there are still multiple issues with it as it
> stands -- I mostly opened it in its current state to spur this discussion.
> I'm was going to go open a handful of tickets so that each individual PR
> and commit are more bite-sized.
>
> On Fri, Sep 28, 2018 at 4:26 PM, Bill Burcham <bb...@pivotal.io> wrote:
>
> > So it sounds like (per Robert) we use the gradle-lint-plugin's
> > unused-dependency rule to warn us of unused dependencies. That handles
> one
> > side of the equation (only list *necessary* dependencies).
> >
> > I haven't heard anyone mention tool support for the other side of the
> > equation: ensuring that we list the *sufficient* list of dependencies per
> > build.gradle.
> >
> > Patrick, you just mentioned the java-library plugin's ability to
> > distinguish between api and implementation dependencies. Geode doesn't
> use
> > that plugin yet AFAIK, nor does the PR introduce that plugin right?
> >
> > All this seems of a piece with the march toward modularity, exemplified
> by
> > Java 9 modules. Patrick, is your vision something like:
> >
> > 1. accept this PR
> > …
> > 2. introduce the java-library plugin and segregate dependencies into api
> > vs. implementation ones
> > …
> > N. define Java modules
> >
> >
> > On Fri, Sep 28, 2018 at 10:39 AM Patrick Rhomberg <pr...@apache.org>
> > wrote:
> >
> > > Bill has the heart of it, yes.
> > >
> > > I should have also mentioned that this ties into java-library plugin
> > > configuration, notably that the `compile` configuration is deprecated.
> > > For dependences that we do not wish to leak, we will need to use
> > > `implementation`.  For dependencies which we intentionally elect to
> share
> > > with our consumers, we should use `api`.
> > >
> > > For modularity, it should not break one module's ability to build for
> > > another module to declare a `compile` configuration to be
> > `implementation`,
> > > unless of course that dependency is an active and necessary component
> > part
> > > of that module's consumption.  In that case, it should be declared part
> > of
> > > the modules `api`.
> > > In this vein and as Bill pointed out, a module should have an accurate
> > > representation of its own build-time dependencies.
> > >
> > > In time, this will allow a much improved build experience, since
> > > `implementation` and `api` are positioned for better granularity of
> > > incremental building.
> > >
> > > On Fri, Sep 28, 2018 at 10:23 AM, Robert Houghton <
> rhoughton@pivotal.io>
> > > wrote:
> > >
> > > > Hi Bill,
> > > >
> > > > We are using a Gradle plug-in to identify dependencies that are
> unused,
> > > or
> > > > are declared in the wrong module or scope.
> > > > This is called out by the Gradle documents on improving build [
> > > > https://guides.gradle.org/performance/#avoid_unnecessary_and_unused_
> > > > dependencies].
> > > > The plug-in documentation is here [
> > > > https://github.com/nebula-plugins/gradle-lint-plugin/
> > > > wiki/Unused-Dependency-Rule
> > > > ]
> > > >
> > > > Thank you,
> > > > -Robert Houghton
> > > >
> > > > On Fri, Sep 28, 2018 at 10:18 AM Bill Burcham <bb...@pivotal.io>
> > > wrote:
> > > >
> > > > > From the PR, Anthony, it seems to me that Patrick is proposing that
> > > each
> > > > > build.gradle be explicit about mentioning the "things" it depends
> on.
> > > For
> > > > > example:
> > > > >
> > > > > [image: image.png]
> > > > >
> > > > > Look at how geode-connectors goes from mentioning a few
> dependencies
> > to
> > > > > mentioning many more. The value of this is that it ostensibly shows
> > us
> > > > all
> > > > > the things that geode-connectors actually depends on.
> > > > >
> > > > > The challenge I see is two-fold:
> > > > >
> > > > > • as with Java imports and "unused imports", there is the risk of
> > > listing
> > > > > more dependencies than we actually need
> > > > > • there's also the risk that we still don't have a complete list of
> > > > > dependencies
> > > > >
> > > > > Patrick: do you have tool support for this? Is there a tool that
> can
> > > > > identify or even remove unused dependencies? What process do you
> use
> > > for
> > > > > finding these heretofore hidden dependencies? Did you run gradle
> > > > > app:dependencies?
> > > > >
> > > >
> > >
> >
>